From c0e9795e98fbf4bc989033637cd777e4a4148f2b Mon Sep 17 00:00:00 2001 From: Donnabell Dmello Date: Mon, 24 Oct 2016 20:47:58 -0700 Subject: [PATCH] Adds healthCheckCriticalTimeout to discovery props The healthCheckCriticalTimeout property tells consul to deregister services in critical longer than timeout. Fixes gh-210 --- .../discovery/ConsulDiscoveryProperties.java | 34 ++++----- .../consul/discovery/ConsulLifecycle.java | 1 + .../serviceregistry/ConsulRegistration.java | 3 + .../ConsulLifecycleDefaultCheckTests.java | 68 ++++++++++++++++++ ...oServiceRegistrationDefaultCheckTests.java | 69 +++++++++++++++++++ src/main/bash/travis_install_consul.sh | 2 +- 6 files changed, 157 insertions(+), 20 deletions(-) create mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleDefaultCheckTests.java create mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationDefaultCheckTests.java diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java index 97bb3f70..7c92e760 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java @@ -35,6 +35,7 @@ import lombok.Setter; * Defines configuration for service discovery and registration. * * @author Spencer Gibb + * @author Donnabell Dmello * @author Venil Noronha */ @ConfigurationProperties("spring.cloud.consul.discovery") @@ -65,14 +66,19 @@ public class ConsulDiscoveryProperties { /** Custom health check url to override default */ private String healthCheckUrl; - /** How often to perform the health check (e.g. 10s) */ + /** How often to perform the health check (e.g. 10s), defaults to 10s. */ private String healthCheckInterval = "10s"; - /** Timeout for health check (e.g. 10s) */ + /** Timeout for health check (e.g. 10s). */ private String healthCheckTimeout; - /** IP address to use when accessing service (must also set preferIpAddress - to use) */ + /** + * Timeout to deregister services critical for longer than timeout (e.g. 30m). + * Requires consul version 7.x or higher. + */ + private String healthCheckCriticalTimeout; + + /** IP address to use when accessing service (must also set preferIpAddress to use) */ private String ipAddress; /** Hostname to use when accessing server */ @@ -86,14 +92,10 @@ public class ConsulDiscoveryProperties { private Lifecycle lifecycle = new Lifecycle(); - /** - * Use ip address rather than hostname during registration - */ + /** Use ip address rather than hostname during registration */ private boolean preferIpAddress = false; - /** - * Source of how we will determine the address to use - */ + /** Source of how we will determine the address to use */ private boolean preferAgentAddress = false; private int catalogServicesWatchDelay = 10; @@ -130,9 +132,7 @@ public class ConsulDiscoveryProperties { */ private Map serverListQueryTags = new HashMap<>(); - /** - * Tag to query for in service list if one is not listed in serverListQueryTags. - */ + /** Tag to query for in service list if one is not listed in serverListQueryTags. */ private String defaultQueryTag; /** @@ -141,14 +141,10 @@ public class ConsulDiscoveryProperties { */ private boolean queryPassing = false; - /** - * Register as a service in consul. - */ + /** Register as a service in consul. */ private boolean register = true; - /** - * Register health check in consul. Useful during development of a service. - */ + /** Register health check in consul. Useful during development of a service. */ private boolean registerHealthCheck = true; /** diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java index 0284ab81..043ee6ea 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java @@ -37,6 +37,7 @@ import lombok.extern.slf4j.Slf4j; /** * @author Spencer Gibb + * @author Donnabell Dmello * @author Venil Noronha * * @deprecated See {@link org.springframework.cloud.consul.serviceregistry.ConsulAutoServiceRegistration} diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistration.java index 6a33a118..91fc6aaa 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulRegistration.java @@ -225,6 +225,9 @@ public class ConsulRegistration implements Registration { } check.setInterval(properties.getHealthCheckInterval()); check.setTimeout(properties.getHealthCheckTimeout()); + if (StringUtils.hasText(properties.getHealthCheckCriticalTimeout())) { + check.setDeregisterCriticalServiceAfter(properties.getHealthCheckCriticalTimeout()); + } return check; } diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleDefaultCheckTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleDefaultCheckTests.java new file mode 100644 index 00000000..48088543 --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleDefaultCheckTests.java @@ -0,0 +1,68 @@ +/* + * Copyright 2013-2017 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.consul.discovery; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.junit4.SpringRunner; + +import com.ecwid.consul.v1.ConsulClient; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +/** + * @author Spencer Gibb + * @deprecated remove in Edgware + */ +@Deprecated +@RunWith(SpringRunner.class) +@SpringBootTest(classes = TestPropsConfig.class, + properties = { "spring.application.name=myTestServiceDefaultChecks", + "spring.cloud.consul.discovery.instanceId=myTestServiceDefaultChecks", + "spring.cloud.consul.discovery.healthCheckInterval=19s", + "spring.cloud.consul.discovery.healthCheckTimeout=12s", + "spring.cloud.consul.discovery.healthCheckCriticalTimeout=30m", + }, webEnvironment = RANDOM_PORT) +public class ConsulLifecycleDefaultCheckTests { + + @Autowired + private ConsulClient consul; + + @Autowired + private ConsulDiscoveryProperties properties; + + @Test + public void contextLoads() { + assertThat(properties.getHealthCheckCriticalTimeout()).isEqualTo("30m"); + assertThat(properties.getHealthCheckInterval()).isEqualTo("19s"); + assertThat(properties.getHealthCheckTimeout()).isEqualTo("12s"); + + // I'm unable to find a way to query consul to see the configuration of the health check + // so for now, just sending the new healthCheckCriticalTimeout and having consul accept + // it is going to have to suffice + + //final Response> checksForService = consul.getHealthChecksForService("myTestServiceDefaultChecks", QueryParams.DEFAULT); + //final List checkList = checksForService.getValue(); + //final Response> response2 = consul.getAgentChecks(); + //final Map checks = response2.getValue(); + //final Check check = checks.get("myTestServiceDefaultChecks"); + //Assertions.assertThat(check).isNotNull(); + } +} diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationDefaultCheckTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationDefaultCheckTests.java new file mode 100644 index 00000000..7abc02c4 --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationDefaultCheckTests.java @@ -0,0 +1,69 @@ +/* + * Copyright 2013-2017 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.consul.serviceregistry; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.consul.discovery.ConsulDiscoveryProperties; +import org.springframework.cloud.consul.serviceregistry.ConsulAutoServiceRegistrationCustomizedPropsTests.TestPropsConfig; +import org.springframework.test.context.junit4.SpringRunner; + +import com.ecwid.consul.v1.ConsulClient; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +/** + * @author Spencer Gibb + * @deprecated remove in Edgware + */ +@RunWith(SpringRunner.class) +@SpringBootTest(classes = TestPropsConfig.class, + properties = { "spring.application.name=myTestServiceDefaultChecks", + "spring.cloud.consul.discovery.instanceId=myTestServiceDefaultChecks", + "spring.cloud.consul.discovery.healthCheckCriticalTimeout=30m", + "spring.cloud.consul.discovery.healthCheckInterval=19s", + "spring.cloud.consul.discovery.healthCheckTimeout=12s", + }, webEnvironment = RANDOM_PORT) +public class ConsulAutoServiceRegistrationDefaultCheckTests { + + @Autowired + private ConsulClient consul; + + @Autowired + private ConsulDiscoveryProperties properties; + + @Test + public void contextLoads() { + assertThat(properties.getHealthCheckCriticalTimeout()).isEqualTo("30m"); + assertThat(properties.getHealthCheckInterval()).isEqualTo("19s"); + assertThat(properties.getHealthCheckTimeout()).isEqualTo("12s"); + + // I'm unable to find a way to query consul to see the configuration of the health check + // so for now, just sending the new healthCheckCriticalTimeout and having consul accept + // it is going to have to suffice + + //final Response> checksForService = consul.getHealthChecksForService("myTestServiceDefaultChecks", QueryParams.DEFAULT); + //final List checkList = checksForService.getValue(); + //final Response> response2 = consul.getAgentChecks(); + //final Map checks = response2.getValue(); + //final Check check = checks.get("myTestServiceDefaultChecks"); + //Assertions.assertThat(check).isNotNull(); + } +} diff --git a/src/main/bash/travis_install_consul.sh b/src/main/bash/travis_install_consul.sh index c752d04a..81a13924 100755 --- a/src/main/bash/travis_install_consul.sh +++ b/src/main/bash/travis_install_consul.sh @@ -1,6 +1,6 @@ #!/bin/bash -CONSUL_VER="0.6.3" +CONSUL_VER="0.7.2" CONSUL_ZIP="consul_${CONSUL_VER}_linux_amd64.zip" IGNORE_CERTS="${IGNORE_CERTS:-no}"