Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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

return generateState(endpointSlices);

}

List<EndpointNameAndNamespace> generateState(List<V1EndpointSlice> endpointSlices) {
Stream<V1ObjectReference> references = endpointSlices.stream()
.map(V1EndpointSlice::getEndpoints)
.filter(Objects::nonNull)
.flatMap(List::stream)
.map(V1Endpoint::getTargetRef);

return KubernetesCatalogWatchContext.state(references);

}

private List<V1EndpointSlice> endpointSlices(DiscoveryV1Api api, Map<String, String> labels) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,21 @@ else if (!context.properties().namespaces().isEmpty()) {
endpoints = namespacedEndpoints(coreV1Api, namespace, context.properties().serviceLabels());
}

/**
* <pre>
* - 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)
* </pre>
*/
return generateState(endpoints);

}

/**
* <pre>
* - 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)
* </pre>
*/
List<EndpointNameAndNamespace> generateState(List<V1Endpoints> endpoints) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract as a separate package-private method to be able to test it

Stream<V1ObjectReference> references = endpoints.stream()
.map(V1Endpoints::getSubsets)
.filter(Objects::nonNull)
Expand All @@ -92,7 +97,6 @@ else if (!context.properties().namespaces().isEmpty()) {
.map(V1EndpointAddress::getTargetRef);

return KubernetesCatalogWatchContext.state(references);

}

private List<V1Endpoints> endpoints(CoreV1Api client, Map<String, String> labels) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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<String, String> 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<V1EndpointSlice> endpointSlicesNoEndpoints = endpointSlicesNoEndpoints().getItems();

// even if Endpoints are missing, we do not fail
assertThat(catalogWatch.generateState(endpointSlicesNoEndpoints)).isEmpty();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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<String, String> 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<V1Endpoints> endpoints = endpointsNoSubsets().getItems();

// we do not fail, even if Subsets are not present
List<EndpointNameAndNamespace> result = catalogWatch.generateState(endpoints);
assertThat(result).isEmpty();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -104,6 +105,8 @@ abstract class KubernetesEndpointsAndEndpointSlicesTests {
*/
abstract void testInOneNamespaceWithDoubleLabel();

abstract void testWithoutSubsetsOrEndpoints();

KubernetesCatalogWatch createWatcherInAllNamespacesWithLabels(Map<String, String> labels, Set<String> namespaces,
CoreV1Api coreV1Api, ApiClient apiClient, boolean endpointSlices) {

Expand Down Expand Up @@ -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()
Expand All @@ -182,6 +191,13 @@ V1EndpointSliceList endpointSlices(String name, String namespace) {
.build();
}

V1EndpointSliceList endpointSlicesNoEndpoints() {
List<V1Endpoint> endpoints = null;
return new V1EndpointSliceListBuilder().withKind("V1EndpointSliceList")
.addToItems(new V1EndpointSliceBuilder().withEndpoints(endpoints).build())
.build();
}

static void invokeAndAssert(KubernetesCatalogWatch watch, List<EndpointNameAndNamespace> state) {
watch.catalogServicesWatch();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<EndpointNameAndNamespace> generateState(List<EndpointSlice> endpointSlices) {
Stream<ObjectReference> references = endpointSlices.stream()
.map(EndpointSlice::getEndpoints)
.filter(Objects::nonNull)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the actual fix

.flatMap(List::stream)
.map(Endpoint::getTargetRef);

return Fabric8CatalogWatchContext.state(references);

}

private List<EndpointSlice> endpointSlices(Fabric8CatalogWatchContext context, String namespace,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,23 @@ public List<EndpointNameAndNamespace> apply(Fabric8CatalogWatchContext context)
List<Endpoints> endpoints = endpoints(context.properties(), context.kubernetesClient(),
context.namespaceProvider(), "catalog-watcher", null, ALWAYS_TRUE);

/**
* <pre>
* - 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)
* </pre>
*/
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.
*
* <pre>
* - 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)
* </pre>
*/
List<EndpointNameAndNamespace> generateState(List<Endpoints> endpoints) {
Stream<ObjectReference> references = endpoints.stream()
.map(Endpoints::getSubsets)
.filter(Objects::nonNull)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,16 @@ void afterEach() {
*/
abstract void testTwoNamespacesOutOfThree();

/**
* <pre>
* - 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.
* </pre>
*/
abstract void testWithoutSubsetsOrEndpoints();

KubernetesCatalogWatch createWatcherInAllNamespacesWithLabels(Map<String, String> labels, Set<String> namespaces,
boolean endpointSlices) {

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

Endpoints endpointsWithoutSubsets(String namespace, Map<String, String> 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<EndpointSubset> 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<String, String> labels, String podName) {

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

}

static EndpointSlice endpointSliceWithoutEndpoints(String namespace, Map<String, String> labels, String podName) {

List<Endpoint> 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<EndpointNameAndNamespace> state) {
watch.catalogServicesWatch();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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;
Expand Down
Loading
Loading