diff --git a/.gitignore b/.gitignore index 51a44355..ffdd69f1 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,4 @@ _site/ *.iml .factorypath *.swp -consul \ No newline at end of file +/consul \ No newline at end of file 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 38645b84..1a44895e 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 @@ -16,28 +16,28 @@ package org.springframework.cloud.consul.discovery; -import static org.springframework.cloud.consul.discovery.IpAddressUtils.getCatalogServiceHost; - import java.util.ArrayList; import java.util.List; import java.util.Map; -import org.springframework.boot.autoconfigure.web.ServerProperties; -import org.springframework.cloud.client.DefaultServiceInstance; -import org.springframework.cloud.client.ServiceInstance; -import org.springframework.cloud.client.discovery.DiscoveryClient; -import org.springframework.util.StringUtils; - import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.QueryParams; import com.ecwid.consul.v1.Response; import com.ecwid.consul.v1.agent.model.Member; import com.ecwid.consul.v1.agent.model.Self; import com.ecwid.consul.v1.agent.model.Service; -import com.ecwid.consul.v1.catalog.model.CatalogService; +import com.ecwid.consul.v1.health.model.HealthService; + +import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.util.StringUtils; import lombok.extern.apachecommons.CommonsLog; +import static org.springframework.cloud.consul.discovery.ConsulServerUtils.findHost; + /** * @author Spencer Gibb */ @@ -52,8 +52,7 @@ public class ConsulDiscoveryClient implements DiscoveryClient { private ServerProperties serverProperties; public ConsulDiscoveryClient(ConsulClient client, ConsulLifecycle lifecycle, - ConsulDiscoveryProperties properties, - ServerProperties serverProperties) { + ConsulDiscoveryProperties properties, ServerProperties serverProperties) { this.client = client; this.lifecycle = lifecycle; this.properties = properties; @@ -72,7 +71,7 @@ public class ConsulDiscoveryClient implements DiscoveryClient { String serviceId; Integer port; if (service == null) { - //possibly called before registration + // possibly called before registration log.warn("Unable to locate service in consul agent: " + lifecycle.getServiceId()); @@ -81,7 +80,8 @@ public class ConsulDiscoveryClient implements DiscoveryClient { if (port == 0 && serverProperties.getPort() != null) { port = serverProperties.getPort(); } - } else { + } + else { serviceId = service.getId(); port = service.getPort(); } @@ -91,7 +91,8 @@ public class ConsulDiscoveryClient implements DiscoveryClient { if (member != null) { if (properties.isPreferIpAddress()) { host = member.getAddress(); - } else if (StringUtils.hasText(member.getName())) { + } + else if (StringUtils.hasText(member.getName())) { host = member.getName(); } } @@ -108,12 +109,12 @@ public class ConsulDiscoveryClient implements DiscoveryClient { } private void addInstancesToList(List instances, String serviceId) { - Response> services = client.getCatalogService(serviceId, - QueryParams.DEFAULT); - for (CatalogService service : services.getValue()) { - String host = getCatalogServiceHost(service); - instances.add(new DefaultServiceInstance(serviceId, host, - service.getServicePort(), false)); + Response> services = client.getHealthServices(serviceId, + false, QueryParams.DEFAULT); + for (HealthService service : services.getValue()) { + String host = findHost(service); + instances.add(new DefaultServiceInstance(serviceId, host, service + .getService().getPort(), false)); } } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulPing.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulPing.java new file mode 100644 index 00000000..0e660f69 --- /dev/null +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulPing.java @@ -0,0 +1,39 @@ +/* + * 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. + * 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 com.netflix.loadbalancer.IPing; +import com.netflix.loadbalancer.Server; + +/** + * "Ping" Consul + * i.e. we dont do a real "ping". We just assume that the server is up if Consul says so + * @author Spencer Gibb + */ +public class ConsulPing implements IPing { + @Override + public boolean isAlive(Server server) { + boolean isAlive = true; + + if (server != null && server instanceof ConsulServer) { + ConsulServer consulServer = (ConsulServer) server; + return consulServer.isPassingChecks(); + } + + return isAlive; + } +} diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulRibbonClientConfiguration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulRibbonClientConfiguration.java index 822a52ee..233daa9d 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulRibbonClientConfiguration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulRibbonClientConfiguration.java @@ -16,27 +16,27 @@ package org.springframework.cloud.consul.discovery; -import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses; -import static com.netflix.client.config.CommonClientConfigKey.EnableZoneAffinity; - import javax.annotation.PostConstruct; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.cloud.consul.discovery.filters.ServiceCheckServerListFilter; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - import com.ecwid.consul.v1.ConsulClient; import com.netflix.client.config.IClientConfig; import com.netflix.config.ConfigurationManager; import com.netflix.config.DynamicPropertyFactory; import com.netflix.config.DynamicStringProperty; +import com.netflix.loadbalancer.IPing; import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerList; import com.netflix.loadbalancer.ServerListFilter; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses; +import static com.netflix.client.config.CommonClientConfigKey.EnableZoneAffinity; + /** * Preprocessor that configures defaults for Consul-discovered ribbon clients. Such as: * @zone, NIWSServerListClassName, DeploymentContextBasedVipAddresses, @@ -74,7 +74,13 @@ public class ConsulRibbonClientConfiguration { @Bean public ServerListFilter ribbonServerListFilter() { - return new ServiceCheckServerListFilter(client); + return new HealthServiceServerListFilter(); + } + + @Bean + @ConditionalOnMissingBean + public IPing ribbonPing() { + return new ConsulPing(); } @PostConstruct diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServer.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServer.java index d621b7ac..468de14f 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServer.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServer.java @@ -16,9 +16,10 @@ package org.springframework.cloud.consul.discovery; -import static org.springframework.cloud.consul.discovery.IpAddressUtils.getCatalogServiceHost; +import static org.springframework.cloud.consul.discovery.ConsulServerUtils.findHost; -import com.ecwid.consul.v1.catalog.model.CatalogService; +import com.ecwid.consul.v1.health.model.Check; +import com.ecwid.consul.v1.health.model.HealthService; import com.netflix.loadbalancer.Server; /** @@ -27,17 +28,15 @@ import com.netflix.loadbalancer.Server; public class ConsulServer extends Server { private final MetaInfo metaInfo; - private final String address; - private final String node; + private final HealthService service; - public ConsulServer(final CatalogService service) { - super(getCatalogServiceHost(service), service.getServicePort()); - address = service.getAddress(); - node = service.getNode(); + public ConsulServer(final HealthService healthService) { + super(findHost(healthService), healthService.getService().getPort()); + this.service = healthService; metaInfo = new MetaInfo() { @Override public String getAppName() { - return service.getServiceName(); + return service.getService().getService(); } @Override @@ -52,9 +51,11 @@ public class ConsulServer extends Server { @Override public String getInstanceId() { - return service.getServiceId(); + return service.getService().getId(); } }; + + setAlive(isPassingChecks()); } @Override @@ -62,11 +63,16 @@ public class ConsulServer extends Server { return metaInfo; } - public String getAddress() { - return address; + public HealthService getHealthService() { + return this.service; } - public String getNode() { - return node; + public boolean isPassingChecks() { + for (Check check : this.service.getChecks()) { + if (check.getStatus() != Check.CheckStatus.PASSING) { + return false; + } + } + return true; } } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerList.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerList.java index 213f3556..bceccad9 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerList.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerList.java @@ -23,7 +23,7 @@ import java.util.List; import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.QueryParams; import com.ecwid.consul.v1.Response; -import com.ecwid.consul.v1.catalog.model.CatalogService; +import com.ecwid.consul.v1.health.model.HealthService; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.AbstractServerList; @@ -61,16 +61,29 @@ public class ConsulServerList extends AbstractServerList { if (client == null) { return Collections.emptyList(); } - String tag = this.properties.getServerListQueryTags().get(this.serviceId); // null is ok - Response> response = client.getCatalogService( - this.serviceId, tag, QueryParams.DEFAULT); + String tag = getTag(); // null is ok + Response> response = client.getHealthServices( + this.serviceId, tag, false, QueryParams.DEFAULT); if (response.getValue() == null || response.getValue().isEmpty()) { return Collections.emptyList(); } List servers = new ArrayList<>(); - for (CatalogService service : response.getValue()) { + for (HealthService service : response.getValue()) { servers.add(new ConsulServer(service)); } return servers; } + + private String getTag() { + return this.properties.getServerListQueryTags().get(this.serviceId); + } + + @Override + public String toString() { + final StringBuffer sb = new StringBuffer("ConsulServerList{"); + sb.append("serviceId='").append(serviceId).append('\''); + sb.append(", tag=").append(getTag()); + sb.append('}'); + return sb.toString(); + } } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/IpAddressUtils.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerUtils.java similarity index 62% rename from spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/IpAddressUtils.java rename to spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerUtils.java index 1c600476..57e50f15 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/IpAddressUtils.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerUtils.java @@ -16,31 +16,27 @@ package org.springframework.cloud.consul.discovery; -import java.io.IOException; -import java.net.Inet4Address; -import java.net.InetAddress; -import java.net.NetworkInterface; -import java.util.Enumeration; - -import lombok.SneakyThrows; -import lombok.extern.apachecommons.CommonsLog; +import com.ecwid.consul.v1.health.model.HealthService; import org.springframework.util.StringUtils; -import com.ecwid.consul.v1.catalog.model.CatalogService; +import lombok.extern.apachecommons.CommonsLog; /** * @author Spencer Gibb */ @CommonsLog -public class IpAddressUtils { +public class ConsulServerUtils { - public static String getCatalogServiceHost(CatalogService service) { - if (StringUtils.hasText(service.getServiceAddress())) { - return service.getServiceAddress(); - } else if (StringUtils.hasText(service.getAddress())) { + public static String findHost(HealthService healthService) { + HealthService.Service service = healthService.getService(); + HealthService.Node node = healthService.getNode(); + + if (StringUtils.hasText(service.getAddress())) { return service.getAddress(); + } else if (StringUtils.hasText(node.getAddress())) { + return node.getAddress(); } - return service.getNode(); + return node.getNode(); } } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilter.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilter.java new file mode 100644 index 00000000..0b522f71 --- /dev/null +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilter.java @@ -0,0 +1,53 @@ +/* + * 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. + * 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 java.util.ArrayList; +import java.util.List; + +import com.netflix.loadbalancer.Server; +import com.netflix.loadbalancer.ServerListFilter; + +import lombok.extern.apachecommons.CommonsLog; + +/** + * ServerList implementaion that filters ConsulServers based on if all their Health Checks are PASSING. + * @author Spencer Gibb + */ +@CommonsLog +public class HealthServiceServerListFilter implements ServerListFilter { + @Override + public List getFilteredListOfServers(List servers) { + List filtered = new ArrayList<>(); + + for (Server server : servers) { + if (server instanceof ConsulServer) { + ConsulServer consulServer = (ConsulServer) server; + + if (consulServer.isPassingChecks()) { + filtered.add(server); + } + + } else { + log.debug("Unable to determine aliveness of server type "+server.getClass()+", "+server); + filtered.add(server); + } + } + + return filtered; + } +} diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/AliveServerListFilter.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/AliveServerListFilter.java index 39d486c7..c98267e8 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/AliveServerListFilter.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/AliveServerListFilter.java @@ -33,6 +33,7 @@ import com.netflix.loadbalancer.ServerListFilter; * live in both). * @author nicu marasoiu on 10.03.2015. */ +@Deprecated public class AliveServerListFilter implements ServerListFilter { private FilteringAgentClient filteringAgentClient; @@ -46,7 +47,7 @@ public class AliveServerListFilter implements ServerListFilter { List filteredServers = new ArrayList<>(); for (Server server : servers) { ConsulServer consulServer = ConsulServer.class.cast(server); - if (liveNodes.contains(consulServer.getAddress())) { + if (liveNodes.contains(consulServer.getHealthService().getService().getAddress())) { filteredServers.add(server); } } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/FilteringAgentClient.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/FilteringAgentClient.java index 157b4886..37ea304c 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/FilteringAgentClient.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/FilteringAgentClient.java @@ -26,6 +26,7 @@ import org.springframework.cloud.consul.model.SerfStatusEnum; import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.agent.model.Member; +@Deprecated public class FilteringAgentClient { private static final int ALIVE_STATUS = SerfStatusEnum.StatusAlive.getCode(); diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/ServiceCheckServerListFilter.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/ServiceCheckServerListFilter.java index 7e8a77e1..7449cc97 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/ServiceCheckServerListFilter.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/filters/ServiceCheckServerListFilter.java @@ -28,6 +28,7 @@ import com.netflix.loadbalancer.ServerListFilter; /** * Created by nicu on 12.03.2015. */ +@Deprecated public class ServiceCheckServerListFilter implements ServerListFilter { private ConsulClient client; diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilterTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilterTests.java new file mode 100644 index 00000000..dd3a7609 --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilterTests.java @@ -0,0 +1,70 @@ +/* + * 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. + * 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 java.util.ArrayList; +import java.util.List; + +import com.ecwid.consul.v1.health.model.Check; +import com.ecwid.consul.v1.health.model.HealthService; +import com.netflix.loadbalancer.Server; + +import org.junit.Test; + +import static com.ecwid.consul.v1.health.model.Check.CheckStatus.*; +import static org.junit.Assert.assertThat; +import static org.hamcrest.Matchers.*; + +/** + * @author Spencer Gibb + */ +public class HealthServiceServerListFilterTests { + + @Test + public void testGetFilteredListOfServers() { + HealthServiceServerListFilter filter = new HealthServiceServerListFilter(); + + ArrayList servers = new ArrayList<>(); + servers.add(newServer(PASSING)); + servers.add(newServer(PASSING)); + servers.add(newServer(CRITICAL)); + servers.add(newServer(WARNING)); + + List filtered = filter.getFilteredListOfServers(servers); + assertThat("wrong # of filtered servers", filtered, hasSize(2)); + } + + private ConsulServer newServer(Check.CheckStatus checkStatus) { + HealthService healthService = new HealthService(); + HealthService.Node node = new HealthService.Node(); + node.setAddress("nodeaddr"+checkStatus.name()); + node.setNode("nodenode"+checkStatus.name()); + healthService.setNode(node); + HealthService.Service service = new HealthService.Service(); + service.setAddress("serviceaddr"+checkStatus.name()); + service.setId("serviceid"+checkStatus.name()); + service.setPort(8080); + service.setService("serviceservice"+checkStatus.name()); + healthService.setService(service); + ArrayList checks = new ArrayList<>(); + Check check = new Check(); + check.setStatus(checkStatus); + checks.add(check); + healthService.setChecks(checks); + return new ConsulServer(healthService); + } +}