diff --git a/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointSlicesCatalogWatch.java b/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointSlicesCatalogWatch.java index 3177302c78..313c8143e1 100644 --- a/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointSlicesCatalogWatch.java +++ b/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointSlicesCatalogWatch.java @@ -20,6 +20,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.Function; import java.util.stream.Stream; @@ -73,13 +74,18 @@ else if (!context.properties().namespaces().isEmpty()) { endpointSlices = namespacedEndpointSlices(api, namespace, context.properties().serviceLabels()); } + return generateState(endpointSlices); + + } + + List generateState(List endpointSlices) { Stream references = endpointSlices.stream() .map(V1EndpointSlice::getEndpoints) + .filter(Objects::nonNull) .flatMap(List::stream) .map(V1Endpoint::getTargetRef); return KubernetesCatalogWatchContext.state(references); - } private List endpointSlices(DiscoveryV1Api api, Map labels) { diff --git a/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsCatalogWatch.java b/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsCatalogWatch.java index 1d3fcac51b..efc7c6d856 100644 --- a/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsCatalogWatch.java +++ b/spring-cloud-kubernetes-client-discovery/src/main/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsCatalogWatch.java @@ -72,16 +72,21 @@ else if (!context.properties().namespaces().isEmpty()) { endpoints = namespacedEndpoints(coreV1Api, namespace, context.properties().serviceLabels()); } - /** - *
-		 *   - An "V1Endpoints" holds a List of V1EndpointSubset.
-		 *   - A single V1EndpointSubset holds a List of V1EndpointAddress
-		 *
-		 *   - (The union of all V1EndpointSubsets is the Set of all V1Endpoints)
-		 *   - Set of V1Endpoints is the cartesian product of :
-		 *     V1EndpointSubset::getAddresses and V1EndpointSubset::getPorts (each is a List)
-		 * 
- */ + return generateState(endpoints); + + } + + /** + *
+	 *   - An "V1Endpoints" holds a List of V1EndpointSubset.
+	 *   - A single V1EndpointSubset holds a List of V1EndpointAddress
+	 *
+	 *   - (The union of all V1EndpointSubsets is the Set of all V1Endpoints)
+	 *   - Set of V1Endpoints is the cartesian product of :
+	 *     V1EndpointSubset::getAddresses and V1EndpointSubset::getPorts (each is a List)
+	 * 
+ */ + List generateState(List endpoints) { Stream references = endpoints.stream() .map(V1Endpoints::getSubsets) .filter(Objects::nonNull) @@ -92,7 +97,6 @@ else if (!context.properties().namespaces().isEmpty()) { .map(V1EndpointAddress::getTargetRef); return KubernetesCatalogWatchContext.state(references); - } private List endpoints(CoreV1Api client, Map labels) { diff --git a/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointSlicesTests.java b/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointSlicesTests.java index d5ff1a185e..6ba264b194 100644 --- a/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointSlicesTests.java +++ b/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointSlicesTests.java @@ -25,6 +25,8 @@ import com.github.tomakehurst.wiremock.client.WireMock; import io.kubernetes.client.openapi.ApiClient; import io.kubernetes.client.openapi.JSON; +import io.kubernetes.client.openapi.models.V1EndpointSlice; +import io.kubernetes.client.openapi.models.V1EndpointSliceList; import io.kubernetes.client.util.ClientBuilder; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; @@ -38,6 +40,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; +import static org.assertj.core.api.Assertions.assertThat; /** * Test cases for the Endpoint Slices support @@ -191,4 +194,36 @@ void testInOneNamespaceWithDoubleLabel() { invokeAndAssert(watch, List.of(new EndpointNameAndNamespace("a", "b"))); } + @Test + @Override + void testWithoutSubsetsOrEndpoints() { + + // even though we set Endpoints here to null, when + // deserializing, it will be an empty List. + // I'm going to leave the test here in case the client changes in this regard. + // 'generateStateEndpointsWithoutEndpoints' test covers the null Subsets anyway + V1EndpointSliceList endpointSlices = endpointSlicesNoEndpoints(); + + stubFor(get("/apis/discovery.k8s.io/v1/namespaces/b/endpointslices?labelSelector=key%3Dvalue%2Ckey1%3Dvalue1") + .willReturn(aResponse().withStatus(200).withBody(new JSON().serialize(endpointSlices)))); + // otherwise the stub might fail + LinkedHashMap map = new LinkedHashMap<>(); + map.put("key", "value"); + map.put("key1", "value1"); + KubernetesCatalogWatch watch = createWatcherInSpecificNamespaceWithLabels("b", map, null, apiClient, + USE_ENDPOINT_SLICES); + + invokeAndAssert(watch, List.of()); + } + + @Test + void generateStateEndpointsWithoutEndpoints() { + + KubernetesEndpointSlicesCatalogWatch catalogWatch = new KubernetesEndpointSlicesCatalogWatch(); + List endpointSlicesNoEndpoints = endpointSlicesNoEndpoints().getItems(); + + // even if Endpoints are missing, we do not fail + assertThat(catalogWatch.generateState(endpointSlicesNoEndpoints)).isEmpty(); + } + } diff --git a/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointsTests.java b/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointsTests.java index a0a2e93bae..9df4a001d1 100644 --- a/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointsTests.java +++ b/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesCatalogWatchEndpointsTests.java @@ -25,6 +25,7 @@ import com.github.tomakehurst.wiremock.client.WireMock; import io.kubernetes.client.openapi.JSON; import io.kubernetes.client.openapi.apis.CoreV1Api; +import io.kubernetes.client.openapi.models.V1Endpoints; import io.kubernetes.client.util.ClientBuilder; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; @@ -38,6 +39,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; +import static org.assertj.core.api.Assertions.assertThat; /** * Test cases for the Endpoints support @@ -191,4 +193,30 @@ void testInOneNamespaceWithDoubleLabel() { invokeAndAssert(watch, List.of(new EndpointNameAndNamespace("a", "b"))); } + @Test + @Override + void testWithoutSubsetsOrEndpoints() { + stubFor(get("/api/v1/namespaces/b/endpoints?labelSelector=key%3Dvalue%2Ckey1%3Dvalue1") + .willReturn(aResponse().withStatus(200).withBody(new JSON().serialize(endpointsNoSubsets())))); + // otherwise the stub might fail + LinkedHashMap map = new LinkedHashMap<>(); + map.put("key", "value"); + map.put("key1", "value1"); + KubernetesCatalogWatch watch = createWatcherInSpecificNamespaceWithLabels("b", map, coreV1Api, null, + USE_ENDPOINT_SLICES); + + invokeAndAssert(watch, List.of()); + } + + @Test + void generateStateEndpointsWithoutSubsets() { + + KubernetesEndpointsCatalogWatch catalogWatch = new KubernetesEndpointsCatalogWatch(); + List endpoints = endpointsNoSubsets().getItems(); + + // we do not fail, even if Subsets are not present + List result = catalogWatch.generateState(endpoints); + assertThat(result).isEmpty(); + } + } diff --git a/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsAndEndpointSlicesTests.java b/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsAndEndpointSlicesTests.java index 0fa40fd936..ba09c0e133 100644 --- a/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsAndEndpointSlicesTests.java +++ b/spring-cloud-kubernetes-client-discovery/src/test/java/org/springframework/cloud/kubernetes/client/discovery/catalog/KubernetesEndpointsAndEndpointSlicesTests.java @@ -22,6 +22,7 @@ import io.kubernetes.client.openapi.ApiClient; import io.kubernetes.client.openapi.apis.CoreV1Api; +import io.kubernetes.client.openapi.models.V1Endpoint; import io.kubernetes.client.openapi.models.V1EndpointAddressBuilder; import io.kubernetes.client.openapi.models.V1EndpointBuilder; import io.kubernetes.client.openapi.models.V1EndpointSliceBuilder; @@ -104,6 +105,8 @@ abstract class KubernetesEndpointsAndEndpointSlicesTests { */ abstract void testInOneNamespaceWithDoubleLabel(); + abstract void testWithoutSubsetsOrEndpoints(); + KubernetesCatalogWatch createWatcherInAllNamespacesWithLabels(Map labels, Set namespaces, CoreV1Api coreV1Api, ApiClient apiClient, boolean endpointSlices) { @@ -174,6 +177,12 @@ V1EndpointsList endpoints(String name, String namespace) { .build(); } + V1EndpointsList endpointsNoSubsets() { + return new V1EndpointsListBuilder().addToItems(new V1EndpointsBuilder().build()) + .withKind("EndpointsList") + .build(); + } + V1EndpointSliceList endpointSlices(String name, String namespace) { return new V1EndpointSliceListBuilder() .addToItems(new V1EndpointSliceBuilder().addToEndpoints(new V1EndpointBuilder() @@ -182,6 +191,13 @@ V1EndpointSliceList endpointSlices(String name, String namespace) { .build(); } + V1EndpointSliceList endpointSlicesNoEndpoints() { + List endpoints = null; + return new V1EndpointSliceListBuilder().withKind("V1EndpointSliceList") + .addToItems(new V1EndpointSliceBuilder().withEndpoints(endpoints).build()) + .build(); + } + static void invokeAndAssert(KubernetesCatalogWatch watch, List state) { watch.catalogServicesWatch(); diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointSliceV1CatalogWatch.java b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointSliceV1CatalogWatch.java index 22f4d88f05..5936b807c0 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointSliceV1CatalogWatch.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointSliceV1CatalogWatch.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.function.Function; import java.util.stream.Stream; @@ -72,13 +73,21 @@ else if (!context.properties().namespaces().isEmpty()) { endpointSlices = endpointSlices(context, namespace, client); } + return generateState(endpointSlices); + } + + /** + * This one is visible for testing, especially since fabric8 mock client will save + * null subsets as empty lists, thus blocking some unit test. + */ + List generateState(List endpointSlices) { Stream references = endpointSlices.stream() .map(EndpointSlice::getEndpoints) + .filter(Objects::nonNull) .flatMap(List::stream) .map(Endpoint::getTargetRef); return Fabric8CatalogWatchContext.state(references); - } private List endpointSlices(Fabric8CatalogWatchContext context, String namespace, diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsCatalogWatch.java b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsCatalogWatch.java index 2e7791c2cf..c50b0127f1 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsCatalogWatch.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsCatalogWatch.java @@ -44,16 +44,23 @@ public List apply(Fabric8CatalogWatchContext context) List endpoints = endpoints(context.properties(), context.kubernetesClient(), context.namespaceProvider(), "catalog-watcher", null, ALWAYS_TRUE); - /** - *
-		 *   - An "Endpoints" holds a List of EndpointSubset.
-		 *   - A single EndpointSubset holds a List of EndpointAddress
-		 *
-		 *   - (The union of all EndpointSubsets is the Set of all Endpoints)
-		 *   - Set of Endpoints is the cartesian product of :
-		 *     EndpointSubset::getAddresses and EndpointSubset::getPorts (each is a List)
-		 * 
- */ + return generateState(endpoints); + } + + /** + * This one is visible for testing, especially since fabric8 mock client will save + * null subsets as empty lists, thus blocking some unit test. + * + *
+	 *   - An "Endpoints" holds a List of EndpointSubset.
+	 *   - A single EndpointSubset holds a List of EndpointAddress
+	 *
+	 *   - (The union of all EndpointSubsets is the Set of all Endpoints)
+	 *   - Set of Endpoints is the cartesian product of :
+	 *     EndpointSubset::getAddresses and EndpointSubset::getPorts (each is a List)
+	 * 
+ */ + List generateState(List endpoints) { Stream references = endpoints.stream() .map(Endpoints::getSubsets) .filter(Objects::nonNull) diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsAndEndpointSlicesTests.java b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsAndEndpointSlicesTests.java index 4f4a55144f..3bd1b8c990 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsAndEndpointSlicesTests.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8EndpointsAndEndpointSlicesTests.java @@ -235,6 +235,16 @@ void afterEach() { */ abstract void testTwoNamespacesOutOfThree(); + /** + *
+	 *      - in the old API (plain Endpoints), tests that subsets are missing
+	 *        and we do not fail.
+	 *      - in the new API (EndpointSlices), tests that Endpoints are missing
+	 *        and we do not fail.
+	 * 
+ */ + abstract void testWithoutSubsetsOrEndpoints(); + KubernetesCatalogWatch createWatcherInAllNamespacesWithLabels(Map labels, Set namespaces, boolean endpointSlices) { @@ -309,6 +319,21 @@ void endpoints(String namespace, Map labels, String podName) { mockClient().endpoints().inNamespace(namespace).resource(endpoints).create(); } + Endpoints endpointsWithoutSubsets(String namespace, Map labels, String podName) { + + // though we set it to null here, the mock client when creating it + // will set it to an empty list. I will keep it like this, may be client changes + // in the future and we have the case still covered by a test + List endpointSubsets = null; + + Endpoints endpoints = new EndpointsBuilder() + .withMetadata(new ObjectMetaBuilder().withLabels(labels).withName("endpoints-" + podName).build()) + .withSubsets(endpointSubsets) + .build(); + mockClient().endpoints().inNamespace(namespace).resource(endpoints).create(); + return endpoints; + } + void service(String namespace, Map labels, String podName) { Service service = new ServiceBuilder() @@ -335,6 +360,23 @@ static void endpointSlice(String namespace, Map labels, String p } + static EndpointSlice endpointSliceWithoutEndpoints(String namespace, Map labels, String podName) { + + List endpoints = null; + + EndpointSlice slice = new EndpointSliceBuilder() + .withMetadata(new ObjectMetaBuilder().withNamespace(namespace) + .withName("slice-" + podName) + .withLabels(labels) + .build()) + .withEndpoints(endpoints) + .build(); + + mockClient().discovery().v1().endpointSlices().inNamespace(namespace).resource(slice).create(); + return slice; + + } + static void invokeAndAssert(KubernetesCatalogWatch watch, List state) { watch.catalogServicesWatch(); diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesCatalogWatchEndpointSlicesTests.java b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesCatalogWatchEndpointSlicesTests.java index 62b213fbb6..4f85c33f75 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesCatalogWatchEndpointSlicesTests.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesCatalogWatchEndpointSlicesTests.java @@ -20,12 +20,15 @@ import java.util.Map; import java.util.Set; +import io.fabric8.kubernetes.api.model.discovery.v1.EndpointSlice; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient; import org.junit.jupiter.api.Test; import org.springframework.cloud.kubernetes.commons.discovery.EndpointNameAndNamespace; +import static org.assertj.core.api.Assertions.assertThat; + /** * Some tests that use the fabric8 mock client, using EndpointSlices * @@ -178,6 +181,29 @@ void testTwoNamespacesOutOfThree() { new EndpointNameAndNamespace("podF", "namespaceB"))); } + @Test + @Override + void testWithoutSubsetsOrEndpoints() { + KubernetesCatalogWatch watch = createWatcherInSpecificNamespacesWithLabels(Set.of("namespaceA"), + Map.of("color", "blue"), ENDPOINT_SLICES); + + endpointSliceWithoutEndpoints("namespaceA", Map.of("color", "blue"), "podA"); + + // we do not fail here, even if Endpoints are not present. + invokeAndAssert(watch, List.of()); + } + + @Test + void generateStateEndpointsWithoutEndpoints() { + + Fabric8EndpointSliceV1CatalogWatch catalogWatch = new Fabric8EndpointSliceV1CatalogWatch(); + EndpointSlice sliceNoEndpoints = endpointSliceWithoutEndpoints("namespaceA", Map.of("color", "blue"), "podA"); + + + // even if Endpoints are missing, we do not fail + assertThat(catalogWatch.generateState(List.of(sliceNoEndpoints))).isEmpty(); + } + // work-around for : https://github.com/fabric8io/kubernetes-client/issues/4649 static KubernetesClient endpointSlicesMockClient() { return mockClient; diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesCatalogWatchEndpointsTests.java b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesCatalogWatchEndpointsTests.java index 07267b4031..40eb804e28 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesCatalogWatchEndpointsTests.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesCatalogWatchEndpointsTests.java @@ -20,6 +20,7 @@ import java.util.Map; import java.util.Set; +import io.fabric8.kubernetes.api.model.Endpoints; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient; import org.junit.jupiter.api.AfterEach; @@ -27,6 +28,8 @@ import org.springframework.cloud.kubernetes.commons.discovery.EndpointNameAndNamespace; +import static org.assertj.core.api.Assertions.assertThat; + /** * Tests for endpoints based catalog watch * @@ -235,6 +238,32 @@ void testTwoNamespacesOutOfThree() { new EndpointNameAndNamespace("podF", "namespaceB"))); } + @Test + @Override + void testWithoutSubsetsOrEndpoints() { + KubernetesCatalogWatch watch = createWatcherInSpecificNamespacesWithLabels(Set.of("namespaceA"), + Map.of("color", "blue"), ENDPOINT_SLICES); + + endpointsWithoutSubsets("namespaceA", Map.of("color", "blue"), "podA"); + // we do not fail here, even if Subsets are not present + invokeAndAssert(watch, List.of()); + } + + @Test + void generateStateEndpointsWithoutSubsets() { + + Fabric8EndpointsCatalogWatch catalogWatch = new Fabric8EndpointsCatalogWatch(); + + // though we set it to null here, the mock client when creating it + // will set it to an empty list. I will keep it like this, may be client changes + // in the future and we have the case still covered by a test + Endpoints endpoints = endpointsWithoutSubsets("c", Map.of(), "d"); + + // we do not fail, even if Subsets are not present + List result = catalogWatch.generateState(List.of(endpoints)); + assertThat(result).isEmpty(); + } + // work-around for : https://github.com/fabric8io/kubernetes-client/issues/4649 static KubernetesClient endpointsMockClient() { return mockClient;