Skip to content

Commit 6ace201

Browse files
ssirowa14ssirowa14
committed
SPARK-59925: DriverResourceDecorator overwrites resource-defined labels with driver pod labels
1 parent eb66854 commit 6ace201

2 files changed

Lines changed: 163 additions & 0 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ public <T extends HasMetadata> T decorate(T resource) {
7373
new ObjectMetaBuilder(resource.getMetadata())
7474
.addToOwnerReferences(buildOwnerReferenceTo(driverPod))
7575
.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())
7680
.build();
7781
resource.setMetadata(metaData);
7882
}

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

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.spark.k8s.operator.decorators;
21+
22+
import static org.junit.jupiter.api.Assertions.assertEquals;
23+
import static org.junit.jupiter.api.Assertions.assertNotNull;
24+
import static org.junit.jupiter.api.Assertions.assertTrue;
25+
26+
import java.util.Map;
27+
28+
import io.fabric8.kubernetes.api.model.OwnerReference;
29+
import io.fabric8.kubernetes.api.model.Pod;
30+
import io.fabric8.kubernetes.api.model.PodBuilder;
31+
import io.fabric8.kubernetes.api.model.Service;
32+
import io.fabric8.kubernetes.api.model.ServiceBuilder;
33+
import org.junit.jupiter.api.Test;
34+
35+
class DriverResourceDecoratorTest {
36+
37+
private static Pod driverPod() {
38+
return new PodBuilder()
39+
.withKind("Pod")
40+
.withApiVersion("v1")
41+
.withNewMetadata()
42+
.withName("spark-driver")
43+
.withUid("driver-uid")
44+
.addToLabels("spark-app-name", "app1")
45+
.addToLabels("istio.io/use-waypoint", "none")
46+
.endMetadata()
47+
.build();
48+
}
49+
50+
/**
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.
53+
*/
54+
@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();
65+
66+
new DriverResourceDecorator(driverPod()).decorate(service);
67+
68+
assertEquals(
69+
"waypoint-ns1", service.getMetadata().getLabels().get("istio.io/use-waypoint"));
70+
}
71+
72+
/** Driver-pod labels whose key is absent on the resource are still propagated (GC identity). */
73+
@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();
84+
85+
new DriverResourceDecorator(driverPod()).decorate(service);
86+
87+
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"));
90+
}
91+
92+
/** A resource with no labels of its own simply inherits the driver-pod labels (no NPE). */
93+
@Test
94+
void resourceWithoutLabelsInheritsDriverPodLabels() {
95+
Service service =
96+
new ServiceBuilder()
97+
.withNewMetadata()
98+
.withName("app1-driver-svc")
99+
.endMetadata()
100+
.withNewSpec()
101+
.endSpec()
102+
.build();
103+
104+
new DriverResourceDecorator(driverPod()).decorate(service);
105+
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"));
109+
}
110+
111+
@Test
112+
void addsOwnerReferenceToDriver() {
113+
Service service =
114+
new ServiceBuilder()
115+
.withNewMetadata()
116+
.withName("app1-connect-svc")
117+
.endMetadata()
118+
.withNewSpec()
119+
.endSpec()
120+
.build();
121+
122+
new DriverResourceDecorator(driverPod()).decorate(service);
123+
124+
assertNotNull(service.getMetadata().getOwnerReferences());
125+
assertEquals(1, service.getMetadata().getOwnerReferences().size());
126+
OwnerReference ref = service.getMetadata().getOwnerReferences().get(0);
127+
assertEquals("spark-driver", ref.getName());
128+
assertEquals("driver-uid", ref.getUid());
129+
assertTrue(ref.getBlockOwnerDeletion());
130+
}
131+
132+
/**
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).
135+
*/
136+
@Test
137+
void skipsWhenOwnerReferenceAlreadyExists() {
138+
Service service =
139+
new ServiceBuilder()
140+
.withNewMetadata()
141+
.withName("app1-connect-svc")
142+
.addToLabels("istio.io/use-waypoint", "waypoint-ns1")
143+
.addNewOwnerReference()
144+
.withKind("Pod")
145+
.withName("spark-driver")
146+
.withUid("driver-uid")
147+
.endOwnerReference()
148+
.endMetadata()
149+
.withNewSpec()
150+
.endSpec()
151+
.build();
152+
153+
new DriverResourceDecorator(driverPod()).decorate(service);
154+
155+
assertEquals(1, service.getMetadata().getOwnerReferences().size());
156+
assertEquals(
157+
"waypoint-ns1", service.getMetadata().getLabels().get("istio.io/use-waypoint"));
158+
}
159+
}

0 commit comments

Comments
 (0)