diff --git a/spring-cloud-kubernetes-client-loadbalancer/src/main/java/org/springframework/cloud/kubernetes/client/loadbalancer/KubernetesClientServiceInstanceMapper.java b/spring-cloud-kubernetes-client-loadbalancer/src/main/java/org/springframework/cloud/kubernetes/client/loadbalancer/KubernetesClientServiceInstanceMapper.java index 83293741d..6c30cde8c 100644 --- a/spring-cloud-kubernetes-client-loadbalancer/src/main/java/org/springframework/cloud/kubernetes/client/loadbalancer/KubernetesClientServiceInstanceMapper.java +++ b/spring-cloud-kubernetes-client-loadbalancer/src/main/java/org/springframework/cloud/kubernetes/client/loadbalancer/KubernetesClientServiceInstanceMapper.java @@ -18,12 +18,14 @@ import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import io.kubernetes.client.openapi.models.V1ObjectMeta; import io.kubernetes.client.openapi.models.V1Service; import io.kubernetes.client.openapi.models.V1ServicePort; import io.kubernetes.client.openapi.models.V1ServiceSpec; +import org.apache.commons.logging.LogFactory; import org.springframework.cloud.kubernetes.commons.discovery.DefaultKubernetesServiceInstance; import org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils; @@ -34,8 +36,11 @@ import org.springframework.cloud.kubernetes.commons.discovery.ServicePortSecureResolver; import org.springframework.cloud.kubernetes.commons.loadbalancer.KubernetesLoadBalancerProperties; import org.springframework.cloud.kubernetes.commons.loadbalancer.KubernetesServiceInstanceMapper; +import org.springframework.core.log.LogAccessor; import org.springframework.util.StringUtils; +import static java.util.Optional.ofNullable; +import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.PORT_NAME_PROPERTY; import static org.springframework.cloud.kubernetes.commons.discovery.ServicePortSecureResolver.Input; /** @@ -43,6 +48,9 @@ */ public class KubernetesClientServiceInstanceMapper implements KubernetesServiceInstanceMapper { + private static final LogAccessor LOG = new LogAccessor( + LogFactory.getLog(KubernetesClientServiceInstanceMapper.class)); + /** * empty on purpose, load balancer implementation does not need them. */ @@ -63,29 +71,49 @@ public KubernetesClientServiceInstanceMapper(KubernetesLoadBalancerProperties pr @Override public KubernetesServiceInstance map(V1Service service) { - final V1ObjectMeta meta = service.getMetadata(); + V1ObjectMeta metadata = service.getMetadata(); + + List ports = ofNullable(service.getSpec()).map(V1ServiceSpec::getPorts).orElse(List.of()); + V1ServicePort port; + + if (ports.isEmpty()) { + LOG.warn(() -> "service : " + metadata.getName() + " does not have any ServicePort(s)," + + " will not consider it for load balancing"); + return null; + } - final List ports = service.getSpec().getPorts(); - V1ServicePort port = null; if (ports.size() == 1) { + LOG.debug(() -> "single ServicePort found, will use it as-is " + "(without checking " + PORT_NAME_PROPERTY + + ")"); port = ports.get(0); } - else if (ports.size() > 1 && StringUtils.hasText(this.properties.getPortName())) { - Optional optPort = ports.stream() - .filter(it -> properties.getPortName().endsWith(it.getName())).findAny(); - if (optPort.isPresent()) { - port = optPort.get(); + else { + String portNameFromProperties = properties.getPortName(); + if (StringUtils.hasText(portNameFromProperties)) { + Optional optionalPort = ports.stream() + .filter(x -> Objects.equals(x.getName(), portNameFromProperties)).findAny(); + if (optionalPort.isPresent()) { + LOG.debug(() -> "found port name that matches : " + portNameFromProperties); + port = optionalPort.get(); + } + else { + logWarning(portNameFromProperties); + port = ports.get(0); + } + } + else { + LOG.warn(() -> PORT_NAME_PROPERTY + " is not set, as such will not consider service with name : " + + metadata.getName()); + return null; } } - if (port == null) { - return null; - } + String host = KubernetesServiceInstanceMapper.createHost(service.getMetadata().getName(), service.getMetadata().getNamespace(), properties.getClusterDomain()); boolean secure = secure(port, service); - return new DefaultKubernetesServiceInstance(meta.getUid(), meta.getName(), host, port.getPort(), + return new DefaultKubernetesServiceInstance(metadata.getUid(), metadata.getName(), host, port.getPort(), serviceMetadata(service), secure); } @@ -94,6 +122,7 @@ private Map serviceMetadata(V1Service service) { V1ServiceSpec serviceSpec = service.getSpec(); ServiceMetadata serviceMetadata = new ServiceMetadata(metadata.getName(), metadata.getNamespace(), serviceSpec.getType(), metadata.getLabels(), metadata.getAnnotations()); + return DiscoveryClientUtils.serviceInstanceMetadata(PORTS_DATA, serviceMetadata, discoveryProperties); } @@ -104,4 +133,9 @@ private boolean secure(V1ServicePort port, V1Service service) { return resolver.resolve(input); } + private void logWarning(String portNameFromProperties) { + LOG.warn(() -> "Did not find a port name that is equal to the value " + portNameFromProperties); + LOG.warn(() -> "Will return 'first' port found, which is non-deterministic"); + } + } diff --git a/spring-cloud-kubernetes-client-loadbalancer/src/test/java/org/springframework/cloud/kubernetes/client/loadbalancer/KubernetesClientServiceInstanceMapperTests.java b/spring-cloud-kubernetes-client-loadbalancer/src/test/java/org/springframework/cloud/kubernetes/client/loadbalancer/KubernetesClientServiceInstanceMapperTests.java index c825641b0..885ee7e5e 100644 --- a/spring-cloud-kubernetes-client-loadbalancer/src/test/java/org/springframework/cloud/kubernetes/client/loadbalancer/KubernetesClientServiceInstanceMapperTests.java +++ b/spring-cloud-kubernetes-client-loadbalancer/src/test/java/org/springframework/cloud/kubernetes/client/loadbalancer/KubernetesClientServiceInstanceMapperTests.java @@ -25,8 +25,12 @@ import io.kubernetes.client.openapi.models.V1ServicePort; import io.kubernetes.client.openapi.models.V1ServicePortBuilder; import io.kubernetes.client.openapi.models.V1ServiceSpecBuilder; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; import org.springframework.cloud.kubernetes.commons.discovery.DefaultKubernetesServiceInstance; import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties; import org.springframework.cloud.kubernetes.commons.discovery.KubernetesServiceInstance; @@ -37,6 +41,7 @@ /** * @author Ryan Baxter */ +@ExtendWith(OutputCaptureExtension.class) class KubernetesClientServiceInstanceMapperTests { @Test @@ -99,6 +104,94 @@ void multiplePortsSecure() { assertThat(serviceInstance).isEqualTo(result); } + @Test + void testEmptyPorts(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName("https"); + KubernetesClientServiceInstanceMapper mapper = new KubernetesClientServiceInstanceMapper(loadBalancerProperties, + KubernetesDiscoveryProperties.DEFAULT); + + Map annotations = Map.of("org.springframework.cloud", "true"); + Map labels = Map.of("beta", "true"); + List servicePorts = List.of(); + V1Service service = createService("database", "default", annotations, labels, servicePorts); + KubernetesServiceInstance serviceInstance = mapper.map(service); + Assertions.assertNull(serviceInstance); + Assertions.assertTrue(output.getOut().contains( + "service : database does not have any ServicePort(s), will not consider it for load balancing")); + } + + @Test + void singlePortNameMatchesProperty(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName("http"); + KubernetesClientServiceInstanceMapper mapper = new KubernetesClientServiceInstanceMapper(loadBalancerProperties, + KubernetesDiscoveryProperties.DEFAULT); + + Map annotations = Map.of("org.springframework.cloud", "true"); + Map labels = Map.of("beta", "true"); + List servicePorts = List.of(new V1ServicePortBuilder().withName("http").withPort(80).build()); + V1Service service = createService("database", "default", annotations, labels, servicePorts); + KubernetesServiceInstance serviceInstance = mapper.map(service); + Assertions.assertNotNull(serviceInstance); + Assertions.assertTrue(output.getOut().contains("single ServicePort found, " + + "will use it as-is (without checking 'spring.cloud.kubernetes.loadbalancer.portName')")); + } + + @Test + void singlePortNameDoesNotMatchProperty(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName("http-api"); + KubernetesClientServiceInstanceMapper mapper = new KubernetesClientServiceInstanceMapper(loadBalancerProperties, + KubernetesDiscoveryProperties.DEFAULT); + + Map annotations = Map.of("org.springframework.cloud", "true"); + Map labels = Map.of("beta", "true"); + List servicePorts = List.of(new V1ServicePortBuilder().withName("http").withPort(80).build()); + V1Service service = createService("database", "default", annotations, labels, servicePorts); + KubernetesServiceInstance serviceInstance = mapper.map(service); + Assertions.assertNotNull(serviceInstance); + Assertions.assertTrue(output.getOut().contains("single ServicePort found, " + + "will use it as-is (without checking 'spring.cloud.kubernetes.loadbalancer.portName')")); + } + + @Test + void multiplePortsNameMatchesProperty(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName("http"); + KubernetesClientServiceInstanceMapper mapper = new KubernetesClientServiceInstanceMapper(loadBalancerProperties, + KubernetesDiscoveryProperties.DEFAULT); + + Map annotations = Map.of("org.springframework.cloud", "true"); + Map labels = Map.of("beta", "true"); + List servicePorts = List.of(new V1ServicePortBuilder().withName("http").withPort(80).build(), + new V1ServicePortBuilder().withName("https").withPort(443).build()); + V1Service service = createService("database", "default", annotations, labels, servicePorts); + KubernetesServiceInstance serviceInstance = mapper.map(service); + Assertions.assertNotNull(serviceInstance); + Assertions.assertTrue(output.getOut().contains("found port name that matches : http")); + Assertions.assertEquals(serviceInstance.getPort(), 80); + } + + @Test + void multiplePortsNameDoesNotMatchProperty(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName("http"); + KubernetesClientServiceInstanceMapper mapper = new KubernetesClientServiceInstanceMapper(loadBalancerProperties, + KubernetesDiscoveryProperties.DEFAULT); + + Map annotations = Map.of("org.springframework.cloud", "true"); + Map labels = Map.of("beta", "true"); + List servicePorts = List.of(new V1ServicePortBuilder().withName("http-api").withPort(80).build(), + new V1ServicePortBuilder().withName("https").withPort(443).build()); + V1Service service = createService("database", "default", annotations, labels, servicePorts); + KubernetesServiceInstance serviceInstance = mapper.map(service); + Assertions.assertNotNull(serviceInstance); + Assertions.assertTrue(output.getOut().contains("Did not find a port name that is equal to the value http")); + Assertions.assertTrue(output.getOut().contains("Will return 'first' port found, which is non-deterministic")); + Assertions.assertTrue(serviceInstance.getPort() == 80 || serviceInstance.getPort() == 443); + } + private V1Service createService(String name, String namespace, Map annotations, Map labels, List servicePorts) { return new V1ServiceBuilder() diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/KubernetesDiscoveryConstants.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/KubernetesDiscoveryConstants.java index 59d599baa..5177b4c22 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/KubernetesDiscoveryConstants.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/discovery/KubernetesDiscoveryConstants.java @@ -98,4 +98,9 @@ private KubernetesDiscoveryConstants() { public static final String CATALOG_WATCH_PROPERTY_WITH_DEFAULT_VALUE = CATALOG_WATCH_PROPERTY_NAME + ":" + CATALOG_WATCHER_DEFAULT_DELAY; + /** + * load balancer port name property. + */ + public static final String PORT_NAME_PROPERTY = "'spring.cloud.kubernetes.loadbalancer.portName'"; + } diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapper.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapper.java index 9b81e5f81..bf6e7a87c 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapper.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/loadbalancer/KubernetesServiceInstanceMapper.java @@ -18,10 +18,7 @@ import java.util.StringJoiner; -import org.apache.commons.logging.LogFactory; - import org.springframework.cloud.kubernetes.commons.discovery.KubernetesServiceInstance; -import org.springframework.core.log.LogAccessor; import org.springframework.util.StringUtils; /** @@ -29,11 +26,6 @@ */ public interface KubernetesServiceInstanceMapper { - /** - * Logger instance. - */ - LogAccessor LOG = new LogAccessor(LogFactory.getLog(KubernetesServiceInstanceMapper.class)); - KubernetesServiceInstance map(T service); static String createHost(String serviceName, String namespace, String clusterDomain) { diff --git a/spring-cloud-kubernetes-fabric8-loadbalancer/src/main/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapper.java b/spring-cloud-kubernetes-fabric8-loadbalancer/src/main/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapper.java index fab705ec1..6f44a85ce 100644 --- a/spring-cloud-kubernetes-fabric8-loadbalancer/src/main/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapper.java +++ b/spring-cloud-kubernetes-fabric8-loadbalancer/src/main/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapper.java @@ -39,6 +39,7 @@ import org.springframework.core.log.LogAccessor; import org.springframework.util.StringUtils; +import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.PORT_NAME_PROPERTY; import static org.springframework.cloud.kubernetes.commons.discovery.ServicePortSecureResolver.Input; /** @@ -48,8 +49,6 @@ */ public class Fabric8ServiceInstanceMapper implements KubernetesServiceInstanceMapper { - private static final String PORT_NAME_PROPERTY = "'spring.cloud.kubernetes.loadbalancer.portName'"; - private static final LogAccessor LOG = new LogAccessor(LogFactory.getLog(Fabric8ServiceInstanceMapper.class)); /**