Avoid retrying on same instance (#834)

* cherry-pick switching to properties

* Pass information on previous ServiceInstance to RequestContext.

# Conflicts:
#	spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicy.java

* Add a RoundRobinLoadBalancer implementation that avoids same service instance while retrying.

* Wrap instances in ArrayList. Add tests.

* Enable AvoidPreviousInstanceRoundRobinLoadBalancer by default if SpringRetry on classpath.

* Fix failing tests. Add javadocs and author tags.

* Fix properties.

* Add documentation.

* Fix docs after review.

* Fix docs after review.

* Handle avoiding previous instance with ServiceInstanceListSupplier in place of LoadBalancer.

* Fix property name.

* Change spelling.

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java

@@ -16,7 +16,10 @@
 
 package org.springframework.cloud.loadbalancer.annotation;
 
+import org.springframework.boot.autoconfigure.AutoConfigureAfter;
+import org.springframework.boot.autoconfigure.condition.AllNestedConditions;
 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.ConditionalOnProperty;
 import org.springframework.cloud.client.ConditionalOnBlockingDiscoveryEnabled;
@@ -26,14 +29,18 @@ import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient;
 import org.springframework.cloud.loadbalancer.core.ReactorLoadBalancer;
+import org.springframework.cloud.loadbalancer.core.RetryAwareServiceInstanceListSupplier;
 import org.springframework.cloud.loadbalancer.core.RoundRobinLoadBalancer;
 import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier;
 import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Conditional;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Primary;
 import org.springframework.core.annotation.Order;
 import org.springframework.core.env.Environment;
+import org.springframework.retry.support.RetryTemplate;
 
 /**
  * @author Spencer Gibb
@@ -129,4 +136,42 @@ public class LoadBalancerClientConfiguration {
 
 	}
 
+	@Configuration(proxyBeanMethods = false)
+	@ConditionalOnBlockingDiscoveryEnabled
+	@ConditionalOnClass(RetryTemplate.class)
+	@Conditional(OnAvoidPreviousInstanceAndRetryEnabledCondition.class)
+	@AutoConfigureAfter(BlockingSupportConfiguration.class)
+	@ConditionalOnBean(ServiceInstanceListSupplier.class)
+	public static class BlockingRetryConfiguration {
+
+		@Bean
+		@ConditionalOnBean(DiscoveryClient.class)
+		@Primary
+		public ServiceInstanceListSupplier retryAwareDiscoveryClientServiceInstanceListSupplier(
+				ServiceInstanceListSupplier delegate) {
+			return new RetryAwareServiceInstanceListSupplier(delegate);
+		}
+
+	}
+
+	static final class OnAvoidPreviousInstanceAndRetryEnabledCondition extends AllNestedConditions {
+
+		private OnAvoidPreviousInstanceAndRetryEnabledCondition() {
+			super(ConfigurationPhase.REGISTER_BEAN);
+		}
+
+		@ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.enabled", havingValue = "true",
+				matchIfMissing = true)
+		static class LoadBalancerRetryEnabled {
+
+		}
+
+		@ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.avoid-previous-instance", havingValue = "true",
+				matchIfMissing = true)
+		static class AvoidPreviousInstanceEnabled {
+
+		}
+
+	}
+
 }

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryFactory.java

@@ -18,8 +18,8 @@ package org.springframework.cloud.loadbalancer.blocking.retry;
 
 import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory;
 import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy;
-import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties;
 import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser;
+import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties;
 import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
 
 /**
@@ -31,15 +31,15 @@ import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalanc
  */
 public class BlockingLoadBalancedRetryFactory implements LoadBalancedRetryFactory {
 
-	private final LoadBalancerRetryProperties retryProperties;
+	private final LoadBalancerProperties loadBalancerProperties;
 
-	public BlockingLoadBalancedRetryFactory(LoadBalancerRetryProperties retryProperties) {
-		this.retryProperties = retryProperties;
+	public BlockingLoadBalancedRetryFactory(LoadBalancerProperties loadBalancerProperties) {
+		this.loadBalancerProperties = loadBalancerProperties;
 	}
 
 	@Override
 	public LoadBalancedRetryPolicy createRetryPolicy(String serviceId, ServiceInstanceChooser serviceInstanceChooser) {
-		return new BlockingLoadBalancedRetryPolicy(serviceId, serviceInstanceChooser, retryProperties);
+		return new BlockingLoadBalancedRetryPolicy(loadBalancerProperties);
 	}
 
 }

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicy.java

@@ -18,8 +18,7 @@ package org.springframework.cloud.loadbalancer.blocking.retry;
 
 import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext;
 import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy;
-import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties;
-import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser;
+import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties;
 import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
 import org.springframework.http.HttpMethod;
 
@@ -33,37 +32,30 @@ import org.springframework.http.HttpMethod;
  */
 public class BlockingLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy {
 
-	private final LoadBalancerRetryProperties retryProperties;
-
-	private final ServiceInstanceChooser serviceInstanceChooser;
-
-	private final String serviceId;
+	private final LoadBalancerProperties properties;
 
 	private int sameServerCount = 0;
 
 	private int nextServerCount = 0;
 
-	public BlockingLoadBalancedRetryPolicy(String serviceId, ServiceInstanceChooser serviceInstanceChooser,
-			LoadBalancerRetryProperties retryProperties) {
-		this.serviceId = serviceId;
-		this.serviceInstanceChooser = serviceInstanceChooser;
-		this.retryProperties = retryProperties;
+	public BlockingLoadBalancedRetryPolicy(LoadBalancerProperties properties) {
+		this.properties = properties;
 	}
 
 	public boolean canRetry(LoadBalancedRetryContext context) {
 		HttpMethod method = context.getRequest().getMethod();
-		return HttpMethod.GET.equals(method) || retryProperties.isRetryOnAllOperations();
+		return HttpMethod.GET.equals(method) || properties.getRetry().isRetryOnAllOperations();
 	}
 
 	@Override
 	public boolean canRetrySameServer(LoadBalancedRetryContext context) {
-		return sameServerCount < retryProperties.getMaxRetriesOnSameServiceInstance() && canRetry(context);
+		return sameServerCount < properties.getRetry().getMaxRetriesOnSameServiceInstance() && canRetry(context);
 	}
 
 	@Override
 	public boolean canRetryNextServer(LoadBalancedRetryContext context) {
 		// After the failure, we increment first and then check, hence the equality check
-		return nextServerCount <= retryProperties.getMaxRetriesOnNextServiceInstance() && canRetry(context);
+		return nextServerCount <= properties.getRetry().getMaxRetriesOnNextServiceInstance() && canRetry(context);
 	}
 
 	@Override
@@ -81,7 +73,10 @@ public class BlockingLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy
 				context.setExhaustedOnly();
 			}
 			else {
-				context.setServiceInstance(serviceInstanceChooser.choose(serviceId));
+				// We want the service instance to be set by
+				// `RetryLoadBalancerInterceptor`
+				// in order to get the entire data of the request
+				context.setServiceInstance(null);
 			}
 		}
 		else {
@@ -91,7 +86,7 @@ public class BlockingLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy
 
 	@Override
 	public boolean retryableStatusCode(int statusCode) {
-		return retryProperties.getRetryableStatusCodes().contains(statusCode);
+		return properties.getRetry().getRetryableStatusCodes().contains(statusCode);
 	}
 
 }

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java

@@ -25,7 +25,6 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties
 import org.springframework.cloud.client.loadbalancer.AsyncLoadBalancerAutoConfiguration;
 import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory;
 import org.springframework.cloud.client.loadbalancer.LoadBalancerClient;
-import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties;
 import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties;
 import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients;
 import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
@@ -60,13 +59,13 @@ public class BlockingLoadBalancerClientAutoConfiguration {
 
 	@Configuration
 	@ConditionalOnClass(RetryTemplate.class)
-	@EnableConfigurationProperties(LoadBalancerRetryProperties.class)
+	@EnableConfigurationProperties(LoadBalancerProperties.class)
 	protected static class BlockingLoadBalancerRetryConfig {
 
 		@Bean
 		@ConditionalOnMissingBean
-		LoadBalancedRetryFactory loadBalancedRetryFactory(LoadBalancerRetryProperties retryProperties) {
-			return new BlockingLoadBalancedRetryFactory(retryProperties);
+		LoadBalancedRetryFactory loadBalancedRetryFactory(LoadBalancerProperties properties) {
+			return new BlockingLoadBalancedRetryFactory(properties);
 		}
 
 	}

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RetryAwareServiceInstanceListSupplier.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright 2012-2020 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.core;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import reactor.core.publisher.Flux;
+
+import org.springframework.cloud.client.ServiceInstance;
+import org.springframework.cloud.client.loadbalancer.Request;
+import org.springframework.cloud.client.loadbalancer.RetryableRequestContext;
+
+/**
+ * A {@link ServiceInstanceListSupplier} implementation that avoids picking the same
+ * service instance while retrying requests.
+ *
+ * @author Olga Maciaszek-Sharma
+ * @since 3.0.0
+ */
+public class RetryAwareServiceInstanceListSupplier extends DelegatingServiceInstanceListSupplier {
+
+	private final Log LOG = LogFactory.getLog(RetryAwareServiceInstanceListSupplier.class);
+
+	public RetryAwareServiceInstanceListSupplier(ServiceInstanceListSupplier delegate) {
+		super(delegate);
+	}
+
+	@Override
+	public String getServiceId() {
+		return delegate.getServiceId();
+	}
+
+	@Override
+	public Flux<List<ServiceInstance>> get(Request request) {
+		if (!(request.getContext() instanceof RetryableRequestContext)) {
+			return get();
+		}
+		RetryableRequestContext context = (RetryableRequestContext) request.getContext();
+		ServiceInstance previousServiceInstance = context.getPreviousServiceInstance();
+		if (previousServiceInstance == null) {
+			return get();
+		}
+		return get().map(instances -> filteredByPreviousInstance(instances, previousServiceInstance));
+	}
+
+	private List<ServiceInstance> filteredByPreviousInstance(List<ServiceInstance> instances,
+			ServiceInstance previousServiceInstance) {
+		List<ServiceInstance> filteredInstances = new ArrayList<>(instances);
+		if (previousServiceInstance != null) {
+			filteredInstances.remove(previousServiceInstance);
+		}
+		if (filteredInstances.size() > 0) {
+			return filteredInstances;
+		}
+		if (LOG.isWarnEnabled()) {
+			LOG.warn(String.format(
+					"No instances found after removing previously used service instance from the search (%s). Returning all found instances.",
+					previousServiceInstance));
+		}
+		return instances;
+	}
+
+	@Override
+	public Flux<List<ServiceInstance>> get() {
+		return delegate.get();
+	}
+
+}

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java

@@ -41,11 +41,11 @@ public class RoundRobinLoadBalancer implements ReactorServiceInstanceLoadBalance
 
 	private static final Log log = LogFactory.getLog(RoundRobinLoadBalancer.class);
 
-	private final AtomicInteger position;
+	final AtomicInteger position;
 
-	private ObjectProvider<ServiceInstanceListSupplier> serviceInstanceListSupplierProvider;
+	final String serviceId;
 
-	private final String serviceId;
+	ObjectProvider<ServiceInstanceListSupplier> serviceInstanceListSupplierProvider;
 
 	/**
 	 * @param serviceInstanceListSupplierProvider a provider of
@@ -78,12 +78,14 @@ public class RoundRobinLoadBalancer implements ReactorServiceInstanceLoadBalance
 	public Mono<Response<ServiceInstance>> choose(Request request) {
 		ServiceInstanceListSupplier supplier = serviceInstanceListSupplierProvider
 				.getIfAvailable(NoopServiceInstanceListSupplier::new);
-		return supplier.get().next().map(this::getInstanceResponse);
+		return supplier.get(request).next().map(this::getInstanceResponse);
 	}
 
-	private Response<ServiceInstance> getInstanceResponse(List<ServiceInstance> instances) {
+	Response<ServiceInstance> getInstanceResponse(List<ServiceInstance> instances) {
 		if (instances.isEmpty()) {
-			log.warn("No servers available for service: " + this.serviceId);
+			if (log.isWarnEnabled()) {
+				log.warn("No servers available for service: " + serviceId);
+			}
 			return new EmptyResponse();
 		}
 		// TODO: enforce order?

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplier.java

@@ -24,6 +24,7 @@ import reactor.core.publisher.Flux;
 
 import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
+import org.springframework.cloud.client.loadbalancer.Request;
 import org.springframework.core.env.Environment;
 
 import static org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory.PROPERTY_NAME;
@@ -38,6 +39,10 @@ public interface ServiceInstanceListSupplier extends Supplier<Flux<List<ServiceI
 
 	String getServiceId();
 
+	default Flux<List<ServiceInstance>> get(Request request) {
+		return get();
+	}
+
 	static ServiceInstanceListSupplierBuilder builder() {
 		return new ServiceInstanceListSupplierBuilder();
 	}

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java

@@ -172,6 +172,12 @@ public final class ServiceInstanceListSupplierBuilder {
 		return this;
 	}
 
+	public ServiceInstanceListSupplierBuilder withRetryAwareness() {
+		DelegateCreator creator = (context, delegate) -> new RetryAwareServiceInstanceListSupplier(delegate);
+		creators.add(creator);
+		return this;
+	}
+
 	/**
 	 * Builds the {@link ServiceInstanceListSupplier} hierarchy.
 	 * @param context application context

spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/ServiceInstanceListSuppliers.java

@@ -26,7 +26,7 @@ import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier;
 
 /**
- * Utility class for service instances.
+ * Utility class for service instance list suppliers.
  *
  * @author Spencer Gibb
  * @author Olga Maciaszek-Sharma

spring-cloud-loadbalancer/src/main/resources/META-INF/additional-spring-configuration-metadata.json

@@ -21,6 +21,12 @@
 			"name": "spring.cloud.loadbalancer.cache.enabled",
 			"description": "Enables Spring Cloud LoadBalancer caching mechanism.",
 			"type": "java.lang.Boolean"
+		},
+		{
+			"defaultValue": true,
+			"name": "spring.cloud.loadbalancer.retry.avoid-previous-instance",
+			"description": "Enables wrapping ServiceInstanceListSupplier beans with `RetryAwareServiceInstanceListSupplier` if Spring-Retry is in the classpath.",
+			"type": "java.lang.Boolean"
 		}
 	]
 }

spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfigurationTests.java

@@ -19,6 +19,7 @@ package org.springframework.cloud.loadbalancer.annotation;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.boot.autoconfigure.AutoConfigurations;
+import org.springframework.boot.test.context.FilteredClassLoader;
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
 import org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClientAutoConfiguration;
 import org.springframework.cloud.client.discovery.composite.reactive.ReactiveCompositeDiscoveryClientAutoConfiguration;
@@ -29,10 +30,12 @@ import org.springframework.cloud.loadbalancer.core.CachingServiceInstanceListSup
 import org.springframework.cloud.loadbalancer.core.DelegatingServiceInstanceListSupplier;
 import org.springframework.cloud.loadbalancer.core.DiscoveryClientServiceInstanceListSupplier;
 import org.springframework.cloud.loadbalancer.core.HealthCheckServiceInstanceListSupplier;
+import org.springframework.cloud.loadbalancer.core.RetryAwareServiceInstanceListSupplier;
 import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier;
 import org.springframework.cloud.loadbalancer.core.ZonePreferenceServiceInstanceListSupplier;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.retry.support.RetryTemplate;
 import org.springframework.web.reactive.function.client.WebClient;
 
 import static org.assertj.core.api.BDDAssertions.then;
@@ -50,6 +53,12 @@ class LoadBalancerClientConfigurationTests {
 					LoadBalancerClientConfiguration.class));
 
 	ApplicationContextRunner blockingDiscoveryClientRunner = new ApplicationContextRunner()
+			.withClassLoader(new FilteredClassLoader(RetryTemplate.class))
+			.withConfiguration(AutoConfigurations.of(CompositeDiscoveryClientAutoConfiguration.class,
+					LoadBalancerCacheAutoConfiguration.class, LoadBalancerAutoConfiguration.class,
+					LoadBalancerClientConfiguration.class));
+
+	ApplicationContextRunner blockingDiscoveryClientRunnerWithRetry = new ApplicationContextRunner()
 			.withConfiguration(AutoConfigurations.of(CompositeDiscoveryClientAutoConfiguration.class,
 					LoadBalancerCacheAutoConfiguration.class, LoadBalancerAutoConfiguration.class,
 					LoadBalancerClientConfiguration.class));
@@ -126,6 +135,30 @@ class LoadBalancerClientConfigurationTests {
 				});
 	}
 
+	@Test
+	void shouldWrapWithRetryAwareSupplierWhenRetryTemplateOnClasspath() {
+		blockingDiscoveryClientRunnerWithRetry.run(context -> {
+			ServiceInstanceListSupplier supplier = context.getBean(ServiceInstanceListSupplier.class);
+			then(supplier).isInstanceOf(RetryAwareServiceInstanceListSupplier.class);
+			then(((DelegatingServiceInstanceListSupplier) supplier).getDelegate())
+					.isInstanceOf(CachingServiceInstanceListSupplier.class);
+			then(((DelegatingServiceInstanceListSupplier) ((DelegatingServiceInstanceListSupplier) supplier)
+					.getDelegate()).getDelegate()).isInstanceOf(DiscoveryClientServiceInstanceListSupplier.class);
+		});
+	}
+
+	@Test
+	void shouldNotWrapWithRetryAwareSupplierWhenRetryTemplateOnClasspath() {
+		blockingDiscoveryClientRunner.withPropertyValues("spring.cloud.loadbalancer.retry.avoidPreviousInstance=false")
+				.run(context -> {
+					ServiceInstanceListSupplier supplier = context.getBean(ServiceInstanceListSupplier.class);
+					then(supplier).isInstanceOf(CachingServiceInstanceListSupplier.class);
+					then(((DelegatingServiceInstanceListSupplier) supplier).getDelegate())
+							.isInstanceOf(DiscoveryClientServiceInstanceListSupplier.class);
+				});
+
+	}
+
 	@Configuration
 	protected static class TestConfig {
 

spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicyTests.java

@@ -23,7 +23,7 @@ import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext;
-import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties;
+import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties;
 import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpRequest;
@@ -47,7 +47,7 @@ class BlockingLoadBalancedRetryPolicyTests {
 
 	private final LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class);
 
-	private final LoadBalancerRetryProperties retryProperties = new LoadBalancerRetryProperties();
+	private final LoadBalancerProperties properties = new LoadBalancerProperties();
 
 	private final UnsupportedOperationException exception = new UnsupportedOperationException();
 
@@ -59,8 +59,8 @@ class BlockingLoadBalancedRetryPolicyTests {
 
 	@Test
 	void shouldExecuteIndicatedNumberOfSameAndNextInstanceRetriesAndCloseRetryContext() {
-		retryProperties.setMaxRetriesOnSameServiceInstance(1);
-		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties);
+		properties.getRetry().setMaxRetriesOnSameServiceInstance(1);
+		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(properties);
 
 		assertThat(retryPolicy.canRetrySameServer(context)).isTrue();
 		assertThat(retryPolicy.canRetryNextServer(context)).isTrue();
@@ -88,7 +88,7 @@ class BlockingLoadBalancedRetryPolicyTests {
 	void shouldNotRetryWhenMethodNotGet() {
 		when(httpRequest.getMethod()).thenReturn(HttpMethod.POST);
 		when(context.getRequest()).thenReturn(httpRequest);
-		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties);
+		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(properties);
 
 		boolean canRetry = retryPolicy.canRetry(context);
 
@@ -99,8 +99,8 @@ class BlockingLoadBalancedRetryPolicyTests {
 	void shouldRetryOnPostWhenEnabled() {
 		when(httpRequest.getMethod()).thenReturn(HttpMethod.POST);
 		when(context.getRequest()).thenReturn(httpRequest);
-		retryProperties.setRetryOnAllOperations(true);
-		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties);
+		properties.getRetry().setRetryOnAllOperations(true);
+		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(properties);
 
 		boolean canRetry = retryPolicy.canRetry(context);
 
@@ -109,16 +109,16 @@ class BlockingLoadBalancedRetryPolicyTests {
 
 	@Test
 	void shouldResolveRetryableStatusCode() {
-		retryProperties.setRetryableStatusCodes(new HashSet<>(Arrays.asList(404, 502)));
-		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties);
+		properties.getRetry().setRetryableStatusCodes(new HashSet<>(Arrays.asList(404, 502)));
+		BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(properties);
 
 		boolean retryableStatusCode = retryPolicy.retryableStatusCode(404);
 
 		assertThat(retryableStatusCode).isTrue();
 	}
 
-	private BlockingLoadBalancedRetryPolicy getRetryPolicy(LoadBalancerRetryProperties retryProperties) {
-		return new BlockingLoadBalancedRetryPolicy("test", loadBalancerClient, retryProperties);
+	private BlockingLoadBalancedRetryPolicy getRetryPolicy(LoadBalancerProperties properties) {
+		return new BlockingLoadBalancedRetryPolicy(properties);
 	}
 
 }

spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/RetryAwareServiceInstanceListSupplierTests.java

@@ -0,0 +1,81 @@
+/*
+ * Copyright 2012-2020 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.core;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.cloud.client.DefaultServiceInstance;
+import org.springframework.cloud.client.ServiceInstance;
+import org.springframework.cloud.client.loadbalancer.DefaultRequest;
+import org.springframework.cloud.client.loadbalancer.RetryableRequestContext;
+import org.springframework.cloud.loadbalancer.support.ServiceInstanceListSuppliers;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link RetryAwareServiceInstanceListSupplier}.
+ *
+ * @author Olga Maciaszek-Sharma
+ */
+class RetryAwareServiceInstanceListSupplierTests {
+
+	private final String serviceId = "test";
+
+	private static DefaultServiceInstance instance(String serviceId, String host, boolean secure) {
+		return new DefaultServiceInstance(serviceId, serviceId, host, 80, secure);
+	}
+
+	@Test
+	void shouldReturnEmptyListIfNoInstances() {
+		ServiceInstanceListSupplier delegate = ServiceInstanceListSuppliers.from(serviceId);
+		ServiceInstanceListSupplier supplier = new RetryAwareServiceInstanceListSupplier(delegate);
+
+		List<ServiceInstance> returnedInstances = supplier.get(new DefaultRequest<>(new RetryableRequestContext(null)))
+				.blockFirst();
+
+		assertThat(returnedInstances).isEmpty();
+	}
+
+	@Test
+	void shouldReturnFilteredInstances() {
+		ServiceInstance firstInstance = instance(serviceId, "1host", false);
+		ServiceInstance secondInstance = instance(serviceId, "2host-secure", true);
+		ServiceInstanceListSupplier delegate = ServiceInstanceListSuppliers.from(serviceId, firstInstance,
+				secondInstance);
+		ServiceInstanceListSupplier supplier = new RetryAwareServiceInstanceListSupplier(delegate);
+
+		List<ServiceInstance> returnedInstances = supplier
+				.get(new DefaultRequest<>(new RetryableRequestContext(firstInstance))).blockFirst();
+
+		assertThat(returnedInstances).containsExactly(secondInstance);
+	}
+
+	@Test
+	void shouldReturnAllInstancesIfFilteredInstancesEmpty() {
+		ServiceInstance firstInstance = instance(serviceId, "1host", false);
+		ServiceInstanceListSupplier delegate = ServiceInstanceListSuppliers.from(serviceId, firstInstance);
+		ServiceInstanceListSupplier supplier = new RetryAwareServiceInstanceListSupplier(delegate);
+
+		List<ServiceInstance> returnedInstances = supplier
+				.get(new DefaultRequest<>(new RetryableRequestContext(firstInstance))).blockFirst();
+
+		assertThat(returnedInstances).containsExactly(firstInstance);
+	}
+
+}