From 184eec0f5846a8c15ae43cea1bdc4dbcaaf1617c Mon Sep 17 00:00:00 2001 From: Venil Noronha Date: Fri, 4 Nov 2016 09:15:14 -0700 Subject: [PATCH] Enables failFast setting for consul service registration (#238) Adds `spring.cloud.consul.discovery.failFast` which defaults to `true` (current behaviour). fixes gh-175 --- .../discovery/ConsulDiscoveryProperties.java | 9 ++++++++- .../consul/discovery/ConsulLifecycle.java | 20 +++++++++++++++---- .../ConsulLifecycleCustomizedPropsTests.java | 13 ++++++++++-- .../discovery/ConsulLifecycleTests.java | 13 ++++++++++-- 4 files changed, 46 insertions(+), 9 deletions(-) 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 c08b66bb..15152b34 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 @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2016 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. @@ -35,6 +35,7 @@ import lombok.Setter; * Defines configuration for service discovery and registration. * * @author Spencer Gibb + * @author Venil Noronha */ @ConfigurationProperties("spring.cloud.consul.discovery") @Data @@ -138,6 +139,12 @@ public class ConsulDiscoveryProperties { */ private boolean registerHealthCheck = true; + /** + * Throw exceptions during service registration if true, otherwise, log + * warnings (defaults to true). + */ + private boolean failFast = true; + @SuppressWarnings("unused") private ConsulDiscoveryProperties() {} 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 3ad850cf..4d7a1693 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 @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2016 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. @@ -24,8 +24,10 @@ import javax.servlet.ServletContext; import org.springframework.cloud.client.discovery.AbstractDiscoveryLifecycle; import org.springframework.retry.annotation.Retryable; import org.springframework.util.Assert; +import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; +import com.ecwid.consul.ConsulException; import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.agent.model.NewService; @@ -33,6 +35,7 @@ import lombok.extern.slf4j.Slf4j; /** * @author Spencer Gibb + * @author Venil Noronha */ @Slf4j public class ConsulLifecycle extends AbstractDiscoveryLifecycle { @@ -170,9 +173,18 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { protected void register(NewService newService) { log.info("Registering service with consul: {}", newService.toString()); - client.agentServiceRegister(newService, properties.getAclToken()); - if (ttlConfig.isEnabled() && ttlScheduler != null) { - ttlScheduler.add(newService); + try { + client.agentServiceRegister(newService, properties.getAclToken()); + if (ttlConfig.isEnabled() && ttlScheduler != null) { + ttlScheduler.add(newService); + } + } + catch (ConsulException e) { + if (this.properties.isFailFast()) { + log.error("Error registering service with consul: {}", newService.toString(), e); + ReflectionUtils.rethrowRuntimeException(e); + } + log.warn("Failfast is false. Error registering service with consul: {}", newService.toString(), e); } } diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleCustomizedPropsTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleCustomizedPropsTests.java index 7710a2bb..2d2473bd 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleCustomizedPropsTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleCustomizedPropsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2016 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. @@ -42,10 +42,12 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; /** * @author Spencer Gibb + * @author Venil Noronha */ @RunWith(SpringJUnit4ClassRunner.class) @FixMethodOrder(MethodSorters.NAME_ASCENDING) @@ -55,7 +57,8 @@ import static org.junit.Assert.assertThat; "spring.cloud.consul.discovery.port=4452", "spring.cloud.consul.discovery.hostname=myhost", "spring.cloud.consul.discovery.ipAddress=10.0.0.1", - "spring.cloud.consul.discovery.registerHealthCheck=false", }, randomPort = true) + "spring.cloud.consul.discovery.registerHealthCheck=false", + "spring.cloud.consul.discovery.failFast=false" }, randomPort = true) public class ConsulLifecycleCustomizedPropsTests { @Autowired @@ -87,6 +90,12 @@ public class ConsulLifecycleCustomizedPropsTests { List checks = checkResponse.getValue(); assertThat("checks was wrong size", checks, hasSize(0)); } + + @Test + public void testFailFastDisabled() { + assertFalse("property failFast was wrong", this.properties.isFailFast()); + } + } @Configuration diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleTests.java index b1cf55e1..2855bb01 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2016 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. @@ -16,8 +16,10 @@ package org.springframework.cloud.consul.discovery; +import com.ecwid.consul.ConsulException; import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.Response; +import com.ecwid.consul.v1.agent.model.NewService; import com.ecwid.consul.v1.agent.model.Service; import org.junit.FixMethodOrder; import org.junit.Test; @@ -43,11 +45,13 @@ import static org.junit.Assert.assertNotNull; /** * @author Spencer Gibb + * @author Venil Noronha */ @RunWith(SpringJUnit4ClassRunner.class) @FixMethodOrder(MethodSorters.NAME_ASCENDING) @SpringApplicationConfiguration(classes = TestConfig.class) -@WebIntegrationTest(value = "spring.application.name=myTestService1-F::something", randomPort = true) +@WebIntegrationTest(value = { "spring.application.name=myTestService1-F::something", + "spring.cloud.consul.discovery.failFast=true" }, randomPort = true) public class ConsulLifecycleTests { @Autowired @@ -83,6 +87,11 @@ public class ConsulLifecycleTests { assertEquals("ab-c1", ConsulLifecycle.normalizeForDns("ab::c1")); } + @Test(expected = ConsulException.class) + public void testFailFastEnabled() { + lifecycle.register(new NewService()); + } + @Test(expected = IllegalArgumentException.class) public void normalizedFailsIfFirstCharIsNumber() { ConsulLifecycle.normalizeForDns("9abc");