From 1485c031ff27a3a7b843869b65d9c70a711636b8 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 2 Nov 2015 23:58:55 +0100 Subject: [PATCH] Dependency based load balancer fixes. Removes dead instances and recognizes new instances. fixes gh-45 --- ...ZookeeperDiscoveryClientConfiguration.java | 8 +- .../ZookeeperRibbonClientConfiguration.java | 31 +++-- .../DependenciesBasedLoadBalancer.java | 12 +- .../dependency/ZookeeperDependencies.java | 11 +- .../CustomZookeeperServiceDiscovery.groovy | 43 +++++++ .../dependency/DependencyConfig.groovy | 104 +++++++++++++++++ .../dependency/StickyRuleISpec.groovy | 9 ++ ...eeperDiscoveryWithDependenciesISpec.groovy | 106 +++++------------- ...DiscoveryWithDyingDependenciesISpec.groovy | 80 +++++++++++++ .../discovery/test/TestRibbonClient.groovy | 30 ++--- .../test/TestServiceRestClient.groovy | 6 +- ...DefaultDependencyWatcherSpringISpec.groovy | 60 +++------- .../src/test/resources/application-client.yml | 6 + .../src/test/resources/application-server.yml | 2 + 14 files changed, 348 insertions(+), 160 deletions(-) create mode 100644 spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/CustomZookeeperServiceDiscovery.groovy create mode 100644 spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/DependencyConfig.groovy create mode 100644 spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDyingDependenciesISpec.groovy create mode 100644 spring-cloud-zookeeper-discovery/src/test/resources/application-client.yml create mode 100644 spring-cloud-zookeeper-discovery/src/test/resources/application-server.yml diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClientConfiguration.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClientConfiguration.java index 2aec67f0..2cd91cf3 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClientConfiguration.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClientConfiguration.java @@ -56,13 +56,13 @@ public class ZookeeperDiscoveryClientConfiguration { } @Bean - public ZookeeperLifecycle zookeeperLifecycle() { - return new ZookeeperLifecycle(zookeeperDiscoveryProperties(), zookeeperServiceDiscovery()); + public ZookeeperLifecycle zookeeperLifecycle(ZookeeperServiceDiscovery zookeeperServiceDiscovery) { + return new ZookeeperLifecycle(zookeeperDiscoveryProperties(), zookeeperServiceDiscovery); } @Bean - public ZookeeperDiscoveryClient zookeeperDiscoveryClient() { - return new ZookeeperDiscoveryClient(zookeeperServiceDiscovery(), zookeeperDependencies); + public ZookeeperDiscoveryClient zookeeperDiscoveryClient(ZookeeperServiceDiscovery zookeeperServiceDiscovery) { + return new ZookeeperDiscoveryClient(zookeeperServiceDiscovery, zookeeperDependencies); } @Bean diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperRibbonClientConfiguration.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperRibbonClientConfiguration.java index 69b6fbcf..6f3dc681 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperRibbonClientConfiguration.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperRibbonClientConfiguration.java @@ -16,11 +16,16 @@ package org.springframework.cloud.zookeeper.discovery; -import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses; -import static com.netflix.client.config.CommonClientConfigKey.EnableZoneAffinity; - import javax.annotation.PostConstruct; +import com.netflix.client.config.IClientConfig; +import com.netflix.config.ConfigurationManager; +import com.netflix.config.DynamicPropertyFactory; +import com.netflix.config.DynamicStringProperty; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.IPing; +import com.netflix.loadbalancer.PingUrl; +import com.netflix.loadbalancer.ServerList; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -31,12 +36,8 @@ import org.springframework.cloud.zookeeper.discovery.dependency.ZookeeperDepende import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import com.netflix.client.config.IClientConfig; -import com.netflix.config.ConfigurationManager; -import com.netflix.config.DynamicPropertyFactory; -import com.netflix.config.DynamicStringProperty; -import com.netflix.loadbalancer.ILoadBalancer; -import com.netflix.loadbalancer.ServerList; +import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses; +import static com.netflix.client.config.CommonClientConfigKey.EnableZoneAffinity; /** * Preprocessor that configures defaults for zookeeper-discovered ribbon clients. Such as: @@ -74,8 +75,16 @@ public class ZookeeperRibbonClientConfiguration { @ConditionalOnMissingBean @ConditionalOnDependenciesPassed @ConditionalOnProperty(value = "spring.cloud.zookeeper.dependencies.ribbon.loadbalancer", matchIfMissing = true) - public ILoadBalancer dependenciesBasedLoadBalancer(ZookeeperDependencies zookeeperDependencies, ServerList serverList) { - return new DependenciesBasedLoadBalancer(zookeeperDependencies, serverList); + public ILoadBalancer dependenciesBasedLoadBalancer(ZookeeperDependencies zookeeperDependencies, + ServerList serverList, IClientConfig config, IPing iPing) { + return new DependenciesBasedLoadBalancer(zookeeperDependencies, serverList, config, iPing); + } + + @Bean + @ConditionalOnMissingBean + @ConditionalOnDependenciesPassed + public IPing healthCheckingRule(ZookeeperDependencies zookeeperDependencies) { + return new PingUrl(false, zookeeperDependencies.getDefaultHealthEndpoint()); } @Bean diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/DependenciesBasedLoadBalancer.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/DependenciesBasedLoadBalancer.java index e6a93c76..a0b02ee8 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/DependenciesBasedLoadBalancer.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/DependenciesBasedLoadBalancer.java @@ -19,7 +19,9 @@ package org.springframework.cloud.zookeeper.discovery.dependency; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import com.netflix.loadbalancer.BaseLoadBalancer; +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.DynamicServerListLoadBalancer; +import com.netflix.loadbalancer.IPing; import com.netflix.loadbalancer.IRule; import com.netflix.loadbalancer.RandomRule; import com.netflix.loadbalancer.RoundRobinRule; @@ -35,15 +37,18 @@ import lombok.extern.slf4j.Slf4j; * @author Marcin Grzejszczak, 4financeIT */ @Slf4j -public class DependenciesBasedLoadBalancer extends BaseLoadBalancer { +public class DependenciesBasedLoadBalancer extends DynamicServerListLoadBalancer { private final Map ruleCache = new ConcurrentHashMap<>(); private final ZookeeperDependencies zookeeperDependencies; - public DependenciesBasedLoadBalancer(ZookeeperDependencies zookeeperDependencies, ServerList serverList) { + public DependenciesBasedLoadBalancer(ZookeeperDependencies zookeeperDependencies, ServerList serverList, IClientConfig config, IPing iPing) { + super(config); this.zookeeperDependencies = zookeeperDependencies; setServersList(serverList.getInitialListOfServers()); + setPing(iPing); + setServerListImpl(serverList); } @Override @@ -56,6 +61,7 @@ public class DependenciesBasedLoadBalancer extends BaseLoadBalancer { }; cacheEntryIfMissing(keyAsString, dependency); log.debug("Will try to retrieve dependency for key [{}]. Current cache contents [{}]", keyAsString, this.ruleCache); + updateListOfServers(); return this.ruleCache.get(keyAsString).choose(key); } diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependencies.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependencies.java index 2be752cd..d7e2930f 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependencies.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependencies.java @@ -23,11 +23,12 @@ import java.util.Map; import javax.annotation.PostConstruct; -import lombok.Data; - +import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.util.StringUtils; +import lombok.Data; + /** * Representation of this service's dependencies in Zookeeper * @@ -48,6 +49,12 @@ public class ZookeeperDependencies { */ private Map dependencies = new LinkedHashMap<>(); + /** + * Default health endpoint that will be checked to verify that a dependency is alive + */ + @Value("${spring.cloud.zookeeper.dependencies.ribbon.loadbalancer.defaulthealthendpoint:/health}") + private String defaultHealthEndpoint; + @PostConstruct public void init() { if (StringUtils.hasText(prefix) && !prefix.endsWith("/")) { diff --git a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/CustomZookeeperServiceDiscovery.groovy b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/CustomZookeeperServiceDiscovery.groovy new file mode 100644 index 00000000..94a301ca --- /dev/null +++ b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/CustomZookeeperServiceDiscovery.groovy @@ -0,0 +1,43 @@ +package org.springframework.cloud.zookeeper.discovery + +import javax.annotation.PreDestroy + +import org.apache.curator.framework.CuratorFramework +import org.apache.curator.x.discovery.ServiceDiscoveryBuilder +import org.apache.curator.x.discovery.ServiceInstance +import org.apache.curator.x.discovery.UriSpec + +class CustomZookeeperServiceDiscovery extends ZookeeperServiceDiscovery { + + private final String applicationName + + CustomZookeeperServiceDiscovery(String applicationName, CuratorFramework curator) { + super(curator, null, null) + this.applicationName = applicationName + build() + } + + @Override + void build() { + setPort(10) + def instance = ServiceInstance.builder().uriSpec(new UriSpec("{scheme}://{address}:{port}/")) + .address('anyUrl') + .port(10) + .name(applicationName) + .build() + getServiceInstanceRef().set(instance) + def discovery = ServiceDiscoveryBuilder + .builder(Void) + .basePath('/') + .client(getCurator()) + .thisInstance(instance) + .build() + getServiceDiscoveryRef().set(discovery) + discovery.start() + } + + @PreDestroy + void close() { + getServiceDiscoveryRef().get().close() + } +} \ No newline at end of file diff --git a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/DependencyConfig.groovy b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/DependencyConfig.groovy new file mode 100644 index 00000000..9bdc8ca7 --- /dev/null +++ b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/DependencyConfig.groovy @@ -0,0 +1,104 @@ +package org.springframework.cloud.zookeeper.discovery.dependency +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.actuate.autoconfigure.EndpointMBeanExportAutoConfiguration +import org.springframework.boot.autoconfigure.EnableAutoConfiguration +import org.springframework.boot.context.embedded.EmbeddedServletContainerInitializedEvent +import org.springframework.cloud.client.discovery.EnableDiscoveryClient +import org.springframework.cloud.client.loadbalancer.LoadBalanced +import org.springframework.cloud.netflix.feign.EnableFeignClients +import org.springframework.cloud.netflix.feign.FeignClient +import org.springframework.cloud.zookeeper.discovery.ZookeeperServiceDiscovery +import org.springframework.cloud.zookeeper.discovery.test.CommonTestConfig +import org.springframework.cloud.zookeeper.discovery.test.TestRibbonClient +import org.springframework.context.ApplicationListener +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration +import org.springframework.context.annotation.Import +import org.springframework.web.bind.annotation.RequestHeader +import org.springframework.web.bind.annotation.RequestMapping +import org.springframework.web.bind.annotation.RequestMethod +import org.springframework.web.bind.annotation.RestController +import org.springframework.web.client.RestTemplate + +@Configuration +@EnableAutoConfiguration(exclude = [EndpointMBeanExportAutoConfiguration]) +@Import(CommonTestConfig) +@EnableDiscoveryClient +@EnableFeignClients(basePackageClasses = [AliasUsingFeignClient, IdUsingFeignClient]) +class DependencyConfig { + + @Autowired ZookeeperServiceDiscovery zookeeperServiceDiscovery + + @Bean + TestRibbonClient testRibbonClient(@LoadBalanced RestTemplate restTemplate) { + return new TestRibbonClient(restTemplate) + } + + @Bean + PingController pingController() { + return new PingController(portListener()) + } + + @Bean + PortListener portListener() { + return new PortListener() + } + +} + +class PortListener implements ApplicationListener { + + private int port + + @Override + public void onApplicationEvent(EmbeddedServletContainerInitializedEvent event) { + this.port = event.getEmbeddedServletContainer().getPort() + } + + public int getPort() { + return port + } + +} + +@FeignClient("someAlias") +interface AliasUsingFeignClient { + @RequestMapping(method = RequestMethod.GET, value = "/beans") + String getBeans() + + @RequestMapping(method = RequestMethod.GET, value = "/checkHeaders") + String checkHeaders() +} + +@FeignClient("nameWithoutAlias") +interface IdUsingFeignClient { + @RequestMapping(method = RequestMethod.GET, value = "/beans") + String getBeans() +} + +@RestController +class PingController { + + private final PortListener portListener + + PingController(PortListener portListener) { + this.portListener = portListener + } + + @RequestMapping('/ping') String ping() { + return 'pong' + } + + @RequestMapping('/port') Integer port() { + return portListener.port + } + + @RequestMapping('/checkHeaders') String checkHeaders(@RequestHeader('Content-Type') String contentType, + @RequestHeader('header1') Collection header1, + @RequestHeader('header2') Collection header2) { + assert contentType == 'application/vnd.newsletter.v1+json' + assert header1 == ['value1'] as Set + assert header2 == ['value2'] as Set + return 'ok' + } +} diff --git a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/StickyRuleISpec.groovy b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/StickyRuleISpec.groovy index 6e4bbd16..e4cfd4ca 100644 --- a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/StickyRuleISpec.groovy +++ b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/StickyRuleISpec.groovy @@ -14,6 +14,8 @@ * limitations under the License. */ package org.springframework.cloud.zookeeper.discovery.dependency +import com.netflix.loadbalancer.IPing +import com.netflix.loadbalancer.NoOpPing import org.apache.curator.framework.CuratorFramework import org.apache.curator.test.TestingServer import org.springframework.beans.factory.annotation.Autowired @@ -34,6 +36,7 @@ import org.springframework.test.context.ContextConfiguration import org.springframework.util.SocketUtils import spock.lang.Specification import spock.util.concurrent.PollingConditions +import spock.util.environment.RestoreSystemProperties @ContextConfiguration(classes = Config, loader = SpringApplicationContextLoader) @ActiveProfiles('loadbalancerclient') @@ -48,7 +51,10 @@ class StickyRuleISpec extends Specification implements PollingUtils { conditions = new PollingConditions() } + @RestoreSystemProperties def 'should use sticky load balancing strategy taken from Zookeeper dependencies'() { + given: + System.setProperty('spring.cloud.zookeeper.dependencies.ribbon.loadbalancer.checkping', 'false') expect: thereAreTwoRegisteredServices() URI uri = getUriForAlias() @@ -91,5 +97,8 @@ class StickyRuleISpec extends Specification implements PollingUtils { return new TestServiceRegistrar(SocketUtils.findAvailableTcpPort(), curatorFramework) } + @Bean IPing noOpPing() { + return new NoOpPing() + } } } \ No newline at end of file diff --git a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDependenciesISpec.groovy b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDependenciesISpec.groovy index 01ca1ca5..96919ea3 100644 --- a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDependenciesISpec.groovy +++ b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDependenciesISpec.groovy @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://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, @@ -14,30 +14,20 @@ * limitations under the License. */ package org.springframework.cloud.zookeeper.discovery.dependency +import org.apache.curator.framework.CuratorFramework import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.autoconfigure.EnableAutoConfiguration import org.springframework.boot.test.SpringApplicationContextLoader import org.springframework.boot.test.WebIntegrationTest import org.springframework.cloud.client.ServiceInstance import org.springframework.cloud.client.discovery.DiscoveryClient -import org.springframework.cloud.client.discovery.EnableDiscoveryClient -import org.springframework.cloud.client.loadbalancer.LoadBalanced -import org.springframework.cloud.netflix.feign.EnableFeignClients -import org.springframework.cloud.netflix.feign.FeignClient -import org.springframework.cloud.zookeeper.discovery.test.CommonTestConfig -import org.springframework.cloud.zookeeper.discovery.test.TestRibbonClient import org.springframework.cloud.zookeeper.discovery.PollingUtils -import org.springframework.context.annotation.Bean +import org.springframework.cloud.zookeeper.discovery.test.TestRibbonClient import org.springframework.context.annotation.Configuration import org.springframework.context.annotation.Import import org.springframework.context.annotation.Profile import org.springframework.test.context.ActiveProfiles import org.springframework.test.context.ContextConfiguration -import org.springframework.web.bind.annotation.RequestHeader -import org.springframework.web.bind.annotation.RequestMapping -import org.springframework.web.bind.annotation.RequestMethod -import org.springframework.web.bind.annotation.RestController -import org.springframework.web.client.RestTemplate import spock.lang.Specification import spock.util.concurrent.PollingConditions @@ -50,7 +40,9 @@ class ZookeeperDiscoveryWithDependenciesISpec extends Specification implements P @Autowired DiscoveryClient discoveryClient @Autowired AliasUsingFeignClient aliasUsingFeignClient @Autowired IdUsingFeignClient idUsingFeignClient - @Autowired ZookeeperDependencies zookeeperDependencies + @Autowired ZookeeperDependencies zookeeperDependencies + @Autowired Config dependencyConfig + @Autowired CuratorFramework curatorFramework PollingConditions conditions = new PollingConditions() def 'should find an instance via path when alias is not found'() { @@ -105,28 +97,28 @@ class ZookeeperDiscoveryWithDependenciesISpec extends Specification implements P } } - def 'should have path equal to alias'() { - given: - def dependency = zookeeperDependencies.getDependencyForAlias('aliasIsPath') - expect: - dependency.path == 'aliasIsPath' - } + def 'should have path equal to alias'() { + given: + def dependency = zookeeperDependencies.getDependencyForAlias('aliasIsPath') + expect: + dependency.path == 'aliasIsPath' + } - def 'should have alias equal to path'() { - given: - def dependency = zookeeperDependencies.getDependencyForPath('aliasIsPath') - expect: - dependency.path == 'aliasIsPath' - } + def 'should have alias equal to path'() { + given: + def dependency = zookeeperDependencies.getDependencyForPath('aliasIsPath') + expect: + dependency.path == 'aliasIsPath' + } - def 'should have path set via string constructor'() { - given: - def dependency = zookeeperDependencies.getDependencyForAlias('anotherAlias') - expect: - dependency.path == 'myPath' - } + def 'should have path set via string constructor'() { + given: + def dependency = zookeeperDependencies.getDependencyForAlias('anotherAlias') + expect: + dependency.path == 'myPath' + } - private boolean callingServiceAtBeansEndpointIsNotEmpty() { + private boolean callingServiceAtBeansEndpointIsNotEmpty() { return !testRibbonClient.callService('someAlias', 'beans').empty } @@ -140,55 +132,9 @@ class ZookeeperDiscoveryWithDependenciesISpec extends Specification implements P @Configuration @EnableAutoConfiguration - @Import(CommonTestConfig) - @EnableDiscoveryClient - @EnableFeignClients(clients = [AliasUsingFeignClient, IdUsingFeignClient]) + @Import(DependencyConfig) @Profile('dependencies') static class Config { - - @Bean - TestRibbonClient testRibbonClient(@LoadBalanced RestTemplate restTemplate) { - return new TestRibbonClient(restTemplate) - } - - @Bean - PingController pingController() { - return new PingController() - } - - } - - @FeignClient("someAlias") - public static interface AliasUsingFeignClient { - @RequestMapping(method = RequestMethod.GET, value = "/beans") - String getBeans(); - - @RequestMapping(method = RequestMethod.GET, value = "/checkHeaders") - String checkHeaders(); - } - - @FeignClient("nameWithoutAlias") - public static interface IdUsingFeignClient { - @RequestMapping(method = RequestMethod.GET, value = "/beans") - String getBeans(); - } - - @RestController - @Profile('dependencies') - static class PingController { - - @RequestMapping('/ping') String ping() { - return 'pong' - } - - @RequestMapping('/checkHeaders') String checkHeaders(@RequestHeader('Content-Type') String contentType, - @RequestHeader('header1') Collection header1, - @RequestHeader('header2') Collection header2) { - assert contentType == 'application/vnd.newsletter.v1+json' - assert header1 == ['value1'] as Set - assert header2 == ['value2'] as Set - return 'ok' - } } } \ No newline at end of file diff --git a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDyingDependenciesISpec.groovy b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDyingDependenciesISpec.groovy new file mode 100644 index 00000000..45a69853 --- /dev/null +++ b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDyingDependenciesISpec.groovy @@ -0,0 +1,80 @@ +/* + * Copyright 2013-2015 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 + * + * http://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.zookeeper.discovery.dependency +import org.apache.curator.test.TestingServer +import org.springframework.boot.autoconfigure.EnableAutoConfiguration +import org.springframework.boot.builder.SpringApplicationBuilder +import org.springframework.cloud.client.discovery.EnableDiscoveryClient +import org.springframework.cloud.zookeeper.discovery.PollingUtils +import org.springframework.cloud.zookeeper.discovery.test.TestRibbonClient +import org.springframework.context.ApplicationContext +import org.springframework.context.ConfigurableApplicationContext +import org.springframework.context.annotation.Configuration +import org.springframework.context.annotation.Import +import org.springframework.util.SocketUtils +import spock.lang.Issue +import spock.lang.Specification +import spock.util.concurrent.PollingConditions +import spock.util.environment.RestoreSystemProperties + +class ZookeeperDiscoveryWithDyingDependenciesISpec extends Specification implements PollingUtils { + + PollingConditions conditions = new PollingConditions() + + @Issue("#45") + @RestoreSystemProperties + def "should refresh a dependency in Ribbon when the dependency has de-registered and registered in Zookeeper"() { + given: + int zookeeperPort = SocketUtils.findAvailableTcpPort() + TestingServer testingServer = new TestingServer(zookeeperPort) + System.setProperty('spring.jmx.enabled', 'false') + System.setProperty('spring.cloud.zookeeper.connectString', "127.0.0.1:$zookeeperPort") + and: + ConfigurableApplicationContext serverContext = contextWithProfile('server') + ConfigurableApplicationContext clientContext = contextWithProfile('client') + and: + Integer portBeforeDying = callServiceAtPortEndpoint(clientContext) + and: + serverContext = restartContext(serverContext, 'server') + expect: + callServiceAtPortEndpoint(clientContext) != portBeforeDying + cleanup: + serverContext?.close() + clientContext?.close() + testingServer?.close() + } + + private ConfigurableApplicationContext contextWithProfile(String profile) { + return new SpringApplicationBuilder(Config).profiles(profile).build().run() + } + + private ConfigurableApplicationContext restartContext(ConfigurableApplicationContext configurableApplicationContext, String profile) { + configurableApplicationContext.close() + return contextWithProfile(profile) + } + + private Integer callServiceAtPortEndpoint(ApplicationContext applicationContext) { + return applicationContext.getBean(TestRibbonClient).callService('testInstance', 'port', Integer) + } + + @Configuration + @EnableDiscoveryClient + @EnableAutoConfiguration + @Import(DependencyConfig) + static class Config { + } + +} \ No newline at end of file diff --git a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/test/TestRibbonClient.groovy b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/test/TestRibbonClient.groovy index 6ce6f287..61b7550c 100644 --- a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/test/TestRibbonClient.groovy +++ b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/test/TestRibbonClient.groovy @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://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, @@ -20,20 +20,24 @@ import org.springframework.web.client.RestTemplate class TestRibbonClient extends TestServiceRestClient { - private final String thisAppName + private final String thisAppName - TestRibbonClient(RestTemplate restTemplate) { - super(restTemplate) - this.thisAppName = 'someName' - } + TestRibbonClient(RestTemplate restTemplate) { + super(restTemplate) + this.thisAppName = 'someName' + } - TestRibbonClient(RestTemplate restTemplate, String thisAppName) { - super(restTemplate) - this.thisAppName = thisAppName - } + TestRibbonClient(RestTemplate restTemplate, String thisAppName) { + super(restTemplate) + this.thisAppName = thisAppName + } - String thisHealthCheck() { - return restTemplate.getForObject("http://$thisAppName/health", String) - } + String thisHealthCheck() { + return restTemplate.getForObject("http://$thisAppName/health", String) + } + + Integer thisPort() { + return restTemplate.getForObject("http://$thisAppName/port", Integer) + } } \ No newline at end of file diff --git a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/test/TestServiceRestClient.groovy b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/test/TestServiceRestClient.groovy index a1af3fd8..63692a00 100644 --- a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/test/TestServiceRestClient.groovy +++ b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/test/TestServiceRestClient.groovy @@ -27,8 +27,12 @@ class TestServiceRestClient { this.restTemplate = restTemplate } + public T callService(String alias, String endpoint, Class clazz) { + return restTemplate.getForObject("http://$alias/$endpoint", clazz) + } + String callService(String alias, String endpoint) { - return restTemplate.getForObject("http://$alias/$endpoint", String) + return callService(alias, endpoint, String) } String callOnUrl(String url, String endpoint) { diff --git a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/watcher/DefaultDependencyWatcherSpringISpec.groovy b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/watcher/DefaultDependencyWatcherSpringISpec.groovy index fc7fae66..e16cffd4 100644 --- a/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/watcher/DefaultDependencyWatcherSpringISpec.groovy +++ b/spring-cloud-zookeeper-discovery/src/test/groovy/org/springframework/cloud/zookeeper/discovery/watcher/DefaultDependencyWatcherSpringISpec.groovy @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://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, @@ -14,17 +14,16 @@ * limitations under the License. */ package org.springframework.cloud.zookeeper.discovery.watcher + import org.apache.curator.framework.CuratorFramework import org.apache.curator.framework.CuratorFrameworkFactory import org.apache.curator.retry.ExponentialBackoffRetry import org.apache.curator.test.TestingServer import org.apache.curator.x.discovery.ServiceCache -import org.apache.curator.x.discovery.ServiceDiscoveryBuilder -import org.apache.curator.x.discovery.ServiceInstance -import org.apache.curator.x.discovery.UriSpec import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.autoconfigure.EnableAutoConfiguration import org.springframework.boot.test.SpringApplicationContextLoader +import org.springframework.cloud.zookeeper.discovery.CustomZookeeperServiceDiscovery import org.springframework.cloud.zookeeper.discovery.PollingUtils import org.springframework.cloud.zookeeper.discovery.ZookeeperServiceDiscovery import org.springframework.cloud.zookeeper.discovery.watcher.presence.DependencyPresenceOnStartupVerifier @@ -39,15 +38,13 @@ import org.springframework.util.SocketUtils import spock.lang.Specification import spock.util.concurrent.PollingConditions -import javax.annotation.PreDestroy - @ContextConfiguration(classes = Config, loader = SpringApplicationContextLoader) @ActiveProfiles('watcher') class DefaultDependencyWatcherSpringISpec extends Specification implements PollingUtils { @Autowired AssertableDependencyPresenceOnStartupVerifier dependencyPresenceOnStartupVerifier @Autowired AssertableDependencyWatcherListener dependencyWatcherListener - @Autowired ZookeeperServiceDiscovery serviceDiscovery + @Autowired ZookeeperServiceDiscovery serviceDiscovery PollingConditions conditions def setup() { @@ -61,7 +58,7 @@ class DefaultDependencyWatcherSpringISpec extends Specification implements Polli def 'should verify that dependency watcher listener is successfully registered and operational'() { when: - serviceDiscovery.serviceDiscovery.unregisterService(serviceDiscovery.serviceInstance) + serviceDiscovery.serviceDiscovery.unregisterService(serviceDiscovery.serviceInstance) then: conditions.eventually willPass { @@ -84,10 +81,10 @@ class DefaultDependencyWatcherSpringISpec extends Specification implements Polli return new TestingServer(SocketUtils.findAvailableTcpPort()) } - @Bean - ZookeeperServiceDiscovery zookeeperServiceDiscovery() { - return new MyZookeeperServiceDiscovery(curatorFramework()) - } + @Bean + ZookeeperServiceDiscovery zookeeperServiceDiscovery() { + return new MyZookeeperServiceDiscovery(curatorFramework()) + } @Bean(initMethod = 'start', destroyMethod = 'close') CuratorFramework curatorFramework() { @@ -106,40 +103,11 @@ class DefaultDependencyWatcherSpringISpec extends Specification implements Polli } - static class MyZookeeperServiceDiscovery extends ZookeeperServiceDiscovery { - MyZookeeperServiceDiscovery(CuratorFramework curator) { - super(curator, null, null) - build() - } - - @Override - void build() { - setPort(10) - - - def instance = ServiceInstance.builder().uriSpec(new UriSpec("{scheme}://{address}:{port}/")) - .address('anyUrl') - .port(10) - .name('testInstance') - .build() - getServiceInstanceRef().set(instance) - - - def discovery = ServiceDiscoveryBuilder - .builder(Void) - .basePath('/') - .client(getCurator()) - .thisInstance(instance) - .build() - getServiceDiscoveryRef().set(discovery) - discovery.start() - } - - @PreDestroy - void close() { - getServiceDiscoveryRef().get().close() - } - } + static class MyZookeeperServiceDiscovery extends CustomZookeeperServiceDiscovery { + MyZookeeperServiceDiscovery(CuratorFramework curator) { + super('testInstance', curator) + } + } static class AssertableDependencyWatcherListener implements DependencyWatcherListener { diff --git a/spring-cloud-zookeeper-discovery/src/test/resources/application-client.yml b/spring-cloud-zookeeper-discovery/src/test/resources/application-client.yml new file mode 100644 index 00000000..85c4d170 --- /dev/null +++ b/spring-cloud-zookeeper-discovery/src/test/resources/application-client.yml @@ -0,0 +1,6 @@ +server.port: 0 +spring.application.name: client +spring.cloud.zookeeper: + dependencies: + testInstance: + path: /server \ No newline at end of file diff --git a/spring-cloud-zookeeper-discovery/src/test/resources/application-server.yml b/spring-cloud-zookeeper-discovery/src/test/resources/application-server.yml new file mode 100644 index 00000000..ba230fe2 --- /dev/null +++ b/spring-cloud-zookeeper-discovery/src/test/resources/application-server.yml @@ -0,0 +1,2 @@ +server.port: 0 +spring.application.name: server \ No newline at end of file