From c046499065657fb99f3d66fd65f6905a343e41f5 Mon Sep 17 00:00:00 2001 From: erabii Date: Wed, 9 Aug 2023 02:30:09 +0300 Subject: [PATCH] More simplifications in fabric8 discovery implementation (#1403) --- .../discovery/DiscoveryClientUtils.java | 54 ++++---- ...viceInstance.java => ServiceMetadata.java} | 4 +- .../discovery/DiscoveryClientUtilsTests.java | 130 +++++++++++------- ...Fabric8KubernetesDiscoveryClientUtils.java | 12 +- .../discovery/KubernetesDiscoveryClient.java | 43 +++--- 5 files changed, 133 insertions(+), 110 deletions(-) rename spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/{ServiceMetadataForServiceInstance.java => ServiceMetadata.java} (82%) diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtils.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtils.java index e3bbed35..cbd45983 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtils.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtils.java @@ -59,41 +59,41 @@ public final class DiscoveryClientUtils { * - service type * */ - public static Map serviceMetadata(String serviceId, Map serviceLabels, - Map serviceAnnotations, Map portsData, - KubernetesDiscoveryProperties properties, String namespace, String serviceType) { - Map serviceMetadata = new HashMap<>(); + public static Map serviceInstanceMetadata(Map portsData, + ServiceMetadata serviceMetadata, KubernetesDiscoveryProperties properties) { + Map result = new HashMap<>(); KubernetesDiscoveryProperties.Metadata metadataProps = properties.metadata(); if (metadataProps.addLabels()) { - Map labelMetadata = keysWithPrefix(serviceLabels, metadataProps.labelsPrefix()); - LOG.debug(() -> "Adding labels metadata: " + labelMetadata + " for serviceId: " + serviceId); - serviceMetadata.putAll(labelMetadata); + Map labelMetadata = keysWithPrefix(serviceMetadata.labels(), metadataProps.labelsPrefix()); + LOG.debug(() -> "Adding labels metadata: " + labelMetadata + " for serviceId: " + serviceMetadata.name()); + result.putAll(labelMetadata); } if (metadataProps.addAnnotations()) { - Map annotationMetadata = keysWithPrefix(serviceAnnotations, + Map annotationMetadata = keysWithPrefix(serviceMetadata.annotations(), metadataProps.annotationsPrefix()); - LOG.debug(() -> "Adding annotations metadata: " + annotationMetadata + " for serviceId: " + serviceId); - serviceMetadata.putAll(annotationMetadata); + LOG.debug(() -> "Adding annotations metadata: " + annotationMetadata + " for serviceId: " + + serviceMetadata.name()); + result.putAll(annotationMetadata); } if (metadataProps.addPorts()) { Map portMetadata = keysWithPrefix(portsData, properties.metadata().portsPrefix()); if (!portMetadata.isEmpty()) { - LOG.debug(() -> "Adding port metadata: " + portMetadata + " for serviceId : " + serviceId); + LOG.debug(() -> "Adding port metadata: " + portMetadata + " for serviceId : " + serviceMetadata.name()); } - serviceMetadata.putAll(portMetadata); + result.putAll(portMetadata); } - serviceMetadata.put(NAMESPACE_METADATA_KEY, namespace); - serviceMetadata.put(SERVICE_TYPE, serviceType); - return serviceMetadata; + result.put(NAMESPACE_METADATA_KEY, serviceMetadata.namespace()); + result.put(SERVICE_TYPE, serviceMetadata.type()); + return result; } public static ServicePortNameAndNumber endpointsPort(LinkedHashMap endpointsPorts, - String serviceId, KubernetesDiscoveryProperties properties, Map serviceLabels) { + ServiceMetadata serviceMetadata, KubernetesDiscoveryProperties properties) { if (endpointsPorts.size() == 0) { - LOG.debug(() -> "no ports found for service : " + serviceId + ", will return zero"); + LOG.debug(() -> "no ports found for service : " + serviceMetadata.name() + ", will return zero"); return new ServicePortNameAndNumber(0, "http"); } @@ -106,7 +106,7 @@ public final class DiscoveryClientUtils { else { Optional portData; - String primaryPortName = primaryPortName(properties, serviceLabels, serviceId); + String primaryPortName = primaryPortName(properties, serviceMetadata.labels(), serviceMetadata.name()); Map existingPorts = endpointsPorts.entrySet().stream() .filter(entry -> StringUtils.hasText(entry.getKey())) @@ -136,11 +136,9 @@ public final class DiscoveryClientUtils { } public static ServiceInstance serviceInstance(@Nullable ServicePortSecureResolver servicePortSecureResolver, - ServiceMetadataForServiceInstance serviceMetadataForServiceInstance, - Supplier instanceIdAndHost, + ServiceMetadata serviceMetadata, Supplier instanceIdAndHost, Function podLabelsAndMetadata, ServicePortNameAndNumber portData, - String serviceId, Map serviceMetadata, String namespace, - KubernetesDiscoveryProperties properties) { + Map serviceInstanceMetadata, KubernetesDiscoveryProperties properties) { InstanceIdHostPodName data = instanceIdAndHost.get(); @@ -150,15 +148,15 @@ public final class DiscoveryClientUtils { } else { secured = servicePortSecureResolver.resolve(new ServicePortSecureResolver.Input(portData, - serviceMetadataForServiceInstance.name(), serviceMetadataForServiceInstance.labels(), - serviceMetadataForServiceInstance.annotations())); + serviceMetadata.name(), serviceMetadata.labels(), serviceMetadata.annotations())); } - Map> podMetadata = podMetadata(data.podName(), serviceMetadata, properties, - podLabelsAndMetadata); + Map> podMetadata = podMetadata(data.podName(), serviceInstanceMetadata, + properties, podLabelsAndMetadata); - return new DefaultKubernetesServiceInstance(data.instanceId(), serviceId, data.host(), portData.portNumber(), - serviceMetadata, secured, namespace, null, podMetadata); + return new DefaultKubernetesServiceInstance(data.instanceId(), serviceMetadata.name(), + data.host(), portData.portNumber(), serviceInstanceMetadata, secured, + serviceMetadata.namespace(), null, podMetadata); } /** diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/ServiceMetadataForServiceInstance.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/ServiceMetadata.java similarity index 82% rename from spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/ServiceMetadataForServiceInstance.java rename to spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/ServiceMetadata.java index 600db6c8..e6e4e724 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/ServiceMetadataForServiceInstance.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/ServiceMetadata.java @@ -19,11 +19,11 @@ package org.springframework.cloud.kubernetes.commons.discovery; import java.util.Map; /** - * Holds service name, labels and annotations. + * Holds service name, namespace, spec.type, labels and annotations. * * @author wind57 * */ -public record ServiceMetadataForServiceInstance(String name, Map labels, +public record ServiceMetadata(String name, String namespace, String type, Map labels, Map annotations) { } diff --git a/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtilsTests.java b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtilsTests.java index c58d450a..c2e37f2c 100644 --- a/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtilsTests.java +++ b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/discovery/DiscoveryClientUtilsTests.java @@ -30,6 +30,11 @@ import org.springframework.boot.test.system.CapturedOutput; import org.springframework.boot.test.system.OutputCaptureExtension; import org.springframework.cloud.client.ServiceInstance; +import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.endpointsPort; +import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.podMetadata; +import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.primaryPortName; +import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.serviceInstance; +import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.serviceInstanceMetadata; import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.PRIMARY_PORT_NAME_LABEL_KEY; /** @@ -62,9 +67,10 @@ class DiscoveryClientUtilsTests { labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), "", metadata, 0, false, false); + ServiceMetadata serviceMetadata = new ServiceMetadata("my-service", namespace, "ClusterIP", serviceLabels, + serviceAnnotations); - Map result = DiscoveryClientUtils.serviceMetadata("my-service", serviceLabels, - serviceAnnotations, portsData, properties, namespace, "ClusterIP"); + Map result = serviceInstanceMetadata(portsData, serviceMetadata, properties); Assertions.assertEquals(result.size(), 2); Assertions.assertEquals(result, Map.of("k8s_namespace", "default", "type", "ClusterIP")); } @@ -93,9 +99,10 @@ class DiscoveryClientUtilsTests { labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), "", metadata, 0, false, false); + ServiceMetadata serviceMetadata = new ServiceMetadata("my-service", namespace, "ClusterIP", serviceLabels, + serviceAnnotations); - Map result = DiscoveryClientUtils.serviceMetadata("my-service", serviceLabels, - serviceAnnotations, portsData, properties, namespace, "ClusterIP"); + Map result = serviceInstanceMetadata(portsData, serviceMetadata, properties); Assertions.assertEquals(result.size(), 2); Assertions.assertEquals(result, Map.of("k8s_namespace", "default", "type", "ClusterIP")); } @@ -124,9 +131,10 @@ class DiscoveryClientUtilsTests { labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), "", metadata, 0, false, false); + ServiceMetadata serviceMetadata = new ServiceMetadata("my-service", namespace, "ClusterIP", serviceLabels, + serviceAnnotations); - Map result = DiscoveryClientUtils.serviceMetadata("my-service", serviceLabels, - serviceAnnotations, portsData, properties, namespace, "ClusterIP"); + Map result = serviceInstanceMetadata(portsData, serviceMetadata, properties); Assertions.assertEquals(result.size(), 3); Assertions.assertEquals(result, Map.of("a", "b", "k8s_namespace", "default", "type", "ClusterIP")); @@ -159,9 +167,10 @@ class DiscoveryClientUtilsTests { labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), "", metadata, 0, false, false); + ServiceMetadata serviceMetadata = new ServiceMetadata("my-service", namespace, "ClusterIP", serviceLabels, + serviceAnnotations); - Map result = DiscoveryClientUtils.serviceMetadata("my-service", serviceLabels, - serviceAnnotations, portsData, properties, namespace, "ClusterIP"); + Map result = serviceInstanceMetadata(portsData, serviceMetadata, properties); Assertions.assertEquals(result.size(), 4); Assertions.assertEquals(result, @@ -196,9 +205,10 @@ class DiscoveryClientUtilsTests { labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), "", metadata, 0, false, false); + ServiceMetadata serviceMetadata = new ServiceMetadata("my-service", namespace, "ClusterIP", serviceLabels, + serviceAnnotations); - Map result = DiscoveryClientUtils.serviceMetadata("my-service", serviceLabels, - serviceAnnotations, portsData, properties, namespace, "ClusterIP"); + Map result = serviceInstanceMetadata(portsData, serviceMetadata, properties); Assertions.assertEquals(result.size(), 3); Assertions.assertEquals(result, Map.of("aa", "bb", "k8s_namespace", "default", "type", "ClusterIP")); @@ -230,9 +240,10 @@ class DiscoveryClientUtilsTests { labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), "", metadata, 0, false, false); + ServiceMetadata serviceMetadata = new ServiceMetadata("my-service", namespace, "ClusterIP", serviceLabels, + serviceAnnotations); - Map result = DiscoveryClientUtils.serviceMetadata("my-service", serviceLabels, - serviceAnnotations, portsData, properties, namespace, "ClusterIP"); + Map result = serviceInstanceMetadata(portsData, serviceMetadata, properties); Assertions.assertEquals(result.size(), 4); Assertions.assertEquals(result, @@ -267,9 +278,10 @@ class DiscoveryClientUtilsTests { labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), "", metadata, 0, false, false); + ServiceMetadata serviceMetadata = new ServiceMetadata("my-service", namespace, "ClusterIP", serviceLabels, + serviceAnnotations); - Map result = DiscoveryClientUtils.serviceMetadata("my-service", serviceLabels, - serviceAnnotations, portsData, properties, namespace, "ClusterIP"); + Map result = serviceInstanceMetadata(portsData, serviceMetadata, properties); Assertions.assertEquals(result.size(), 6); Assertions.assertEquals(result, Map.of("annotation-aa", "bb", "annotation-cc", "dd", "label-a", "b", "label-c", @@ -308,9 +320,10 @@ class DiscoveryClientUtilsTests { labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), "", metadata, 0, false, false); + ServiceMetadata serviceMetadata = new ServiceMetadata("my-service", namespace, "ClusterIP", serviceLabels, + serviceAnnotations); - Map result = DiscoveryClientUtils.serviceMetadata("my-service", serviceLabels, - serviceAnnotations, portsData, properties, namespace, "ClusterIP"); + Map result = serviceInstanceMetadata(portsData, serviceMetadata, properties); Assertions.assertEquals(result.size(), 3); Assertions.assertEquals(result, Map.of("https", "8080", "k8s_namespace", "default", "type", "ClusterIP")); @@ -341,9 +354,10 @@ class DiscoveryClientUtilsTests { labelsPrefix, addAnnotations, annotationsPrefix, addPorts, portsPrefix); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), "", metadata, 0, false, false); + ServiceMetadata serviceMetadata = new ServiceMetadata("my-service", namespace, "ClusterIP", serviceLabels, + serviceAnnotations); - Map result = DiscoveryClientUtils.serviceMetadata("my-service", serviceLabels, - serviceAnnotations, portsData, properties, namespace, "ClusterIP"); + Map result = serviceInstanceMetadata(portsData, serviceMetadata, properties); Assertions.assertEquals(result.size(), 4); Assertions.assertEquals(result, @@ -366,7 +380,7 @@ class DiscoveryClientUtilsTests { Map serviceLabels = Map.of(); - String result = DiscoveryClientUtils.primaryPortName(properties, serviceLabels, "abc"); + String result = primaryPortName(properties, serviceLabels, "abc"); Assertions.assertNull(result); Assertions.assertTrue(output.getOut().contains( "did not find a primary-port-name in neither properties nor service labels for service with ID : abc")); @@ -388,7 +402,7 @@ class DiscoveryClientUtilsTests { Map serviceLabels = Map.of(); - String result = DiscoveryClientUtils.primaryPortName(properties, serviceLabels, "abc"); + String result = primaryPortName(properties, serviceLabels, "abc"); Assertions.assertNotNull(result); Assertions.assertEquals(result, primaryPortName); Assertions.assertTrue(output.getOut().contains("will use primaryPortName : https for service with ID = abc")); @@ -407,7 +421,7 @@ class DiscoveryClientUtilsTests { Map serviceLabels = Map.of(PRIMARY_PORT_NAME_LABEL_KEY, "https"); KubernetesDiscoveryProperties properties = KubernetesDiscoveryProperties.DEFAULT; - String result = DiscoveryClientUtils.primaryPortName(properties, serviceLabels, "abc"); + String result = primaryPortName(properties, serviceLabels, "abc"); Assertions.assertNotNull(result); Assertions.assertEquals(result, "https"); Assertions.assertTrue(output.getOut().contains("will use primaryPortName : https for service with ID = abc")); @@ -428,7 +442,7 @@ class DiscoveryClientUtilsTests { KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L, true, "", Set.of(), Map.of(), primaryPortName, null, 0, false); - String result = DiscoveryClientUtils.primaryPortName(properties, serviceLabels, "abc"); + String result = primaryPortName(properties, serviceLabels, "abc"); Assertions.assertNotNull(result); Assertions.assertEquals(result, "http"); Assertions.assertTrue(output.getOut().contains("will use primaryPortName : http for service with ID = abc")); @@ -446,9 +460,10 @@ class DiscoveryClientUtilsTests { LinkedHashMap endpointsPorts = new LinkedHashMap<>(); Map serviceLabels = Map.of(); + ServiceMetadata serviceMetadata = new ServiceMetadata(serviceId, "default", "ClusterIP", serviceLabels, + Map.of()); - ServicePortNameAndNumber portData = DiscoveryClientUtils.endpointsPort(endpointsPorts, serviceId, properties, - serviceLabels); + ServicePortNameAndNumber portData = endpointsPort(endpointsPorts, serviceMetadata, properties); Assertions.assertEquals(portData.portNumber(), 0); Assertions.assertEquals(portData.portName(), "http"); Assertions.assertTrue(output.getOut().contains("no ports found for service : spring-k8s, will return zero")); @@ -467,9 +482,10 @@ class DiscoveryClientUtilsTests { LinkedHashMap endpointsPorts = new LinkedHashMap<>(); endpointsPorts.put("http", 8080); Map serviceLabels = Map.of(); + ServiceMetadata serviceMetadata = new ServiceMetadata(serviceId, "default", "ClusterIP", serviceLabels, + Map.of()); - ServicePortNameAndNumber portData = DiscoveryClientUtils.endpointsPort(endpointsPorts, serviceId, properties, - serviceLabels); + ServicePortNameAndNumber portData = endpointsPort(endpointsPorts, serviceMetadata, properties); Assertions.assertEquals(portData.portNumber(), 8080); Assertions.assertEquals(portData.portName(), "http"); Assertions.assertTrue(output.getOut().contains("endpoint ports has a single entry, using port : 8080")); @@ -490,8 +506,10 @@ class DiscoveryClientUtilsTests { endpointsPorts.put("not-http-or-https", 8081); Map serviceLabels = Map.of(); - ServicePortNameAndNumber portData = DiscoveryClientUtils.endpointsPort(endpointsPorts, serviceId, properties, - serviceLabels); + ServiceMetadata serviceMetadata = new ServiceMetadata(serviceId, "default", "ClusterIP", serviceLabels, + Map.of()); + + ServicePortNameAndNumber portData = endpointsPort(endpointsPorts, serviceMetadata, properties); Assertions.assertEquals(portData.portNumber(), 8080); Assertions.assertNull(portData.portName()); Assertions.assertTrue(output.getOut().contains( @@ -524,8 +542,10 @@ class DiscoveryClientUtilsTests { endpointsPorts.put("two", 8081); Map serviceLabels = Map.of(); - ServicePortNameAndNumber portData = DiscoveryClientUtils.endpointsPort(endpointsPorts, serviceId, properties, - serviceLabels); + ServiceMetadata serviceMetadata = new ServiceMetadata(serviceId, "default", "ClusterIP", serviceLabels, + Map.of()); + + ServicePortNameAndNumber portData = endpointsPort(endpointsPorts, serviceMetadata, properties); Assertions.assertEquals(portData.portNumber(), 8080); Assertions.assertEquals(portData.portName(), "one"); Assertions.assertTrue( @@ -558,8 +578,10 @@ class DiscoveryClientUtilsTests { endpointsPorts.put("two", 8081); Map serviceLabels = Map.of(); - ServicePortNameAndNumber portData = DiscoveryClientUtils.endpointsPort(endpointsPorts, serviceId, properties, - serviceLabels); + ServiceMetadata serviceMetadata = new ServiceMetadata(serviceId, "default", "ClusterIP", serviceLabels, + Map.of()); + + ServicePortNameAndNumber portData = endpointsPort(endpointsPorts, serviceMetadata, properties); Assertions.assertEquals(portData.portNumber(), 8081); Assertions.assertEquals(portData.portName(), "two"); Assertions.assertTrue( @@ -587,8 +609,10 @@ class DiscoveryClientUtilsTests { endpointsPorts.put("https", 8082); Map serviceLabels = Map.of(); - ServicePortNameAndNumber portData = DiscoveryClientUtils.endpointsPort(endpointsPorts, serviceId, properties, - serviceLabels); + ServiceMetadata serviceMetadata = new ServiceMetadata(serviceId, "default", "ClusterIP", serviceLabels, + Map.of()); + + ServicePortNameAndNumber portData = endpointsPort(endpointsPorts, serviceMetadata, properties); Assertions.assertEquals(portData.portNumber(), 8082); Assertions.assertEquals(portData.portName(), "https"); Assertions.assertTrue( @@ -617,8 +641,10 @@ class DiscoveryClientUtilsTests { endpointsPorts.put("http", 8082); Map serviceLabels = Map.of(); - ServicePortNameAndNumber portData = DiscoveryClientUtils.endpointsPort(endpointsPorts, serviceId, properties, - serviceLabels); + ServiceMetadata serviceMetadata = new ServiceMetadata(serviceId, "default", "ClusterIP", serviceLabels, + Map.of()); + + ServicePortNameAndNumber portData = endpointsPort(endpointsPorts, serviceMetadata, properties); Assertions.assertEquals(portData.portNumber(), 8082); Assertions.assertEquals(portData.portName(), "http"); Assertions.assertTrue( @@ -635,13 +661,13 @@ class DiscoveryClientUtilsTests { ServicePortSecureResolver resolver = new ServicePortSecureResolver(properties); ServicePortNameAndNumber portData = new ServicePortNameAndNumber(8080, "http"); - ServiceMetadataForServiceInstance forServiceInstance = new ServiceMetadataForServiceInstance("my-service", - Map.of(), Map.of()); + ServiceMetadata forServiceInstance = new ServiceMetadata("my-service", "k8s", "ClusterIP", Map.of(), + Map.of()); InstanceIdHostPodName instanceIdHostPodName = new InstanceIdHostPodName("123", "127.0.0.1", null); Map serviceMetadata = Map.of("a", "b"); - ServiceInstance serviceInstance = DiscoveryClientUtils.serviceInstance(resolver, forServiceInstance, - () -> instanceIdHostPodName, null, portData, "my-service", serviceMetadata, "k8s", properties); + ServiceInstance serviceInstance = serviceInstance(resolver, forServiceInstance, () -> instanceIdHostPodName, + null, portData, serviceMetadata, properties); Assertions.assertTrue(serviceInstance instanceof DefaultKubernetesServiceInstance); DefaultKubernetesServiceInstance defaultInstance = (DefaultKubernetesServiceInstance) serviceInstance; Assertions.assertEquals(defaultInstance.getInstanceId(), "123"); @@ -663,13 +689,13 @@ class DiscoveryClientUtilsTests { false, "", Set.of(), Map.of(), "", KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, false, false); ServicePortNameAndNumber portData = new ServicePortNameAndNumber(-1, "http"); - ServiceMetadataForServiceInstance forServiceInstance = new ServiceMetadataForServiceInstance("my-service", - Map.of(), Map.of()); + ServiceMetadata forServiceInstance = new ServiceMetadata("my-service", "k8s", "ClusterIP", Map.of(), + Map.of()); InstanceIdHostPodName instanceIdHostPodName = new InstanceIdHostPodName("123", "spring.io", null); Map serviceMetadata = Map.of("a", "b"); - ServiceInstance serviceInstance = DiscoveryClientUtils.serviceInstance(null, forServiceInstance, - () -> instanceIdHostPodName, null, portData, "my-service", serviceMetadata, "k8s", properties); + ServiceInstance serviceInstance = serviceInstance(null, forServiceInstance, () -> instanceIdHostPodName, null, + portData, serviceMetadata, properties); Assertions.assertTrue(serviceInstance instanceof DefaultKubernetesServiceInstance); DefaultKubernetesServiceInstance defaultInstance = (DefaultKubernetesServiceInstance) serviceInstance; @@ -700,7 +726,7 @@ class DiscoveryClientUtilsTests { false, "", Set.of(), Map.of(), "", metadata, 0, false, false); Function podLabelsAndMetadata = x -> null; - Map> result = DiscoveryClientUtils.podMetadata(podName, serviceMetadata, properties, + Map> result = podMetadata(podName, serviceMetadata, properties, podLabelsAndMetadata); Assertions.assertTrue(result.isEmpty()); } @@ -721,7 +747,7 @@ class DiscoveryClientUtilsTests { false, "", Set.of(), Map.of(), "", metadata, 0, false, false); Function podLabelsAndMetadata = x -> null; - Map> result = DiscoveryClientUtils.podMetadata(podName, serviceMetadata, properties, + Map> result = podMetadata(podName, serviceMetadata, properties, podLabelsAndMetadata); Assertions.assertTrue(result.isEmpty()); } @@ -747,7 +773,7 @@ class DiscoveryClientUtilsTests { false, "", Set.of(), Map.of(), "", metadata, 0, false, false); Function podLabelsAndMetadata = x -> null; - Map> result = DiscoveryClientUtils.podMetadata(podName, serviceMetadata, properties, + Map> result = podMetadata(podName, serviceMetadata, properties, podLabelsAndMetadata); Assertions.assertTrue(result.isEmpty()); } @@ -776,7 +802,7 @@ class DiscoveryClientUtilsTests { PodLabelsAndAnnotations both = new PodLabelsAndAnnotations(Map.of(), Map.of("c", "d")); Function podLabelsAndMetadata = x -> both; - Map> result = DiscoveryClientUtils.podMetadata(podName, serviceMetadata, properties, + Map> result = podMetadata(podName, serviceMetadata, properties, podLabelsAndMetadata); Assertions.assertTrue(result.isEmpty()); @@ -807,7 +833,7 @@ class DiscoveryClientUtilsTests { PodLabelsAndAnnotations both = new PodLabelsAndAnnotations(Map.of("a", "b"), Map.of("c", "d")); Function podLabelsAndMetadata = x -> both; - Map> result = DiscoveryClientUtils.podMetadata(podName, serviceMetadata, properties, + Map> result = podMetadata(podName, serviceMetadata, properties, podLabelsAndMetadata); Assertions.assertEquals(result.size(), 1); Assertions.assertEquals(result.get("labels"), Map.of("a", "b")); @@ -839,7 +865,7 @@ class DiscoveryClientUtilsTests { PodLabelsAndAnnotations both = new PodLabelsAndAnnotations(Map.of("a", "b"), Map.of()); Function podLabelsAndMetadata = x -> both; - Map> result = DiscoveryClientUtils.podMetadata(podName, serviceMetadata, properties, + Map> result = podMetadata(podName, serviceMetadata, properties, podLabelsAndMetadata); Assertions.assertTrue(result.isEmpty()); @@ -870,7 +896,7 @@ class DiscoveryClientUtilsTests { PodLabelsAndAnnotations both = new PodLabelsAndAnnotations(Map.of("a", "b"), Map.of("c", "d")); Function podLabelsAndMetadata = x -> both; - Map> result = DiscoveryClientUtils.podMetadata(podName, serviceMetadata, properties, + Map> result = podMetadata(podName, serviceMetadata, properties, podLabelsAndMetadata); Assertions.assertEquals(result.size(), 1); Assertions.assertEquals(result.get("annotations"), Map.of("c", "d")); @@ -902,7 +928,7 @@ class DiscoveryClientUtilsTests { PodLabelsAndAnnotations both = new PodLabelsAndAnnotations(Map.of("a", "b"), Map.of("c", "d")); Function podLabelsAndMetadata = x -> both; - Map> result = DiscoveryClientUtils.podMetadata(podName, serviceMetadata, properties, + Map> result = podMetadata(podName, serviceMetadata, properties, podLabelsAndMetadata); Assertions.assertEquals(result.size(), 2); Assertions.assertEquals(result.get("annotations"), Map.of("c", "d")); diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtils.java b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtils.java index 0613591c..f9c13b9a 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtils.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/Fabric8KubernetesDiscoveryClientUtils.java @@ -30,8 +30,10 @@ import io.fabric8.kubernetes.api.model.EndpointPort; import io.fabric8.kubernetes.api.model.EndpointSubset; import io.fabric8.kubernetes.api.model.Endpoints; import io.fabric8.kubernetes.api.model.EndpointsList; +import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServiceList; +import io.fabric8.kubernetes.api.model.ServiceSpec; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.FilterNested; import io.fabric8.kubernetes.client.dsl.FilterWatchListDeletable; @@ -42,7 +44,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider; import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties; -import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadataForServiceInstance; +import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadata; import org.springframework.cloud.kubernetes.fabric8.Fabric8Utils; import org.springframework.core.log.LogAccessor; import org.springframework.util.CollectionUtils; @@ -215,9 +217,11 @@ final class Fabric8KubernetesDiscoveryClientUtils { return result; } - static ServiceMetadataForServiceInstance forServiceInstance(Service service) { - return new ServiceMetadataForServiceInstance(service.getMetadata().getName(), service.getMetadata().getLabels(), - service.getMetadata().getAnnotations()); + static ServiceMetadata serviceMetadata(Service service) { + ObjectMeta metadata = service.getMetadata(); + ServiceSpec serviceSpec = service.getSpec(); + return new ServiceMetadata(metadata.getName(), metadata.getNamespace(), serviceSpec.getType(), + metadata.getLabels(), metadata.getAnnotations()); } /** diff --git a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java index 7b7284dd..d2fe2642 100644 --- a/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java +++ b/spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java @@ -17,6 +17,7 @@ package org.springframework.cloud.kubernetes.fabric8.discovery; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -25,7 +26,6 @@ import java.util.function.Predicate; import io.fabric8.kubernetes.api.model.EndpointAddress; import io.fabric8.kubernetes.api.model.EndpointSubset; import io.fabric8.kubernetes.api.model.Endpoints; -import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.client.KubernetesClient; import org.apache.commons.logging.LogFactory; @@ -33,24 +33,25 @@ import org.apache.commons.logging.LogFactory; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider; -import org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils; import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties; -import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadataForServiceInstance; +import org.springframework.cloud.kubernetes.commons.discovery.ServiceMetadata; import org.springframework.cloud.kubernetes.commons.discovery.ServicePortNameAndNumber; import org.springframework.cloud.kubernetes.commons.discovery.ServicePortSecureResolver; import org.springframework.context.EnvironmentAware; import org.springframework.core.env.Environment; import org.springframework.core.log.LogAccessor; +import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.endpointsPort; import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.serviceInstance; +import static org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils.serviceInstanceMetadata; import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.EXTERNAL_NAME; import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8InstanceIdHostPodNameSupplier.externalName; import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8InstanceIdHostPodNameSupplier.nonExternalName; import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.addresses; import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.endpointSubsetPortsData; import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.endpoints; -import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.forServiceInstance; import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.portsData; +import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.serviceMetadata; import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8KubernetesDiscoveryClientUtils.services; import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8PodLabelsAndAnnotationsSupplier.externalName; import static org.springframework.cloud.kubernetes.fabric8.discovery.Fabric8PodLabelsAndAnnotationsSupplier.nonExternalName; @@ -120,7 +121,7 @@ public class KubernetesDiscoveryClient implements DiscoveryClient, EnvironmentAw List instances = new ArrayList<>(); for (EndpointSubsetNS es : subsetsNS) { // subsetsNS are only those that matched the serviceId - instances.addAll(getNamespaceServiceInstances(es, serviceId)); + instances.addAll(serviceInstances(es, serviceId)); } if (properties.includeExternalNameServices()) { @@ -129,19 +130,15 @@ public class KubernetesDiscoveryClient implements DiscoveryClient, EnvironmentAw s -> s.getSpec().getType().equals(EXTERNAL_NAME), Map.of("metadata.name", serviceId), "fabric8-discovery"); for (Service service : services) { - ObjectMeta serviceMetadata = service.getMetadata(); - Map result = DiscoveryClientUtils.serviceMetadata(serviceId, - serviceMetadata.getLabels(), serviceMetadata.getAnnotations(), Map.of(), properties, - serviceMetadata.getNamespace(), service.getSpec().getType()); + ServiceMetadata serviceMetadata = serviceMetadata(service); + Map serviceInstanceMetadata = serviceInstanceMetadata(Map.of(), serviceMetadata, + properties); - ServiceMetadataForServiceInstance forServiceInstance = forServiceInstance(service); Fabric8InstanceIdHostPodNameSupplier supplierOne = externalName(service); Fabric8PodLabelsAndAnnotationsSupplier supplierTwo = externalName(); - ServiceInstance externalNameServiceInstance = serviceInstance(null, forServiceInstance, supplierOne, - supplierTwo, new ServicePortNameAndNumber(-1, null), serviceId, result, - service.getMetadata().getNamespace(), properties); - + ServiceInstance externalNameServiceInstance = serviceInstance(null, serviceMetadata, supplierOne, + supplierTwo, new ServicePortNameAndNumber(-1, null), serviceInstanceMetadata, properties); instances.add(externalNameServiceInstance); } } @@ -153,7 +150,7 @@ public class KubernetesDiscoveryClient implements DiscoveryClient, EnvironmentAw return endpoints(properties, client, namespaceProvider, "fabric8-discovery", serviceId, adapter.filter()); } - private List getNamespaceServiceInstances(EndpointSubsetNS es, String serviceId) { + private List serviceInstances(EndpointSubsetNS es, String serviceId) { List subsets = es.endpointSubset(); if (subsets.isEmpty()) { @@ -165,26 +162,24 @@ public class KubernetesDiscoveryClient implements DiscoveryClient, EnvironmentAw List instances = new ArrayList<>(); Service service = client.services().inNamespace(namespace).withName(serviceId).get(); - ObjectMeta serviceMetadata = service.getMetadata(); + ServiceMetadata serviceMetadata = serviceMetadata(service); + Map portsData = portsData(subsets); - Map result = DiscoveryClientUtils.serviceMetadata(serviceId, serviceMetadata.getLabels(), - serviceMetadata.getAnnotations(), portsData(subsets), properties, serviceMetadata.getNamespace(), - service.getSpec().getType()); + Map serviceInstanceMetadata = serviceInstanceMetadata(portsData, serviceMetadata, properties); for (EndpointSubset endpointSubset : subsets) { - ServicePortNameAndNumber portData = DiscoveryClientUtils.endpointsPort( - endpointSubsetPortsData(endpointSubset), serviceId, properties, service.getMetadata().getLabels()); + LinkedHashMap endpointsPortData = endpointSubsetPortsData(endpointSubset); + ServicePortNameAndNumber portData = endpointsPort(endpointsPortData, serviceMetadata, properties); List addresses = addresses(endpointSubset, properties); for (EndpointAddress endpointAddress : addresses) { - ServiceMetadataForServiceInstance forServiceInstance = forServiceInstance(service); Fabric8InstanceIdHostPodNameSupplier supplierOne = nonExternalName(endpointAddress, service); Fabric8PodLabelsAndAnnotationsSupplier supplierTwo = nonExternalName(client, namespace); - ServiceInstance serviceInstance = serviceInstance(servicePortSecureResolver, forServiceInstance, - supplierOne, supplierTwo, portData, serviceId, result, namespace, properties); + ServiceInstance serviceInstance = serviceInstance(servicePortSecureResolver, serviceMetadata, + supplierOne, supplierTwo, portData, serviceInstanceMetadata, properties); instances.add(serviceInstance); } }