From c60df0f18e674f737c9862171275e6c795fd7b82 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Mon, 12 Aug 2019 12:21:38 +0200 Subject: [PATCH] Fix gh 491 gh 553 non reactive loadbalancer client (#590) * Provide non-reactive LB client implemenation to use with RestTemplate. Fixes gh-491. Fixes gh-553. * Add javadoc. --- .../main/asciidoc/spring-cloud-commons.adoc | 11 +- .../{reactive => }/LoadBalancerUriTools.java | 12 +- ...torLoadBalancerExchangeFilterFunction.java | 1 + .../LoadBalancerUriToolsTests.java | 2 +- .../client/BlockingLoadBalancerClient.java | 93 ++++++++ ...ngLoadBalancerClientAutoConfiguration.java | 86 +++++++ .../main/resources/META-INF/spring.factories | 4 +- .../BlockingLoadBalancerClientTests.java | 211 ++++++++++++++++++ 8 files changed, 413 insertions(+), 7 deletions(-) rename spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/{reactive => }/LoadBalancerUriTools.java (88%) rename spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/{reactive => }/LoadBalancerUriToolsTests.java (99%) create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClient.java create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index 410ac5de..de9f9f32 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -336,7 +336,7 @@ For instance, Eureka's supported statuses are `UP`, `DOWN`, `OUT_OF_SERVICE`, an === Spring RestTemplate as a Load Balancer Client -`RestTemplate` can be automatically configured to use ribbon. +`RestTemplate` can be automatically configured to use a Load-balancer client under the hood. To create a load-balanced `RestTemplate`, create a `RestTemplate` `@Bean` and use the `@LoadBalanced` qualifier, as shown in the following example: [source,java,indent=0] @@ -369,6 +369,15 @@ The URI needs to use a virtual host name (that is, a service name, not a host na The Ribbon client is used to create a full physical address. See {githubroot}/spring-cloud-netflix/blob/master/spring-cloud-netflix-ribbon/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java[RibbonAutoConfiguration] for details of how the `RestTemplate` is set up. +IMPORTANT: In order to use a load-balanced `RestTemplate`, you need to have a load-balancer implementation in your classpath. +The recommended implementation is `BlockingLoadBalancerClient` +- add `org.springframework.cloud:spring-cloud-loadbalancer` in order to use it. +The +`RibbonLoadBalancerClient` also can be used, but it's now under maintenance and we do not recommend adding it to new projects. + +WARNING: If you want to use `BlockingLoadBalancerClient`, make sure you do not have +`RibbonLoadBalancerClient` in the project classpath, as for backward compatibility reasons, it will be used by default. + === Spring WebClient as a Load Balancer Client `WebClient` can be automatically configured to use a load-balancer client. diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerUriTools.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerUriTools.java similarity index 88% rename from spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerUriTools.java rename to spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerUriTools.java index a78621b0..7eda1846 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerUriTools.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerUriTools.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.cloud.client.loadbalancer.reactive; +package org.springframework.cloud.client.loadbalancer; import java.net.URI; import java.util.HashMap; @@ -29,7 +29,7 @@ import org.springframework.web.util.UriComponentsBuilder; * @author Olga Maciaszek-Sharma * @since 2.2.0 */ -final class LoadBalancerUriTools { +public final class LoadBalancerUriTools { private LoadBalancerUriTools() { throw new IllegalStateException("Can't instantiate a utility class"); @@ -82,7 +82,13 @@ final class LoadBalancerUriTools { return 80; } - static URI reconstructURI(ServiceInstance serviceInstance, URI original) { + /** + * Modifies the URI in order to redirect the request to a service instance of choice. + * @param serviceInstance the {@link ServiceInstance} to redirect the request to. + * @param original the {@link URI} from the original request + * @return the modified {@link URI} + */ + public static URI reconstructURI(ServiceInstance serviceInstance, URI original) { if (serviceInstance == null) { throw new IllegalArgumentException("Service Instance cannot be null."); } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunction.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunction.java index 7946c9c9..16f4ab8b 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunction.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunction.java @@ -24,6 +24,7 @@ import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.LoadBalancerUriTools; import org.springframework.http.HttpStatus; import org.springframework.web.reactive.function.client.ClientRequest; import org.springframework.web.reactive.function.client.ClientResponse; diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerUriToolsTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerUriToolsTests.java similarity index 99% rename from spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerUriToolsTests.java rename to spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerUriToolsTests.java index 59f93a01..bedcc252 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerUriToolsTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerUriToolsTests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.cloud.client.loadbalancer.reactive; +package org.springframework.cloud.client.loadbalancer; import java.net.URI; import java.util.LinkedHashMap; diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClient.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClient.java new file mode 100644 index 00000000..159a07bf --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClient.java @@ -0,0 +1,93 @@ +/* + * Copyright 2012-2019 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 + * + * https://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.loadbalancer.blocking.client; + +import java.io.IOException; +import java.net.URI; + +import reactor.core.publisher.Mono; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.client.loadbalancer.LoadBalancerRequest; +import org.springframework.cloud.client.loadbalancer.LoadBalancerUriTools; +import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; +import org.springframework.cloud.client.loadbalancer.reactive.Response; +import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; +import org.springframework.util.ReflectionUtils; + +/** + * The default {@link LoadBalancerClient} implementation. + * + * @author Olga Maciaszek-Sharma + * @since 2.2.0 + */ +public class BlockingLoadBalancerClient implements LoadBalancerClient { + + private final LoadBalancerClientFactory loadBalancerClientFactory; + + public BlockingLoadBalancerClient( + LoadBalancerClientFactory loadBalancerClientFactory) { + this.loadBalancerClientFactory = loadBalancerClientFactory; + } + + @Override + public T execute(String serviceId, LoadBalancerRequest request) + throws IOException { + ServiceInstance serviceInstance = choose(serviceId); + if (serviceInstance == null) { + throw new IllegalStateException("No instances available for " + serviceId); + } + return execute(serviceId, serviceInstance, request); + } + + @Override + public T execute(String serviceId, ServiceInstance serviceInstance, + LoadBalancerRequest request) throws IOException { + try { + return request.apply(serviceInstance); + } + catch (IOException iOException) { + throw iOException; + } + catch (Exception exception) { + ReflectionUtils.rethrowRuntimeException(exception); + } + return null; + } + + @Override + public URI reconstructURI(ServiceInstance serviceInstance, URI original) { + return LoadBalancerUriTools.reconstructURI(serviceInstance, original); + } + + @Override + public ServiceInstance choose(String serviceId) { + ReactiveLoadBalancer loadBalancer = loadBalancerClientFactory + .getInstance(serviceId); + if (loadBalancer == null) { + return null; + } + Response loadBalancerResponse = Mono.from(loadBalancer.choose()) + .block(); + if (loadBalancerResponse == null) { + return null; + } + return loadBalancerResponse.getServer(); + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java new file mode 100644 index 00000000..1aaee4ad --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java @@ -0,0 +1,86 @@ +/* + * Copyright 2012-2019 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 + * + * https://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.loadbalancer.config; + +import javax.annotation.PostConstruct; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.boot.autoconfigure.AutoConfigureAfter; +import org.springframework.boot.autoconfigure.AutoConfigureBefore; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; +import org.springframework.cloud.client.loadbalancer.AsyncLoadBalancerAutoConfiguration; +import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients; +import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; +import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.web.client.RestTemplate; + +/** + * An autoconfiguration for {@link BlockingLoadBalancerClient}. + * + * @author Olga Maciaszek-Sharma + * @since 2.2.0 + */ +@Configuration +@LoadBalancerClients +@AutoConfigureAfter(LoadBalancerAutoConfiguration.class) +@AutoConfigureBefore({ + org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration.class, + AsyncLoadBalancerAutoConfiguration.class }) +public class BlockingLoadBalancerClientAutoConfiguration { + + @Bean + @ConditionalOnClass( + name = "org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient") + public RibbonWarnLogger ribbonWarnLogger() { + return new RibbonWarnLogger(); + } + + @Bean + @ConditionalOnBean(LoadBalancerClientFactory.class) + @ConditionalOnClass(RestTemplate.class) + @ConditionalOnMissingBean + @ConditionalOnMissingClass("org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient") + public LoadBalancerClient loadBalancerClient( + LoadBalancerClientFactory loadBalancerClientFactory) { + return new BlockingLoadBalancerClient(loadBalancerClientFactory); + } + +} + +class RibbonWarnLogger { + + private static final Log LOG = LogFactory.getLog(RibbonWarnLogger.class); + + @PostConstruct + void logWarning() { + if (LOG.isWarnEnabled()) { + LOG.warn( + "You already have RibbonLoadBalancerClient on your classpath. It will be used by default. To use " + + BlockingLoadBalancerClient.class.getSimpleName() + + " remove spring-cloud-starter-netflix-ribbon from your project."); + } + } + +} diff --git a/spring-cloud-loadbalancer/src/main/resources/META-INF/spring.factories b/spring-cloud-loadbalancer/src/main/resources/META-INF/spring.factories index b12b173d..9fa6cb80 100644 --- a/spring-cloud-loadbalancer/src/main/resources/META-INF/spring.factories +++ b/spring-cloud-loadbalancer/src/main/resources/META-INF/spring.factories @@ -1,4 +1,4 @@ # AutoConfiguration org.springframework.boot.autoconfigure.EnableAutoConfiguration=\ -org.springframework.cloud.loadbalancer.config.LoadBalancerAutoConfiguration - +org.springframework.cloud.loadbalancer.config.LoadBalancerAutoConfiguration,\ +org.springframework.cloud.loadbalancer.config.BlockingLoadBalancerClientAutoConfiguration diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java new file mode 100644 index 00000000..51b66cf0 --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java @@ -0,0 +1,211 @@ +/* + * Copyright 2012-2019 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 + * + * https://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.loadbalancer.blocking.client; + +import java.io.IOException; +import java.net.URI; +import java.util.Collections; +import java.util.List; +import java.util.Random; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import reactor.core.publisher.Mono; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties; +import org.springframework.cloud.client.loadbalancer.LoadBalancerRequest; +import org.springframework.cloud.client.loadbalancer.reactive.DefaultResponse; +import org.springframework.cloud.client.loadbalancer.reactive.EmptyResponse; +import org.springframework.cloud.client.loadbalancer.reactive.Request; +import org.springframework.cloud.client.loadbalancer.reactive.Response; +import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients; +import org.springframework.cloud.loadbalancer.core.ReactorLoadBalancer; +import org.springframework.cloud.loadbalancer.core.ReactorServiceInstanceLoadBalancer; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit.jupiter.SpringExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +/** + * Tests for {@link BlockingLoadBalancerClient}. + * + * @author Olga Maciaszek-Sharma + */ +@SpringBootTest +@ExtendWith(SpringExtension.class) +class BlockingLoadBalancerClientTests { + + @Autowired + private BlockingLoadBalancerClient loadBalancerClient; + + @Autowired + private SimpleDiscoveryProperties properties; + + @BeforeEach + void setUp() { + properties.getInstances().put("myservice", + Collections.singletonList( + new SimpleDiscoveryProperties.SimpleServiceInstance( + URI.create("https://test.example:9999")))); + } + + @Test + void correctServiceInstanceChosen() { + ServiceInstance serviceInstance = loadBalancerClient.choose("myservice"); + assertThat(serviceInstance.getHost()).isEqualTo("test.example"); + } + + @Test + void nullReturnedIfInstanceMissing() { + ServiceInstance serviceInstance = loadBalancerClient.choose("unknownservice"); + assertThat(serviceInstance).isNull(); + } + + @Test + void requestExecutedAgainstCorrectInstance() throws IOException { + final String result = "result"; + Object actualResult = loadBalancerClient.execute("myservice", + (LoadBalancerRequest) instance -> { + assertThat(instance.getHost()).isEqualTo("test.example"); + return result; + }); + assertThat(actualResult).isEqualTo(result); + } + + @Test + void exceptionThrownIfInstanceNotAvailableForRequestExecution() throws IOException { + try { + final String result = "result"; + Object actualResult = loadBalancerClient.execute("unknownservice", + (LoadBalancerRequest) instance -> result); + assertThat(actualResult).isEqualTo(result); + fail("Should have thrown exception."); + } + catch (Exception exception) { + assertThat(exception).isNotNull(); + assertThat(exception).isInstanceOf(IllegalStateException.class); + assertThat(exception).hasMessage("No instances available for unknownservice"); + } + } + + @Test + void exceptionRethrownAsRuntime() { + try { + loadBalancerClient.execute("myservice", instance -> { + assertThat(instance.getHost()).isEqualTo("test.example"); + throw new Exception("Should throw exception."); + }); + fail("Should have thrown exception."); + } + catch (Exception exception) { + assertThat(exception).isNotNull(); + assertThat(exception).isInstanceOf(RuntimeException.class); + } + } + + @Test + void IOExceptionRethrown() { + try { + loadBalancerClient.execute("myservice", instance -> { + assertThat(instance.getHost()).isEqualTo("test.example"); + throw new IOException("Should throw IO exception."); + }); + fail("Should have thrown exception."); + } + catch (Exception exception) { + assertThat(exception).isNotNull(); + assertThat(exception).isInstanceOf(IOException.class); + } + } + + @Configuration + @EnableAutoConfiguration + @SpringBootConfiguration + @LoadBalancerClients({ + @org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient( + name = "myservice", configuration = MyServiceConfig.class), + @org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient( + name = "unknownservice", + configuration = UnknownServiceConfig.class) }) + protected static class Config { + + } + + protected static class MyServiceConfig { + + @Bean + ReactorLoadBalancer reactiveLoadBalancer( + DiscoveryClient discoveryClient) { + return new DiscoveryClientBasedReactiveLoadBalancer("myservice", + discoveryClient); + } + + } + + protected static class UnknownServiceConfig { + + @Bean + ReactorLoadBalancer reactiveLoadBalancer( + DiscoveryClient discoveryClient) { + return new DiscoveryClientBasedReactiveLoadBalancer("unknownservice", + discoveryClient); + } + + } + +} + +class DiscoveryClientBasedReactiveLoadBalancer + implements ReactorServiceInstanceLoadBalancer { + + private final Random random = new Random(); + + private final String serviceId; + + private final DiscoveryClient discoveryClient; + + DiscoveryClientBasedReactiveLoadBalancer(String serviceId, + DiscoveryClient discoveryClient) { + this.serviceId = serviceId; + this.discoveryClient = discoveryClient; + } + + @Override + public Mono> choose() { + List instances = discoveryClient.getInstances(serviceId); + if (instances.size() == 0) { + return Mono.just(new EmptyResponse()); + } + int instanceIdx = this.random.nextInt(instances.size()); + return Mono.just(new DefaultResponse(instances.get(instanceIdx))); + } + + @Override + public Mono> choose(Request request) { + return choose(); + } + +}