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
2016-01-25 10:17:57 +00:00
parent 92a35ba77f
commit dbb6059558
5 changed files with 65 additions and 80 deletions
--- 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<ServiceInstance> 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() + "/";
 	}

 }
--- 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()));

 	}

--- 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);
--- 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;
 		}

--- 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);
-		setup("spring.cloud.config.discovery.enabled=true");
-		assertEquals(
-				1,
-				this.context
-						.getBeanNamesForType(DiscoveryClientConfigServiceBootstrapConfiguration.class).length);
-		Mockito.verify(this.client).getNextServerFromEureka("CONFIGSERVER", false);
-		ConfigClientProperties locator = this.context
-				.getBean(ConfigClientProperties.class);
-		assertEquals("http://foo:7001/", 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))
+		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("http://foo:8877/", locator.getRawUri());
+	}
+
+	@Test
+	public void secureWhenRequested() throws Exception {
+		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).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,