From 8c054083ccecc5a78768d7fee92f12c16c95a03a Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Thu, 19 Dec 2019 17:27:40 +0100 Subject: [PATCH] Use configuration properties instead of environment. (#662) * Use configuration properties instead of environment. * Fix after review. --- docs/src/main/asciidoc/_configprops.adoc | 1 + .../main/asciidoc/spring-cloud-commons.adoc | 13 +++--- .../reactive/LoadBalancerProperties.java | 44 +++++++++++++++++++ .../LoadBalancerClientConfiguration.java | 9 +++- ...PreferenceServiceInstanceListSupplier.java | 10 ++--- ...renceServiceInstanceListSupplierTests.java | 17 +++---- 6 files changed, 72 insertions(+), 22 deletions(-) create mode 100644 spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java diff --git a/docs/src/main/asciidoc/_configprops.adoc b/docs/src/main/asciidoc/_configprops.adoc index 7b4901df..8f17cbd9 100644 --- a/docs/src/main/asciidoc/_configprops.adoc +++ b/docs/src/main/asciidoc/_configprops.adoc @@ -31,6 +31,7 @@ |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.retry.enabled | true | |spring.cloud.loadbalancer.ribbon.enabled | true | Causes `RibbonLoadBalancerClient` to be used by default. +|spring.cloud.loadbalancer.zone | | |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 4a1e6ed6..3ebf1287 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -881,6 +881,8 @@ We use `DiscoveryClient`-specific `zone` configuration (for example, `eureka.ins NOTE: You can also override `DiscoveryClient`-specific zone setup by setting the value of `spring.cloud.loadbalancer.zone` property. +WARNING: For the time being, only Eureka Discovery Client is instrumented to set the LoadBalancer zone. For other discovery client, set the `spring.cloud.loadbalancer.zone` property. More instrumentations coming shortly. + NOTE: To determine the zone of a retrieved `ServiceInstance`, we check the value under the `"zone"` key in its metadata map. The `ZonePreferenceServiceInstanceListSupplier` filters retrieved instances and only returns the ones within the same zone. @@ -901,11 +903,12 @@ public class CustomLoadBalancerConfiguration { @Bean public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier( ReactiveDiscoveryClient discoveryClient, Environment environment, + LoadBalancerProperties loadBalancerProperties, ApplicationContext context) { DiscoveryClientServiceInstanceListSupplier firstDelegate = new DiscoveryClientServiceInstanceListSupplier( discoveryClient, environment); ZonePreferenceServiceInstanceListSupplier delegate = new ZonePreferenceServiceInstanceListSupplier(firstDelegate, - environment); + loadBalancerProperties); ObjectProvider cacheManagerProvider = context .getBeanProvider(LoadBalancerCacheManager.class); if (cacheManagerProvider.getIfAvailable() != null) { @@ -914,8 +917,7 @@ public class CustomLoadBalancerConfiguration { } return delegate; } - -} + } ---- [[spring-cloud-loadbalancer-starter]] @@ -952,10 +954,9 @@ public class MyConfiguration { ---- ==== -You can use this feature to instantiate different implementations of `ServiceInstanceListSupplier` or `ReactorLoadBalancer`, -either written by you, or provided by us as alternatives (for example `ZonePreferenceServiceInstanceListSupplier`) to override the default setup. +You can use this feature to instantiate different implementations of `ServiceInstanceListSupplier` or `ReactorLoadBalancer`, either written by you, or provided by us as alternatives (for example `ZonePreferenceServiceInstanceListSupplier`) to override the default setup. -You can see an example of a custom cofiguration <>. +You can see an example of a custom configuration <>. NOTE: The annotation `value` arguments (`stores` in the example above) specifies the service id of the service that we should send the requests to with the given custom configuration. 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 new file mode 100644 index 00000000..989310f2 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerProperties.java @@ -0,0 +1,44 @@ +/* + * 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.client.loadbalancer.reactive; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * A {@link ConfigurationProperties} bean for Spring Cloud LoadBalancer. + * + * @author Olga Maciaszek-Sharma + * @since 2.2.1 + */ +@ConfigurationProperties("spring.cloud.loadbalancer") +public class LoadBalancerProperties { + + /** + * A {@link String} representation of the zone used for filtering + * instances by zoned load-balancing implementations. + */ + private String 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/annotation/LoadBalancerClientConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java index 35fcc800..62d5b9a7 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 @@ -26,6 +26,7 @@ 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; @@ -48,12 +49,18 @@ import org.springframework.core.env.Environment; * @author Tim Ysewyn */ @Configuration(proxyBeanMethods = false) -@EnableConfigurationProperties +@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/core/ZonePreferenceServiceInstanceListSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplier.java index a05ba3f7..19192d59 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 java.util.Map; import reactor.core.publisher.Flux; import org.springframework.cloud.client.ServiceInstance; -import org.springframework.core.env.Environment; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; /** * An implementation of {@link ServiceInstanceListSupplier} that filters instances @@ -42,14 +42,14 @@ public class ZonePreferenceServiceInstanceListSupplier private final ServiceInstanceListSupplier delegate; - private final Environment environment; + private final LoadBalancerProperties loadBalancerProperties; private String zone; public ZonePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate, - Environment environment) { + LoadBalancerProperties loadBalancerProperties) { this.delegate = delegate; - this.environment = environment; + this.loadBalancerProperties = loadBalancerProperties; } @Override @@ -64,7 +64,7 @@ public class ZonePreferenceServiceInstanceListSupplier private List filteredByZone(List serviceInstances) { if (zone == null) { - zone = environment.getProperty("spring.cloud.loadbalancer.zone"); + zone = loadBalancerProperties.getZone(); } if (zone != null) { List filteredInstances = new ArrayList<>(); 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 9c3d6ef2..bdc1f56d 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 reactor.core.publisher.Flux; import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; -import org.springframework.core.env.Environment; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; 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 Environment environment = mock(Environment.class); + private LoadBalancerProperties loadBalancerProperties = new LoadBalancerProperties(); private ZonePreferenceServiceInstanceListSupplier supplier = new ZonePreferenceServiceInstanceListSupplier( - delegate, environment); + delegate, loadBalancerProperties); private ServiceInstance first = serviceInstance("test-1", buildZoneMetadata("zone1")); @@ -63,8 +63,7 @@ class ZonePreferenceServiceInstanceListSupplierTests { @Test void shouldFilterInstancesByZone() { - when(environment.getProperty("spring.cloud.loadbalancer.zone")) - .thenReturn("zone1"); + loadBalancerProperties.setZone("zone1"); when(delegate.get()).thenReturn( Flux.just(Arrays.asList(first, second, third, fourth, fifth))); @@ -79,8 +78,7 @@ class ZonePreferenceServiceInstanceListSupplierTests { @Test void shouldReturnAllInstancesIfNoZoneInstances() { - when(environment.getProperty("spring.cloud.loadbalancer.zone")) - .thenReturn("zone1"); + loadBalancerProperties.setZone("zone1"); when(delegate.get()).thenReturn(Flux.just(Arrays.asList(third, fourth))); List filtered = supplier.get().blockFirst(); @@ -91,8 +89,7 @@ class ZonePreferenceServiceInstanceListSupplierTests { @Test void shouldNotThrowNPEIfNullInstanceMetadata() { - when(environment.getProperty("spring.cloud.loadbalancer.zone")) - .thenReturn("zone1"); + loadBalancerProperties.setZone("zone1"); when(delegate.get()).thenReturn( Flux.just(Collections.singletonList(serviceInstance("test-6", null)))); assertThatCode(() -> supplier.get().blockFirst()).doesNotThrowAnyException(); @@ -100,7 +97,7 @@ class ZonePreferenceServiceInstanceListSupplierTests { @Test void shouldReturnAllInstancesIfNoZone() { - when(environment.getProperty("spring.cloud.loadbalancer.zone")).thenReturn(null); + loadBalancerProperties.setZone(null); when(delegate.get()) .thenReturn(Flux.just(Arrays.asList(first, second, third, fourth)));