Skip to content

Commit b16f279

Browse files
ryanjbaxterryanjbaxter
authored
Merge pull request #1972 from wind57/fix-1971
Fix 1971
2 parents bd8d127 + c7aaca0 commit b16f279

File tree

10 files changed

+225
-23
lines changed

10 files changed

+225
-23
lines changed

‎spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointSlicesCatalogWatch.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Collections;
2121
import java.util.List;
2222
import java.util.Map;
23+
import java.util.Objects;
2324
import java.util.function.Function;
2425
import java.util.stream.Stream;
2526

@@ -73,13 +74,18 @@ else if (!context.properties().namespaces().isEmpty()) {
7374
endpointSlices = namespacedEndpointSlices(api, namespace, context.properties().serviceLabels());
7475
}
7576

77+
return generateState(endpointSlices);
78+
79+
}
80+
81+
List<EndpointNameAndNamespace> generateState(List<V1EndpointSlice> endpointSlices) {
7682
Stream<V1ObjectReference> references = endpointSlices.stream()
7783
.map(V1EndpointSlice::getEndpoints)
84+
.filter(Objects::nonNull)
7885
.flatMap(List::stream)
7986
.map(V1Endpoint::getTargetRef);
8087

8188
return KubernetesCatalogWatchContext.state(references);
82-
8389
}
8490

8591
private List<V1EndpointSlice> endpointSlices(DiscoveryV1Api api, Map<String, String> labels) {

‎spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsCatalogWatch.java‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,21 @@ else if (!context.properties().namespaces().isEmpty()) {
7272
endpoints = namespacedEndpoints(coreV1Api, namespace, context.properties().serviceLabels());
7373
}
7474

75-
/**
76-
* <pre>
77-
* - An "V1Endpoints" holds a List of V1EndpointSubset.
78-
* - A single V1EndpointSubset holds a List of V1EndpointAddress
79-
*
80-
* - (The union of all V1EndpointSubsets is the Set of all V1Endpoints)
81-
* - Set of V1Endpoints is the cartesian product of :
82-
* V1EndpointSubset::getAddresses and V1EndpointSubset::getPorts (each is a List)
83-
* </pre>
84-
*/
75+
return generateState(endpoints);
76+
77+
}
78+
79+
/**
80+
* <pre>
81+
* - An "V1Endpoints" holds a List of V1EndpointSubset.
82+
* - A single V1EndpointSubset holds a List of V1EndpointAddress
83+
*
84+
* - (The union of all V1EndpointSubsets is the Set of all V1Endpoints)
85+
* - Set of V1Endpoints is the cartesian product of :
86+
* V1EndpointSubset::getAddresses and V1EndpointSubset::getPorts (each is a List)
87+
* </pre>
88+
*/
89+
List<EndpointNameAndNamespace> generateState(List<V1Endpoints> endpoints) {
8590
Stream<V1ObjectReference> references = endpoints.stream()
8691
.map(V1Endpoints::getSubsets)
8792
.filter(Objects::nonNull)
@@ -92,7 +97,6 @@ else if (!context.properties().namespaces().isEmpty()) {
9297
.map(V1EndpointAddress::getTargetRef);
9398

9499
return KubernetesCatalogWatchContext.state(references);
95-
96100
}
97101

98102
private List<V1Endpoints> endpoints(CoreV1Api client, Map<String, String> labels) {

‎spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointSlicesTests.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import com.github.tomakehurst.wiremock.client.WireMock;
2626
import io.kubernetes.client.openapi.ApiClient;
2727
import io.kubernetes.client.openapi.JSON;
28+
import io.kubernetes.client.openapi.models.V1EndpointSlice;
29+
import io.kubernetes.client.openapi.models.V1EndpointSliceList;
2830
import io.kubernetes.client.util.ClientBuilder;
2931
import org.junit.jupiter.api.AfterAll;
3032
import org.junit.jupiter.api.AfterEach;
@@ -38,6 +40,7 @@
3840
import static com.github.tomakehurst.wiremock.client.WireMock.get;
3941
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
4042
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
43+
import static org.assertj.core.api.Assertions.assertThat;
4144

4245
/**
4346
* Test cases for the Endpoint Slices support
@@ -191,4 +194,36 @@ void testInOneNamespaceWithDoubleLabel() {
191194
invokeAndAssert(watch, List.of(new EndpointNameAndNamespace("a", "b")));
192195
}
193196

197+
@Test
198+
@Override
199+
void testWithoutSubsetsOrEndpoints() {
200+
201+
// even though we set Endpoints here to null, when
202+
// deserializing, it will be an empty List.
203+
// I'm going to leave the test here in case the client changes in this regard.
204+
// 'generateStateEndpointsWithoutEndpoints' test covers the null Subsets anyway
205+
V1EndpointSliceList endpointSlices = endpointSlicesNoEndpoints();
206+
207+
stubFor(get("/apis/discovery.k8s.io/v1/namespaces/b/endpointslices?labelSelector=key%3Dvalue%2Ckey1%3Dvalue1")
208+
.willReturn(aResponse().withStatus(200).withBody(new JSON().serialize(endpointSlices))));
209+
// otherwise the stub might fail
210+
LinkedHashMap<String, String> map = new LinkedHashMap<>();
211+
map.put("key", "value");
212+
map.put("key1", "value1");
213+
KubernetesCatalogWatch watch = createWatcherInSpecificNamespaceWithLabels("b", map, null, apiClient,
214+
USE_ENDPOINT_SLICES);
215+
216+
invokeAndAssert(watch, List.of());
217+
}
218+
219+
@Test
220+
void generateStateEndpointsWithoutEndpoints() {
221+
222+
KubernetesEndpointSlicesCatalogWatch catalogWatch = new KubernetesEndpointSlicesCatalogWatch();
223+
List<V1EndpointSlice> endpointSlicesNoEndpoints = endpointSlicesNoEndpoints().getItems();
224+
225+
// even if Endpoints are missing, we do not fail
226+
assertThat(catalogWatch.generateState(endpointSlicesNoEndpoints)).isEmpty();
227+
}
228+
194229
}

‎spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointsTests.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.github.tomakehurst.wiremock.client.WireMock;
2626
import io.kubernetes.client.openapi.JSON;
2727
import io.kubernetes.client.openapi.apis.CoreV1Api;
28+
import io.kubernetes.client.openapi.models.V1Endpoints;
2829
import io.kubernetes.client.util.ClientBuilder;
2930
import org.junit.jupiter.api.AfterAll;
3031
import org.junit.jupiter.api.AfterEach;
@@ -38,6 +39,7 @@
3839
import static com.github.tomakehurst.wiremock.client.WireMock.get;
3940
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
4041
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
42+
import static org.assertj.core.api.Assertions.assertThat;
4143

4244
/**
4345
* Test cases for the Endpoints support
@@ -191,4 +193,30 @@ void testInOneNamespaceWithDoubleLabel() {
191193
invokeAndAssert(watch, List.of(new EndpointNameAndNamespace("a", "b")));
192194
}
193195

196+
@Test
197+
@Override
198+
void testWithoutSubsetsOrEndpoints() {
199+
stubFor(get("/api/v1/namespaces/b/endpoints?labelSelector=key%3Dvalue%2Ckey1%3Dvalue1")
200+
.willReturn(aResponse().withStatus(200).withBody(new JSON().serialize(endpointsNoSubsets()))));
201+
// otherwise the stub might fail
202+
LinkedHashMap<String, String> map = new LinkedHashMap<>();
203+
map.put("key", "value");
204+
map.put("key1", "value1");
205+
KubernetesCatalogWatch watch = createWatcherInSpecificNamespaceWithLabels("b", map, coreV1Api, null,
206+
USE_ENDPOINT_SLICES);
207+
208+
invokeAndAssert(watch, List.of());
209+
}
210+
211+
@Test
212+
void generateStateEndpointsWithoutSubsets() {
213+
214+
KubernetesEndpointsCatalogWatch catalogWatch = new KubernetesEndpointsCatalogWatch();
215+
List<V1Endpoints> endpoints = endpointsNoSubsets().getItems();
216+
217+
// we do not fail, even if Subsets are not present
218+
List<EndpointNameAndNamespace> result = catalogWatch.generateState(endpoints);
219+
assertThat(result).isEmpty();
220+
}
221+
194222
}

‎spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsAndEndpointSlicesTests.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import io.kubernetes.client.openapi.ApiClient;
2424
import io.kubernetes.client.openapi.apis.CoreV1Api;
25+
import io.kubernetes.client.openapi.models.V1Endpoint;
2526
import io.kubernetes.client.openapi.models.V1EndpointAddressBuilder;
2627
import io.kubernetes.client.openapi.models.V1EndpointBuilder;
2728
import io.kubernetes.client.openapi.models.V1EndpointSliceBuilder;
@@ -104,6 +105,8 @@ abstract class KubernetesEndpointsAndEndpointSlicesTests {
104105
*/
105106
abstract void testInOneNamespaceWithDoubleLabel();
106107

108+
abstract void testWithoutSubsetsOrEndpoints();
109+
107110
KubernetesCatalogWatch createWatcherInAllNamespacesWithLabels(Map<String, String> labels, Set<String> namespaces,
108111
CoreV1Api coreV1Api, ApiClient apiClient, boolean endpointSlices) {
109112

@@ -174,6 +177,12 @@ V1EndpointsList endpoints(String name, String namespace) {
174177
.build();
175178
}
176179

180+
V1EndpointsList endpointsNoSubsets() {
181+
return new V1EndpointsListBuilder().addToItems(new V1EndpointsBuilder().build())
182+
.withKind("EndpointsList")
183+
.build();
184+
}
185+
177186
V1EndpointSliceList endpointSlices(String name, String namespace) {
178187
return new V1EndpointSliceListBuilder()
179188
.addToItems(new V1EndpointSliceBuilder().addToEndpoints(new V1EndpointBuilder()
@@ -182,6 +191,13 @@ V1EndpointSliceList endpointSlices(String name, String namespace) {
182191
.build();
183192
}
184193

194+
V1EndpointSliceList endpointSlicesNoEndpoints() {
195+
List<V1Endpoint> endpoints = null;
196+
return new V1EndpointSliceListBuilder().withKind("V1EndpointSliceList")
197+
.addToItems(new V1EndpointSliceBuilder().withEndpoints(endpoints).build())
198+
.build();
199+
}
200+
185201
static void invokeAndAssert(KubernetesCatalogWatch watch, List<EndpointNameAndNamespace> state) {
186202
watch.catalogServicesWatch();
187203

‎spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointSliceV1CatalogWatch.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.util.ArrayList;
2020
import java.util.List;
21+
import java.util.Objects;
2122
import java.util.function.Function;
2223
import java.util.stream.Stream;
2324

@@ -72,13 +73,21 @@ else if (!context.properties().namespaces().isEmpty()) {
7273
endpointSlices = endpointSlices(context, namespace, client);
7374
}
7475

76+
return generateState(endpointSlices);
77+
}
78+
79+
/**
80+
* This one is visible for testing, especially since fabric8 mock client will save
81+
* null subsets as empty lists, thus blocking some unit test.
82+
*/
83+
List<EndpointNameAndNamespace> generateState(List<EndpointSlice> endpointSlices) {
7584
Stream<ObjectReference> references = endpointSlices.stream()
7685
.map(EndpointSlice::getEndpoints)
86+
.filter(Objects::nonNull)
7787
.flatMap(List::stream)
7888
.map(Endpoint::getTargetRef);
7989

8090
return Fabric8CatalogWatchContext.state(references);
81-
8291
}
8392

8493
private List<EndpointSlice> endpointSlices(Fabric8CatalogWatchContext context, String namespace,

‎spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsCatalogWatch.java‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,23 @@ public List<EndpointNameAndNamespace> apply(Fabric8CatalogWatchContext context)
4444
List<Endpoints> endpoints = endpoints(context.properties(), context.kubernetesClient(),
4545
context.namespaceProvider(), "catalog-watcher", null, ALWAYS_TRUE);
4646

47-
/**
48-
* <pre>
49-
* - An "Endpoints" holds a List of EndpointSubset.
50-
* - A single EndpointSubset holds a List of EndpointAddress
51-
*
52-
* - (The union of all EndpointSubsets is the Set of all Endpoints)
53-
* - Set of Endpoints is the cartesian product of :
54-
* EndpointSubset::getAddresses and EndpointSubset::getPorts (each is a List)
55-
* </pre>
56-
*/
47+
return generateState(endpoints);
48+
}
49+
50+
/**
51+
* This one is visible for testing, especially since fabric8 mock client will save
52+
* null subsets as empty lists, thus blocking some unit test.
53+
*
54+
* <pre>
55+
* - An "Endpoints" holds a List of EndpointSubset.
56+
* - A single EndpointSubset holds a List of EndpointAddress
57+
*
58+
* - (The union of all EndpointSubsets is the Set of all Endpoints)
59+
* - Set of Endpoints is the cartesian product of :
60+
* EndpointSubset::getAddresses and EndpointSubset::getPorts (each is a List)
61+
* </pre>
62+
*/
63+
List<EndpointNameAndNamespace> generateState(List<Endpoints> endpoints) {
5764
Stream<ObjectReference> references = endpoints.stream()
5865
.map(Endpoints::getSubsets)
5966
.filter(Objects::nonNull)

‎spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsAndEndpointSlicesTests.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,16 @@ void afterEach() {
235235
*/
236236
abstract void testTwoNamespacesOutOfThree();
237237

238+
/**
239+
* <pre>
240+
* - in the old API (plain Endpoints), tests that subsets are missing
241+
* and we do not fail.
242+
* - in the new API (EndpointSlices), tests that Endpoints are missing
243+
* and we do not fail.
244+
* </pre>
245+
*/
246+
abstract void testWithoutSubsetsOrEndpoints();
247+
238248
KubernetesCatalogWatch createWatcherInAllNamespacesWithLabels(Map<String, String> labels, Set<String> namespaces,
239249
boolean endpointSlices) {
240250

@@ -309,6 +319,21 @@ void endpoints(String namespace, Map<String, String> labels, String podName) {
309319
mockClient().endpoints().inNamespace(namespace).resource(endpoints).create();
310320
}
311321

322+
Endpoints endpointsWithoutSubsets(String namespace, Map<String, String> labels, String podName) {
323+
324+
// though we set it to null here, the mock client when creating it
325+
// will set it to an empty list. I will keep it like this, may be client changes
326+
// in the future and we have the case still covered by a test
327+
List<EndpointSubset> endpointSubsets = null;
328+
329+
Endpoints endpoints = new EndpointsBuilder()
330+
.withMetadata(new ObjectMetaBuilder().withLabels(labels).withName("endpoints-" + podName).build())
331+
.withSubsets(endpointSubsets)
332+
.build();
333+
mockClient().endpoints().inNamespace(namespace).resource(endpoints).create();
334+
return endpoints;
335+
}
336+
312337
void service(String namespace, Map<String, String> labels, String podName) {
313338

314339
Service service = new ServiceBuilder()
@@ -335,6 +360,23 @@ static void endpointSlice(String namespace, Map<String, String> labels, String p
335360

336361
}
337362

363+
static EndpointSlice endpointSliceWithoutEndpoints(String namespace, Map<String, String> labels, String podName) {
364+
365+
List<Endpoint> endpoints = null;
366+
367+
EndpointSlice slice = new EndpointSliceBuilder()
368+
.withMetadata(new ObjectMetaBuilder().withNamespace(namespace)
369+
.withName("slice-" + podName)
370+
.withLabels(labels)
371+
.build())
372+
.withEndpoints(endpoints)
373+
.build();
374+
375+
mockClient().discovery().v1().endpointSlices().inNamespace(namespace).resource(slice).create();
376+
return slice;
377+
378+
}
379+
338380
static void invokeAndAssert(KubernetesCatalogWatch watch, List<EndpointNameAndNamespace> state) {
339381
watch.catalogServicesWatch();
340382

‎spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesCatalogWatchEndpointSlicesTests.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@
2020
import java.util.Map;
2121
import java.util.Set;
2222

23+
import io.fabric8.kubernetes.api.model.discovery.v1.EndpointSlice;
2324
import io.fabric8.kubernetes.client.KubernetesClient;
2425
import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient;
2526
import org.junit.jupiter.api.Test;
2627

2728
import org.springframework.cloud.kubernetes.commons.discovery.EndpointNameAndNamespace;
2829

30+
import static org.assertj.core.api.Assertions.assertThat;
31+
2932
/**
3033
* Some tests that use the fabric8 mock client, using EndpointSlices
3134
*
@@ -178,6 +181,29 @@ void testTwoNamespacesOutOfThree() {
178181
new EndpointNameAndNamespace("podF", "namespaceB")));
179182
}
180183

184+
@Test
185+
@Override
186+
void testWithoutSubsetsOrEndpoints() {
187+
KubernetesCatalogWatch watch = createWatcherInSpecificNamespacesWithLabels(Set.of("namespaceA"),
188+
Map.of("color", "blue"), ENDPOINT_SLICES);
189+
190+
endpointSliceWithoutEndpoints("namespaceA", Map.of("color", "blue"), "podA");
191+
192+
// we do not fail here, even if Endpoints are not present.
193+
invokeAndAssert(watch, List.of());
194+
}
195+
196+
@Test
197+
void generateStateEndpointsWithoutEndpoints() {
198+
199+
Fabric8EndpointSliceV1CatalogWatch catalogWatch = new Fabric8EndpointSliceV1CatalogWatch();
200+
EndpointSlice sliceNoEndpoints = endpointSliceWithoutEndpoints("namespaceA", Map.of("color", "blue"), "podA");
201+
202+
203+
// even if Endpoints are missing, we do not fail
204+
assertThat(catalogWatch.generateState(List.of(sliceNoEndpoints))).isEmpty();
205+
}
206+
181207
// work-around for : https://github.com/fabric8io/kubernetes-client/issues/4649
182208
static KubernetesClient endpointSlicesMockClient() {
183209
return mockClient;

0 commit comments

Comments
 (0)