K8s client lb cleanup 3 (#1627)
This commit is contained in:
@@ -18,12 +18,14 @@ package org.springframework.cloud.kubernetes.client.loadbalancer;
|
||||
|
||||
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.ServicePortNameAnd
|
||||
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 @@ import static org.springframework.cloud.kubernetes.commons.discovery.ServicePort
|
||||
*/
|
||||
public class KubernetesClientServiceInstanceMapper implements KubernetesServiceInstanceMapper<V1Service> {
|
||||
|
||||
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 class KubernetesClientServiceInstanceMapper implements KubernetesServiceI
|
||||
|
||||
@Override
|
||||
public KubernetesServiceInstance map(V1Service service) {
|
||||
final V1ObjectMeta meta = service.getMetadata();
|
||||
V1ObjectMeta metadata = service.getMetadata();
|
||||
|
||||
final List<V1ServicePort> ports = service.getSpec().getPorts();
|
||||
V1ServicePort port = null;
|
||||
if (ports.size() == 1) {
|
||||
port = ports.get(0);
|
||||
}
|
||||
else if (ports.size() > 1 && StringUtils.hasText(this.properties.getPortName())) {
|
||||
Optional<V1ServicePort> optPort = ports.stream()
|
||||
.filter(it -> properties.getPortName().endsWith(it.getName())).findAny();
|
||||
if (optPort.isPresent()) {
|
||||
port = optPort.get();
|
||||
}
|
||||
}
|
||||
if (port == null) {
|
||||
List<V1ServicePort> 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;
|
||||
}
|
||||
|
||||
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<V1ServicePort> 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);
|
||||
}
|
||||
|
||||
@@ -94,6 +122,7 @@ public class KubernetesClientServiceInstanceMapper implements KubernetesServiceI
|
||||
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 @@ public class KubernetesClientServiceInstanceMapper implements KubernetesServiceI
|
||||
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");
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -25,8 +25,12 @@ import io.kubernetes.client.openapi.models.V1ServiceBuilder;
|
||||
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 @@ import static org.assertj.core.api.Assertions.assertThat;
|
||||
/**
|
||||
* @author Ryan Baxter
|
||||
*/
|
||||
@ExtendWith(OutputCaptureExtension.class)
|
||||
class KubernetesClientServiceInstanceMapperTests {
|
||||
|
||||
@Test
|
||||
@@ -99,6 +104,94 @@ class KubernetesClientServiceInstanceMapperTests {
|
||||
assertThat(serviceInstance).isEqualTo(result);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testEmptyPorts(CapturedOutput output) {
|
||||
KubernetesLoadBalancerProperties loadBalancerProperties = new KubernetesLoadBalancerProperties();
|
||||
loadBalancerProperties.setPortName("https");
|
||||
KubernetesClientServiceInstanceMapper mapper = new KubernetesClientServiceInstanceMapper(loadBalancerProperties,
|
||||
KubernetesDiscoveryProperties.DEFAULT);
|
||||
|
||||
Map<String, String> annotations = Map.of("org.springframework.cloud", "true");
|
||||
Map<String, String> labels = Map.of("beta", "true");
|
||||
List<V1ServicePort> 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<String, String> annotations = Map.of("org.springframework.cloud", "true");
|
||||
Map<String, String> labels = Map.of("beta", "true");
|
||||
List<V1ServicePort> 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<String, String> annotations = Map.of("org.springframework.cloud", "true");
|
||||
Map<String, String> labels = Map.of("beta", "true");
|
||||
List<V1ServicePort> 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<String, String> annotations = Map.of("org.springframework.cloud", "true");
|
||||
Map<String, String> labels = Map.of("beta", "true");
|
||||
List<V1ServicePort> 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<String, String> annotations = Map.of("org.springframework.cloud", "true");
|
||||
Map<String, String> labels = Map.of("beta", "true");
|
||||
List<V1ServicePort> 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<String, String> annotations,
|
||||
Map<String, String> labels, List<V1ServicePort> servicePorts) {
|
||||
return new V1ServiceBuilder()
|
||||
|
||||
@@ -98,4 +98,9 @@ public final class 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'";
|
||||
|
||||
}
|
||||
|
||||
@@ -18,10 +18,7 @@ package org.springframework.cloud.kubernetes.commons.loadbalancer;
|
||||
|
||||
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 @@ import org.springframework.util.StringUtils;
|
||||
*/
|
||||
public interface KubernetesServiceInstanceMapper<T> {
|
||||
|
||||
/**
|
||||
* Logger instance.
|
||||
*/
|
||||
LogAccessor LOG = new LogAccessor(LogFactory.getLog(KubernetesServiceInstanceMapper.class));
|
||||
|
||||
KubernetesServiceInstance map(T service);
|
||||
|
||||
static String createHost(String serviceName, String namespace, String clusterDomain) {
|
||||
|
||||
@@ -39,6 +39,7 @@ 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.KubernetesDiscoveryConstants.PORT_NAME_PROPERTY;
|
||||
import static org.springframework.cloud.kubernetes.commons.discovery.ServicePortSecureResolver.Input;
|
||||
|
||||
/**
|
||||
@@ -48,8 +49,6 @@ import static org.springframework.cloud.kubernetes.commons.discovery.ServicePort
|
||||
*/
|
||||
public class Fabric8ServiceInstanceMapper implements KubernetesServiceInstanceMapper<Service> {
|
||||
|
||||
private static final String PORT_NAME_PROPERTY = "'spring.cloud.kubernetes.loadbalancer.portName'";
|
||||
|
||||
private static final LogAccessor LOG = new LogAccessor(LogFactory.getLog(Fabric8ServiceInstanceMapper.class));
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user