Drop overlapping configuration in blocking implementation (#1524)

This commit is contained in:
erabii
2023-11-30 23:14:23 +02:00
committed by GitHub
parent 0e7215e23e
commit 4bba72fc82
7 changed files with 65 additions and 30 deletions

View File

@@ -60,7 +60,8 @@ public record KubernetesDiscoveryProperties(
@DefaultValue Metadata metadata, @DefaultValue Metadata metadata,
@DefaultValue("" + DEFAULT_ORDER) int order, @DefaultValue("" + DEFAULT_ORDER) int order,
boolean useEndpointSlices, boolean useEndpointSlices,
boolean includeExternalNameServices) { boolean includeExternalNameServices,
String discoveryServerUrl) {
// @formatter:on // @formatter:on
/** /**
@@ -80,7 +81,20 @@ public record KubernetesDiscoveryProperties(
@DefaultValue Map<String, String> serviceLabels, String primaryPortName, @DefaultValue Metadata metadata, @DefaultValue Map<String, String> serviceLabels, String primaryPortName, @DefaultValue Metadata metadata,
@DefaultValue("" + DEFAULT_ORDER) int order, boolean useEndpointSlices) { @DefaultValue("" + DEFAULT_ORDER) int order, boolean useEndpointSlices) {
this(enabled, allNamespaces, namespaces, waitCacheReady, cacheLoadingTimeoutSeconds, includeNotReadyAddresses, this(enabled, allNamespaces, namespaces, waitCacheReady, cacheLoadingTimeoutSeconds, includeNotReadyAddresses,
filter, knownSecurePorts, serviceLabels, primaryPortName, metadata, order, useEndpointSlices, false); filter, knownSecurePorts, serviceLabels, primaryPortName, metadata, order, useEndpointSlices, false,
null);
}
public KubernetesDiscoveryProperties(@DefaultValue("true") boolean enabled, boolean allNamespaces,
@DefaultValue Set<String> namespaces, @DefaultValue("true") boolean waitCacheReady,
@DefaultValue("60") long cacheLoadingTimeoutSeconds, boolean includeNotReadyAddresses, String filter,
@DefaultValue({ "443", "8443" }) Set<Integer> knownSecurePorts,
@DefaultValue Map<String, String> serviceLabels, String primaryPortName, @DefaultValue Metadata metadata,
@DefaultValue("" + DEFAULT_ORDER) int order, boolean useEndpointSlices,
boolean includeExternalNameServices) {
this(enabled, allNamespaces, namespaces, waitCacheReady, cacheLoadingTimeoutSeconds, includeNotReadyAddresses,
filter, knownSecurePorts, serviceLabels, primaryPortName, metadata, order, useEndpointSlices,
includeExternalNameServices, null);
} }
/** /**
@@ -88,7 +102,7 @@ public record KubernetesDiscoveryProperties(
*/ */
public static final KubernetesDiscoveryProperties DEFAULT = new KubernetesDiscoveryProperties(true, false, Set.of(), public static final KubernetesDiscoveryProperties DEFAULT = new KubernetesDiscoveryProperties(true, false, Set.of(),
true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, false, true, 60, false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, false,
false); false, null);
/** /**
* @param addLabels include labels as metadata * @param addLabels include labels as metadata

View File

@@ -54,6 +54,7 @@ class KubernetesDiscoveryPropertiesTests {
assertThat(props.order()).isZero(); assertThat(props.order()).isZero();
assertThat(props.useEndpointSlices()).isFalse(); assertThat(props.useEndpointSlices()).isFalse();
assertThat(props.includeExternalNameServices()).isFalse(); assertThat(props.includeExternalNameServices()).isFalse();
assertThat(props.discoveryServerUrl()).isNull();
}); });
} }
@@ -66,7 +67,8 @@ class KubernetesDiscoveryPropertiesTests {
"spring.cloud.kubernetes.discovery.use-endpoint-slices=true", "spring.cloud.kubernetes.discovery.use-endpoint-slices=true",
"spring.cloud.kubernetes.discovery.namespaces[0]=ns1", "spring.cloud.kubernetes.discovery.namespaces[0]=ns1",
"spring.cloud.kubernetes.discovery.namespaces[1]=ns2", "spring.cloud.kubernetes.discovery.namespaces[1]=ns2",
"spring.cloud.kubernetes.discovery.include-external-name-services=true") "spring.cloud.kubernetes.discovery.include-external-name-services=true",
"spring.cloud.kubernetes.discovery.discovery-server-url=http://example")
.run(context -> { .run(context -> {
KubernetesDiscoveryProperties props = context.getBean(KubernetesDiscoveryProperties.class); KubernetesDiscoveryProperties props = context.getBean(KubernetesDiscoveryProperties.class);
assertThat(props).isNotNull(); assertThat(props).isNotNull();
@@ -87,6 +89,7 @@ class KubernetesDiscoveryPropertiesTests {
assertThat(props.order()).isZero(); assertThat(props.order()).isZero();
assertThat(props.useEndpointSlices()).isTrue(); assertThat(props.useEndpointSlices()).isTrue();
assertThat(props.includeExternalNameServices()).isTrue(); assertThat(props.includeExternalNameServices()).isTrue();
assertThat(props.discoveryServerUrl()).isEqualTo("http://example");
}); });
} }

View File

@@ -73,14 +73,13 @@ class ConfigServerBootstrapper extends KubernetesConfigServerBootstrapper {
} }
private KubernetesConfigServerInstanceProvider getInstanceProvider(Binder binder, BindHandler bindHandler) { private KubernetesConfigServerInstanceProvider getInstanceProvider(Binder binder, BindHandler bindHandler) {
KubernetesDiscoveryClientProperties kubernetesDiscoveryClientProperties = binder KubernetesDiscoveryProperties kubernetesDiscoveryProperties = binder
.bind(KubernetesDiscoveryProperties.PREFIX, Bindable.of(KubernetesDiscoveryClientProperties.class), .bind(KubernetesDiscoveryProperties.PREFIX, Bindable.of(KubernetesDiscoveryProperties.class),
bindHandler) bindHandler)
.orElseGet(KubernetesDiscoveryClientProperties::new); .orElseGet(() -> KubernetesDiscoveryProperties.DEFAULT);
KubernetesDiscoveryClientBlockingAutoConfiguration autoConfiguration = KubernetesDiscoveryClientBlockingAutoConfiguration autoConfiguration = new KubernetesDiscoveryClientBlockingAutoConfiguration();
new KubernetesDiscoveryClientBlockingAutoConfiguration();
DiscoveryClient discoveryClient = autoConfiguration DiscoveryClient discoveryClient = autoConfiguration
.kubernetesDiscoveryClient(autoConfiguration.restTemplate(), kubernetesDiscoveryClientProperties); .kubernetesDiscoveryClient(autoConfiguration.restTemplate(), kubernetesDiscoveryProperties);
return discoveryClient::getInstances; return discoveryClient::getInstances;
} }

View File

@@ -23,6 +23,7 @@ import java.util.stream.Collectors;
import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.DiscoveryClient;
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
import org.springframework.web.client.RestTemplate; import org.springframework.web.client.RestTemplate;
@@ -39,6 +40,7 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
private final String discoveryServerUrl; private final String discoveryServerUrl;
@Deprecated(forRemoval = true)
public KubernetesDiscoveryClient(RestTemplate rest, KubernetesDiscoveryClientProperties properties) { public KubernetesDiscoveryClient(RestTemplate rest, KubernetesDiscoveryClientProperties properties) {
if (!StringUtils.hasText(properties.getDiscoveryServerUrl())) { if (!StringUtils.hasText(properties.getDiscoveryServerUrl())) {
throw new DiscoveryServerUrlInvalidException(); throw new DiscoveryServerUrlInvalidException();
@@ -49,6 +51,16 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
this.discoveryServerUrl = properties.getDiscoveryServerUrl(); this.discoveryServerUrl = properties.getDiscoveryServerUrl();
} }
KubernetesDiscoveryClient(RestTemplate rest, KubernetesDiscoveryProperties kubernetesDiscoveryProperties) {
if (!StringUtils.hasText(kubernetesDiscoveryProperties.discoveryServerUrl())) {
throw new DiscoveryServerUrlInvalidException();
}
this.rest = rest;
this.emptyNamespaces = kubernetesDiscoveryProperties.namespaces().isEmpty();
this.namespaces = kubernetesDiscoveryProperties.namespaces();
this.discoveryServerUrl = kubernetesDiscoveryProperties.discoveryServerUrl();
}
@Override @Override
public String description() { public String description() {
return "Kubernetes Discovery Client"; return "Kubernetes Discovery Client";
@@ -56,8 +68,8 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
@Override @Override
public List<ServiceInstance> getInstances(String serviceId) { public List<ServiceInstance> getInstances(String serviceId) {
KubernetesServiceInstance[] responseBody = rest.getForEntity( KubernetesServiceInstance[] responseBody = rest
discoveryServerUrl + "/apps/" + serviceId, KubernetesServiceInstance[].class).getBody(); .getForEntity(discoveryServerUrl + "/apps/" + serviceId, KubernetesServiceInstance[].class).getBody();
if (responseBody != null && responseBody.length > 0) { if (responseBody != null && responseBody.length > 0) {
return Arrays.stream(responseBody).filter(this::matchNamespaces).collect(Collectors.toList()); return Arrays.stream(responseBody).filter(this::matchNamespaces).collect(Collectors.toList());
} }
@@ -79,8 +91,8 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
} }
private boolean matchNamespaces(Service service) { private boolean matchNamespaces(Service service) {
return service.getServiceInstances().isEmpty() || return service.getServiceInstances().isEmpty()
service.getServiceInstances().stream().anyMatch(this::matchNamespaces); || service.getServiceInstances().stream().anyMatch(this::matchNamespaces);
} }
} }

View File

@@ -24,6 +24,7 @@ import org.springframework.cloud.kubernetes.commons.PodUtils;
import org.springframework.cloud.kubernetes.commons.discovery.ConditionalOnSpringCloudKubernetesBlockingDiscovery; import org.springframework.cloud.kubernetes.commons.discovery.ConditionalOnSpringCloudKubernetesBlockingDiscovery;
import org.springframework.cloud.kubernetes.commons.discovery.ConditionalOnSpringCloudKubernetesBlockingDiscoveryHealthInitializer; import org.springframework.cloud.kubernetes.commons.discovery.ConditionalOnSpringCloudKubernetesBlockingDiscoveryHealthInitializer;
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryClientHealthIndicatorInitializer; import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryClientHealthIndicatorInitializer;
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
@@ -35,8 +36,7 @@ import org.springframework.web.client.RestTemplate;
@Configuration(proxyBeanMethods = false) @Configuration(proxyBeanMethods = false)
@ConditionalOnSpringCloudKubernetesBlockingDiscovery @ConditionalOnSpringCloudKubernetesBlockingDiscovery
@EnableConfigurationProperties({ DiscoveryClientHealthIndicatorProperties.class, @EnableConfigurationProperties({ DiscoveryClientHealthIndicatorProperties.class, KubernetesDiscoveryProperties.class })
KubernetesDiscoveryClientProperties.class })
class KubernetesDiscoveryClientBlockingAutoConfiguration { class KubernetesDiscoveryClientBlockingAutoConfiguration {
@Bean @Bean
@@ -48,7 +48,7 @@ class KubernetesDiscoveryClientBlockingAutoConfiguration {
@Bean @Bean
@ConditionalOnMissingBean @ConditionalOnMissingBean
KubernetesDiscoveryClient kubernetesDiscoveryClient(RestTemplate restTemplate, KubernetesDiscoveryClient kubernetesDiscoveryClient(RestTemplate restTemplate,
KubernetesDiscoveryClientProperties properties) { KubernetesDiscoveryProperties properties) {
return new KubernetesDiscoveryClient(restTemplate, properties); return new KubernetesDiscoveryClient(restTemplate, properties);
} }

View File

@@ -22,7 +22,11 @@ import org.springframework.boot.context.properties.ConfigurationProperties;
/** /**
* @author Ryan Baxter * @author Ryan Baxter
* @deprecated use
* {@link org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties}
* instead.
*/ */
@Deprecated(forRemoval = true)
@ConfigurationProperties("spring.cloud.kubernetes.discovery") @ConfigurationProperties("spring.cloud.kubernetes.discovery")
public class KubernetesDiscoveryClientProperties { public class KubernetesDiscoveryClientProperties {

View File

@@ -33,6 +33,7 @@ import org.junit.jupiter.params.provider.MethodSource;
import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.boot.web.client.RestTemplateBuilder;
import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
import org.springframework.web.client.RestTemplate; import org.springframework.web.client.RestTemplate;
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
@@ -114,8 +115,9 @@ class KubernetesDiscoveryClientTests {
@Test @Test
void getInstances() { void getInstances() {
RestTemplate rest = new RestTemplateBuilder().build(); RestTemplate rest = new RestTemplateBuilder().build();
KubernetesDiscoveryClientProperties properties = new KubernetesDiscoveryClientProperties(); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60,
properties.setDiscoveryServerUrl(wireMockServer.baseUrl()); false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, false, false,
wireMockServer.baseUrl());
KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(rest, properties); KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(rest, properties);
assertThat(discoveryClient.getServices()).contains("test-svc-1", "test-svc-3"); assertThat(discoveryClient.getServices()).contains("test-svc-1", "test-svc-3");
} }
@@ -123,8 +125,9 @@ class KubernetesDiscoveryClientTests {
@Test @Test
void getServices() { void getServices() {
RestTemplate rest = new RestTemplateBuilder().build(); RestTemplate rest = new RestTemplateBuilder().build();
KubernetesDiscoveryClientProperties properties = new KubernetesDiscoveryClientProperties(); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, Set.of(), true, 60,
properties.setDiscoveryServerUrl(wireMockServer.baseUrl()); false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, false, false,
wireMockServer.baseUrl());
KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(rest, properties); KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(rest, properties);
Map<String, String> metadata = new HashMap<>(); Map<String, String> metadata = new HashMap<>();
metadata.put("spring", "true"); metadata.put("spring", "true");
@@ -140,9 +143,9 @@ class KubernetesDiscoveryClientTests {
@MethodSource("servicesFilteredByNamespacesSource") @MethodSource("servicesFilteredByNamespacesSource")
void getServicesFilteredByNamespaces(Set<String> namespaces, List<String> expectedServices) { void getServicesFilteredByNamespaces(Set<String> namespaces, List<String> expectedServices) {
RestTemplate rest = new RestTemplateBuilder().build(); RestTemplate rest = new RestTemplateBuilder().build();
KubernetesDiscoveryClientProperties properties = new KubernetesDiscoveryClientProperties(); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, namespaces, true, 60,
properties.setNamespaces(namespaces); false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, false, false,
properties.setDiscoveryServerUrl(wireMockServer.baseUrl()); wireMockServer.baseUrl());
KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(rest, properties); KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(rest, properties);
assertThat(discoveryClient.getServices()).containsExactlyInAnyOrderElementsOf(expectedServices); assertThat(discoveryClient.getServices()).containsExactlyInAnyOrderElementsOf(expectedServices);
} }
@@ -151,9 +154,9 @@ class KubernetesDiscoveryClientTests {
@MethodSource("instancesFilteredByNamespacesSource") @MethodSource("instancesFilteredByNamespacesSource")
void getInstancesFilteredByNamespaces(Set<String> namespaces, String serviceId, List<String> expectedInstances) { void getInstancesFilteredByNamespaces(Set<String> namespaces, String serviceId, List<String> expectedInstances) {
RestTemplate rest = new RestTemplateBuilder().build(); RestTemplate rest = new RestTemplateBuilder().build();
KubernetesDiscoveryClientProperties properties = new KubernetesDiscoveryClientProperties(); KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties(true, true, namespaces, true, 60,
properties.setNamespaces(namespaces); false, null, Set.of(), Map.of(), null, KubernetesDiscoveryProperties.Metadata.DEFAULT, 0, false, false,
properties.setDiscoveryServerUrl(wireMockServer.baseUrl()); wireMockServer.baseUrl());
KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(rest, properties); KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(rest, properties);
assertThat(discoveryClient.getInstances(serviceId)).map(ServiceInstance::getInstanceId) assertThat(discoveryClient.getInstances(serviceId)).map(ServiceInstance::getInstanceId)
.containsExactlyInAnyOrderElementsOf(expectedInstances); .containsExactlyInAnyOrderElementsOf(expectedInstances);
@@ -161,9 +164,9 @@ class KubernetesDiscoveryClientTests {
private static Stream<Arguments> servicesFilteredByNamespacesSource() { private static Stream<Arguments> servicesFilteredByNamespacesSource() {
return Stream.of(Arguments.of(Set.of(), List.of("test-svc-1", "test-svc-3")), return Stream.of(Arguments.of(Set.of(), List.of("test-svc-1", "test-svc-3")),
Arguments.of(Set.of("namespace1", "namespace2"), List.of("test-svc-1", "test-svc-3")), Arguments.of(Set.of("namespace1", "namespace2"), List.of("test-svc-1", "test-svc-3")),
Arguments.of(Set.of("namespace1"), List.of("test-svc-1")), Arguments.of(Set.of("namespace1"), List.of("test-svc-1")),
Arguments.of(Set.of("namespace2", "does-not-exist"), List.of("test-svc-3"))); Arguments.of(Set.of("namespace2", "does-not-exist"), List.of("test-svc-3")));
} }
private static Stream<Arguments> instancesFilteredByNamespacesSource() { private static Stream<Arguments> instancesFilteredByNamespacesSource() {