From ecf12aa0e71a1fbef1350dbf83964e829a0eb8f6 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Tue, 14 Mar 2017 12:24:38 -0600 Subject: [PATCH] Makes ServerListUpdater respect configuration. By default, `LoadBalancerBuilder` built a load-balancer with a `PollingServerListUpdater`. This change introduces a `ServerListUpdater` bean which creates a `PollingServerListUpdater(IClientConfig)` so that `ribbon.ServerListRefreshInterval` is respected. fixes gh-1304 --- .../main/asciidoc/spring-cloud-netflix.adoc | 1 + .../ribbon/RibbonClientConfiguration.java | 18 +++++--- .../RibbonClientConfigurationTests.java | 46 +++++++++++++++++-- 3 files changed, 53 insertions(+), 12 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index df550743..dacac0e5 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -737,6 +737,7 @@ Spring Cloud Netflix provides the following beans by default for ribbon * `ServerList` ribbonServerList: `ConfigurationBasedServerList` * `ServerListFilter` ribbonServerListFilter: `ZonePreferenceServerListFilter` * `ILoadBalancer` ribbonLoadBalancer: `ZoneAwareLoadBalancer` +* `ServerListUpdater` ribbonServerListUpdater: `PollingServerListUpdater` Creating a bean of one of those type and placing it in a `@RibbonClient` configuration (such as `FooConfiguration` above) allows you to override each diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java index 402b6f06..d33c214e 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java @@ -48,10 +48,11 @@ import com.netflix.loadbalancer.DummyPing; import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.IPing; import com.netflix.loadbalancer.IRule; -import com.netflix.loadbalancer.LoadBalancerBuilder; +import com.netflix.loadbalancer.PollingServerListUpdater; import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerList; import com.netflix.loadbalancer.ServerListFilter; +import com.netflix.loadbalancer.ServerListUpdater; import com.netflix.loadbalancer.ZoneAvoidanceRule; import com.netflix.loadbalancer.ZoneAwareLoadBalancer; import com.netflix.niws.client.http.RestClient; @@ -226,19 +227,22 @@ public class RibbonClientConfiguration { } } + @Bean + @ConditionalOnMissingBean + public ServerListUpdater ribbonServerListUpdater(IClientConfig config) { + return new PollingServerListUpdater(config); + } + @Bean @ConditionalOnMissingBean public ILoadBalancer ribbonLoadBalancer(IClientConfig config, ServerList serverList, ServerListFilter serverListFilter, - IRule rule, IPing ping) { + IRule rule, IPing ping, ServerListUpdater serverListUpdater) { if (this.propertiesFactory.isSet(ILoadBalancer.class, name)) { return this.propertiesFactory.get(ILoadBalancer.class, config, name); } - ZoneAwareLoadBalancer balancer = LoadBalancerBuilder.newBuilder() - .withClientConfig(config).withRule(rule).withPing(ping) - .withServerListFilter(serverListFilter).withDynamicServerList(serverList) - .buildDynamicServerListLoadBalancer(); - return balancer; + return new ZoneAwareLoadBalancer<>(config, rule, ping, serverList, + serverListFilter, serverListUpdater); } @Bean diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java index be0bbe45..df835d11 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java @@ -20,26 +20,38 @@ import java.net.URI; import java.util.ArrayList; import java.util.List; -import com.netflix.client.AbstractLoadBalancerAwareClient; -import com.netflix.niws.client.http.RestClient; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.ListableBeanFactory; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.test.util.EnvironmentTestUtils; import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration.OverrideRestClient; +import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; +import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.util.ReflectionTestUtils; +import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.PollingServerListUpdater; import com.netflix.loadbalancer.Server; -import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; -import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; -import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import com.netflix.loadbalancer.ServerListUpdater; +import com.netflix.loadbalancer.ZoneAwareLoadBalancer; +import com.netflix.niws.client.http.RestClient; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.when; @@ -184,6 +196,30 @@ public class RibbonClientConfigurationTests { requiredType).length > 0; } + @Test + public void testLoadBalancerConstruction() { + ConfigurableApplicationContext context = new SpringApplicationBuilder(TestLBConfig.class).properties( + "test.ribbon.ServerListRefreshInterval=999") + .run(); + + SpringClientFactory clientFactory = context.getBean(SpringClientFactory.class); + ILoadBalancer loadBalancer = clientFactory.getInstance("test", ILoadBalancer.class); + assertThat(loadBalancer, is(instanceOf(ZoneAwareLoadBalancer.class))); + ZoneAwareLoadBalancer lb = (ZoneAwareLoadBalancer) loadBalancer; + ServerListUpdater serverListUpdater = (PollingServerListUpdater) ReflectionTestUtils.getField(loadBalancer, "serverListUpdater"); + Long refreshIntervalMs = (Long) ReflectionTestUtils.getField(serverListUpdater, "refreshIntervalMs"); + assertThat(refreshIntervalMs, equalTo(999L)); + + ServerListUpdater updater = clientFactory.getInstance("test", ServerListUpdater.class); + assertThat(updater, is(sameInstance(serverListUpdater))); + + context.close(); + } + + @Configuration + @EnableAutoConfiguration + protected static class TestLBConfig { } + static class TestRestClient extends OverrideRestClient { private TestRestClient(IClientConfig ncc) {