Skip to content

Commit 8a2ed9e

Browse files
ssirowa14ssirowa14
committed
Rebase Conflict Resolution
1 parent 6ace201 commit 8a2ed9e

4 files changed

Lines changed: 156 additions & 59 deletions

File tree

‎docs/migration_guide.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,18 @@ affect building the project, CI, tests or examples.
159159
`driverStartTimeoutMillis` was disabled. Since `DriverStartTimedOut` is an infrastructure failure
160160
while the others are not, `restartPolicy: OnInfrastructureFailure` no longer restarts such an
161161
application. Use `OnFailure` to restart it.
162+
- Since 1.1.0, a label set on a secondary resource of the driver takes precedence over a label of
163+
the same key on the driver pod, instead of being overwritten by it. This affects the user-defined
164+
labels in `spec.driverServiceIngressList` (`serviceMetadata`, `ingressMetadata`),
165+
`spec.driverHttpRouteList` (`serviceMetadata`, `httpRouteMetadata`), `spec.driverGrpcRouteList`
166+
(`serviceMetadata`, `grpcRouteMetadata`) and `spec.configMapSpecs`, as well as the other
167+
Kubernetes resources of the driver, e.g. its config map and on-demand PVCs. 1.0 overwrote these
168+
labels with the driver-pod labels of the same key, including the ones injected into the driver pod
169+
by admission webhooks. The operator- and Spark-managed identity labels, i.e. any `spark.operator/`
170+
key, `spark-role`, `spark-version`, `spark-app-selector` and `spark-app-name`, still take the
171+
driver-pod value, so the secondary-to-primary mapping and the pod and service selectors keep
172+
working. To restore the behavior before 1.1.0 for a given key, remove it from the secondary
173+
resource so it inherits the driver-pod label.
162174

163175
### SparkCluster
164176

‎spark-operator-api/src/main/java/org/apache/spark/k8s/operator/Constants.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ public final class Constants {
8686
/** The label for the Spark version. */
8787
public static final String LABEL_SPARK_VERSION_NAME = "spark-version";
8888

89+
/** The label for the Spark application ID (Spark-defined, used as the service selector). */
90+
public static final String LABEL_SPARK_APP_SELECTOR = "spark-app-selector";
91+
92+
/** The label for the Spark application name (Spark-defined). */
93+
public static final String LABEL_SPARK_APP_NAME = "spark-app-name";
94+
8995
/** A dummy field for sentinel resources. */
9096
public static final String SENTINEL_RESOURCE_DUMMY_FIELD = "sentinel.dummy.number";
9197

‎spark-operator/src/main/java/org/apache/spark/k8s/operator/decorators/DriverResourceDecorator.java‎

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,18 @@
1919

2020
package org.apache.spark.k8s.operator.decorators;
2121

22+
import static org.apache.spark.k8s.operator.Constants.LABEL_SPARK_APPLICATION_NAME;
23+
import static org.apache.spark.k8s.operator.Constants.LABEL_SPARK_APP_NAME;
24+
import static org.apache.spark.k8s.operator.Constants.LABEL_SPARK_APP_SELECTOR;
25+
import static org.apache.spark.k8s.operator.Constants.LABEL_SPARK_OPERATOR_NAME;
26+
import static org.apache.spark.k8s.operator.Constants.LABEL_SPARK_ROLE_NAME;
27+
import static org.apache.spark.k8s.operator.Constants.LABEL_SPARK_VERSION_NAME;
2228
import static org.apache.spark.k8s.operator.utils.ModelUtils.buildOwnerReferenceTo;
2329

30+
import java.util.Map;
31+
import java.util.Set;
32+
import java.util.stream.Collectors;
33+
2434
import io.fabric8.kubernetes.api.model.HasMetadata;
2535
import io.fabric8.kubernetes.api.model.ObjectMeta;
2636
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
@@ -45,10 +55,33 @@
4555
public class DriverResourceDecorator implements ResourceDecorator {
4656
private final Pod driverPod;
4757

58+
/**
59+
* Operator- and Spark-managed identity labels whose driver-pod value must always propagate to
60+
* secondary resources unchanged. A resource's own label of the same key
61+
* must never override them. Any label key under the {@value #MANAGED_LABEL_PREFIX} prefix is
62+
* protected the same way (which already covers the operator-namespaced keys listed here).
63+
*/
64+
private static final Set<String> MANAGED_LABEL_KEYS =
65+
Set.of(
66+
LABEL_SPARK_OPERATOR_NAME,
67+
LABEL_SPARK_APPLICATION_NAME,
68+
LABEL_SPARK_ROLE_NAME,
69+
LABEL_SPARK_VERSION_NAME,
70+
LABEL_SPARK_APP_SELECTOR,
71+
LABEL_SPARK_APP_NAME);
72+
73+
/** Any label key starting with this prefix is treated as operator-managed and protected. */
74+
private static final String MANAGED_LABEL_PREFIX = "spark.operator/";
75+
4876
/**
4977
* Decorates a Kubernetes resource by adding an owner reference to the driver pod. This ensures
5078
* that secondary resources are garbage collected when the driver pod is deleted.
5179
*
80+
* <p>Label precedence: driver-pod labels are copied onto the resource; a label already set on the
81+
* resource takes precedence on key collision, except for operator- and Spark-managed identity
82+
* labels (see {@link #MANAGED_LABEL_KEYS} and {@link #MANAGED_LABEL_PREFIX}), for which the
83+
* driver-pod value always wins.
84+
*
5285
* @param resource The resource to decorate.
5386
* @param <T> The type of the resource, extending HasMetadata.
5487
* @return The decorated resource.
@@ -73,13 +106,38 @@ public <T extends HasMetadata> T decorate(T resource) {
73106
new ObjectMetaBuilder(resource.getMetadata())
74107
.addToOwnerReferences(buildOwnerReferenceTo(driverPod))
75108
.addToLabels(driverPod.getMetadata().getLabels())
76-
// Re-apply the resource's own labels last so a label set explicitly on
77-
// the resource (e.g. a per-service label) wins over the driver-pod label
78-
// of the same key, rather than being overwritten by it.
79-
.addToLabels(resource.getMetadata().getLabels())
109+
.addToLabels(overridableLabels(resource.getMetadata().getLabels()))
80110
.build();
81111
resource.setMetadata(metaData);
82112
}
83113
return resource;
84114
}
115+
116+
/**
117+
* Filters the resource's own labels down to the ones that are allowed to override a driver-pod
118+
* label of the same key, dropping operator- and Spark-managed identity labels (see {@link
119+
* #MANAGED_LABEL_KEYS} and {@link #MANAGED_LABEL_PREFIX}).
120+
*
121+
* @param resourceLabels the resource's own labels (may be {@code null}).
122+
* @return the subset of labels safe to re-apply on top of the driver-pod labels.
123+
*/
124+
private static Map<String, String> overridableLabels(Map<String, String> resourceLabels) {
125+
if (resourceLabels == null || resourceLabels.isEmpty()) {
126+
return Map.of();
127+
}
128+
return resourceLabels.entrySet().stream()
129+
.filter(entry -> !isManagedLabel(entry.getKey()))
130+
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
131+
}
132+
133+
/**
134+
* Returns whether the given label key is operator- or Spark-managed and must therefore keep the
135+
* driver-pod value rather than be overridden by the resource.
136+
*
137+
* @param key the label key to check.
138+
* @return {@code true} if the key is managed and must not be overridden.
139+
*/
140+
private static boolean isManagedLabel(String key) {
141+
return MANAGED_LABEL_KEYS.contains(key) || key.startsWith(MANAGED_LABEL_PREFIX);
142+
}
85143
}

‎spark-operator/src/test/java/org/apache/spark/k8s/operator/decorators/DriverResourceDecoratorTest.java‎

Lines changed: 76 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
package org.apache.spark.k8s.operator.decorators;
2121

22+
import static org.apache.spark.k8s.operator.Constants.LABEL_SPARK_APPLICATION_NAME;
23+
import static org.apache.spark.k8s.operator.Constants.LABEL_SPARK_OPERATOR_NAME;
24+
import static org.apache.spark.k8s.operator.Constants.LABEL_SPARK_ROLE_NAME;
2225
import static org.junit.jupiter.api.Assertions.assertEquals;
2326
import static org.junit.jupiter.api.Assertions.assertNotNull;
2427
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -41,83 +44,100 @@ private static Pod driverPod() {
4144
.withNewMetadata()
4245
.withName("spark-driver")
4346
.withUid("driver-uid")
44-
.addToLabels("spark-app-name", "app1")
45-
.addToLabels("istio.io/use-waypoint", "none")
47+
.addToLabels(LABEL_SPARK_OPERATOR_NAME, "spark-operator")
48+
.addToLabels(LABEL_SPARK_APPLICATION_NAME, "app1")
49+
// Prefix-matched managed label that is not one of the exact-match keys.
50+
.addToLabels("spark.operator/submission-id", "sub-1")
51+
.addToLabels(LABEL_SPARK_ROLE_NAME, "driver")
52+
.addToLabels("spark-app-selector", "spark-app-selector-1")
53+
.addToLabels("overlapping-label", "driver-value")
54+
.endMetadata()
55+
.build();
56+
}
57+
58+
/** Builds a Service carrying only the given metadata labels (decorator reads metadata only). */
59+
private static Service service(Map<String, String> labels) {
60+
return new ServiceBuilder()
61+
.withNewMetadata()
62+
.withName("app1-connect-svc")
63+
.addToLabels(labels)
4664
.endMetadata()
4765
.build();
4866
}
4967

5068
/**
51-
* Regression: a label set explicitly on the resource (e.g. a per-service waypoint label) must win
52-
* over a driver-pod label of the same key, not be overwritten by it.
69+
* Driver-pod labels whose key is absent on the resource are propagated onto the resource, while a
70+
* label the resource sets itself still wins on key collision (e.g. a per-service waypoint label).
5371
*/
5472
@Test
55-
void perServiceLabelWinsOverDriverPodLabel() {
56-
Service service =
57-
new ServiceBuilder()
58-
.withNewMetadata()
59-
.withName("app1-connect-svc")
60-
.addToLabels("istio.io/use-waypoint", "waypoint-ns1")
61-
.endMetadata()
62-
.withNewSpec()
63-
.endSpec()
64-
.build();
73+
void driverLabelsPropagatedAndResourceLabelWinsOnCollision() {
74+
Service service = service(Map.of("overlapping-label", "resource-value"));
6575

6676
new DriverResourceDecorator(driverPod()).decorate(service);
6777

6878
assertEquals(
69-
"waypoint-ns1", service.getMetadata().getLabels().get("istio.io/use-waypoint"));
79+
Map.of(
80+
LABEL_SPARK_OPERATOR_NAME, "spark-operator",
81+
LABEL_SPARK_APPLICATION_NAME, "app1",
82+
"spark.operator/submission-id", "sub-1",
83+
LABEL_SPARK_ROLE_NAME, "driver",
84+
"spark-app-selector", "spark-app-selector-1",
85+
"overlapping-label", "resource-value"),
86+
service.getMetadata().getLabels());
7087
}
7188

72-
/** Driver-pod labels whose key is absent on the resource are still propagated (GC identity). */
89+
/** A resource whose label map is null inherits the driver-pod labels without an NPE. */
7390
@Test
74-
void driverPodOnlyLabelsArePropagated() {
75-
Service service =
76-
new ServiceBuilder()
77-
.withNewMetadata()
78-
.withName("app1-connect-svc")
79-
.addToLabels("istio.io/use-waypoint", "waypoint-ns1")
80-
.endMetadata()
81-
.withNewSpec()
82-
.endSpec()
83-
.build();
91+
void resourceWithoutLabelsInheritsDriverPodLabels() {
92+
Service service = service(Map.of());
93+
// withNewMetadata() seeds an empty map, so clear it to exercise the null-labels path.
94+
service.getMetadata().setLabels(null);
8495

8596
new DriverResourceDecorator(driverPod()).decorate(service);
8697

8798
Map<String, String> labels = service.getMetadata().getLabels();
88-
assertEquals("app1", labels.get("spark-app-name"));
89-
assertEquals("waypoint-ns1", labels.get("istio.io/use-waypoint"));
99+
assertEquals("app1", labels.get(LABEL_SPARK_APPLICATION_NAME));
100+
assertEquals("driver", labels.get(LABEL_SPARK_ROLE_NAME));
90101
}
91102

92-
/** A resource with no labels of its own simply inherits the driver-pod labels (no NPE). */
103+
/**
104+
* Managed identity labels (exact-match keys and anything under the {@code spark.operator} prefix)
105+
* must keep the driver-pod value even when the resource sets a different value, so owner mapping
106+
* and selectors are preserved, while a non-managed label is still allowed to win.
107+
*/
93108
@Test
94-
void resourceWithoutLabelsInheritsDriverPodLabels() {
109+
void managedLabelsAreNotOverriddenByResource() {
95110
Service service =
96-
new ServiceBuilder()
97-
.withNewMetadata()
98-
.withName("app1-driver-svc")
99-
.endMetadata()
100-
.withNewSpec()
101-
.endSpec()
102-
.build();
111+
service(
112+
Map.of(
113+
LABEL_SPARK_OPERATOR_NAME, "rogue-operator",
114+
LABEL_SPARK_APPLICATION_NAME, "rogue-app",
115+
"spark.operator/submission-id", "rogue-sub",
116+
LABEL_SPARK_ROLE_NAME, "rogue-role",
117+
"spark-app-selector", "rogue-selector",
118+
"overlapping-label", "resource-value"));
103119

104120
new DriverResourceDecorator(driverPod()).decorate(service);
105121

106-
Map<String, String> labels = service.getMetadata().getLabels();
107-
assertEquals("none", labels.get("istio.io/use-waypoint"));
108-
assertEquals("app1", labels.get("spark-app-name"));
122+
assertEquals(
123+
Map.of(
124+
LABEL_SPARK_OPERATOR_NAME, "spark-operator",
125+
LABEL_SPARK_APPLICATION_NAME, "app1",
126+
"spark.operator/submission-id", "sub-1",
127+
LABEL_SPARK_ROLE_NAME, "driver",
128+
"spark-app-selector", "spark-app-selector-1",
129+
"overlapping-label", "resource-value"),
130+
service.getMetadata().getLabels());
109131
}
110132

133+
/**
134+
* A resource with no existing owner reference gets exactly one pointing at the driver pod (by
135+
* name and uid, with blockOwnerDeletion), which is what drives cascading garbage collection when
136+
* the driver pod is deleted.
137+
*/
111138
@Test
112139
void addsOwnerReferenceToDriver() {
113-
Service service =
114-
new ServiceBuilder()
115-
.withNewMetadata()
116-
.withName("app1-connect-svc")
117-
.endMetadata()
118-
.withNewSpec()
119-
.endSpec()
120-
.build();
140+
Service service = service(Map.of());
121141

122142
new DriverResourceDecorator(driverPod()).decorate(service);
123143

@@ -130,30 +150,31 @@ void addsOwnerReferenceToDriver() {
130150
}
131151

132152
/**
133-
* When the resource already carries the driver-pod owner reference, decoration is a no-op, so an
134-
* existing per-service label is left untouched (and no duplicate owner reference is added).
153+
* When the resource already carries the driver-pod owner reference, decoration is a no-op: the
154+
* labels are left exactly as they were (no driver-pod labels added) and no duplicate owner
155+
* reference is appended.
135156
*/
136157
@Test
137158
void skipsWhenOwnerReferenceAlreadyExists() {
138159
Service service =
139160
new ServiceBuilder()
140161
.withNewMetadata()
141162
.withName("app1-connect-svc")
142-
.addToLabels("istio.io/use-waypoint", "waypoint-ns1")
163+
.addToLabels("overlapping-label", "resource-value")
143164
.addNewOwnerReference()
144165
.withKind("Pod")
145166
.withName("spark-driver")
146167
.withUid("driver-uid")
147168
.endOwnerReference()
148169
.endMetadata()
149-
.withNewSpec()
150-
.endSpec()
151170
.build();
152171

153172
new DriverResourceDecorator(driverPod()).decorate(service);
154173

155-
assertEquals(1, service.getMetadata().getOwnerReferences().size());
174+
// Labels are untouched: no driver-pod labels were merged in.
156175
assertEquals(
157-
"waypoint-ns1", service.getMetadata().getLabels().get("istio.io/use-waypoint"));
176+
Map.of("overlapping-label", "resource-value"), service.getMetadata().getLabels());
177+
// And no duplicate owner reference was appended.
178+
assertEquals(1, service.getMetadata().getOwnerReferences().size());
158179
}
159180
}

0 commit comments

Comments
 (0)