From 381a0970450398533fbe8fe4d6ad42b6aa48c9b5 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 15 Jun 2018 14:16:23 -0400 Subject: [PATCH 1/7] Does not set accept-encoding header when zuul proxies request if already present. Fixes #2998 (#3011) --- .../netflix/zuul/filters/ProxyRequestHelper.java | 4 +++- .../zuul/filters/ProxyRequestHelperTests.java | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java index 0747f0314..72e3cd07f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java @@ -145,7 +145,9 @@ public class ProxyRequestHelper { for (String header : zuulRequestHeaders.keySet()) { headers.set(header, zuulRequestHeaders.get(header)); } - headers.set(HttpHeaders.ACCEPT_ENCODING, "gzip"); + if(!headers.containsKey(HttpHeaders.ACCEPT_ENCODING)) { + headers.set(HttpHeaders.ACCEPT_ENCODING, "gzip"); + } return headers; } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java index 25f391d87..aa9ae4224 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java @@ -211,6 +211,20 @@ public class ProxyRequestHelperTests { assertThat(acceptEncodings, contains("gzip")); } + @Test + public void buildZuulRequestHeadersRequestsAcceptEncoding() { + MockHttpServletRequest request = new MockHttpServletRequest("", "/"); + request.addHeader("accept-encoding", "identity"); + + ProxyRequestHelper helper = new ProxyRequestHelper(); + + MultiValueMap headers = helper.buildZuulRequestHeaders(request); + + List acceptEncodings = headers.get("accept-encoding"); + assertThat(acceptEncodings, hasSize(1)); + assertThat(acceptEncodings, contains("identity")); + } + @Test public void setResponseLowercase() throws IOException { MockHttpServletRequest request = new MockHttpServletRequest("POST", "/"); From 2df81038f9033c77b03283407bb8864491543f2a Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 15 Jun 2018 14:31:53 -0400 Subject: [PATCH 2/7] Adds .jdk8 file back --- spring-cloud-netflix-turbine-stream/.jdk8 | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 spring-cloud-netflix-turbine-stream/.jdk8 diff --git a/spring-cloud-netflix-turbine-stream/.jdk8 b/spring-cloud-netflix-turbine-stream/.jdk8 new file mode 100644 index 000000000..e69de29bb From 8c7116b2d80e3802835e6666c1ba445837cc810a Mon Sep 17 00:00:00 2001 From: Yongsung Yoon Date: Fri, 22 Jun 2018 01:39:41 +0900 Subject: [PATCH 3/7] Add support cluster query param in TurbineStream (#3001) --- .../main/asciidoc/spring-cloud-netflix.adoc | 18 +++ .../stream/TurbineStreamConfiguration.java | 24 +++ .../TurbineStreamConfigurationTest.java | 138 ++++++++++++++++++ 3 files changed, 180 insertions(+) create mode 100644 spring-cloud-netflix-turbine-stream/src/test/java/org/springframework/cloud/netflix/turbine/stream/TurbineStreamConfigurationTest.java diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 545bdee48..e11ed6043 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -726,6 +726,24 @@ You can then point the Hystrix Dashboard to the Turbine Stream Server instead of Spring Cloud provides a `spring-cloud-starter-netflix-turbine-stream` that has all the dependencies you need to get a Turbine Stream server running - just add the Stream binder of your choice, e.g. `spring-cloud-starter-stream-rabbit`. You need Java 8 to run the app because it is Netty-based. +Turbine Stream server also supports the `cluster` parameter. +Unlike Turbine server, Turbine Stream uses eureka serviceIds as cluster names and these are not configurable. + +If Turbine Stream server is running on port 8989 on `my.turbine.server` and you have two eureka serviceIds `customers` and `products` in your environment, the following URLs will be available on your Turbine Stream server. `default` and empty cluster name will provide all metrics that Turbine Stream server receives. + +---- +http://my.turbine.sever:8989/turbine.stream?cluster=customers +http://my.turbine.sever:8989/turbine.stream?cluster=products +http://my.turbine.sever:8989/turbine.stream?cluster=default +http://my.turbine.sever:8989/turbine.stream +---- + +So, you can use eureka serviceIds as cluster names for your Turbine dashboard (or any compatible dashboard). +You don’t need to configure any properties like `turbine.appConfig`, `turbine.clusterNameExpression` and `turbine.aggregator.clusterConfig` for your Turbine Stream server. + +NOTE: Turbine Stream server gathers all metrics from the configured input channel with Spring Cloud Stream. It means that it doesn’t gather Hystrix metrics actively from each instance. It just can provide metrics that were already gathered into the input channel by each instance. + + [[spring-cloud-ribbon]] == Client Side Load Balancer: Ribbon diff --git a/spring-cloud-netflix-turbine-stream/src/main/java/org/springframework/cloud/netflix/turbine/stream/TurbineStreamConfiguration.java b/spring-cloud-netflix-turbine-stream/src/main/java/org/springframework/cloud/netflix/turbine/stream/TurbineStreamConfiguration.java index b1fc78df4..13e25754f 100644 --- a/spring-cloud-netflix-turbine-stream/src/main/java/org/springframework/cloud/netflix/turbine/stream/TurbineStreamConfiguration.java +++ b/spring-cloud-netflix-turbine-stream/src/main/java/org/springframework/cloud/netflix/turbine/stream/TurbineStreamConfiguration.java @@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.turbine.stream; import java.nio.charset.StandardCharsets; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -46,6 +47,7 @@ import org.springframework.util.SocketUtils; import static io.reactivex.netty.pipeline.PipelineConfigurators.serveSseConfigurator; import rx.Observable; +import rx.functions.Func1; import rx.subjects.PublishSubject; /** @@ -58,6 +60,10 @@ public class TurbineStreamConfiguration implements SmartLifecycle { private static final Log log = LogFactory.getLog(TurbineStreamConfiguration.class); + private static final String CLUSTER_PARAM = "cluster"; + private static final String DEFAULT_CLUSTER = "default"; + private static final String INSTANCE_ID_KEY = "instanceId"; + private AtomicBoolean running = new AtomicBoolean(false); @Autowired @@ -103,6 +109,7 @@ public class TurbineStreamConfiguration implements SmartLifecycle { response.getHeaders().setHeader("Content-Type", "text/event-stream"); return output.doOnUnsubscribe( () -> log.info("Unsubscribing RxNetty server connection")) + .filter(createClusterPredicate(request.getQueryParameters())) .flatMap(data -> response.writeAndFlush(new ServerSentEvent( null, Unpooled.copiedBuffer("message", @@ -113,6 +120,23 @@ public class TurbineStreamConfiguration implements SmartLifecycle { return httpServer; } + Func1, Boolean> createClusterPredicate(Map> queryParameters) { + List clusterNames = queryParameters.get(CLUSTER_PARAM); + if ((clusterNames == null) || (clusterNames.isEmpty()) || (clusterNames.contains(DEFAULT_CLUSTER))) { + return (data) -> true; // always true + } + + return (data) -> { + String instanceId = (String) data.get(INSTANCE_ID_KEY); + if (instanceId == null) { + return true; // ping or unknown metric data. should be passed + } + + return clusterNames.stream() + .anyMatch(clusterName -> instanceId.toLowerCase().startsWith(clusterName.toLowerCase() + ":")); + }; + } + @Override public boolean isAutoStartup() { return true; diff --git a/spring-cloud-netflix-turbine-stream/src/test/java/org/springframework/cloud/netflix/turbine/stream/TurbineStreamConfigurationTest.java b/spring-cloud-netflix-turbine-stream/src/test/java/org/springframework/cloud/netflix/turbine/stream/TurbineStreamConfigurationTest.java new file mode 100644 index 000000000..184c21484 --- /dev/null +++ b/spring-cloud-netflix-turbine-stream/src/test/java/org/springframework/cloud/netflix/turbine/stream/TurbineStreamConfigurationTest.java @@ -0,0 +1,138 @@ +/* + * Copyright 2013-2018 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.netflix.turbine.stream; + +import org.junit.Before; +import org.junit.Test; +import rx.Observable; +import rx.functions.Func1; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; + +import static org.junit.Assert.assertEquals; + +/** + * @author Yongsung Yoon + */ +public class TurbineStreamConfigurationTest { + TurbineStreamConfiguration turbineStreamConfiguration; + List> testMetricList; + + @Before + public void setUp() { + turbineStreamConfiguration = new TurbineStreamConfiguration(); + testMetricList = createBasicTestMetricList(); + } + + private List> createBasicTestMetricList() { + List> testDataList = new ArrayList<>(); + HashMap map = new HashMap<>(); + map.put("instanceId", "abc:127.0.0.1:8080"); + map.put("type", "HystrixCommand"); + testDataList.add(map); + + map = new HashMap<>(); + map.put("instanceId", "def:127.0.0.1:8080"); + map.put("type", "HystrixCommand"); + testDataList.add(map); + + map = new HashMap<>(); + map.put("instanceId", "xyz:127.0.0.1:8080"); + map.put("type", "HystrixThreadPool"); + testDataList.add(map); + + map = new HashMap<>(); + map.put("type", "ping"); + testDataList.add(map); + + map = new HashMap<>(); + map.put("dummy", "data"); + testDataList.add(map); + + return testDataList; + } + + @Test + public void shouldReturnAlwaysTruePredicateWithEmptyQueryParam() { + Func1, Boolean> clusterPredicate = turbineStreamConfiguration.createClusterPredicate(Collections.emptyMap()); + + assertThatGivenPredicateReturnsTrueAsExpectedCount(5, clusterPredicate); // all + } + + @Test + public void shouldReturnAlwaysTruePredicateIfQueryParamsContainDefault() { + Map> queryMap = new HashMap<>(); + queryMap.put("cluster", Arrays.asList("default", "garbage")); + + Func1, Boolean> clusterPredicate = turbineStreamConfiguration.createClusterPredicate(queryMap); + + assertThatGivenPredicateReturnsTrueAsExpectedCount(5, clusterPredicate); // all + } + + @Test + public void shouldReturnPredicateForGivenClusterName() { + Map> queryMap = new HashMap<>(); + queryMap.put("cluster", Arrays.asList("abc")); + + Func1, Boolean> clusterPredicate = turbineStreamConfiguration.createClusterPredicate(queryMap); + + assertThatGivenPredicateReturnsTrueAsExpectedCount(3, clusterPredicate); // abc + ping + dummy + } + + @Test + public void shouldReturnPredicateForGivenMultipleClusterNames() { + Map> queryMap = new HashMap<>(); + queryMap.put("cluster", Arrays.asList("abc", "xyz")); + + Func1, Boolean> clusterPredicate = turbineStreamConfiguration.createClusterPredicate(queryMap); + + assertThatGivenPredicateReturnsTrueAsExpectedCount(4, clusterPredicate); // abc + xyz + ping + dummy + } + + @Test + public void shouldReturnPredicateForUnknownClusterName() { + Map> queryMap = new HashMap<>(); + queryMap.put("cluster", Arrays.asList("ttt", "eee")); + + Func1, Boolean> clusterPredicate = turbineStreamConfiguration.createClusterPredicate(queryMap); + + assertThatGivenPredicateReturnsTrueAsExpectedCount(2, clusterPredicate); // ping + dummy + } + + void assertThatGivenPredicateReturnsTrueAsExpectedCount(int expectedCount, Func1, Boolean> predicate) { + assertEquals(expectedCount, countEmittedMetricsWithPredicate(predicate)); + } + + int countEmittedMetricsWithPredicate(Func1, Boolean> predicate) { + try { + return Observable.from(this.testMetricList) + .filter(predicate) + .count() + .toBlocking() + .single(); + + } catch (NoSuchElementException ex) { + return 0; + } + } +} From 7b75fe934e6254170b6553fa78d02361a855680b Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 25 Jun 2018 09:05:28 -0400 Subject: [PATCH 4/7] Return 404 upstream when using Eureka and RestTemplate. Fixes #2861 (#3023) --- .../RestTemplateTransportClientFactory.java | 19 +++++++++++++++++++ .../http/EurekaServerMockApplication.java | 16 ++++++++++------ .../RestTemplateEurekaHttpClientTest.java | 6 ++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/http/RestTemplateTransportClientFactory.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/http/RestTemplateTransportClientFactory.java index 9258a3116..ffb181de0 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/http/RestTemplateTransportClientFactory.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/http/RestTemplateTransportClientFactory.java @@ -19,8 +19,10 @@ package org.springframework.cloud.netflix.eureka.http; import java.net.URI; import java.net.URISyntaxException; +import org.springframework.http.HttpStatus; import org.springframework.http.client.support.BasicAuthorizationInterceptor; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; +import org.springframework.web.client.DefaultResponseErrorHandler; import org.springframework.web.client.RestTemplate; import com.fasterxml.jackson.databind.BeanDescription; @@ -74,6 +76,7 @@ public class RestTemplateTransportClientFactory implements TransportClientFactor } restTemplate.getMessageConverters().add(0, mappingJacksonHttpMessageConverter()); + restTemplate.setErrorHandler(new ErrorHanlder()); return restTemplate; } @@ -134,4 +137,20 @@ public class RestTemplateTransportClientFactory implements TransportClientFactor public void shutdown() { } + class ErrorHanlder extends DefaultResponseErrorHandler { + @Override + protected boolean hasError(HttpStatus statusCode) { + /** + * When the Eureka server restarts and a client tries to sent a heartbeat the server + * will respond with a 404. By default RestTemplate will throw an exception in this case. + * What we want is to return the 404 to the upstream code so it will send another registration + * request to the server. + */ + if(statusCode.is4xxClientError()) { + return false; + } + return super.hasError(statusCode); + } + } + } diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/http/EurekaServerMockApplication.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/http/EurekaServerMockApplication.java index 63976782f..45d95462f 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/http/EurekaServerMockApplication.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/http/EurekaServerMockApplication.java @@ -20,6 +20,7 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; @@ -82,13 +83,16 @@ public class EurekaServerMockApplication { @ResponseStatus(HttpStatus.OK) @PutMapping(value = "/apps/{appName}/{id}", params = { "status", "lastDirtyTimestamp" }) - public InstanceInfo sendHeartBeat(@PathVariable String appName, - @PathVariable String id, @RequestParam String status, - @RequestParam String lastDirtyTimestamp, - @RequestParam(required = false) String overriddenstatus) { - return new InstanceInfo(null, null, null, null, null, null, null, null, null, + public ResponseEntity sendHeartBeat(@PathVariable String appName, + @PathVariable String id, @RequestParam String status, + @RequestParam String lastDirtyTimestamp, + @RequestParam(required = false) String overriddenstatus) { + if("fourOFour".equals(appName)) { + return new ResponseEntity(HttpStatus.NOT_FOUND); + } + return new ResponseEntity(new InstanceInfo(null, null, null, null, null, null, null, null, null, null, null, null, null, 0, null, null, null, null, null, null, null, 0l, - 0l, null, null); + 0l, null, null), HttpStatus.OK); } @ResponseStatus(HttpStatus.OK) diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/http/RestTemplateEurekaHttpClientTest.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/http/RestTemplateEurekaHttpClientTest.java index 4545d62cb..4d357d732 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/http/RestTemplateEurekaHttpClientTest.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/http/RestTemplateEurekaHttpClientTest.java @@ -91,6 +91,12 @@ public class RestTemplateEurekaHttpClientTest { .sendHeartBeat("test", "test", info, null).getStatusCode()); } + @Test + public void testSendHeartBeatFourOFour() { + Assert.assertEquals(HttpStatus.NOT_FOUND.value(), eurekaHttpClient + .sendHeartBeat("fourOFour", "test", info, null).getStatusCode()); + } + @Test public void testStatusUpdate() { Assert.assertEquals(HttpStatus.OK.value(), eurekaHttpClient From ab7b0fb3a828171a896bc5189d153d4986c2cf7c Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 25 Jun 2018 09:30:48 -0400 Subject: [PATCH 5/7] Clarify health and status url context path docs. Fixes #2804. --- docs/src/main/asciidoc/spring-cloud-netflix.adoc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index e11ed6043..20f106191 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -108,21 +108,23 @@ The status page and health indicators for a Eureka instance default to useful endpoints in a Spring Boot Actuator application. You need to change these, even for an Actuator application if you use a non-default context path or servlet path -(e.g. `server.servletPath=/foo`) or management endpoint path -(e.g. `management.contextPath=/admin`). Example: +(e.g. `server.servletPath=/foo`) Example: .application.yml ---- eureka: instance: - statusPageUrlPath: ${management.context-path}/info - healthCheckUrlPath: ${management.context-path}/health + statusPageUrlPath: ${server.servletPath}/info + healthCheckUrlPath: ${server.servletPath}/health ---- These links show up in the metadata that is consumed by clients, and used in some scenarios to decide whether to send requests to your application, so it's helpful if they are accurate. +NOTE: In Dalston it was also required to set the status and health check URLs when changing +that management context path. This requirement was removed beinging in Edgware. + === Registering a Secure Application If your app wants to be contacted over HTTPS you can set two flags in From 8af3ae8af8e5a6438556666cc0bd99f849401e3b Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 26 Jun 2018 10:36:46 -0400 Subject: [PATCH 6/7] Use management metadata in sidecar. Fixes 2859. (#3028) --- .../DefaultManagementMetadataProvider.java | 2 +- .../netflix/sidecar/SidecarConfiguration.java | 47 +++++++++++++++---- .../netflix/sidecar/SidecarApplication.java | 26 ++++++++++ .../sidecar/SidecarApplicationTests.java | 31 ++++++++++++ 4 files changed, 95 insertions(+), 11 deletions(-) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProvider.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProvider.java index 2dc5c623f..745c8c1e7 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProvider.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProvider.java @@ -40,7 +40,7 @@ public class DefaultManagementMetadataProvider implements ManagementMetadataProv return port != null && port == RANDOM_PORT; } - private String getHealthCheckUrl(EurekaInstanceConfigBean instance, int serverPort, String serverContextPath, + protected String getHealthCheckUrl(EurekaInstanceConfigBean instance, int serverPort, String serverContextPath, String managementContextPath, Integer managementPort, boolean isSecure) { String healthCheckUrlPath = instance.getHealthCheckUrlPath(); String healthCheckUrl = getUrl(instance, serverPort, serverContextPath, managementContextPath, diff --git a/spring-cloud-netflix-sidecar/src/main/java/org/springframework/cloud/netflix/sidecar/SidecarConfiguration.java b/spring-cloud-netflix-sidecar/src/main/java/org/springframework/cloud/netflix/sidecar/SidecarConfiguration.java index a3b551249..73da73aa0 100644 --- a/spring-cloud-netflix-sidecar/src/main/java/org/springframework/cloud/netflix/sidecar/SidecarConfiguration.java +++ b/spring-cloud-netflix-sidecar/src/main/java/org/springframework/cloud/netflix/sidecar/SidecarConfiguration.java @@ -20,14 +20,17 @@ import static org.springframework.cloud.commons.util.IdUtils.getDefaultInstanceI import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.netflix.eureka.EurekaInstanceConfigBean; +import org.springframework.cloud.netflix.eureka.metadata.DefaultManagementMetadataProvider; +import org.springframework.cloud.netflix.eureka.metadata.ManagementMetadata; +import org.springframework.cloud.netflix.eureka.metadata.ManagementMetadataProvider; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.env.ConfigurableEnvironment; @@ -36,8 +39,7 @@ import org.springframework.util.StringUtils; import com.netflix.appinfo.HealthCheckHandler; import com.netflix.discovery.EurekaClientConfig; -import java.net.InetAddress; -import java.net.UnknownHostException; +import java.util.Map; /** * Sidecar Configuration that setting up {@link com.netflix.appinfo.EurekaInstanceConfig}. @@ -80,8 +82,17 @@ public class SidecarConfiguration { @Autowired private InetUtils inetUtils; - @Value("${management.port:${MANAGEMENT_PORT:${server.port:${SERVER_PORT:${PORT:8080}}}}}") - private int managementPort = 8080; + @Value(value = "${management.port:${MANAGEMENT_PORT:#{null}}}") + private Integer managementPort; + + @Value("${server.port:${SERVER_PORT:${PORT:8080}}}") + private int serverPort = 8080; + + @Value("${management.context-path:${MANAGEMENT_CONTEXT_PATH:#{null}}}") + private String managementContextPath; + + @Value("${server.context-path:${SERVER_CONTEXT_PATH:/}}") + private String serverContextPath = "/"; @Value("${eureka.instance.hostname:${EUREKA_INSTANCE_HOSTNAME:}}") private String hostname; @@ -90,7 +101,13 @@ public class SidecarConfiguration { private ConfigurableEnvironment env; @Bean - public EurekaInstanceConfigBean eurekaInstanceConfigBean() { + @ConditionalOnMissingBean + public ManagementMetadataProvider serviceManagementMetadataProvider() { + return new DefaultManagementMetadataProvider(); + } + + @Bean + public EurekaInstanceConfigBean eurekaInstanceConfigBean(ManagementMetadataProvider managementMetadataProvider) { EurekaInstanceConfigBean config = new EurekaInstanceConfigBean(inetUtils); RelaxedPropertyResolver springPropertyResolver = new RelaxedPropertyResolver(env, "spring.application."); String springAppName = springPropertyResolver.getProperty("name"); @@ -114,10 +131,20 @@ public class SidecarConfiguration { config.setIpAddress(ipAddress); } String scheme = config.getSecurePortEnabled() ? "https" : "http"; - config.setStatusPageUrl(scheme + "://" + config.getHostname() + ":" - + this.managementPort + config.getStatusPageUrlPath()); - config.setHealthCheckUrl(scheme + "://" + config.getHostname() + ":" - + this.managementPort + config.getHealthCheckUrlPath()); + ManagementMetadata metadata = managementMetadataProvider.get(config, serverPort, + serverContextPath, managementContextPath, managementPort); + + if(metadata != null) { + config.setStatusPageUrl(metadata.getStatusPageUrl()); + config.setHealthCheckUrl(metadata.getHealthCheckUrl()); + if(config.isSecurePortEnabled()) { + config.setSecureHealthCheckUrl(metadata.getSecureHealthCheckUrl()); + } + Map metadataMap = config.getMetadataMap(); + if (metadataMap.get("management.port") == null) { + metadataMap.put("management.port", String.valueOf(metadata.getManagementPort())); + } + } config.setHomePageUrl(scheme + "://" + config.getHostname() + ":" + port + config.getHomePageUrlPath()); return config; diff --git a/spring-cloud-netflix-sidecar/src/test/java/org/springframework/cloud/netflix/sidecar/SidecarApplication.java b/spring-cloud-netflix-sidecar/src/test/java/org/springframework/cloud/netflix/sidecar/SidecarApplication.java index 63fe51291..27fe3f660 100644 --- a/spring-cloud-netflix-sidecar/src/test/java/org/springframework/cloud/netflix/sidecar/SidecarApplication.java +++ b/spring-cloud-netflix-sidecar/src/test/java/org/springframework/cloud/netflix/sidecar/SidecarApplication.java @@ -18,6 +18,11 @@ package org.springframework.cloud.netflix.sidecar; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.cloud.netflix.eureka.EurekaInstanceConfigBean; +import org.springframework.cloud.netflix.eureka.metadata.DefaultManagementMetadataProvider; +import org.springframework.cloud.netflix.eureka.metadata.ManagementMetadata; +import org.springframework.cloud.netflix.eureka.metadata.ManagementMetadataProvider; +import org.springframework.context.annotation.Bean; import org.springframework.web.bind.annotation.RestController; @SpringBootApplication @@ -29,4 +34,25 @@ public class SidecarApplication { SpringApplication.run(SidecarApplication.class, args); } + @Bean + public ManagementMetadataProvider managementMetadataProvider() { + //The default management metadata provider checks for random ports, we dont care about this in tests + return new DefaultManagementMetadataProvider() { + @Override + public ManagementMetadata get(EurekaInstanceConfigBean instance, int serverPort, String serverContextPath, String managementContextPath, Integer managementPort) { + String healthCheckUrl = getHealthCheckUrl(instance, serverPort, serverContextPath, + managementContextPath, managementPort, false); + String statusPageUrl = getStatusPageUrl(instance, serverPort, serverContextPath, + managementContextPath, managementPort); + + ManagementMetadata metadata = new ManagementMetadata(healthCheckUrl, statusPageUrl, managementPort == null ? serverPort : managementPort); + if(instance.isSecurePortEnabled()) { + metadata.setSecureHealthCheckUrl(getHealthCheckUrl(instance, serverPort, serverContextPath, + managementContextPath, managementPort, true)); + } + return metadata; + } + }; + } + } diff --git a/spring-cloud-netflix-sidecar/src/test/java/org/springframework/cloud/netflix/sidecar/SidecarApplicationTests.java b/spring-cloud-netflix-sidecar/src/test/java/org/springframework/cloud/netflix/sidecar/SidecarApplicationTests.java index 2a64dea73..0ce61d242 100644 --- a/spring-cloud-netflix-sidecar/src/test/java/org/springframework/cloud/netflix/sidecar/SidecarApplicationTests.java +++ b/spring-cloud-netflix-sidecar/src/test/java/org/springframework/cloud/netflix/sidecar/SidecarApplicationTests.java @@ -80,4 +80,35 @@ public class SidecarApplicationTests { } } + @RunWith(SpringJUnit4ClassRunner.class) + @SpringBootTest(classes = SidecarApplication.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { + "spring.application.name=mytest", "spring.cloud.client.hostname=mhhost", "spring.application.instance_id=1", + "eureka.instance.hostname=mhhost1", "sidecar.hostname=mhhost2", "sidecar.port=7000", "sidecar.ipAddress=127.0.0.1", + "management.context-path=/foo"}) + public static class ManagementContextPathStatusAndHealthCheckUrls { + @Autowired + EurekaInstanceConfigBean config; + + @Test + public void testStatusAndHealthCheckUrls() { + assertThat(this.config.getStatusPageUrl(), equalTo("http://mhhost2:0/foo/info")); + assertThat(this.config.getHealthCheckUrl(), equalTo("http://mhhost2:0/foo/health")); + } + } + + @RunWith(SpringJUnit4ClassRunner.class) + @SpringBootTest(classes = SidecarApplication.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { + "spring.application.name=mytest", "spring.cloud.client.hostname=mhhost", "spring.application.instance_id=1", + "eureka.instance.hostname=mhhost1", "sidecar.hostname=mhhost2", "sidecar.port=7000", "sidecar.ipAddress=127.0.0.1", + "server.context-path=/foo"}) + public static class ServerContextPathStatusAndHealthCheckUrls { + @Autowired + EurekaInstanceConfigBean config; + + @Test + public void testStatusAndHealthCheckUrls() { + assertThat(this.config.getStatusPageUrl(), equalTo("http://mhhost2:0/foo/info")); + assertThat(this.config.getHealthCheckUrl(), equalTo("http://mhhost2:0/foo/health")); + } + } } From c181163a754065d4427c936b17f2fa140ecf7536 Mon Sep 17 00:00:00 2001 From: Taras Danylchuk Date: Wed, 27 Jun 2018 18:46:34 +0300 Subject: [PATCH 7/7] fixes https://github.com/spring-cloud/spring-cloud-netflix/issues/2972 (#3018) root cause: cached bean trying to re-register resolution summary: get re-created bean from context to re-register --- .../eureka/EurekaClientAutoConfiguration.java | 47 +++++++---- .../serviceregistry/EurekaRegistration.java | 6 +- .../EurekaClientAutoConfigurationTests.java | 80 +++++++++++++------ 3 files changed, 87 insertions(+), 46 deletions(-) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java index c6404436f..7ab63051b 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java @@ -26,6 +26,7 @@ import java.lang.annotation.Target; import java.net.MalformedURLException; import java.util.Map; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.autoconfigure.AutoConfigureAfter; @@ -94,9 +95,6 @@ import com.netflix.discovery.EurekaClientConfig; "org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationAutoConfiguration"}) public class EurekaClientAutoConfiguration { - @Autowired(required = false) - private HealthCheckHandler healthCheckHandler; - @Bean public HasFeatures eurekaFeature() { return HasFeatures.namedFeature("Eureka Client", EurekaClient.class); @@ -203,18 +201,8 @@ public class EurekaClientAutoConfiguration { @Bean @ConditionalOnBean(AutoServiceRegistrationProperties.class) @ConditionalOnProperty(value = "spring.cloud.service-registry.auto-registration.enabled", matchIfMissing = true) - public EurekaRegistration eurekaRegistration(EurekaClient eurekaClient, CloudEurekaInstanceConfig instanceConfig, ApplicationInfoManager applicationInfoManager) { - return EurekaRegistration.builder(instanceConfig) - .with(applicationInfoManager) - .with(eurekaClient) - .with(healthCheckHandler) - .build(); - } - - @Bean - @ConditionalOnBean(AutoServiceRegistrationProperties.class) - @ConditionalOnProperty(value = "spring.cloud.service-registry.auto-registration.enabled", matchIfMissing = true) - public EurekaAutoServiceRegistration eurekaAutoServiceRegistration(ApplicationContext context, EurekaServiceRegistry registry, EurekaRegistration registration) { + public EurekaAutoServiceRegistration eurekaAutoServiceRegistration(ApplicationContext context, EurekaServiceRegistry registry, + EurekaRegistration registration) { return new EurekaAutoServiceRegistration(context, registry, registration); } @@ -242,6 +230,20 @@ public class EurekaClientAutoConfiguration { InstanceInfo instanceInfo = new InstanceInfoFactory().create(config); return new ApplicationInfoManager(config, instanceInfo); } + + @Bean + @ConditionalOnBean(AutoServiceRegistrationProperties.class) + @ConditionalOnProperty(value = "spring.cloud.service-registry.auto-registration.enabled", matchIfMissing = true) + public EurekaRegistration eurekaRegistration(EurekaClient eurekaClient, + CloudEurekaInstanceConfig instanceConfig, + ApplicationInfoManager applicationInfoManager, + @Autowired(required = false) HealthCheckHandler healthCheckHandler) { + return EurekaRegistration.builder(instanceConfig) + .with(applicationInfoManager) + .with(eurekaClient) + .with(healthCheckHandler) + .build(); + } } @Configuration @@ -273,6 +275,21 @@ public class EurekaClientAutoConfiguration { return new ApplicationInfoManager(config, instanceInfo); } + @Bean + @org.springframework.cloud.context.config.annotation.RefreshScope + @ConditionalOnBean(AutoServiceRegistrationProperties.class) + @ConditionalOnProperty(value = "spring.cloud.service-registry.auto-registration.enabled", matchIfMissing = true) + public EurekaRegistration eurekaRegistration(EurekaClient eurekaClient, + CloudEurekaInstanceConfig instanceConfig, + ApplicationInfoManager applicationInfoManager, + @Autowired(required = false) HealthCheckHandler healthCheckHandler) { + return EurekaRegistration.builder(instanceConfig) + .with(applicationInfoManager) + .with(eurekaClient) + .with(healthCheckHandler) + .build(); + } + } @Target({ ElementType.TYPE, ElementType.METHOD }) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaRegistration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaRegistration.java index dd9f991a3..c591e4d1e 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaRegistration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaRegistration.java @@ -44,7 +44,7 @@ import com.netflix.discovery.EurekaClientConfig; /** * @author Spencer Gibb */ -public class EurekaRegistration implements Registration, Closeable { +public class EurekaRegistration implements Registration { private static final Log log = LogFactory.getLog(EurekaRegistration.class); private final EurekaClient eurekaClient; @@ -201,8 +201,4 @@ public class EurekaRegistration implements Registration, Closeable { return this.instanceConfig.getSecurePort(); } - @Override - public void close() throws IOException { - this.eurekaClient.shutdown(); - } } diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java index a787475fb..a6e20b8d0 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java @@ -16,27 +16,32 @@ package org.springframework.cloud.netflix.eureka; -import java.io.IOException; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.After; import org.junit.Test; import org.mockito.Mockito; + +import org.springframework.aop.framework.Advised; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.SearchStrategy; import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration; +import org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationProperties; import org.springframework.cloud.commons.util.UtilAutoConfiguration; +import org.springframework.cloud.context.refresh.ContextRefresher; import org.springframework.cloud.context.scope.GenericScope; -import org.springframework.cloud.netflix.eureka.serviceregistry.EurekaRegistration; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.test.util.ReflectionTestUtils; import com.netflix.appinfo.ApplicationInfoManager; +import com.netflix.appinfo.HealthCheckHandler; import com.netflix.discovery.EurekaClient; import com.netflix.discovery.EurekaClientConfig; import com.netflix.discovery.shared.transport.jersey.EurekaJerseyClient; @@ -45,8 +50,6 @@ import com.sun.jersey.client.apache4.ApacheHttpClient4; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; import static org.springframework.boot.test.util.EnvironmentTestUtils.addEnvironment; /** @@ -366,6 +369,43 @@ public class EurekaClientAutoConfigurationTests { .startsWith(GenericScope.class.getName()+"$LockedScopedProxyFactoryBean"); } + @Test + public void shouldReregisterHealthCheckHandlerAfterRefresh() throws Exception { + addEnvironment(this.context, "eureka.client.healthcheck.enabled=true"); + setupContext(RefreshAutoConfiguration.class, AutoServiceRegistrationConfiguration.class); + + EurekaClient oldEurekaClient = getLazyInitEurekaClient(); + + HealthCheckHandler healthCheckHandler = this.context.getBean("eurekaHealthCheckHandler", HealthCheckHandler.class); + + assertThat(healthCheckHandler).isInstanceOf(EurekaHealthCheckHandler.class); + assertThat(oldEurekaClient.getHealthCheckHandler()).isSameAs(healthCheckHandler); + + ContextRefresher refresher = this.context.getBean("contextRefresher", ContextRefresher.class); + refresher.refresh(); + + EurekaClient newEurekaClient = getLazyInitEurekaClient(); + HealthCheckHandler newHealthCheckHandler = this.context.getBean("eurekaHealthCheckHandler", HealthCheckHandler.class); + + assertThat(healthCheckHandler).isSameAs(newHealthCheckHandler); + assertThat(oldEurekaClient).isNotSameAs(newEurekaClient); + assertThat(newEurekaClient.getHealthCheckHandler()).isSameAs(healthCheckHandler); + } + + @Test + public void shouldCloseDiscoveryClient() throws Exception { + addEnvironment(this.context, "eureka.client.healthcheck.enabled=true"); + setupContext(RefreshAutoConfiguration.class, AutoServiceRegistrationConfiguration.class); + + AtomicBoolean isShutdown = (AtomicBoolean) ReflectionTestUtils.getField(getLazyInitEurekaClient(), "isShutdown"); + + assertThat(isShutdown.get()).isFalse(); + + this.context.close(); + + assertThat(isShutdown.get()).isTrue(); + } + @Test public void basicAuth() { addEnvironment(this.context, "server.port=8989", @@ -425,16 +465,6 @@ public class EurekaClientAutoConfigurationTests { } } - @Test - public void eurekaRegistrationClosed() throws IOException { - setupContext(TestEurekaRegistrationConfiguration.class); - if (this.context != null) { - EurekaRegistration registration = this.context.getBean(EurekaRegistration.class); - this.context.close(); - verify(registration).close(); - } - } - private void testNonSecurePort(String propName) { addEnvironment(this.context, propName + ":8888"); setupContext(); @@ -452,6 +482,10 @@ public class EurekaClientAutoConfigurationTests { return this.context.getBean(EurekaInstanceConfigBean.class); } + private EurekaClient getLazyInitEurekaClient() throws Exception { + return (EurekaClient)((Advised) this.context.getBean("eurekaClient", EurekaClient.class)).getTargetSource().getTarget(); + } + @Configuration @EnableConfigurationProperties @Import({ UtilAutoConfiguration.class, EurekaClientAutoConfiguration.class }) @@ -481,18 +515,6 @@ public class EurekaClientAutoConfigurationTests { } } - @Configuration - protected static class TestEurekaRegistrationConfiguration { - - @Bean - public EurekaRegistration eurekaRegistration(EurekaClient eurekaClient, CloudEurekaInstanceConfig instanceConfig, ApplicationInfoManager applicationInfoManager) { - return spy(EurekaRegistration.builder(instanceConfig) - .with(applicationInfoManager) - .with(eurekaClient) - .build()); - } - } - @Configuration protected static class MockClientConfiguration { @@ -515,4 +537,10 @@ public class EurekaClientAutoConfigurationTests { return Mockito.mock(ApacheHttpClient4.class); } } + + @Configuration + @EnableConfigurationProperties(AutoServiceRegistrationProperties.class) + public static class AutoServiceRegistrationConfiguration { + + } }