diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java index 24ce7726..a332b6eb 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java @@ -47,7 +47,7 @@ import lombok.extern.apachecommons.CommonsLog; public class ConsulDiscoveryClient implements DiscoveryClient { interface LocalResolver { - String getServiceId(); + String getInstanceId(); Integer getPort(); } @@ -62,8 +62,8 @@ public class ConsulDiscoveryClient implements DiscoveryClient { ConsulDiscoveryProperties properties) { this(client, properties, new LocalResolver() { @Override - public String getServiceId() { - return lifecycle.getServiceId(); + public String getInstanceId() { + return lifecycle.getInstanceId(); } @Override @@ -92,24 +92,24 @@ public class ConsulDiscoveryClient implements DiscoveryClient { @Override public ServiceInstance getLocalServiceInstance() { Response> agentServices = client.getAgentServices(); - Service service = agentServices.getValue().get(localResolver.getServiceId()); - String serviceId; + Service service = agentServices.getValue().get(localResolver.getInstanceId()); + String instanceId; Integer port; Map metadata; String host = "localhost"; // if we have a response from consul, that is the ultimate source of truth if (service != null) { - serviceId = service.getId(); + instanceId = service.getId(); port = service.getPort(); host = service.getAddress(); metadata = getMetadata(service.getTags()); } else { // possibly called before registration, use configuration or best guess log.warn("Unable to locate service in consul agent: " - + localResolver.getServiceId()); + + localResolver.getInstanceId()); - serviceId = localResolver.getServiceId(); + instanceId = localResolver.getInstanceId(); port = localResolver.getPort(); if (port == 0 && serverProperties != null && serverProperties.getPort() != null) { @@ -128,7 +128,7 @@ public class ConsulDiscoveryClient implements DiscoveryClient { } } - return new DefaultServiceInstance(serviceId, host, port, false, metadata); + return new DefaultServiceInstance(instanceId, host, port, false, metadata); } private String getAgentHost() { diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientConfiguration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientConfiguration.java index e437eaea..f19c6775 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientConfiguration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientConfiguration.java @@ -83,14 +83,14 @@ public class ConsulDiscoveryClientConfiguration { } @Override - public String getServiceId() { + public String getInstanceId() { ConsulRegistration registration = getBean(ConsulRegistration.class); if (registration != null) { - return registration.getServiceId(); + return registration.getInstanceId(); } ConsulLifecycle lifecycle = getBean(ConsulLifecycle.class); if (lifecycle != null) { - return lifecycle.getServiceId(); + return lifecycle.getInstanceId(); } throw new IllegalStateException("Must have one of ConsulRegistration or ConsulLifecycle"); } 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 043ee6ea..cfe8e152 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 @@ -16,14 +16,12 @@ package org.springframework.cloud.consul.discovery; -import java.util.List; - import javax.servlet.ServletContext; import org.springframework.beans.BeansException; import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.cloud.client.discovery.AbstractDiscoveryLifecycle; -import org.springframework.cloud.consul.serviceregistry.ConsulRegistration; +import org.springframework.cloud.consul.serviceregistry.ConsulAutoRegistration; import org.springframework.context.ApplicationContext; import org.springframework.retry.annotation.Retryable; import org.springframework.util.Assert; @@ -110,7 +108,8 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { return; } Assert.notNull(service.getPort(), "service.port has not been set"); - ConsulRegistration registration = ConsulRegistration.lifecycleRegistration(service.getPort(), this.properties, getContext(), this.servletContext, this.ttlConfig); + ConsulAutoRegistration registration = ConsulAutoRegistration.lifecycleRegistration(service.getPort(), + getServiceId(), this.properties, getContext(), this.servletContext, this.ttlConfig); if (registration.getService().getPort() == null) { // not set by properties registration.initializePort(service.getPort()); } @@ -119,15 +118,16 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { register(service); } - private NewService.Check createCheck(Integer port) { - return ConsulRegistration.createCheck(port, this.ttlConfig, this.properties); + @Deprecated + public String getServiceId() { + return getInstanceId(); } - public String getServiceId() { + public String getInstanceId() { // cache instanceId, so on refresh this won't get recomputed // this is a problem if ${random.value} is used if (this.instanceId == null) { - this.instanceId = ConsulRegistration.getServiceId(properties, getContext()); + this.instanceId = ConsulAutoRegistration.getInstanceId(properties, getContext()); } return this.instanceId; } @@ -138,7 +138,7 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { return; } - ConsulRegistration registration = ConsulRegistration.managementRegistration(this.properties, getContext(), this.ttlConfig); + ConsulAutoRegistration registration = ConsulAutoRegistration.managementRegistration(this.properties, getContext(), this.ttlConfig); register(registration.getService()); } @@ -175,10 +175,6 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { deregister(getManagementServiceId()); } - private List createTags() { - return ConsulRegistration.createTags(this.properties, this.servletContext); - } - private void deregister(String serviceId) { if (!this.properties.isRegister()) { return; @@ -197,35 +193,35 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { @Override protected String getAppName() { - return ConsulRegistration.getAppName(this.properties, this.propertyResolver); + return ConsulAutoRegistration.getAppName(this.properties, this.propertyResolver); } /** * @return the serviceId of the Management Service */ public String getManagementServiceId() { - return ConsulRegistration.normalizeForDns(getContext().getId()) + SEPARATOR + properties.getManagementSuffix(); + return ConsulAutoRegistration.normalizeForDns(getContext().getId()) + SEPARATOR + properties.getManagementSuffix(); } /** * @return the service name of the Management Service */ public String getManagementServiceName() { - return ConsulRegistration.normalizeForDns(getAppName()) + SEPARATOR + properties.getManagementSuffix(); + return ConsulAutoRegistration.normalizeForDns(getAppName()) + SEPARATOR + properties.getManagementSuffix(); } /** * @return the port of the Management Service */ protected Integer getManagementPort() { - return ConsulRegistration.getManagementPort(this.properties, getContext()); + return ConsulAutoRegistration.getManagementPort(this.properties, getContext()); } /** - * @deprecated See {@link org.springframework.cloud.consul.serviceregistry.ConsulRegistration#normalizeForDns(String)} + * @deprecated See {@link org.springframework.cloud.consul.serviceregistry.ConsulAutoRegistration#normalizeForDns(String)} */ @Deprecated public static String normalizeForDns(String s) { - return ConsulRegistration.normalizeForDns(s); + return ConsulAutoRegistration.normalizeForDns(s); } } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/TtlScheduler.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/TtlScheduler.java index b46976b7..eae5da4f 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/TtlScheduler.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/TtlScheduler.java @@ -50,25 +50,30 @@ public class TtlScheduler { this.client = client; } + @Deprecated + public void add(final NewService service) { + add(service.getId()); + } + /** * Add a service to the checks loop. */ - public void add(final NewService service) { + public void add(String instanceId) { ScheduledFuture task = scheduler.scheduleAtFixedRate(new ConsulHeartbeatTask( - service.getId()), configuration.computeHearbeatInterval() + instanceId), configuration.computeHearbeatInterval() .toStandardDuration().getMillis()); - ScheduledFuture previousTask = serviceHeartbeats.put(service.getId(), task); + ScheduledFuture previousTask = serviceHeartbeats.put(instanceId, task); if (previousTask != null) { previousTask.cancel(true); } } - public void remove(String serviceId) { - ScheduledFuture task = serviceHeartbeats.get(serviceId); + public void remove(String instanceId) { + ScheduledFuture task = serviceHeartbeats.get(instanceId); if (task != null) { task.cancel(true); } - serviceHeartbeats.remove(serviceId); + serviceHeartbeats.remove(instanceId); } private class ConsulHeartbeatTask implements Runnable { @@ -87,4 +92,4 @@ public class TtlScheduler { log.debug("Sending consul heartbeat for: " + checkId); } } -} \ No newline at end of file +} diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java new file mode 100644 index 00000000..97d762fb --- /dev/null +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java @@ -0,0 +1,261 @@ +/* + * 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 java.util.LinkedList; +import java.util.List; + +import javax.servlet.ServletContext; + +import org.springframework.boot.bind.RelaxedPropertyResolver; +import org.springframework.cloud.client.discovery.ManagementServerPortUtils; +import org.springframework.cloud.client.serviceregistry.ServiceRegistry; +import org.springframework.cloud.consul.discovery.ConsulDiscoveryProperties; +import org.springframework.cloud.consul.discovery.HeartbeatProperties; +import org.springframework.context.ApplicationContext; +import org.springframework.util.Assert; +import org.springframework.util.StringUtils; + +import com.ecwid.consul.v1.agent.model.NewService; + +/** + * @author Spencer Gibb + */ +public class ConsulAutoRegistration extends ConsulRegistration { + + public static final char SEPARATOR = '-'; + + private final ConsulDiscoveryProperties properties; + private final ApplicationContext context; + private final HeartbeatProperties heartbeatProperties; + + public ConsulAutoRegistration(NewService service, ConsulDiscoveryProperties properties, ApplicationContext context, HeartbeatProperties heartbeatProperties) { + super(service); + this.properties = properties; + this.context = context; + this.heartbeatProperties = heartbeatProperties; + } + + public void initializePort(int knownPort) { + if (getService().getPort() == null) { + // not set by properties + getService().setPort(knownPort); + } + // we might not have a port until now, so this is the earliest we + // can create a check + + setCheck(getService(), this.properties, this.context, this.heartbeatProperties); + } + + public ConsulAutoRegistration managementRegistration() { + return managementRegistration(this.properties, this.context, this.heartbeatProperties); + } + + public static ConsulAutoRegistration registration(ConsulDiscoveryProperties properties, ApplicationContext context, + ServletContext servletContext, HeartbeatProperties heartbeatProperties) { + RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver(context.getEnvironment()); + + NewService service = new NewService(); + String appName = getAppName(properties, propertyResolver); + service.setId(getInstanceId(properties, context)); + if(!properties.isPreferAgentAddress()) { + service.setAddress(properties.getHostname()); + } + service.setName(normalizeForDns(appName)); + service.setTags(createTags(properties, servletContext)); + + if (properties.getPort() != null) { + service.setPort(properties.getPort()); + } + + return new ConsulAutoRegistration(service, properties, context, heartbeatProperties); + } + + @Deprecated //TODO: do I need this here, or should I just copy what I need back into lifecycle? + public static ConsulAutoRegistration lifecycleRegistration(Integer port, String instanceId, ConsulDiscoveryProperties properties, ApplicationContext context, + ServletContext servletContext, HeartbeatProperties heartbeatProperties) { + RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver(context.getEnvironment()); + + NewService service = new NewService(); + String appName = getAppName(properties, propertyResolver); + service.setId(instanceId); + if(!properties.isPreferAgentAddress()) { + service.setAddress(properties.getHostname()); + } + service.setName(normalizeForDns(appName)); + service.setTags(createTags(properties, servletContext)); + + // If an alternate external port is specified, register using it instead + if (properties.getPort() != null) { + service.setPort(properties.getPort()); + } else { + service.setPort(port); + } + + Assert.notNull(service.getPort(), "service.port may not be null"); + + setCheck(service, properties, context, heartbeatProperties); + + return new ConsulAutoRegistration(service, properties, context, heartbeatProperties); + } + + public static void setCheck(NewService service, ConsulDiscoveryProperties properties, ApplicationContext context, HeartbeatProperties heartbeatProperties) { + if (properties.isRegisterHealthCheck()) { + Integer checkPort; + if (shouldRegisterManagement(properties, context)) { + checkPort = getManagementPort(properties, context); + } else { + checkPort = service.getPort(); + } + Assert.notNull(checkPort, "checkPort may not be null"); + service.setCheck(createCheck(checkPort, heartbeatProperties, properties)); + } + } + + public static ConsulAutoRegistration managementRegistration(ConsulDiscoveryProperties properties, ApplicationContext context, + HeartbeatProperties heartbeatProperties) { + RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver(context.getEnvironment()); + NewService management = new NewService(); + management.setId(getManagementServiceId(properties, context)); + management.setAddress(properties.getHostname()); + management.setName(getManagementServiceName(properties, propertyResolver)); + management.setPort(getManagementPort(properties, context)); + management.setTags(properties.getManagementTags()); + if (properties.isRegisterHealthCheck()) { + management.setCheck(createCheck(getManagementPort(properties, context), heartbeatProperties, properties)); + } + return new ConsulAutoRegistration(management, properties, context, heartbeatProperties); + } + + public static String getInstanceId(ConsulDiscoveryProperties properties, ApplicationContext context) { + if (!StringUtils.hasText(properties.getInstanceId())) { + return normalizeForDns(context.getId()); + } else { + return normalizeForDns(properties.getInstanceId()); + } + } + + public static String normalizeForDns(String s) { + if (s == null || !Character.isLetter(s.charAt(0)) + || !Character.isLetterOrDigit(s.charAt(s.length()-1))) { + throw new IllegalArgumentException("Consul service ids must not be empty, must start with a letter, end with a letter or digit, and have as interior characters only letters, digits, and hyphen"); + } + + StringBuilder normalized = new StringBuilder(); + Character prev = null; + for (char curr : s.toCharArray()) { + Character toAppend = null; + if (Character.isLetterOrDigit(curr)) { + toAppend = curr; + } else if (prev == null || !(prev == SEPARATOR)) { + toAppend = SEPARATOR; + } + if (toAppend != null) { + normalized.append(toAppend); + prev = toAppend; + } + } + + return normalized.toString(); + } + + + public static List createTags(ConsulDiscoveryProperties properties, ServletContext servletContext) { + List tags = new LinkedList<>(properties.getTags()); + if(servletContext != null + && StringUtils.hasText(servletContext.getContextPath()) + && StringUtils.hasText(servletContext.getContextPath().replaceAll("/", ""))) { + tags.add("contextPath=" + servletContext.getContextPath()); + } + if (!StringUtils.isEmpty(properties.getInstanceZone())) { + tags.add(properties.getDefaultZoneMetadataName() + "=" + properties.getInstanceZone()); + } + if (!StringUtils.isEmpty(properties.getInstanceGroup())) { + tags.add("group=" + properties.getInstanceGroup()); + } + return tags; + } + + public static NewService.Check createCheck(Integer port, HeartbeatProperties ttlConfig, + ConsulDiscoveryProperties properties) { + NewService.Check check = new NewService.Check(); + if (ttlConfig.isEnabled()) { + check.setTtl(ttlConfig.getTtl()); + return check; + } + + Assert.notNull(port, "createCheck port must not be null"); + Assert.isTrue(port > 0, "createCheck port must be greater than 0"); + + if (properties.getHealthCheckUrl() != null) { + check.setHttp(properties.getHealthCheckUrl()); + } else { + check.setHttp(String.format("%s://%s:%s%s", properties.getScheme(), + properties.getHostname(), port, + properties.getHealthCheckPath())); + } + check.setInterval(properties.getHealthCheckInterval()); + check.setTimeout(properties.getHealthCheckTimeout()); + if (StringUtils.hasText(properties.getHealthCheckCriticalTimeout())) { + check.setDeregisterCriticalServiceAfter(properties.getHealthCheckCriticalTimeout()); + } + return check; + } + + /** + * @return the app name, currently the spring.application.name property + */ + public static String getAppName(ConsulDiscoveryProperties properties, RelaxedPropertyResolver propertyResolver) { + String appName = properties.getServiceName(); + if (!StringUtils.isEmpty(appName)) { + return appName; + } + return propertyResolver.getProperty("spring.application.name", "application"); + } + + /** + * @return if the management service should be registered with the {@link ServiceRegistry} + */ + public static boolean shouldRegisterManagement(ConsulDiscoveryProperties properties, ApplicationContext context) { + return getManagementPort(properties, context) != null && ManagementServerPortUtils.isDifferent(context); + } + + /** + * @return the serviceId of the Management Service + */ + public static String getManagementServiceId(ConsulDiscoveryProperties properties, ApplicationContext context) { + return normalizeForDns(context.getId()) + SEPARATOR + properties.getManagementSuffix(); + } + + /** + * @return the service name of the Management Service + */ + public static String getManagementServiceName(ConsulDiscoveryProperties properties, RelaxedPropertyResolver propertyResolver) { + return normalizeForDns(getAppName(properties, propertyResolver)) + SEPARATOR + properties.getManagementSuffix(); + } + + /** + * @return the port of the Management Service + */ + public static Integer getManagementPort(ConsulDiscoveryProperties properties, ApplicationContext context) { + // If an alternate external port is specified, use it instead + if (properties.getManagementPort() != null) { + return properties.getManagementPort(); + } + return ManagementServerPortUtils.getPort(context); + } +} diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistration.java index 86a9964c..72deb984 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistration.java @@ -32,10 +32,10 @@ public class ConsulAutoServiceRegistration extends AbstractAutoServiceRegistrati private static Log log = LogFactory.getLog(ConsulAutoServiceRegistration.class); private ConsulDiscoveryProperties properties; - private ConsulRegistration registration; + private ConsulAutoRegistration registration; public ConsulAutoServiceRegistration(ConsulServiceRegistry serviceRegistry, ConsulDiscoveryProperties properties, - ConsulRegistration registration) { + ConsulAutoRegistration registration) { super(serviceRegistry); this.properties = properties; this.registration = registration; @@ -56,13 +56,13 @@ public class ConsulAutoServiceRegistration extends AbstractAutoServiceRegistrati } @Override - protected ConsulRegistration getRegistration() { + protected ConsulAutoRegistration getRegistration() { Assert.notNull(this.registration.getService().getPort(), "service.port has not been set"); return this.registration; } @Override - protected ConsulRegistration getManagementRegistration() { + protected ConsulAutoRegistration getManagementRegistration() { return this.registration.managementRegistration(); } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationAutoConfiguration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationAutoConfiguration.java index ed897101..08de5812 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationAutoConfiguration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationAutoConfiguration.java @@ -43,15 +43,15 @@ public class ConsulAutoServiceRegistrationAutoConfiguration { @Bean @ConditionalOnMissingBean - public ConsulAutoServiceRegistration consulAutoServiceRegistration(ConsulServiceRegistry registry, ConsulDiscoveryProperties properties, ConsulRegistration consulRegistration) { + public ConsulAutoServiceRegistration consulAutoServiceRegistration(ConsulServiceRegistry registry, ConsulDiscoveryProperties properties, ConsulAutoRegistration consulRegistration) { return new ConsulAutoServiceRegistration(registry, properties, consulRegistration); } @Bean @ConditionalOnMissingBean - public ConsulRegistration consulRegistration(ConsulDiscoveryProperties properties, ApplicationContext applicationContext, + public ConsulAutoRegistration consulRegistration(ConsulDiscoveryProperties properties, ApplicationContext applicationContext, ServletContext servletContext, HeartbeatProperties heartbeatProperties) { - return ConsulRegistration.registration(properties, applicationContext, servletContext, heartbeatProperties); + return ConsulAutoRegistration.registration(properties, applicationContext, servletContext, heartbeatProperties); } } 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 91fc6aaa..b2ae9404 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 @@ -16,20 +16,7 @@ package org.springframework.cloud.consul.serviceregistry; -import java.util.LinkedList; -import java.util.List; - -import javax.servlet.ServletContext; - -import org.springframework.boot.bind.RelaxedPropertyResolver; -import org.springframework.cloud.client.discovery.ManagementServerPortUtils; import org.springframework.cloud.client.serviceregistry.Registration; -import org.springframework.cloud.client.serviceregistry.ServiceRegistry; -import org.springframework.cloud.consul.discovery.ConsulDiscoveryProperties; -import org.springframework.cloud.consul.discovery.HeartbeatProperties; -import org.springframework.context.ApplicationContext; -import org.springframework.util.Assert; -import org.springframework.util.StringUtils; import com.ecwid.consul.v1.agent.model.NewService; @@ -38,243 +25,22 @@ import com.ecwid.consul.v1.agent.model.NewService; */ public class ConsulRegistration implements Registration { - public static final char SEPARATOR = '-'; - private final NewService service; - private final ConsulDiscoveryProperties properties; - private final ApplicationContext context; - private final HeartbeatProperties heartbeatProperties; - private String instanceId; - public ConsulRegistration(NewService service, ConsulDiscoveryProperties properties, ApplicationContext context, HeartbeatProperties heartbeatProperties) { + public ConsulRegistration(NewService service) { this.service = service; - this.properties = properties; - this.context = context; - this.heartbeatProperties = heartbeatProperties; - - // cache instanceId, so on refresh this won't get recomputed - // this is a problem if ${random.value} is used - this.instanceId = ConsulRegistration.getServiceId(properties, context); - } - - public String getInstanceId() { - return this.instanceId; - } - - public void initializePort(int knownPort) { - if (getService().getPort() == null) { - // not set by properties - getService().setPort(knownPort); - } - // we might not have a port until now, so this is the earliest we - // can create a check - - setCheck(this.service, this.properties, this.context, this.heartbeatProperties); - } - - public ConsulRegistration managementRegistration() { - return managementRegistration(this.properties, this.context, this.heartbeatProperties); - } - - public static ConsulRegistration registration(ConsulDiscoveryProperties properties, ApplicationContext context, - ServletContext servletContext, HeartbeatProperties heartbeatProperties) { - RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver(context.getEnvironment()); - - NewService service = new NewService(); - String appName = getAppName(properties, propertyResolver); - service.setId(getServiceId(properties, context)); - if(!properties.isPreferAgentAddress()) { - service.setAddress(properties.getHostname()); - } - service.setName(normalizeForDns(appName)); - service.setTags(createTags(properties, servletContext)); - - if (properties.getPort() != null) { - service.setPort(properties.getPort()); - } - - return new ConsulRegistration(service, properties, context, heartbeatProperties); - } - - @Deprecated //TODO: do I need this here, or should I just copy what I need back into lifecycle? - public static ConsulRegistration lifecycleRegistration(Integer port, ConsulDiscoveryProperties properties, ApplicationContext context, - ServletContext servletContext, HeartbeatProperties heartbeatProperties) { - RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver(context.getEnvironment()); - - NewService service = new NewService(); - String appName = getAppName(properties, propertyResolver); - service.setId(getServiceId(properties, context)); - if(!properties.isPreferAgentAddress()) { - service.setAddress(properties.getHostname()); - } - service.setName(normalizeForDns(appName)); - service.setTags(createTags(properties, servletContext)); - - // If an alternate external port is specified, register using it instead - if (properties.getPort() != null) { - service.setPort(properties.getPort()); - } else { - service.setPort(port); - } - - Assert.notNull(service.getPort(), "service.port may not be null"); - - setCheck(service, properties, context, heartbeatProperties); - - return new ConsulRegistration(service, properties, context, heartbeatProperties); - } - - public static void setCheck(NewService service, ConsulDiscoveryProperties properties, ApplicationContext context, HeartbeatProperties heartbeatProperties) { - if (properties.isRegisterHealthCheck()) { - Integer checkPort; - if (shouldRegisterManagement(properties, context)) { - checkPort = getManagementPort(properties, context); - } else { - checkPort = service.getPort(); - } - Assert.notNull(checkPort, "checkPort may not be null"); - service.setCheck(createCheck(checkPort, heartbeatProperties, properties)); - } - } - - public static ConsulRegistration managementRegistration(ConsulDiscoveryProperties properties, ApplicationContext context, - HeartbeatProperties heartbeatProperties) { - RelaxedPropertyResolver propertyResolver = new RelaxedPropertyResolver(context.getEnvironment()); - NewService management = new NewService(); - management.setId(getManagementServiceId(properties, context)); - management.setAddress(properties.getHostname()); - management.setName(getManagementServiceName(properties, propertyResolver)); - management.setPort(getManagementPort(properties, context)); - management.setTags(properties.getManagementTags()); - if (properties.isRegisterHealthCheck()) { - management.setCheck(createCheck(getManagementPort(properties, context), heartbeatProperties, properties)); - } - return new ConsulRegistration(management, properties, context, heartbeatProperties); - } - - public String getServiceId() { - return this.service.getId(); - } - - public static String getServiceId(ConsulDiscoveryProperties properties, ApplicationContext context) { - if (!StringUtils.hasText(properties.getInstanceId())) { - return normalizeForDns(context.getId()); - } else { - return normalizeForDns(properties.getInstanceId()); - } - } - - public static String normalizeForDns(String s) { - if (s == null || !Character.isLetter(s.charAt(0)) - || !Character.isLetterOrDigit(s.charAt(s.length()-1))) { - throw new IllegalArgumentException("Consul service ids must not be empty, must start with a letter, end with a letter or digit, and have as interior characters only letters, digits, and hyphen"); - } - - StringBuilder normalized = new StringBuilder(); - Character prev = null; - for (char curr : s.toCharArray()) { - Character toAppend = null; - if (Character.isLetterOrDigit(curr)) { - toAppend = curr; - } else if (prev == null || !(prev == SEPARATOR)) { - toAppend = SEPARATOR; - } - if (toAppend != null) { - normalized.append(toAppend); - prev = toAppend; - } - } - - return normalized.toString(); - } - - - public static List createTags(ConsulDiscoveryProperties properties, ServletContext servletContext) { - List tags = new LinkedList<>(properties.getTags()); - if(servletContext != null - && StringUtils.hasText(servletContext.getContextPath()) - && StringUtils.hasText(servletContext.getContextPath().replaceAll("/", ""))) { - tags.add("contextPath=" + servletContext.getContextPath()); - } - if (!StringUtils.isEmpty(properties.getInstanceZone())) { - tags.add(properties.getDefaultZoneMetadataName() + "=" + properties.getInstanceZone()); - } - if (!StringUtils.isEmpty(properties.getInstanceGroup())) { - tags.add("group=" + properties.getInstanceGroup()); - } - return tags; - } - - public static NewService.Check createCheck(Integer port, HeartbeatProperties ttlConfig, - ConsulDiscoveryProperties properties) { - NewService.Check check = new NewService.Check(); - if (ttlConfig.isEnabled()) { - check.setTtl(ttlConfig.getTtl()); - return check; - } - - Assert.notNull(port, "createCheck port must not be null"); - Assert.isTrue(port > 0, "createCheck port must be greater than 0"); - - if (properties.getHealthCheckUrl() != null) { - check.setHttp(properties.getHealthCheckUrl()); - } else { - check.setHttp(String.format("%s://%s:%s%s", properties.getScheme(), - properties.getHostname(), port, - properties.getHealthCheckPath())); - } - check.setInterval(properties.getHealthCheckInterval()); - check.setTimeout(properties.getHealthCheckTimeout()); - if (StringUtils.hasText(properties.getHealthCheckCriticalTimeout())) { - check.setDeregisterCriticalServiceAfter(properties.getHealthCheckCriticalTimeout()); - } - return check; - } - - /** - * @return the app name, currently the spring.application.name property - */ - public static String getAppName(ConsulDiscoveryProperties properties, RelaxedPropertyResolver propertyResolver) { - String appName = properties.getServiceName(); - if (!StringUtils.isEmpty(appName)) { - return appName; - } - return propertyResolver.getProperty("spring.application.name", "application"); - } - - /** - * @return if the management service should be registered with the {@link ServiceRegistry} - */ - public static boolean shouldRegisterManagement(ConsulDiscoveryProperties properties, ApplicationContext context) { - return getManagementPort(properties, context) != null && ManagementServerPortUtils.isDifferent(context); - } - - /** - * @return the serviceId of the Management Service - */ - public static String getManagementServiceId(ConsulDiscoveryProperties properties, ApplicationContext context) { - return normalizeForDns(context.getId()) + SEPARATOR + properties.getManagementSuffix(); - } - - /** - * @return the service name of the Management Service - */ - public static String getManagementServiceName(ConsulDiscoveryProperties properties, RelaxedPropertyResolver propertyResolver) { - return normalizeForDns(getAppName(properties, propertyResolver)) + SEPARATOR + properties.getManagementSuffix(); - } - - /** - * @return the port of the Management Service - */ - public static Integer getManagementPort(ConsulDiscoveryProperties properties, ApplicationContext context) { - // If an alternate external port is specified, use it instead - if (properties.getManagementPort() != null) { - return properties.getManagementPort(); - } - return ManagementServerPortUtils.getPort(context); } public NewService getService() { return service; } + + public String getInstanceId() { + return getService().getId(); + } + + public String getServiceId() { + return getService().getName(); + } + } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistry.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistry.java index 339d291d..3e2673d7 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistry.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistry.java @@ -18,6 +18,9 @@ package org.springframework.cloud.consul.serviceregistry; import com.ecwid.consul.ConsulException; import com.ecwid.consul.v1.ConsulClient; +import com.ecwid.consul.v1.QueryParams; +import com.ecwid.consul.v1.Response; +import com.ecwid.consul.v1.health.model.HealthService; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.cloud.client.serviceregistry.ServiceRegistry; @@ -26,6 +29,8 @@ import org.springframework.cloud.consul.discovery.HeartbeatProperties; import org.springframework.cloud.consul.discovery.TtlScheduler; import org.springframework.util.ReflectionUtils; +import java.util.List; + /** * @author Spencer Gibb */ @@ -54,7 +59,7 @@ public class ConsulServiceRegistry implements ServiceRegistry> healthServices = client.getHealthServices(serviceId, + this.properties.getQueryTagForService(serviceId), false, + QueryParams.DEFAULT, this.properties.getAclToken()); + return healthServices.getValue(); } } diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientLocalServiceInstanceTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientLocalServiceInstanceTests.java index aa6f9b9d..c9def5ca 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientLocalServiceInstanceTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientLocalServiceInstanceTests.java @@ -82,7 +82,7 @@ public class ConsulDiscoveryClientLocalServiceInstanceTests { service.setId(SERVICE_ID); service.setTags(Arrays.asList(TAG)); - given(this.lifecycle.getServiceId()).willReturn(SERVICE_ID); + given(this.lifecycle.getInstanceId()).willReturn(SERVICE_ID); given(this.consul.getAgentServices()).willReturn(new Response<>(Collections.singletonMap(SERVICE_ID, service), RAW_RESPONSE)); @@ -138,7 +138,7 @@ public class ConsulDiscoveryClientLocalServiceInstanceTests { } private void mockFromConfig(int port, String address) { - given(this.lifecycle.getServiceId()).willReturn(SERVICE_ID); + given(this.lifecycle.getInstanceId()).willReturn(SERVICE_ID); given(this.lifecycle.getConfiguredPort()).willReturn(port); given(this.properties.getTags()).willReturn(Arrays.asList(TAG)); given(this.properties.getHostname()).willReturn(address); diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleCustomizedTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleCustomizedTests.java index 89bfd51a..f83ac6b6 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleCustomizedTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleCustomizedTests.java @@ -65,15 +65,15 @@ public class ConsulLifecycleCustomizedTests { @Test public void usesCustomConsulLifecycle() { - assertEquals("serviceId is not customized", "foo:bar", lifecycle1.getServiceId()); - assertEquals("serviceId is not customized", "foo:bar", lifecycle2.getServiceId()); + assertEquals("serviceId is not customized", "foo:bar", lifecycle1.getInstanceId()); + assertEquals("serviceId is not customized", "foo:bar", lifecycle2.getInstanceId()); } @Test - public void serviceIdIsCached() { + public void instanceIdIsCached() { // simulate a refresh where instanceId is changed this.properties.setInstanceId("baz"); - assertEquals("serviceId is not cached", "foo:bar", lifecycle2.getServiceId()); + assertEquals("serviceId is not cached", "foo:bar", lifecycle2.getInstanceId()); } @Configuration @@ -98,8 +98,8 @@ public class ConsulLifecycleCustomizedTests { } @Override - public String getServiceId() { - return super.getServiceId()+":bar"; + public String getInstanceId() { + return super.getInstanceId()+":bar"; } } } 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 acefde0e..ed5bb47a 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 @@ -70,11 +70,11 @@ public class ConsulLifecycleTests { public void contextLoads() { Response> response = consul.getAgentServices(); Map services = response.getValue(); - Service service = services.get(lifecycle.getServiceId()); + Service service = services.get(lifecycle.getInstanceId()); assertNotNull("service was null", service); assertNotEquals("service port is 0", 0, service.getPort().intValue()); assertFalse("service id contained invalid character: " + service.getId(), service.getId().contains(":")); - assertEquals("service id was wrong", lifecycle.getServiceId(), service.getId()); + assertEquals("service id was wrong", lifecycle.getInstanceId(), service.getId()); assertEquals("service name was wrong", "myTestService1-F-something", service.getService()); assertFalse("service address must not be empty", StringUtils.isEmpty(service.getAddress())); assertEquals("service address must equals hostname from discovery properties", discoveryProperties.getHostname(), service.getAddress()); diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedTests.java index 98424d21..b8f0418e 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationCustomizedTests.java @@ -59,7 +59,7 @@ public class ConsulAutoServiceRegistrationCustomizedTests { public static class MyTestConfig { @Bean public CustomAutoRegistration consulAutoServiceRegistration(ConsulServiceRegistry serviceRegistry, ConsulDiscoveryProperties properties, - ConsulRegistration registration) { + ConsulAutoRegistration registration) { return new CustomAutoRegistration(serviceRegistry, properties, registration); } } @@ -68,7 +68,7 @@ public class ConsulAutoServiceRegistrationCustomizedTests { @Autowired public CustomAutoRegistration(ConsulServiceRegistry serviceRegistry, ConsulDiscoveryProperties properties, - ConsulRegistration registration) { + ConsulAutoRegistration registration) { super(serviceRegistry, properties, registration); } diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationTests.java index 93e6b310..9f08ffde 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationTests.java @@ -40,7 +40,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; -import static org.springframework.cloud.consul.serviceregistry.ConsulRegistration.normalizeForDns; +import static org.springframework.cloud.consul.serviceregistry.ConsulAutoRegistration.normalizeForDns; /** * @author Spencer Gibb @@ -64,11 +64,11 @@ public class ConsulAutoServiceRegistrationTests { public void contextLoads() { Response> response = consul.getAgentServices(); Map services = response.getValue(); - Service service = services.get(registration.getServiceId()); + Service service = services.get(registration.getInstanceId()); assertNotNull("service was null", service); assertNotEquals("service port is 0", 0, service.getPort().intValue()); assertFalse("service id contained invalid character: " + service.getId(), service.getId().contains(":")); - assertEquals("service id was wrong", registration.getServiceId(), service.getId()); + assertEquals("service id was wrong", registration.getInstanceId(), service.getId()); assertEquals("service name was wrong", "myTestService1-FF-something", service.getService()); assertFalse("service address must not be empty", StringUtils.isEmpty(service.getAddress())); assertEquals("service address must equals hostname from discovery properties", discoveryProperties.getHostname(), service.getAddress()); diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryTests.java new file mode 100644 index 00000000..69ada461 --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulServiceRegistryTests.java @@ -0,0 +1,117 @@ +/* + * 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 java.util.Collections; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.context.embedded.LocalServerPort; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.EnableDiscoveryClient; +import org.springframework.cloud.consul.discovery.ConsulDiscoveryClient; +import org.springframework.test.context.junit4.SpringRunner; + +import com.ecwid.consul.v1.agent.model.NewService; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +/** + * @author Spencer Gibb + */ +@RunWith(SpringRunner.class) +@SpringBootTest(properties = "spring.cloud.consul.discovery.query-passing=true", + webEnvironment = RANDOM_PORT) +public class ConsulServiceRegistryTests { + + @Autowired(required = false) + private ConsulRegistration autoRegistration; + + @Autowired + private ConsulServiceRegistry serviceRegistry; + + @Autowired + private ConsulDiscoveryClient discoveryClient; + + @LocalServerPort + private int port; + + @Test + public void contextLoads() { + + assertThat(autoRegistration).as("autoRegistration created erroneously").isNull(); + + String serviceId = "myNonAutoRegisteredService"; + + NewService service = new NewService(); + service.setAddress("localhost"); + service.setId("myNonAutoRegisteredService-A1"); + service.setName(serviceId); + service.setPort(port); + service.setTags(Collections.singletonList("mytag")); + + ConsulRegistration registration = new ConsulRegistration(service); + Throwable t = null; + try { + serviceRegistry.register(registration); + assertHasInstance(serviceId); + + Object status = serviceRegistry.getStatus(registration); + assertThat(status).isNotNull(); + + // only works if query-passing = true + serviceRegistry.setStatus(registration, "OUT_OF_SERVICE"); + assertEmptyInstances(serviceId); + + serviceRegistry.setStatus(registration, "UP"); + assertHasInstance(serviceId); + } catch (RuntimeException e) { + throw e ; + } finally { + serviceRegistry.deregister(registration); + if (t == null) { // just deregister, test already failed + assertEmptyInstances(serviceId); + } + } + + } + + private void assertHasInstance(String serviceId) { + List instances = discoveryClient.getInstances(serviceId); + assertThat(instances).hasSize(1); + + ServiceInstance instance = instances.get(0); + assertThat(instance.getServiceId()).isEqualTo(serviceId); + } + + private void assertEmptyInstances(String serviceId) { + List instances = discoveryClient.getInstances(serviceId); + assertThat(instances).isEmpty(); + } + + @EnableDiscoveryClient(autoRegister = false) + @SpringBootConfiguration + @EnableAutoConfiguration + protected static class TestConfig { } +} +