From dbb60595588377c2337b14672a41afc9f1915bf4 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Mon, 25 Jan 2016 10:17:57 +0000 Subject: [PATCH] Simplify config server discovery with DiscoverClient metadata Since DiscoverClient now supports metadata natively, we can use that and not rely on Eureka interafces at all. Fixes gh-787. Also fixes https://github.com/spring-cloud/spring-cloud-config/issues/302 --- ...ntConfigServiceBootstrapConfiguration.java | 21 ++---- .../eureka/EurekaInstanceConfigBeanTests.java | 23 +++---- .../eureka/InstanceInfoFactoryTests.java | 13 ++-- ...ntConfigServiceAutoConfigurationTests.java | 9 +-- ...figServiceBootstrapConfigurationTests.java | 65 ++++++++----------- 5 files changed, 58 insertions(+), 73 deletions(-) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceBootstrapConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceBootstrapConfiguration.java index 1ace4eb3..5c834624 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceBootstrapConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceBootstrapConfiguration.java @@ -16,8 +16,6 @@ package org.springframework.cloud.netflix.eureka.config; -import java.util.List; - import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -32,7 +30,6 @@ import org.springframework.context.annotation.Import; import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.context.event.EventListener; -import com.netflix.appinfo.InstanceInfo; import com.netflix.discovery.EurekaClient; import lombok.extern.apachecommons.CommonsLog; @@ -56,21 +53,16 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration { @Autowired private DiscoveryClient client; - @Autowired - private EurekaClient eurekaClient; - @EventListener(ContextRefreshedEvent.class) public void onApplicationEvent(ContextRefreshedEvent event) { refresh(); } - // TODO: re-instate heart beat (maybe? isn't it handled in the child context?) - private void refresh() { try { log.debug("Locating configserver via discovery"); - InstanceInfo server = this.eurekaClient.getNextServerFromEureka( - this.config.getDiscovery().getServiceId(), false); + ServiceInstance server = this.client + .getInstances(this.config.getDiscovery().getServiceId()).get(0); String url = getHomePage(server); if (server.getMetadata().containsKey("password")) { String user = server.getMetadata().get("user"); @@ -93,13 +85,8 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration { } } - private String getHomePage(InstanceInfo server) { - List instances = this.client - .getInstances(this.config.getDiscovery().getServiceId()); - if (instances == null || instances.isEmpty()) { - return server.getHomePageUrl(); - } - return instances.get(0).getUri().toString() + "/"; + private String getHomePage(ServiceInstance server) { + return server.getUri().toString() + "/"; } } diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java index 16bf54e4..c8e88c8d 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java @@ -46,11 +46,12 @@ public class EurekaInstanceConfigBeanTests { private String ipAddress; @Before - public void init() { - InetUtils.HostInfo hostInfo = new InetUtils(new InetUtilsProperties()) - .findFirstNonLoopbackHostInfo(); - this.hostName = hostInfo.getHostname(); - this.ipAddress = hostInfo.getIpAddress(); + public void init() throws Exception { + try (InetUtils utils = new InetUtils(new InetUtilsProperties())) { + InetUtils.HostInfo hostInfo = utils.findFirstNonLoopbackHostInfo(); + this.hostName = hostInfo.getHostname(); + this.ipAddress = hostInfo.getIpAddress(); + } } @After @@ -147,8 +148,8 @@ public class EurekaInstanceConfigBeanTests { @Test public void testDefaultInitialStatus() { setupContext(); - assertEquals("initialStatus wrong", InstanceStatus.UP, getInstanceConfig() - .getInitialStatus()); + assertEquals("initialStatus wrong", InstanceStatus.UP, + getInstanceConfig().getInitialStatus()); } @Test(expected = BeanCreationException.class) @@ -161,8 +162,8 @@ public class EurekaInstanceConfigBeanTests { public void testCustomInitialStatus() { addEnvironment(this.context, "eureka.instance.initial-status:STARTING"); setupContext(); - assertEquals("initialStatus wrong", InstanceStatus.STARTING, getInstanceConfig() - .getInitialStatus()); + assertEquals("initialStatus wrong", InstanceStatus.STARTING, + getInstanceConfig().getInitialStatus()); } @Test @@ -170,8 +171,8 @@ public class EurekaInstanceConfigBeanTests { addEnvironment(this.context, "eureka.instance.preferIpAddress:true"); setupContext(); EurekaInstanceConfigBean instance = getInstanceConfig(); - assertTrue("Wrong hostname: " + instance.getHostname(), getInstanceConfig() - .getHostname().equals(instance.getIpAddress())); + assertTrue("Wrong hostname: " + instance.getHostname(), + getInstanceConfig().getHostname().equals(instance.getIpAddress())); } diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/InstanceInfoFactoryTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/InstanceInfoFactoryTests.java index 1fd9bc18..d9ec7784 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/InstanceInfoFactoryTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/InstanceInfoFactoryTests.java @@ -1,5 +1,7 @@ package org.springframework.cloud.netflix.eureka; +import java.io.IOException; + import org.junit.Test; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -19,10 +21,12 @@ public class InstanceInfoFactoryTests { private AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); @Test - public void instanceIdIsHostNameByDefault() { + public void instanceIdIsHostNameByDefault() throws IOException { InstanceInfo instanceInfo = setupInstance(); - assertEquals(new InetUtils(new InetUtilsProperties()) - .findFirstNonLoopbackHostInfo().getHostname(), instanceInfo.getId()); + try (InetUtils utils = new InetUtils(new InetUtilsProperties())) { + assertEquals(utils.findFirstNonLoopbackHostInfo().getHostname(), + instanceInfo.getId()); + } } @Test @@ -38,8 +42,9 @@ public class InstanceInfoFactoryTests { } private InstanceInfo setupInstance(String... pairs) { - for (String pair : pairs) + for (String pair : pairs) { addEnvironment(this.context, pair); + } this.context.register(PropertyPlaceholderAutoConfiguration.class, TestConfiguration.class); diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceAutoConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceAutoConfigurationTests.java index 4455c6ab..e08f0c88 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceAutoConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceAutoConfigurationTests.java @@ -16,6 +16,8 @@ package org.springframework.cloud.netflix.eureka.config; +import java.util.Arrays; + import org.junit.After; import org.junit.Test; import org.mockito.Mockito; @@ -24,8 +26,6 @@ import org.springframework.boot.test.EnvironmentTestUtils; import org.springframework.cloud.config.client.ConfigClientProperties; import org.springframework.cloud.netflix.eureka.EurekaClientAutoConfiguration; import org.springframework.cloud.netflix.eureka.EurekaDiscoveryClientConfiguration; -import org.springframework.cloud.netflix.eureka.config.DiscoveryClientConfigServiceAutoConfiguration; -import org.springframework.cloud.netflix.eureka.config.DiscoveryClientConfigServiceBootstrapConfiguration; import org.springframework.cloud.util.UtilAutoConfiguration; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -64,7 +64,7 @@ public class DiscoveryClientConfigServiceAutoConfigurationTests { assertEquals(1, this.context.getBeanNamesForType( DiscoveryClientConfigServiceAutoConfiguration.class).length); EurekaClient eurekaClient = this.context.getParent().getBean(EurekaClient.class); - Mockito.verify(eurekaClient, times(2)).getNextServerFromEureka("CONFIGSERVER", + Mockito.verify(eurekaClient, times(2)).getInstancesByVipAddress("CONFIGSERVER", false); Mockito.verify(eurekaClient, times(1)).shutdown(); ConfigClientProperties locator = this.context @@ -99,7 +99,8 @@ public class DiscoveryClientConfigServiceAutoConfigurationTests { public EurekaClient eurekaClient(ApplicationInfoManager manager) { InstanceInfo info = manager.getInfo(); EurekaClient client = Mockito.mock(EurekaClient.class); - given(client.getNextServerFromEureka("CONFIGSERVER", false)).willReturn(info); + given(client.getInstancesByVipAddress("CONFIGSERVER", false)) + .willReturn(Arrays.asList(info)); return client; } diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceBootstrapConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceBootstrapConfigurationTests.java index 33f3d541..621c7fb1 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceBootstrapConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/config/DiscoveryClientConfigServiceBootstrapConfigurationTests.java @@ -16,9 +16,6 @@ package org.springframework.cloud.netflix.eureka.config; -import static org.junit.Assert.assertEquals; -import static org.mockito.BDDMockito.given; - import java.util.Arrays; import org.junit.After; @@ -26,14 +23,15 @@ import org.junit.Test; import org.mockito.Mockito; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.test.EnvironmentTestUtils; +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.config.client.ConfigClientProperties; -import org.springframework.cloud.netflix.eureka.config.DiscoveryClientConfigServiceBootstrapConfiguration; import org.springframework.cloud.util.UtilAutoConfiguration; import org.springframework.context.annotation.AnnotationConfigApplicationContext; -import com.netflix.appinfo.InstanceInfo; -import com.netflix.appinfo.InstanceInfo.PortType; -import com.netflix.discovery.EurekaClient; +import static org.junit.Assert.assertEquals; +import static org.mockito.BDDMockito.given; /** * @author Dave Syer @@ -42,10 +40,9 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { private AnnotationConfigApplicationContext context; - private EurekaClient client = Mockito.mock(EurekaClient.class); + private DiscoveryClient client = Mockito.mock(DiscoveryClient.class); - private InstanceInfo info = InstanceInfo.Builder.newBuilder().setAppName("app") - .setHostName("foo").setHomePageUrl("/", null).build(); + private ServiceInstance info = new DefaultServiceInstance("app", "foo", 8877, false); @After public void close() { @@ -58,40 +55,33 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { public void offByDefault() throws Exception { this.context = new AnnotationConfigApplicationContext( DiscoveryClientConfigServiceBootstrapConfiguration.class); - assertEquals(0, this.context.getBeanNamesForType(EurekaClient.class).length); - assertEquals( - 0, - this.context - .getBeanNamesForType(DiscoveryClientConfigServiceBootstrapConfiguration.class).length); + assertEquals(0, this.context.getBeanNamesForType(DiscoveryClient.class).length); + assertEquals(0, this.context.getBeanNamesForType( + DiscoveryClientConfigServiceBootstrapConfiguration.class).length); } @Test public void onWhenRequested() throws Exception { - given(this.client.getNextServerFromEureka("CONFIGSERVER", false)).willReturn( - this.info); + given(this.client.getInstances("CONFIGSERVER")) + .willReturn(Arrays.asList(this.info)); setup("spring.cloud.config.discovery.enabled=true"); - assertEquals( - 1, - this.context - .getBeanNamesForType(DiscoveryClientConfigServiceBootstrapConfiguration.class).length); - Mockito.verify(this.client).getNextServerFromEureka("CONFIGSERVER", false); + assertEquals(1, this.context.getBeanNamesForType( + DiscoveryClientConfigServiceBootstrapConfiguration.class).length); + Mockito.verify(this.client).getInstances("CONFIGSERVER"); ConfigClientProperties locator = this.context .getBean(ConfigClientProperties.class); - assertEquals("http://foo:7001/", locator.getRawUri()); + assertEquals("http://foo:8877/", locator.getRawUri()); } @Test public void secureWhenRequested() throws Exception { - this.info = InstanceInfo.Builder.newBuilder().setAppName("app").setHostName("foo") - .setHomePageUrl("/", null).enablePort(PortType.SECURE, true).setSecurePort(443).build(); - given(this.client.getNextServerFromEureka("CONFIGSERVER", false)) - .willReturn(this.info); - given(this.client.getInstancesByVipAddress("CONFIGSERVER", false)) + this.info = new DefaultServiceInstance("app", "foo", 443, true); + given(this.client.getInstances("CONFIGSERVER")) .willReturn(Arrays.asList(this.info)); setup("spring.cloud.config.discovery.enabled=true"); assertEquals(1, this.context.getBeanNamesForType( DiscoveryClientConfigServiceBootstrapConfiguration.class).length); - Mockito.verify(this.client).getNextServerFromEureka("CONFIGSERVER", false); + Mockito.verify(this.client).getInstances("CONFIGSERVER"); ConfigClientProperties locator = this.context .getBean(ConfigClientProperties.class); assertEquals("https://foo:443/", locator.getRawUri()); @@ -100,12 +90,12 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { @Test public void setsPasssword() throws Exception { this.info.getMetadata().put("password", "bar"); - given(this.client.getNextServerFromEureka("CONFIGSERVER", false)).willReturn( - this.info); + given(this.client.getInstances("CONFIGSERVER")) + .willReturn(Arrays.asList(this.info)); setup("spring.cloud.config.discovery.enabled=true"); ConfigClientProperties locator = this.context .getBean(ConfigClientProperties.class); - assertEquals("http://foo:7001/", locator.getRawUri()); + assertEquals("http://foo:8877/", locator.getRawUri()); assertEquals("bar", locator.getPassword()); assertEquals("user", locator.getUsername()); } @@ -113,19 +103,20 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { @Test public void setsPath() throws Exception { this.info.getMetadata().put("configPath", "/bar"); - given(this.client.getNextServerFromEureka("CONFIGSERVER", false)).willReturn( - this.info); + given(this.client.getInstances("CONFIGSERVER")) + .willReturn(Arrays.asList(this.info)); setup("spring.cloud.config.discovery.enabled=true"); ConfigClientProperties locator = this.context .getBean(ConfigClientProperties.class); - assertEquals("http://foo:7001/bar", locator.getRawUri()); + assertEquals("http://foo:8877/bar", locator.getRawUri()); } private void setup(String... env) { this.context = new AnnotationConfigApplicationContext(); EnvironmentTestUtils.addEnvironment(this.context, env); - this.context.getDefaultListableBeanFactory().registerSingleton( - "eurekaClient", this.client); + EnvironmentTestUtils.addEnvironment(this.context, "eureka.client.enabled=false"); + this.context.getDefaultListableBeanFactory().registerSingleton("discoveryClient", + this.client); this.context.register(UtilAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, DiscoveryClientConfigServiceBootstrapConfiguration.class,