From 95186267e6646c48e2537a50489d957ff5c43b44 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 17 Jan 2020 15:45:03 -0500 Subject: [PATCH 1/5] Swaps deprecated ConditionalOnEnabledEndpoint for updated annotation. Swaps with ConditionalOnAvailableEndpoint --- .../CommonsClientAutoConfiguration.java | 4 +-- .../ServiceRegistryAutoConfiguration.java | 4 +-- .../CommonsClientAutoConfigurationTests.java | 8 ++++-- ...veCommonsClientAutoConfigurationTests.java | 25 +++++++++++-------- .../RefreshEndpointAutoConfiguration.java | 9 +++---- ...eEnvironmentEndpointAutoConfiguration.java | 6 ++--- .../LifecycleMvcAutoConfigurationTests.java | 12 ++++++--- .../restart/RestartIntegrationTests.java | 1 + 8 files changed, 42 insertions(+), 27 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/CommonsClientAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/CommonsClientAutoConfiguration.java index e5a2e86e2..50c115bfc 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/CommonsClientAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/CommonsClientAutoConfiguration.java @@ -21,7 +21,7 @@ import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnEnabledEndpoint; +import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnAvailableEndpoint; import org.springframework.boot.actuate.endpoint.annotation.Endpoint; import org.springframework.boot.actuate.health.HealthIndicator; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -93,7 +93,7 @@ protected static class ActuatorConfiguration { private List hasFeatures = new ArrayList<>(); @Bean - @ConditionalOnEnabledEndpoint + @ConditionalOnAvailableEndpoint public FeaturesEndpoint featuresEndpoint() { return new FeaturesEndpoint(this.hasFeatures); } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/ServiceRegistryAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/ServiceRegistryAutoConfiguration.java index 4b71800fc..a48390c47 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/ServiceRegistryAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/ServiceRegistryAutoConfiguration.java @@ -17,7 +17,7 @@ package org.springframework.cloud.client.serviceregistry; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnEnabledEndpoint; +import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnAvailableEndpoint; import org.springframework.boot.actuate.endpoint.annotation.Endpoint; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; @@ -39,7 +39,7 @@ protected class ServiceRegistryEndpointConfiguration { private Registration registration; @Bean - @ConditionalOnEnabledEndpoint + @ConditionalOnAvailableEndpoint public ServiceRegistryEndpoint serviceRegistryEndpoint( ServiceRegistry serviceRegistry) { ServiceRegistryEndpoint endpoint = new ServiceRegistryEndpoint( diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/CommonsClientAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/CommonsClientAutoConfigurationTests.java index 58b4475a9..13b3ad0b3 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/CommonsClientAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/CommonsClientAutoConfigurationTests.java @@ -51,6 +51,7 @@ public void beansCreatedNormally() { applicationContextRunner .withConfiguration( AutoConfigurations.of(HealthEndpointAutoConfiguration.class)) + .withPropertyValues("management.endpoints.web.exposure.include=features") .run(ctxt -> { then(ctxt.getBean(DiscoveryClientHealthIndicator.class)).isNotNull(); then(ctxt.getBean(DiscoveryCompositeHealthContributor.class)) @@ -63,7 +64,9 @@ public void beansCreatedNormally() { @Test public void disableAll() { applicationContextRunner - .withPropertyValues("spring.cloud.discovery.enabled=false").run(ctxt -> { + .withPropertyValues("spring.cloud.discovery.enabled=false", + "management.endpoints.web.exposure.include=features") + .run(ctxt -> { assertThat(ctxt) .doesNotHaveBean(DiscoveryClientHealthIndicator.class); assertThat(ctxt) @@ -77,7 +80,8 @@ public void disableAll() { @Test public void disableBlocking() { applicationContextRunner - .withPropertyValues("spring.cloud.discovery.blocking.enabled=false") + .withPropertyValues("spring.cloud.discovery.blocking.enabled=false", + "management.endpoints.web.exposure.include=features") .run(ctxt -> { assertThat(ctxt) .doesNotHaveBean(DiscoveryClientHealthIndicator.class); diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/ReactiveCommonsClientAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/ReactiveCommonsClientAutoConfigurationTests.java index 46062f3dc..5f35aeccc 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/ReactiveCommonsClientAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/ReactiveCommonsClientAutoConfigurationTests.java @@ -46,20 +46,24 @@ public class ReactiveCommonsClientAutoConfigurationTests { @Test public void beansCreatedNormally() { - applicationContextRunner.run(context -> { - then(context.getBean(ReactiveDiscoveryClientHealthIndicator.class)) - .isNotNull(); - then(context.getBean(ReactiveDiscoveryCompositeHealthContributor.class)) - .isNotNull(); - then(context.getBean(FeaturesEndpoint.class)).isNotNull(); - then(context.getBeansOfType(HasFeatures.class).values()).isNotEmpty(); - }); + applicationContextRunner + .withPropertyValues("management.endpoints.web.exposure.include=features") + .run(context -> { + then(context.getBean(ReactiveDiscoveryClientHealthIndicator.class)) + .isNotNull(); + then(context + .getBean(ReactiveDiscoveryCompositeHealthContributor.class)) + .isNotNull(); + then(context.getBean(FeaturesEndpoint.class)).isNotNull(); + then(context.getBeansOfType(HasFeatures.class).values()).isNotEmpty(); + }); } @Test public void disableAll() { applicationContextRunner - .withPropertyValues("spring.cloud.discovery.enabled=false") + .withPropertyValues("spring.cloud.discovery.enabled=false", + "management.endpoints.web.exposure.include=features") .run(context -> { assertThat(context).doesNotHaveBean( ReactiveDiscoveryClientHealthIndicator.class); @@ -74,7 +78,8 @@ public void disableAll() { @Test public void disableReactive() { applicationContextRunner - .withPropertyValues("spring.cloud.discovery.reactive.enabled=false") + .withPropertyValues("spring.cloud.discovery.reactive.enabled=false", + "management.endpoints.web.exposure.include=features") .run(context -> { assertThat(context).doesNotHaveBean( ReactiveDiscoveryClientHealthIndicator.class); diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java index 39ad4a7ee..5f66358c0 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java @@ -20,7 +20,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.autoconfigure.endpoint.EndpointAutoConfiguration; import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnAvailableEndpoint; -import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnEnabledEndpoint; import org.springframework.boot.actuate.autoconfigure.health.ConditionalOnEnabledHealthIndicator; import org.springframework.boot.actuate.health.Health; import org.springframework.boot.autoconfigure.AutoConfigureAfter; @@ -87,7 +86,7 @@ class RestartEndpointWithIntegrationConfiguration { private IntegrationMBeanExporter exporter; @Bean - @ConditionalOnEnabledEndpoint + @ConditionalOnAvailableEndpoint @ConditionalOnMissingBean public RestartEndpoint restartEndpoint() { RestartEndpoint endpoint = new RestartEndpoint(); @@ -104,7 +103,7 @@ public RestartEndpoint restartEndpoint() { class RestartEndpointWithoutIntegrationConfiguration { @Bean - @ConditionalOnEnabledEndpoint + @ConditionalOnAvailableEndpoint @ConditionalOnMissingBean public RestartEndpoint restartEndpointWithoutIntegration() { return new RestartEndpoint(); @@ -118,7 +117,7 @@ class PauseResumeEndpointsConfiguration { @Bean @ConditionalOnBean(RestartEndpoint.class) @ConditionalOnMissingBean - @ConditionalOnEnabledEndpoint + @ConditionalOnAvailableEndpoint public RestartEndpoint.PauseEndpoint pauseEndpoint(RestartEndpoint restartEndpoint) { return restartEndpoint.getPauseEndpoint(); } @@ -126,7 +125,7 @@ public RestartEndpoint.PauseEndpoint pauseEndpoint(RestartEndpoint restartEndpoi @Bean @ConditionalOnBean(RestartEndpoint.class) @ConditionalOnMissingBean - @ConditionalOnEnabledEndpoint + @ConditionalOnAvailableEndpoint public RestartEndpoint.ResumeEndpoint resumeEndpoint( RestartEndpoint restartEndpoint) { return restartEndpoint.getResumeEndpoint(); diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/WritableEnvironmentEndpointAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/WritableEnvironmentEndpointAutoConfiguration.java index 73a11ef71..89ed88fb6 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/WritableEnvironmentEndpointAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/WritableEnvironmentEndpointAutoConfiguration.java @@ -16,7 +16,7 @@ package org.springframework.cloud.autoconfigure; -import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnEnabledEndpoint; +import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnAvailableEndpoint; import org.springframework.boot.actuate.autoconfigure.env.EnvironmentEndpointAutoConfiguration; import org.springframework.boot.actuate.autoconfigure.env.EnvironmentEndpointProperties; import org.springframework.boot.actuate.env.EnvironmentEndpoint; @@ -61,7 +61,7 @@ public WritableEnvironmentEndpointAutoConfiguration( @Bean @ConditionalOnMissingBean - @ConditionalOnEnabledEndpoint + @ConditionalOnAvailableEndpoint public WritableEnvironmentEndpoint environmentEndpoint(Environment environment) { WritableEnvironmentEndpoint endpoint = new WritableEnvironmentEndpoint( environment); @@ -73,7 +73,7 @@ public WritableEnvironmentEndpoint environmentEndpoint(Environment environment) } @Bean - @ConditionalOnEnabledEndpoint + @ConditionalOnAvailableEndpoint public WritableEnvironmentEndpointWebExtension environmentEndpointWebExtension( WritableEnvironmentEndpoint endpoint, EnvironmentManager environment) { return new WritableEnvironmentEndpointWebExtension(endpoint, environment); diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/LifecycleMvcAutoConfigurationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/LifecycleMvcAutoConfigurationTests.java index 07ae7bc6f..da657bfcd 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/LifecycleMvcAutoConfigurationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/LifecycleMvcAutoConfigurationTests.java @@ -63,7 +63,8 @@ public void environmentWebEndpointExtensionGloballyDisabled() { @Test public void environmentWebEndpointExtensionEnabled() { beanCreated("environmentEndpointWebExtension", - "management.endpoint.env.enabled=true"); + "management.endpoint.env.enabled=true", + "management.endpoints.web.exposure.include=env"); } // restartEndpoint @@ -80,7 +81,8 @@ public void restartEndpointGloballyDisabled() { @Test public void restartEndpointEnabled() { beanCreatedAndEndpointEnabled("restartEndpoint", RestartEndpoint.class, - RestartEndpoint::restart, "management.endpoint.restart.enabled=true"); + RestartEndpoint::restart, "management.endpoint.restart.enabled=true", + "management.endpoints.web.exposure.include=restart"); } // pauseEndpoint @@ -105,6 +107,7 @@ public void pauseEndpointEnabled() { beanCreatedAndEndpointEnabled("pauseEndpoint", RestartEndpoint.PauseEndpoint.class, RestartEndpoint.PauseEndpoint::pause, "management.endpoint.restart.enabled=true", + "management.endpoints.web.exposure.include=restart,pause", "management.endpoint.pause.enabled=true"); } @@ -112,12 +115,14 @@ public void pauseEndpointEnabled() { @Test public void resumeEndpointDisabled() { beanNotCreated("resumeEndpoint", "management.endpoint.restart.enabled=true", + "management.endpoints.web.exposure.include=restart", "management.endpoint.resume.enabled=false"); } @Test public void resumeEndpointRestartDisabled() { beanNotCreated("resumeEndpoint", "management.endpoint.restart.enabled=false", + "management.endpoints.web.exposure.include=resume", "management.endpoint.resume.enabled=true"); } @@ -132,7 +137,8 @@ public void resumeEndpointEnabled() { RestartEndpoint.ResumeEndpoint.class, RestartEndpoint.ResumeEndpoint::resume, "management.endpoint.restart.enabled=true", - "management.endpoint.resume.enabled=true"); + "management.endpoint.resume.enabled=true", + "management.endpoints.web.exposure.include=restart,resume"); } private void beanNotCreated(String beanName, String... contextProperties) { diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/restart/RestartIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/restart/RestartIntegrationTests.java index 97856464e..c2024b755 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/restart/RestartIntegrationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/restart/RestartIntegrationTests.java @@ -47,6 +47,7 @@ public void testRestartTwice() throws Exception { this.context = SpringApplication.run(TestConfiguration.class, "--management.endpoint.restart.enabled=true", "--server.port=0", + "--management.endpoints.web.exposure.include=restart", "--spring.liveBeansView.mbeanDomain=livebeans"); RestartEndpoint endpoint = this.context.getBean(RestartEndpoint.class); From e37b383c04890553f40abadab7c57edba8ba6724 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 17 Jan 2020 15:52:35 -0500 Subject: [PATCH 2/5] Migrates to new OutputCaptureRule --- .../RefreshAutoConfigurationMoreClassPathTests.java | 4 ++-- .../cloud/autoconfigure/RefreshAutoConfigurationTests.java | 4 ++-- .../EnvironmentDecryptApplicationInitializerTests.java | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationMoreClassPathTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationMoreClassPathTests.java index 84ea03c5c..6e2d47d6b 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationMoreClassPathTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationMoreClassPathTests.java @@ -23,7 +23,7 @@ import org.springframework.boot.WebApplicationType; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.builder.SpringApplicationBuilder; -import org.springframework.boot.test.rule.OutputCapture; +import org.springframework.boot.test.system.OutputCaptureRule; import org.springframework.cloud.test.ClassPathExclusions; import org.springframework.cloud.test.ModifiedClassPathRunner; import org.springframework.context.ConfigurableApplicationContext; @@ -40,7 +40,7 @@ public class RefreshAutoConfigurationMoreClassPathTests { @Rule - public OutputCapture outputCapture = new OutputCapture(); + public OutputCaptureRule outputCapture = new OutputCaptureRule(); private static ConfigurableApplicationContext getApplicationContext( Class configuration, String... properties) { diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationTests.java index 315b78bcd..174594225 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/autoconfigure/RefreshAutoConfigurationTests.java @@ -25,7 +25,7 @@ import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.boot.test.rule.OutputCapture; +import org.springframework.boot.test.system.OutputCaptureRule; import org.springframework.cloud.context.refresh.ContextRefresher; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Configuration; @@ -38,7 +38,7 @@ public class RefreshAutoConfigurationTests { @Rule - public OutputCapture output = new OutputCapture(); + public OutputCaptureRule output = new OutputCaptureRule(); private static ConfigurableApplicationContext getApplicationContext( WebApplicationType type, Class configuration, String... properties) { diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java index 5638e769b..4bf69df08 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java @@ -24,7 +24,7 @@ import org.junit.Rule; import org.junit.Test; -import org.springframework.boot.test.rule.OutputCapture; +import org.springframework.boot.test.system.OutputCaptureRule; import org.springframework.boot.test.util.TestPropertyValues; import org.springframework.boot.test.util.TestPropertyValues.Type; import org.springframework.context.ApplicationContext; @@ -59,7 +59,7 @@ public class EnvironmentDecryptApplicationInitializerTests { Encryptors.noOpText()); @Rule - public OutputCapture outputCapture = new OutputCapture(); + public OutputCaptureRule outputCapture = new OutputCaptureRule(); @Test public void decryptCipherKey() { From 905af987d4230371c4085cf6aee0c2a0a7a37b4f Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 17 Jan 2020 16:25:02 -0500 Subject: [PATCH 3/5] Migrates to new OutputCaptureRule --- .../CompatibilityVerifierFailureAutoConfigurationTests.java | 4 ++-- .../cloud/configuration/CompatibilityVerifierTests.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/CompatibilityVerifierFailureAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/CompatibilityVerifierFailureAutoConfigurationTests.java index a050b4c82..b1c39fd4f 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/CompatibilityVerifierFailureAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/CompatibilityVerifierFailureAutoConfigurationTests.java @@ -23,7 +23,7 @@ import org.springframework.beans.factory.BeanCreationException; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; -import org.springframework.boot.test.rule.OutputCapture; +import org.springframework.boot.test.system.OutputCaptureRule; import org.springframework.context.annotation.Configuration; import org.springframework.core.NestedExceptionUtils; @@ -35,7 +35,7 @@ public class CompatibilityVerifierFailureAutoConfigurationTests { @Rule - public OutputCapture outputCapture = new OutputCapture(); + public OutputCaptureRule outputCapture = new OutputCaptureRule(); @Test public void contextFailsToLoad() { diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/CompatibilityVerifierTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/CompatibilityVerifierTests.java index e896ecfe4..2746d1709 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/CompatibilityVerifierTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/configuration/CompatibilityVerifierTests.java @@ -23,7 +23,7 @@ import org.junit.Rule; import org.junit.Test; -import org.springframework.boot.test.rule.OutputCapture; +import org.springframework.boot.test.system.OutputCaptureRule; import static org.assertj.core.api.BDDAssertions.then; @@ -33,7 +33,7 @@ public class CompatibilityVerifierTests { @Rule - public OutputCapture outputCapture = new OutputCapture(); + public OutputCaptureRule outputCapture = new OutputCaptureRule(); @Test public void should_not_print_the_report_when_no_errors_were_found() { From 4eda8f96c7523458980bed8813bd7ab90f8ddebd Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Mon, 3 Feb 2020 20:25:38 +0100 Subject: [PATCH 4/5] Add health check loadBalancing implementation. --- docs/src/main/asciidoc/_configprops.adoc | 5 +- .../main/asciidoc/spring-cloud-commons.adoc | 63 +++++++- .../reactive/LoadBalancerProperties.java | 57 +++++++- .../LoadBalancerClientConfiguration.java | 9 -- .../config/LoadBalancerAutoConfiguration.java | 11 ++ .../config/LoadBalancerZoneConfig.java | 42 ++++++ ...ealthCheckServiceInstanceListSupplier.java | 137 ++++++++++++++++++ ...PreferenceServiceInstanceListSupplier.java | 10 +- ...CheckServiceInstanceListSupplierTests.java | 126 ++++++++++++++++ ...renceServiceInstanceListSupplierTests.java | 14 +- 10 files changed, 443 insertions(+), 31 deletions(-) create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerZoneConfig.java create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplier.java create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplierTests.java diff --git a/docs/src/main/asciidoc/_configprops.adoc b/docs/src/main/asciidoc/_configprops.adoc index 1d313be67..9c3d46bb0 100644 --- a/docs/src/main/asciidoc/_configprops.adoc +++ b/docs/src/main/asciidoc/_configprops.adoc @@ -28,10 +28,13 @@ |spring.cloud.inetutils.timeout-seconds | 1 | Timeout, in seconds, for calculating hostname. |spring.cloud.inetutils.use-only-site-local-interfaces | false | Whether to use only interfaces with site local addresses. See {@link InetAddress#isSiteLocalAddress()} for more details. |spring.cloud.loadbalancer.cache.caffeine.spec | | The spec to use to create caches. See CaffeineSpec for more details on the spec format. +|spring.cloud.loadbalancer.cache.capacity | 256 | Initial cache capacity expressed as int. |spring.cloud.loadbalancer.cache.ttl | 30s | Time To Live - time counted from writing of the record, after which cache entries are expired, expressed as a {@link Duration}. The property {@link String} has to be in keeping with the appropriate syntax as specified in Spring Boot StringToDurationConverter. @see StringToDurationConverter.java +|spring.cloud.loadbalancer.health-check.initial-delay | 0 | Initial delay value for the HealthCheck scheduler. +|spring.cloud.loadbalancer.health-check.interval | 30s | Interval for rerunning the HealthCheck scheduler. +|spring.cloud.loadbalancer.health-check.path | | |spring.cloud.loadbalancer.retry.enabled | true | |spring.cloud.loadbalancer.ribbon.enabled | true | Causes `RibbonLoadBalancerClient` to be used by default. -|spring.cloud.loadbalancer.zone | | A {@link String} representation of the zone used for filtering instances by zoned load-balancing implementations. |spring.cloud.refresh.enabled | true | Enables autoconfiguration for the refresh scope and associated features. |spring.cloud.refresh.extra-refreshable | true | Additional class names for beans to post process into refresh scope. |spring.cloud.service-registry.auto-registration.enabled | true | Whether service auto-registration is enabled. Defaults to true. diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index 7a4356606..b85851b9b 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -906,12 +906,71 @@ public class CustomLoadBalancerConfiguration { @Bean public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier( ReactiveDiscoveryClient discoveryClient, Environment environment, - LoadBalancerProperties loadBalancerProperties, + LoadBalancerZoneConfig zoneConfig, ApplicationContext context) { DiscoveryClientServiceInstanceListSupplier firstDelegate = new DiscoveryClientServiceInstanceListSupplier( discoveryClient, environment); ZonePreferenceServiceInstanceListSupplier delegate = new ZonePreferenceServiceInstanceListSupplier(firstDelegate, - loadBalancerProperties); + zoneConfig); + ObjectProvider cacheManagerProvider = context + .getBeanProvider(LoadBalancerCacheManager.class); + if (cacheManagerProvider.getIfAvailable() != null) { + return new CachingServiceInstanceListSupplier(delegate, + cacheManagerProvider.getIfAvailable()); + } + return delegate; + } + } +---- + +=== Instance Health-Check for LoadBalancer + +It is possible to enable a scheduled HealthCheck for the LoadBalancer. The `HealthCheckServiceInstanceListSupplier` +is provided for that. It regularly verifies if the instances provided by a delegate +`ServiceInstanceListSupplier` are still alive and only returns the healthy instances, +unless there are none - then it returns all the retrieved instances. + +TIP: This mechanism is particularly helpful while using the `SimpleDiscoveryClient`. For the +clients backed by an actual Service Registry, it's not necessary to use, as we already get +healthy instances after querying the external ServiceDiscovery. + +The `HealthCheckServiceInstanceListSupplier` uses `InstanceHealthChecker` to verify if the instances are +alive. We provide a default `PingHealthChecker` instance. It uses `WebClient` to execute +requests against the `health` endpoint of the instance. You can also provide your own implementation +of `InstanceHealthChecker` instead. + +`HealthCheckServiceInstanceListSupplier` uses properties prefixed with +`spring.cloud.loadbalancer.healthcheck`. You can set the `initialDelay` and `interval` +for the scheduler. + +For the `PingHealthChecker`, you can set the default path for the healthcheck URL by setting +the value of the `spring.cloud.loadbalancer.healthcheck.path.default`. You can also set a specific value +for any given service by setting the value of the `spring.cloud.loadbalancer.healthcheck.path.[SERVICE_ID]`, +substituting the `[SERVICE_ID]` with the correct ID of your service. If the path is not set, +`/actuator/health` is used by default. + +In order to use the health-check scheduler approach, you will have to instantiate a `HealthCheckServiceInstanceListSupplier` bean in a <>. + +We use delegates to work with `ServiceInstanceListSupplier` beans. +We suggest passing a `DiscoveryClientServiceInstanceListSupplier` delegate in the constructor of `HealthCheckServiceInstanceListSupplier` and, in turn, wrapping the latter with a `CachingServiceInstanceListSupplier` to leverage <>. + +You could use this sample configuration to set it up: + +[[zoned-based-custom-loadbalancer-configuration]] +[source,java,indent=0] +---- +public class CustomLoadBalancerConfiguration { + + @Bean + public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier( + ReactiveDiscoveryClient discoveryClient, Environment environment, + LoadBalancerProperties loadBalancerProperties, + ApplicationContext context, + InstanceHealthChecker healthChecker) { + DiscoveryClientServiceInstanceListSupplier firstDelegate = new DiscoveryClientServiceInstanceListSupplier( + discoveryClient, environment); + HealthCheckServiceInstanceListSupplier delegate = new HealthCheckServiceInstanceListSupplier(firstDelegate, + loadBalancerProperties, healthChecker); ObjectProvider cacheManagerProvider = context .getBeanProvider(LoadBalancerCacheManager.class); if (cacheManagerProvider.getIfAvailable() != null) { diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java index 989310f2b..3e7f99a3f 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java @@ -16,7 +16,11 @@ package org.springframework.cloud.client.loadbalancer.reactive; +import java.time.Duration; +import java.util.Map; + import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.util.LinkedCaseInsensitiveMap; /** * A {@link ConfigurationProperties} bean for Spring Cloud LoadBalancer. @@ -28,17 +32,56 @@ public class LoadBalancerProperties { /** - * A {@link String} representation of the zone used for filtering - * instances by zoned load-balancing implementations. + * Properties for HealthCheckServiceInstanceListSupplier. */ - private String zone; + private HealthCheck healthCheck = new HealthCheck(); + + public HealthCheck getHealthCheck() { + return healthCheck; + } - public String getZone() { - return zone; + public void setHealthCheck(HealthCheck healthCheck) { + this.healthCheck = healthCheck; } - public void setZone(String zone) { - this.zone = zone; + public static class HealthCheck { + + /** + * Initial delay value for the HealthCheck scheduler. + */ + private int initialDelay = 0; + + /** + * Interval for rerunning the HealthCheck scheduler. + */ + private Duration interval = Duration.ofSeconds(30); + + private Map path = new LinkedCaseInsensitiveMap<>(); + + public int getInitialDelay() { + return initialDelay; + } + + public void setInitialDelay(int initialDelay) { + this.initialDelay = initialDelay; + } + + public Map getPath() { + return path; + } + + public void setPath(Map path) { + this.path = path; + } + + public Duration getInterval() { + return interval; + } + + public void setInterval(Duration interval) { + this.interval = interval; + } + } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java index 62d5b9a71..7c6f42828 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java @@ -19,14 +19,12 @@ import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.ConditionalOnBlockingDiscoveryEnabled; import org.springframework.cloud.client.ConditionalOnDiscoveryEnabled; import org.springframework.cloud.client.ConditionalOnReactiveDiscoveryEnabled; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient; -import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.cloud.loadbalancer.cache.LoadBalancerCacheManager; import org.springframework.cloud.loadbalancer.core.CachingServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.CachingServiceInstanceSupplier; @@ -49,18 +47,11 @@ * @author Tim Ysewyn */ @Configuration(proxyBeanMethods = false) -@EnableConfigurationProperties(LoadBalancerProperties.class) @ConditionalOnDiscoveryEnabled public class LoadBalancerClientConfiguration { private static final int REACTIVE_SERVICE_INSTANCE_SUPPLIER_ORDER = 193827465; - @Bean - @ConditionalOnMissingBean - LoadBalancerProperties loadBalancerProperties() { - return new LoadBalancerProperties(); - } - @Bean @ConditionalOnMissingBean public ReactorLoadBalancer reactorServiceInstanceLoadBalancer( diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java index 23d8a79bf..6afa79f74 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerAutoConfiguration.java @@ -22,7 +22,9 @@ import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerBeanPostProcessorAutoConfiguration; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancerAutoConfiguration; import org.springframework.cloud.client.loadbalancer.reactive.ReactorLoadBalancerClientAutoConfiguration; import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClientSpecification; @@ -30,6 +32,7 @@ import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.Environment; /** * @author Spencer Gibb @@ -37,6 +40,7 @@ */ @Configuration(proxyBeanMethods = false) @LoadBalancerClients +@EnableConfigurationProperties(LoadBalancerProperties.class) @AutoConfigureBefore({ ReactorLoadBalancerClientAutoConfiguration.class, LoadBalancerBeanPostProcessorAutoConfiguration.class, ReactiveLoadBalancerAutoConfiguration.class }) @@ -49,6 +53,13 @@ public LoadBalancerAutoConfiguration( this.configurations = configurations; } + @Bean + @ConditionalOnMissingBean + public LoadBalancerZoneConfig zoneConfig(Environment environment) { + return new LoadBalancerZoneConfig( + environment.getProperty("spring.cloud.loadbalancer.zone")); + } + @ConditionalOnMissingBean @Bean public LoadBalancerClientFactory loadBalancerClientFactory() { diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerZoneConfig.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerZoneConfig.java new file mode 100644 index 000000000..befc05dfc --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/LoadBalancerZoneConfig.java @@ -0,0 +1,42 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.config; + +/** + * @author Olga Maciaszek-Sharma + */ +public class LoadBalancerZoneConfig { + + /** + * A {@link String} representation of the zone used for filtering + * instances by zoned load-balancing implementations. + */ + private String zone; + + public LoadBalancerZoneConfig(String zone) { + this.zone = zone; + } + + public String getZone() { + return zone; + } + + public void setZone(String zone) { + this.zone = zone; + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplier.java new file mode 100644 index 000000000..d89db7e8f --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplier.java @@ -0,0 +1,137 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import reactor.core.publisher.Flux; +import reactor.core.publisher.FluxSink; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.http.HttpStatus; +import org.springframework.web.reactive.function.client.WebClient; +import org.springframework.web.util.UriComponentsBuilder; + +/** + * A {@link ServiceInstanceListSupplier} implementation that verifies whether the + * instances are alive and only returns the healthy one, unless there are none. Uses + * {@link WebClient} to ping the health endpoint of the instances. + * + * @author Olga Maciaszek-Sharma + * @since 2.2.0 + */ +public class HealthCheckServiceInstanceListSupplier + implements ServiceInstanceListSupplier { + + private static final Log LOG = LogFactory + .getLog(HealthCheckServiceInstanceListSupplier.class); + + private final ServiceInstanceListSupplier delegate; + + private final LoadBalancerProperties.HealthCheck healthCheck; + + private final WebClient webClient; + + private final String defaultHealthCheckPath; + + private List instances = Collections + .synchronizedList(new ArrayList<>()); + + private List healthyInstances = Collections + .synchronizedList(new ArrayList<>()); + + public HealthCheckServiceInstanceListSupplier(ServiceInstanceListSupplier delegate, + LoadBalancerProperties.HealthCheck healthCheck, WebClient webClient) { + this.delegate = delegate; + this.healthCheck = healthCheck; + defaultHealthCheckPath = healthCheck.getPath().getOrDefault("default", + "/actuator/health"); + this.webClient = webClient; + initInstances(); + + } + + private void initInstances() { + delegate.get().subscribe(delegateInstances -> { + instances.clear(); + instances.addAll(delegateInstances); + }); + + Flux> healthCheckFlux = healthCheckFlux(); + + healthCheckFlux.subscribe(verifiedInstances -> { + healthyInstances.clear(); + healthyInstances.addAll(verifiedInstances); + }); + } + + protected Flux> healthCheckFlux() { + return Flux.create(emitter -> Schedulers + .newSingle("Health Check Verifier: " + getServiceId(), true) + .schedulePeriodically(() -> { + List verifiedInstances = new ArrayList<>(); + Flux.fromIterable(instances).filterWhen(this::isAlive) + .subscribe(serviceInstance -> { + verifiedInstances.add(serviceInstance); + emitter.next(verifiedInstances); + }); + }, healthCheck.getInitialDelay(), healthCheck.getInterval().toMillis(), + TimeUnit.MILLISECONDS), + FluxSink.OverflowStrategy.LATEST); + } + + @Override + public String getServiceId() { + return delegate.getServiceId(); + } + + @Override + public Flux> get() { + if (!healthyInstances.isEmpty()) { + return Flux.defer(() -> Flux.fromIterable(healthyInstances).collectList()); + } + // If there are no healthy instances, it might be better to still retry on all of + // them + if (LOG.isWarnEnabled()) { + LOG.warn( + "No verified healthy instances were found, returning all listed instances."); + } + return Flux.defer(() -> Flux.fromIterable(instances).collectList()); + } + + // Visible for tests + Mono isAlive(ServiceInstance serviceInstance) { + String healthCheckPropertyValue = healthCheck.getPath() + .get(serviceInstance.getServiceId()); + String healthCheckPath = healthCheckPropertyValue != null + ? healthCheckPropertyValue : defaultHealthCheckPath; + return webClient.get() + .uri(UriComponentsBuilder.fromUri(serviceInstance.getUri()) + .path(healthCheckPath).build().toUri()) + .exchange() + .map(clientResponse -> HttpStatus.OK.equals(clientResponse.statusCode())); + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplier.java index 19192d592..7a2b222d5 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplier.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplier.java @@ -23,7 +23,7 @@ import reactor.core.publisher.Flux; import org.springframework.cloud.client.ServiceInstance; -import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.cloud.loadbalancer.config.LoadBalancerZoneConfig; /** * An implementation of {@link ServiceInstanceListSupplier} that filters instances @@ -42,14 +42,14 @@ public class ZonePreferenceServiceInstanceListSupplier private final ServiceInstanceListSupplier delegate; - private final LoadBalancerProperties loadBalancerProperties; + private final LoadBalancerZoneConfig zoneConfig; private String zone; public ZonePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate, - LoadBalancerProperties loadBalancerProperties) { + LoadBalancerZoneConfig zoneConfig) { this.delegate = delegate; - this.loadBalancerProperties = loadBalancerProperties; + this.zoneConfig = zoneConfig; } @Override @@ -64,7 +64,7 @@ public Flux> get() { private List filteredByZone(List serviceInstances) { if (zone == null) { - zone = loadBalancerProperties.getZone(); + zone = zoneConfig.getZone(); } if (zone != null) { List filteredInstances = new ArrayList<>(); diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplierTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplierTests.java new file mode 100644 index 000000000..8f6961a81 --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplierTests.java @@ -0,0 +1,126 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.web.server.LocalServerPort; +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.context.annotation.Configuration; +import org.springframework.mock.env.MockEnvironment; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.reactive.function.client.WebClient; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link HealthCheckServiceInstanceListSupplier}. + * + * @author Olga Maciaszek-Sharma + */ +@ExtendWith(SpringExtension.class) +@SpringBootTest( + classes = HealthCheckServiceInstanceListSupplierTests.TestApplication.class, + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +class HealthCheckServiceInstanceListSupplierTests { + + @LocalServerPort + private int port; + + private final WebClient webClient = WebClient.create(); + + private LoadBalancerProperties.HealthCheck healthCheck = new LoadBalancerProperties.HealthCheck(); + + @SuppressWarnings("ConstantConditions") + @Test + void shouldCheckInstanceWithProvidedHealthCheckPath() { + healthCheck.getPath().put("ignored-service", "/health"); + HealthCheckServiceInstanceListSupplier listSupplier = new HealthCheckServiceInstanceListSupplier( + ServiceInstanceListSupplier.FixedServiceInstanceListSupplier + .with(new MockEnvironment()).build(), + healthCheck, webClient); + ServiceInstance serviceInstance = new DefaultServiceInstance("ignored-service-1", + "ignored-service", "127.0.0.1", port, false); + + boolean alive = listSupplier.isAlive(serviceInstance).block(); + + assertThat(alive).isTrue(); + } + + @SuppressWarnings("ConstantConditions") + @Test + void shouldCheckInstanceWithDefaultHealthCheckPath() { + HealthCheckServiceInstanceListSupplier listSupplier = new HealthCheckServiceInstanceListSupplier( + ServiceInstanceListSupplier.FixedServiceInstanceListSupplier + .with(new MockEnvironment()).build(), + healthCheck, webClient); + ServiceInstance serviceInstance = new DefaultServiceInstance("ignored-service-1", + "ignored-service", "127.0.0.1", port, false); + + boolean alive = listSupplier.isAlive(serviceInstance).block(); + + assertThat(alive).isTrue(); + } + + @SuppressWarnings("ConstantConditions") + @Test + void shouldReturnFalseIfEndpointNotFound() { + healthCheck.getPath().put("ignored-service", "/test"); + HealthCheckServiceInstanceListSupplier listSupplier = new HealthCheckServiceInstanceListSupplier( + ServiceInstanceListSupplier.FixedServiceInstanceListSupplier + .with(new MockEnvironment()).build(), + healthCheck, webClient); + ServiceInstance serviceInstance = new DefaultServiceInstance("ignored-service-1", + "ignored-service", "127.0.0.1", port, false); + + boolean alive = listSupplier.isAlive(serviceInstance).block(); + + assertThat(alive).isFalse(); + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @RestController + static class TestApplication { + + public static void main(String[] args) { + SpringApplication.run( + HealthCheckServiceInstanceListSupplierTests.TestApplication.class, + args); + } + + @GetMapping("/health") + void healthCheck() { + + } + + @GetMapping("/actuator/health") + void defaultHealthCheck() { + + } + + } + +} diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplierTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplierTests.java index bdc1f56d5..b62ab753c 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplierTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplierTests.java @@ -27,7 +27,7 @@ import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; -import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; +import org.springframework.cloud.loadbalancer.config.LoadBalancerZoneConfig; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; @@ -44,10 +44,10 @@ class ZonePreferenceServiceInstanceListSupplierTests { private DiscoveryClientServiceInstanceListSupplier delegate = mock( DiscoveryClientServiceInstanceListSupplier.class); - private LoadBalancerProperties loadBalancerProperties = new LoadBalancerProperties(); + private LoadBalancerZoneConfig zoneConfig = new LoadBalancerZoneConfig(null); private ZonePreferenceServiceInstanceListSupplier supplier = new ZonePreferenceServiceInstanceListSupplier( - delegate, loadBalancerProperties); + delegate, zoneConfig); private ServiceInstance first = serviceInstance("test-1", buildZoneMetadata("zone1")); @@ -63,7 +63,7 @@ class ZonePreferenceServiceInstanceListSupplierTests { @Test void shouldFilterInstancesByZone() { - loadBalancerProperties.setZone("zone1"); + zoneConfig.setZone("zone1"); when(delegate.get()).thenReturn( Flux.just(Arrays.asList(first, second, third, fourth, fifth))); @@ -78,7 +78,7 @@ void shouldFilterInstancesByZone() { @Test void shouldReturnAllInstancesIfNoZoneInstances() { - loadBalancerProperties.setZone("zone1"); + zoneConfig.setZone("zone1"); when(delegate.get()).thenReturn(Flux.just(Arrays.asList(third, fourth))); List filtered = supplier.get().blockFirst(); @@ -89,7 +89,7 @@ void shouldReturnAllInstancesIfNoZoneInstances() { @Test void shouldNotThrowNPEIfNullInstanceMetadata() { - loadBalancerProperties.setZone("zone1"); + zoneConfig.setZone("zone1"); when(delegate.get()).thenReturn( Flux.just(Collections.singletonList(serviceInstance("test-6", null)))); assertThatCode(() -> supplier.get().blockFirst()).doesNotThrowAnyException(); @@ -97,7 +97,7 @@ void shouldNotThrowNPEIfNullInstanceMetadata() { @Test void shouldReturnAllInstancesIfNoZone() { - loadBalancerProperties.setZone(null); + zoneConfig.setZone(null); when(delegate.get()) .thenReturn(Flux.just(Arrays.asList(first, second, third, fourth))); From dafc04b2387418e64a00a1a98a8359dc508b58f3 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Tue, 4 Feb 2020 09:50:48 +0100 Subject: [PATCH 5/5] Make isAlive() method protected. --- .../core/HealthCheckServiceInstanceListSupplier.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplier.java index d89db7e8f..0374a00d8 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplier.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HealthCheckServiceInstanceListSupplier.java @@ -121,8 +121,7 @@ public Flux> get() { return Flux.defer(() -> Flux.fromIterable(instances).collectList()); } - // Visible for tests - Mono isAlive(ServiceInstance serviceInstance) { + protected Mono isAlive(ServiceInstance serviceInstance) { String healthCheckPropertyValue = healthCheck.getPath() .get(serviceInstance.getServiceId()); String healthCheckPath = healthCheckPropertyValue != null