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 99de7559..8f269207 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 @@ -18,12 +18,13 @@ package org.springframework.cloud.kubernetes.fabric8.loadbalancer; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServicePort; -import io.fabric8.kubernetes.client.utils.Utils; +import org.apache.commons.logging.LogFactory; import org.springframework.cloud.kubernetes.commons.discovery.DefaultKubernetesServiceInstance; import org.springframework.cloud.kubernetes.commons.discovery.DiscoveryClientUtils; @@ -35,6 +36,8 @@ import org.springframework.cloud.kubernetes.commons.discovery.ServicePortSecureR import org.springframework.cloud.kubernetes.commons.loadbalancer.KubernetesLoadBalancerProperties; import org.springframework.cloud.kubernetes.commons.loadbalancer.KubernetesServiceInstanceMapper; import org.springframework.cloud.kubernetes.fabric8.Fabric8Utils; +import org.springframework.core.log.LogAccessor; +import org.springframework.util.StringUtils; import static org.springframework.cloud.kubernetes.commons.discovery.ServicePortSecureResolver.Input; @@ -45,6 +48,10 @@ import static org.springframework.cloud.kubernetes.commons.discovery.ServicePort */ 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)); + /** * empty on purpose, load balancer implementation does not need them. */ @@ -65,28 +72,48 @@ public class Fabric8ServiceInstanceMapper implements KubernetesServiceInstanceMa @Override public KubernetesServiceInstance map(Service service) { - ObjectMeta meta = service.getMetadata(); + ObjectMeta metadata = service.getMetadata(); List ports = service.getSpec().getPorts(); - ServicePort port = null; - if (ports.size() == 1) { - port = ports.get(0); - } - else if (ports.size() > 1 && Utils.isNotNullOrEmpty(properties.getPortName())) { - Optional optPort = ports.stream().filter(it -> properties.getPortName().endsWith(it.getName())) - .findAny(); - if (optPort.isPresent()) { - port = optPort.get(); - } - } - if (port == null) { + ServicePort port; + + if (ports.isEmpty()) { + LOG.warn(() -> "service : " + metadata.getName() + " does not have any ServicePort(s)," + + " will not consider it for load balancing"); return 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 { + 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; + } + } + 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); } @@ -102,4 +129,9 @@ public class Fabric8ServiceInstanceMapper implements KubernetesServiceInstanceMa 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-fabric8-loadbalancer/src/test/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapperTests.java b/spring-cloud-kubernetes-fabric8-loadbalancer/src/test/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapperTests.java index 06015a01..04ea207d 100644 --- a/spring-cloud-kubernetes-fabric8-loadbalancer/src/test/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapperTests.java +++ b/spring-cloud-kubernetes-fabric8-loadbalancer/src/test/java/org/springframework/cloud/kubernetes/fabric8/loadbalancer/Fabric8ServiceInstanceMapperTests.java @@ -28,11 +28,15 @@ import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.kubernetes.api.model.ServicePortBuilder; 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.KubernetesDiscoveryProperties; import org.springframework.cloud.kubernetes.commons.discovery.KubernetesServiceInstance; import org.springframework.cloud.kubernetes.commons.loadbalancer.KubernetesLoadBalancerProperties; +@ExtendWith(OutputCaptureExtension.class) class Fabric8ServiceInstanceMapperTests { @Test @@ -134,6 +138,159 @@ class Fabric8ServiceInstanceMapperTests { Assertions.assertEquals(result.get("two"), "2"); } + /** + *
+	 *     service has no ServicePorts
+	 * 
+ */ + @Test + void testMapEmptyPorts(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + KubernetesDiscoveryProperties discoveryProperties = new KubernetesDiscoveryProperties(true, false, Set.of(), + true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, + true); + + List ports = List.of(); + Service service = buildService("test", "test-namespace", "abc", ports, Map.of(), Map.of()); + KubernetesServiceInstance result = new Fabric8ServiceInstanceMapper(loadBalancerProperties, discoveryProperties) + .map(service); + + Assertions.assertNull(result); + Assertions.assertTrue(output.getOut() + .contains("service : test does not have any ServicePort(s), will not consider it for load balancing")); + + } + + /** + *
+	 *     service has a single ServicePort, and its name matches
+	 *     'spring.cloud.kubernetes.loadbalancer.portName'
+	 * 
+ */ + @Test + void testSinglePortsMatchesProperty(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName("my-port-name"); + KubernetesDiscoveryProperties discoveryProperties = new KubernetesDiscoveryProperties(true, false, Set.of(), + true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, + true); + + List ports = List.of(new ServicePortBuilder().withPort(8080).withName("my-port-name").build()); + Service service = buildService("test", "test-namespace", "abc", ports, Map.of(), Map.of()); + KubernetesServiceInstance result = new Fabric8ServiceInstanceMapper(loadBalancerProperties, discoveryProperties) + .map(service); + + Assertions.assertNotNull(result); + Assertions.assertTrue(output.getOut().contains( + "single ServicePort found, will use it as-is (without checking 'spring.cloud.kubernetes.loadbalancer.portName')")); + + } + + /** + *
+	 *     service has a single ServicePort, and its name does not match
+	 *     'spring.cloud.kubernetes.loadbalancer.portName'.
+	 *
+	 *     in this case, service is still considered, because we don't care
+	 *     about the property name when there is a single service port.
+	 * 
+ */ + @Test + void testSinglePortDoesNotMatchProperty(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName("my-different-port-name"); + KubernetesDiscoveryProperties discoveryProperties = new KubernetesDiscoveryProperties(true, false, Set.of(), + true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, + true); + + List ports = List.of(new ServicePortBuilder().withPort(8080).withName("my-port-name").build()); + Service service = buildService("test", "test-namespace", "abc", ports, Map.of(), Map.of()); + KubernetesServiceInstance result = new Fabric8ServiceInstanceMapper(loadBalancerProperties, discoveryProperties) + .map(service); + + Assertions.assertNotNull(result); + Assertions.assertTrue(output.getOut().contains( + "single ServicePort found, will use it as-is (without checking 'spring.cloud.kubernetes.loadbalancer.portName')")); + + } + + /** + *
+	 *     service has multiple ServicePorts, and 'spring.cloud.kubernetes.loadbalancer.portName' is empty.
+	 *     in this case, service will be skipped.
+	 * 
+ */ + @Test + void testMultiplePortsWithoutPortNameProperty(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName(""); + KubernetesDiscoveryProperties discoveryProperties = new KubernetesDiscoveryProperties(true, false, Set.of(), + true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, + true); + + List ports = List.of(new ServicePortBuilder().withPort(8080).withName("one").build(), + new ServicePortBuilder().withPort(8081).withName("two").build()); + Service service = buildService("test", "test-namespace", "abc", ports, Map.of(), Map.of()); + KubernetesServiceInstance result = new Fabric8ServiceInstanceMapper(loadBalancerProperties, discoveryProperties) + .map(service); + + Assertions.assertNull(result); + Assertions.assertTrue(output.getOut().contains( + "'spring.cloud.kubernetes.loadbalancer.portName' is not set, as such will not consider service with name : test")); + + } + + /** + *
+	 *     service has multiple ServicePorts, and 'spring.cloud.kubernetes.loadbalancer.portName' is empty.
+	 *     in this case, service will be skipped.
+	 * 
+ */ + @Test + void testMultiplePortsWithPortNamePropertyMatch(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName("one"); + KubernetesDiscoveryProperties discoveryProperties = new KubernetesDiscoveryProperties(true, false, Set.of(), + true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, + true); + + List ports = List.of(new ServicePortBuilder().withPort(8080).withName("one").build(), + new ServicePortBuilder().withPort(8081).withName("two").build()); + Service service = buildService("test", "test-namespace", "abc", ports, Map.of(), Map.of()); + KubernetesServiceInstance result = new Fabric8ServiceInstanceMapper(loadBalancerProperties, discoveryProperties) + .map(service); + + Assertions.assertNotNull(result); + Assertions.assertTrue(output.getOut().contains("found port name that matches : one")); + + } + + /** + *
+	 *     service has multiple ServicePorts, and 'spring.cloud.kubernetes.loadbalancer.portName' is empty.
+	 *     in this case, service will be skipped.
+	 * 
+ */ + @Test + void testMultiplePortsWithPortNamePropertyNoMatch(CapturedOutput output) { + KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties(); + loadBalancerProperties.setPortName("three"); + KubernetesDiscoveryProperties discoveryProperties = new KubernetesDiscoveryProperties(true, false, Set.of(), + true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, + true); + + List ports = List.of(new ServicePortBuilder().withPort(8080).withName("one").build(), + new ServicePortBuilder().withPort(8081).withName("two").build()); + Service service = buildService("test", "test-namespace", "abc", ports, Map.of(), Map.of()); + KubernetesServiceInstance result = new Fabric8ServiceInstanceMapper(loadBalancerProperties, discoveryProperties) + .map(service); + + Assertions.assertNotNull(result); + Assertions.assertTrue(output.getOut().contains("Did not find a port name that is equal to the value three")); + Assertions.assertTrue(output.getOut().contains("Will return 'first' port found, which is non-deterministic")); + Assertions.assertTrue(result.getPort() == 8081 || result.getPort() == 8080); + } + private Service buildService(String name, String namespace, String uid, int port, String portName, Map labels) { ServicePort servicePort = new ServicePortBuilder().withPort(port).withName(portName).build();