SpringCloud/spring-cloud-kubernetes
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Separate some more logic (#1222)

Browse Source
This commit is contained in:
erabii
2023-02-13 18:14:21 +02:00
committed by GitHub
parent 627a847cc0
commit 16aae515f4
3 changed files with 370 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClient.java
View File

@@ -42,10 +42,8 @@ import org.springframework.util.StringUtils;
import static java.util.stream.Collectors.toMap;
import static org.springframework.cloud.kubernetes.commons.config.ConfigUtils.keysWithPrefix;
import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.HTTP;
import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.HTTPS;
import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.NAMESPACE_METADATA_KEY;
import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.PRIMARY_PORT_NAME_LABEL_KEY;
import static org.springframework.cloud.kubernetes.fabric8.discovery.KubernetesDiscoveryClientUtils.endpointsPort;
import static org.springframework.cloud.kubernetes.fabric8.discovery.KubernetesDiscoveryClientUtils.subsetsFromEndpoints;
/**
@@ -148,12 +146,6 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
final Map<String, String> serviceMetadata = this.getServiceMetadata(service);
KubernetesDiscoveryProperties.Metadata metadataProps = this.properties.metadata();
String primaryPortName = this.properties.primaryPortName();
Map<String, String> labels = service.getMetadata().getLabels();
if (labels != null && labels.containsKey(PRIMARY_PORT_NAME_LABEL_KEY)) {
primaryPortName = labels.get(PRIMARY_PORT_NAME_LABEL_KEY);
}
for (EndpointSubset s : subsets) {
// Extend the service metadata map with per-endpoint port information (if
// requested)
@@ -183,7 +175,7 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
}
for (EndpointAddress endpointAddress : addresses) {
int endpointPort = findEndpointPort(s, serviceId, primaryPortName);
int endpointPort = endpointsPort(s, serviceId, properties, service);
String instanceId = null;
if (endpointAddress.getTargetRef() != null) {
instanceId = endpointAddress.getTargetRef().getUid();
@@ -223,41 +215,6 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
return serviceMetadata;
}
private int findEndpointPort(EndpointSubset s, String serviceId, String primaryPortName) {
List<EndpointPort> endpointPorts = s.getPorts();
if (endpointPorts.size() == 1) {
return endpointPorts.get(0).getPort();
}
else {
Map<String, Integer> ports = endpointPorts.stream().filter(p -> StringUtils.hasText(p.getName()))
.collect(Collectors.toMap(EndpointPort::getName, EndpointPort::getPort));
// This oneliner is looking for a port with a name equal to the primary port
// name specified in the service label
// or in spring.cloud.kubernetes.discovery.primary-port-name, equal to https,
// or equal to http.
// In case no port has been found return -1 to log a warning and fall back to
// the first port in the list.
int discoveredPort = ports.getOrDefault(primaryPortName,
ports.getOrDefault(HTTPS, ports.getOrDefault(HTTP, -1)));
if (discoveredPort == -1) {
if (StringUtils.hasText(primaryPortName)) {
log.warn("Could not find a port named '" + primaryPortName + "', 'https', or 'http' for service '"
+ serviceId + "'.");
}
else {
log.warn("Could not find a port named 'https' or 'http' for service '" + serviceId + "'.");
}
log.warn(
"Make sure that either the primary-port-name label has been added to the service, or that spring.cloud.kubernetes.discovery.primary-port-name has been configured.");
log.warn("Alternatively name the primary port 'https' or 'http'");
log.warn("An incorrect configuration may result in non-deterministic behaviour.");
discoveredPort = endpointPorts.get(0).getPort();
}
return discoveredPort;
}
}
@Override
public List<String> getServices() {
return adapter.apply(client).stream().map(s -> s.getMetadata().getName()).toList();

102
spring-cloud-kubernetes-fabric8-discovery/src/main/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientUtils.java
View File

@@ -17,15 +17,32 @@
package org.springframework.cloud.kubernetes.fabric8.discovery;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.stream.Collectors;
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.Service;
import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
import org.springframework.core.log.LogAccessor;
import org.springframework.util.StringUtils;
import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.HTTP;
import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.HTTPS;
import static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.PRIMARY_PORT_NAME_LABEL_KEY;
/**
* @author wind57
*/
final class KubernetesDiscoveryClientUtils {
private static final LogAccessor LOG = new LogAccessor(LogFactory.getLog(KubernetesDiscoveryClientUtils.class));
private KubernetesDiscoveryClientUtils() {
}
@@ -37,4 +54,89 @@ final class KubernetesDiscoveryClientUtils {
return new EndpointSubsetNS(clientNamespace.get(), List.of());
}
static int endpointsPort(EndpointSubset endpointSubset, String serviceId, KubernetesDiscoveryProperties properties,
Service service) {
List<EndpointPort> endpointPorts = endpointSubset.getPorts();
if (endpointPorts.size() == 1) {
int port = endpointPorts.get(0).getPort();
LOG.debug(() -> "endpoint ports has a single entry, using port : " + port);
return port;
}
else {
Optional<Integer> port;
String primaryPortName = primaryPortName(properties, service, serviceId);
Map<String, Integer> existingPorts = endpointPorts.stream()
.filter(endpointPort -> StringUtils.hasText(endpointPort.getName()))
.collect(Collectors.toMap(EndpointPort::getName, EndpointPort::getPort));
port = fromMap(existingPorts, primaryPortName, "found primary-port-name (with value: '" + primaryPortName
+ "') via properties or service labels to match port");
if (port.isPresent()) {
return port.get();
}
port = fromMap(existingPorts, HTTPS, "found primary-port-name via 'https' to match port");
if (port.isPresent()) {
return port.get();
}
port = fromMap(existingPorts, HTTP, "found primary-port-name via 'http' to match port");
if (port.isPresent()) {
return port.get();
}
logWarnings();
return endpointPorts.get(0).getPort();
}
}
/**
* take primary-port-name from service label "PRIMARY_PORT_NAME_LABEL_KEY" if it
* exists, otherwise from KubernetesDiscoveryProperties if it exists, otherwise null.
*/
static String primaryPortName(KubernetesDiscoveryProperties properties, Service service, String serviceId) {
String primaryPortNameFromProperties = properties.primaryPortName();
Map<String, String> serviceLabels = service.getMetadata().getLabels();
// the value from labels takes precedence over the one from properties
String primaryPortName = Optional
.ofNullable(Optional.ofNullable(serviceLabels).orElse(Map.of()).get(PRIMARY_PORT_NAME_LABEL_KEY))
.orElse(primaryPortNameFromProperties);
if (primaryPortName == null) {
LOG.debug(
() -> "did not find a primary-port-name in neither properties nor service labels for service with ID : "
+ serviceId);
return null;
}
LOG.debug(() -> "will use primaryPortName : " + primaryPortName + " for service with ID = " + serviceId);
return primaryPortName;
}
private static Optional<Integer> fromMap(Map<String, Integer> existingPorts, String key, String message) {
Integer fromPrimaryPortName = existingPorts.get(key);
if (fromPrimaryPortName == null) {
LOG.debug(() -> "not " + message);
return Optional.empty();
}
else {
LOG.debug(() -> message + " : " + fromPrimaryPortName);
return Optional.of(fromPrimaryPortName);
}
}
private static void logWarnings() {
LOG.warn(() -> """
Make sure that either the primary-port-name label has been added to the service,
or spring.cloud.kubernetes.discovery.primary-port-name has been configured.
Alternatively name the primary port 'https' or 'http'
An incorrect configuration may result in non-deterministic behaviour.""");
}
}

266
spring-cloud-kubernetes-fabric8-discovery/src/test/java/org/springframework/cloud/kubernetes/fabric8/discovery/KubernetesDiscoveryClientUtilsTests.java
View File

@@ -17,17 +17,32 @@
package org.springframework.cloud.kubernetes.fabric8.discovery;
import java.util.List;
import java.util.Map;
import java.util.Set;
import io.fabric8.kubernetes.api.model.EndpointPortBuilder;
import io.fabric8.kubernetes.api.model.EndpointSubset;
import io.fabric8.kubernetes.api.model.EndpointSubsetBuilder;
import io.fabric8.kubernetes.api.model.Endpoints;
import io.fabric8.kubernetes.api.model.EndpointsBuilder;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.kubernetes.api.model.Service;
import io.fabric8.kubernetes.api.model.ServiceBuilder;
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 static org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryConstants.PRIMARY_PORT_NAME_LABEL_KEY;
/**
* @author wind57
*/
@ExtendWith(OutputCaptureExtension.class)
class KubernetesDiscoveryClientUtilsTests {
@Test
@@ -58,4 +73,255 @@ class KubernetesDiscoveryClientUtilsTests {
Assertions.assertEquals(result.namespace(), "default");
}
/**
* <pre>
* - properties do not have primary-port-name set
* - service labels do not have primary-port-name set
*
* As such null is returned.
* </pre>
*/
@Test
void testPrimaryPortNameNotFound(CapturedOutput output) {
KubernetesDiscoveryProperties properties = KubernetesDiscoveryProperties.DEFAULT;
Service service = new ServiceBuilder().withMetadata(new ObjectMeta()).build();
String result = KubernetesDiscoveryClientUtils.primaryPortName(properties, service, "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"));
}
/**
* <pre>
* - properties do have primary-port-name set to "https"
* - service labels do not have primary-port-name set
*
* As such "https" is returned.
* </pre>
*/
@Test
void testPrimaryPortNameFoundInProperties(CapturedOutput output) {
String primaryPortName = "https";
KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
true, "", Set.of(), Map.of(), primaryPortName, null, 0, false);
Service service = new ServiceBuilder().withMetadata(new ObjectMeta()).build();
String result = KubernetesDiscoveryClientUtils.primaryPortName(properties, service, "abc");
Assertions.assertNotNull(result);
Assertions.assertEquals(result, primaryPortName);
Assertions.assertTrue(output.getOut().contains("will use primaryPortName : https for service with ID = abc"));
}
/**
* <pre>
* - properties do not have primary-port-name set
* - service labels do have primary-port-name set to "https"
*
* As such "https" is returned.
* </pre>
*/
@Test
void testPrimaryPortNameFoundInLabels(CapturedOutput output) {
Map<String, String> labels = Map.of(PRIMARY_PORT_NAME_LABEL_KEY, "https");
KubernetesDiscoveryProperties properties = KubernetesDiscoveryProperties.DEFAULT;
Service service = new ServiceBuilder().withMetadata(new ObjectMetaBuilder().withLabels(labels).build()).build();
String result = KubernetesDiscoveryClientUtils.primaryPortName(properties, service, "abc");
Assertions.assertNotNull(result);
Assertions.assertEquals(result, "https");
Assertions.assertTrue(output.getOut().contains("will use primaryPortName : https for service with ID = abc"));
}
/**
* <pre>
* - properties do have primary-port-name set to "https"
* - service labels do have primary-port-name set to "http"
*
* As such "http" is returned (labels win).
* </pre>
*/
@Test
void testPrimaryPortNameFoundInBothPropertiesAndLabels(CapturedOutput output) {
String primaryPortName = "https";
Map<String, String> labels = Map.of(PRIMARY_PORT_NAME_LABEL_KEY, "http");
KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
true, "", Set.of(), Map.of(), primaryPortName, null, 0, false);
Service service = new ServiceBuilder().withMetadata(new ObjectMetaBuilder().withLabels(labels).build()).build();
String result = KubernetesDiscoveryClientUtils.primaryPortName(properties, service, "abc");
Assertions.assertNotNull(result);
Assertions.assertEquals(result, "http");
Assertions.assertTrue(output.getOut().contains("will use primaryPortName : http for service with ID = abc"));
}
/**
* <pre>
* - EndpointSubset has a single entry in getPorts.
* </pre>
*/
@Test
void testEndpointsPortSinglePort(CapturedOutput output) {
EndpointSubset endpointSubset = new EndpointSubsetBuilder()
.withPorts(new EndpointPortBuilder().withPort(8080).build()).build();
String serviceId = "spring-k8s";
KubernetesDiscoveryProperties properties = KubernetesDiscoveryProperties.DEFAULT;
Service service = new ServiceBuilder().build();
Integer port = KubernetesDiscoveryClientUtils.endpointsPort(endpointSubset, serviceId, properties, service);
Assertions.assertEquals(port, 8080);
Assertions.assertTrue(output.getOut().contains("endpoint ports has a single entry, using port : 8080"));
}
/**
* <pre>
* - primary-port-name is null.
* </pre>
*/
@Test
void testEndpointsPortNullPrimaryPortName(CapturedOutput output) {
EndpointSubset endpointSubset = new EndpointSubsetBuilder()
.withPorts(new EndpointPortBuilder().withPort(8080).build(),
new EndpointPortBuilder().withPort(8081).build())
.build();
String serviceId = "spring-k8s";
KubernetesDiscoveryProperties properties = KubernetesDiscoveryProperties.DEFAULT;
Service service = new ServiceBuilder().withMetadata(new ObjectMeta()).build();
Integer port = KubernetesDiscoveryClientUtils.endpointsPort(endpointSubset, serviceId, properties, service);
Assertions.assertEquals(port, 8080);
Assertions.assertTrue(output.getOut().contains(
"did not find a primary-port-name in neither properties nor service labels for service with ID : spring-k8s"));
Assertions.assertTrue(output.getOut()
.contains("not found primary-port-name (with value: 'null') via properties or service labels"));
Assertions.assertTrue(output.getOut().contains("not found primary-port-name via 'https' to match port"));
Assertions.assertTrue(output.getOut().contains("not found primary-port-name via 'http' to match port"));
Assertions.assertTrue(output.getOut().contains("""
Make sure that either the primary-port-name label has been added to the service,
or spring.cloud.kubernetes.discovery.primary-port-name has been configured.
Alternatively name the primary port 'https' or 'http'
An incorrect configuration may result in non-deterministic behaviour."""));
}
/**
* <pre>
* - primary-port-name is "three", such a port name does not exist.
* </pre>
*/
@Test
void testEndpointsPortPrimaryPortNameIsPresentButNotFound(CapturedOutput output) {
EndpointSubset endpointSubset = new EndpointSubsetBuilder()
.withPorts(new EndpointPortBuilder().withPort(8080).withName("one").build(),
new EndpointPortBuilder().withPort(8081).withName("two").build())
.build();
String serviceId = "spring-k8s";
String primaryPortName = "three";
KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
true, "", Set.of(), Map.of(), primaryPortName, null, 0, false);
Service service = new ServiceBuilder().withMetadata(new ObjectMeta()).build();
Integer port = KubernetesDiscoveryClientUtils.endpointsPort(endpointSubset, serviceId, properties, service);
Assertions.assertEquals(port, 8080);
Assertions.assertTrue(
output.getOut().contains("will use primaryPortName : three for service with ID = spring-k8s"));
Assertions.assertTrue(output.getOut()
.contains("not found primary-port-name (with value: 'three') via properties or service labels"));
Assertions.assertTrue(output.getOut().contains("not found primary-port-name via 'https' to match port"));
Assertions.assertTrue(output.getOut().contains("not found primary-port-name via 'http' to match port"));
Assertions.assertTrue(output.getOut().contains("""
Make sure that either the primary-port-name label has been added to the service,
or spring.cloud.kubernetes.discovery.primary-port-name has been configured.
Alternatively name the primary port 'https' or 'http'
An incorrect configuration may result in non-deterministic behaviour."""));
}
/**
* <pre>
* - primary-port-name is "two", such a port name exists and matches 8081
* </pre>
*/
@Test
void testEndpointsPortPrimaryPortNameFound(CapturedOutput output) {
EndpointSubset endpointSubset = new EndpointSubsetBuilder()
.withPorts(new EndpointPortBuilder().withPort(8080).withName("one").build(),
new EndpointPortBuilder().withPort(8081).withName("two").build())
.build();
String serviceId = "spring-k8s";
String primaryPortName = "two";
KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
true, "", Set.of(), Map.of(), primaryPortName, null, 0, false);
Service service = new ServiceBuilder().withMetadata(new ObjectMeta()).build();
Integer port = KubernetesDiscoveryClientUtils.endpointsPort(endpointSubset, serviceId, properties, service);
Assertions.assertEquals(port, 8081);
Assertions.assertTrue(
output.getOut().contains("will use primaryPortName : two for service with ID = spring-k8s"));
Assertions.assertTrue(output.getOut().contains(
"found primary-port-name (with value: 'two') via properties or service labels to match port : 8081"));
}
/**
* <pre>
* - primary-port-name is "three", such a port name does not exist.
* - https port exists and this one is returned
* </pre>
*/
@Test
void testEndpointsPortPrimaryPortHttps(CapturedOutput output) {
EndpointSubset endpointSubset = new EndpointSubsetBuilder()
.withPorts(new EndpointPortBuilder().withPort(8080).withName("one").build(),
new EndpointPortBuilder().withPort(8081).withName("two").build(),
new EndpointPortBuilder().withPort(8082).withName("https").build())
.build();
String serviceId = "spring-k8s";
String primaryPortName = "three";
KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
true, "", Set.of(), Map.of(), primaryPortName, null, 0, false);
Service service = new ServiceBuilder().withMetadata(new ObjectMeta()).build();
Integer port = KubernetesDiscoveryClientUtils.endpointsPort(endpointSubset, serviceId, properties, service);
Assertions.assertEquals(port, 8082);
Assertions.assertTrue(
output.getOut().contains("will use primaryPortName : three for service with ID = spring-k8s"));
Assertions.assertTrue(output.getOut().contains(
"not found primary-port-name (with value: 'three') via properties or service labels to match port"));
Assertions.assertTrue(output.getOut().contains("found primary-port-name via 'https' to match port : 8082"));
}
/**
* <pre>
* - primary-port-name is "three", such a port name does not exist.
* - http port exists and this one is returned
* </pre>
*/
@Test
void testEndpointsPortPrimaryPortHttp(CapturedOutput output) {
EndpointSubset endpointSubset = new EndpointSubsetBuilder()
.withPorts(new EndpointPortBuilder().withPort(8080).withName("one").build(),
new EndpointPortBuilder().withPort(8081).withName("two").build(),
new EndpointPortBuilder().withPort(8082).withName("http").build())
.build();
String serviceId = "spring-k8s";
String primaryPortName = "three";
KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60L,
true, "", Set.of(), Map.of(), primaryPortName, null, 0, false);
Service service = new ServiceBuilder().withMetadata(new ObjectMeta()).build();
Integer port = KubernetesDiscoveryClientUtils.endpointsPort(endpointSubset, serviceId, properties, service);
Assertions.assertEquals(port, 8082);
Assertions.assertTrue(
output.getOut().contains("will use primaryPortName : three for service with ID = spring-k8s"));
Assertions.assertTrue(output.getOut().contains(
"not found primary-port-name (with value: 'three') via properties or service labels to match port"));
Assertions.assertTrue(output.getOut().contains("found primary-port-name via 'http' to match port : 8082"));
}
}