Unverified Commit 4479a2b2 by Spencer Gibb

Merge pull request #945 from tkvangorder/master

* pull945: Ribbon retry works & fix Content-Length error.
parents a3ab3259 05351b78
...@@ -79,9 +79,6 @@ public class RibbonAutoConfiguration { ...@@ -79,9 +79,6 @@ public class RibbonAutoConfiguration {
@Autowired @Autowired
private SpringClientFactory springClientFactory; private SpringClientFactory springClientFactory;
@Autowired
private LoadBalancerClient loadBalancerClient;
@Bean @Bean
public RestTemplateCustomizer restTemplateCustomizer( public RestTemplateCustomizer restTemplateCustomizer(
final RibbonClientHttpRequestFactory ribbonClientHttpRequestFactory) { final RibbonClientHttpRequestFactory ribbonClientHttpRequestFactory) {
...@@ -95,8 +92,7 @@ public class RibbonAutoConfiguration { ...@@ -95,8 +92,7 @@ public class RibbonAutoConfiguration {
@Bean @Bean
public RibbonClientHttpRequestFactory ribbonClientHttpRequestFactory() { public RibbonClientHttpRequestFactory ribbonClientHttpRequestFactory() {
return new RibbonClientHttpRequestFactory(this.springClientFactory, return new RibbonClientHttpRequestFactory(this.springClientFactory);
this.loadBalancerClient);
} }
} }
......
...@@ -18,6 +18,8 @@ package org.springframework.cloud.netflix.ribbon; ...@@ -18,6 +18,8 @@ package org.springframework.cloud.netflix.ribbon;
import java.net.URI; import java.net.URI;
import javax.annotation.PostConstruct;
import org.apache.http.client.params.ClientPNames; import org.apache.http.client.params.ClientPNames;
import org.apache.http.client.params.CookiePolicy; import org.apache.http.client.params.CookiePolicy;
import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.annotation.Value;
...@@ -48,6 +50,9 @@ import com.netflix.servo.monitor.Monitors; ...@@ -48,6 +50,9 @@ import com.netflix.servo.monitor.Monitors;
import com.sun.jersey.api.client.Client; import com.sun.jersey.api.client.Client;
import com.sun.jersey.client.apache4.ApacheHttpClient4; import com.sun.jersey.client.apache4.ApacheHttpClient4;
import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses;
import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.setRibbonProperty;
/** /**
* @author Dave Syer * @author Dave Syer
*/ */
...@@ -155,6 +160,11 @@ public class RibbonClientConfiguration { ...@@ -155,6 +160,11 @@ public class RibbonClientConfiguration {
return new DefaultServerIntrospector(); return new DefaultServerIntrospector();
} }
@PostConstruct
public void preprocess() {
setRibbonProperty(name, DeploymentContextBasedVipAddresses.key(), name);
}
static class OverrideRestClient extends RestClient { static class OverrideRestClient extends RestClient {
private ServerIntrospector serverIntrospector; private ServerIntrospector serverIntrospector;
......
...@@ -19,16 +19,12 @@ package org.springframework.cloud.netflix.ribbon; ...@@ -19,16 +19,12 @@ package org.springframework.cloud.netflix.ribbon;
import java.io.IOException; import java.io.IOException;
import java.net.URI; import java.net.URI;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.LoadBalancerClient;
import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient.RibbonServer;
import org.springframework.http.HttpMethod; import org.springframework.http.HttpMethod;
import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequest;
import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.ClientHttpRequestFactory;
import com.netflix.client.config.IClientConfig; import com.netflix.client.config.IClientConfig;
import com.netflix.client.http.HttpRequest; import com.netflix.client.http.HttpRequest;
import com.netflix.loadbalancer.Server;
import com.netflix.niws.client.http.RestClient; import com.netflix.niws.client.http.RestClient;
/** /**
...@@ -38,12 +34,8 @@ public class RibbonClientHttpRequestFactory implements ClientHttpRequestFactory ...@@ -38,12 +34,8 @@ public class RibbonClientHttpRequestFactory implements ClientHttpRequestFactory
private final SpringClientFactory clientFactory; private final SpringClientFactory clientFactory;
private LoadBalancerClient loadBalancer; public RibbonClientHttpRequestFactory(SpringClientFactory clientFactory) {
public RibbonClientHttpRequestFactory(SpringClientFactory clientFactory,
LoadBalancerClient loadBalancer) {
this.clientFactory = clientFactory; this.clientFactory = clientFactory;
this.loadBalancer = loadBalancer;
} }
@Override @Override
...@@ -55,28 +47,11 @@ public class RibbonClientHttpRequestFactory implements ClientHttpRequestFactory ...@@ -55,28 +47,11 @@ public class RibbonClientHttpRequestFactory implements ClientHttpRequestFactory
throw new IOException( throw new IOException(
"Invalid hostname in the URI [" + originalUri.toASCIIString() + "]"); "Invalid hostname in the URI [" + originalUri.toASCIIString() + "]");
} }
ServiceInstance instance = this.loadBalancer.choose(serviceId); IClientConfig clientConfig = this.clientFactory.getClientConfig(serviceId);
if (instance == null) { RestClient client = this.clientFactory.getClient(serviceId, RestClient.class);
throw new IllegalStateException("No instances available for " + serviceId);
}
URI uri = this.loadBalancer.reconstructURI(instance, originalUri);
IClientConfig clientConfig = this.clientFactory
.getClientConfig(instance.getServiceId());
RestClient client = this.clientFactory.getClient(instance.getServiceId(),
RestClient.class);
HttpRequest.Verb verb = HttpRequest.Verb.valueOf(httpMethod.name()); HttpRequest.Verb verb = HttpRequest.Verb.valueOf(httpMethod.name());
RibbonLoadBalancerContext context = this.clientFactory
.getLoadBalancerContext(serviceId);
Server server = null;
if (instance instanceof RibbonServer) {
server = ((RibbonServer) instance).getServer();
}
RibbonStatsRecorder statsRecorder = new RibbonStatsRecorder(context, server);
return new RibbonHttpRequest(uri, verb, client, clientConfig, statsRecorder); return new RibbonHttpRequest(originalUri, verb, client, clientConfig);
} }
} }
...@@ -16,21 +16,22 @@ ...@@ -16,21 +16,22 @@
package org.springframework.cloud.netflix.ribbon; package org.springframework.cloud.netflix.ribbon;
import com.netflix.client.config.IClientConfig;
import com.netflix.client.http.HttpRequest;
import com.netflix.client.http.HttpResponse;
import com.netflix.niws.client.http.RestClient;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.client.AbstractClientHttpRequest;
import org.springframework.http.client.ClientHttpResponse;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStream; import java.io.OutputStream;
import java.net.URI; import java.net.URI;
import java.util.List; import java.util.List;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.client.AbstractClientHttpRequest;
import org.springframework.http.client.ClientHttpResponse;
import com.netflix.client.config.IClientConfig;
import com.netflix.client.http.HttpRequest;
import com.netflix.client.http.HttpResponse;
import com.netflix.niws.client.http.RestClient;
/** /**
* @author Spencer Gibb * @author Spencer Gibb
*/ */
...@@ -42,16 +43,14 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest { ...@@ -42,16 +43,14 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest {
private HttpRequest.Verb verb; private HttpRequest.Verb verb;
private RestClient client; private RestClient client;
private IClientConfig config; private IClientConfig config;
private RibbonStatsRecorder statsRecorder;
private ByteArrayOutputStream outputStream = null; private ByteArrayOutputStream outputStream = null;
public RibbonHttpRequest(URI uri, HttpRequest.Verb verb, RestClient client, public RibbonHttpRequest(URI uri, HttpRequest.Verb verb, RestClient client,
IClientConfig config, RibbonStatsRecorder statsRecorder) { IClientConfig config) {
this.uri = uri; this.uri = uri;
this.verb = verb; this.verb = verb;
this.client = client; this.client = client;
this.config = config; this.config = config;
this.statsRecorder = statsRecorder;
this.builder = HttpRequest.newBuilder().uri(uri).verb(verb); this.builder = HttpRequest.newBuilder().uri(uri).verb(verb);
} }
...@@ -83,11 +82,9 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest { ...@@ -83,11 +82,9 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest {
builder.entity(outputStream.toByteArray()); builder.entity(outputStream.toByteArray());
} }
HttpRequest request = builder.build(); HttpRequest request = builder.build();
HttpResponse response = client.execute(request, config); HttpResponse response = client.executeWithLoadBalancer(request, config);
statsRecorder.recordStats(response);
return new RibbonHttpResponse(response); return new RibbonHttpResponse(response);
} catch (Exception e) { } catch (Exception e) {
statsRecorder.recordStats(e);
throw new IOException(e); throw new IOException(e);
} }
} }
...@@ -96,14 +93,19 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest { ...@@ -96,14 +93,19 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest {
for (String name : headers.keySet()) { for (String name : headers.keySet()) {
// apache http RequestContent pukes if there is a body and // apache http RequestContent pukes if there is a body and
// the dynamic headers are already present // the dynamic headers are already present
if (!isDynamic(name)) { if (isDynamic(name) && outputStream != null) {
continue;
}
//Don't add content-length if the output stream is null. The RibbonClient does this for us.
if (name.equals("Content-Length") && outputStream == null) {
continue;
}
List<String> values = headers.get(name); List<String> values = headers.get(name);
for (String value : values) { for (String value : values) {
builder.header(name, value); builder.header(name, value);
} }
} }
} }
}
private boolean isDynamic(String name) { private boolean isDynamic(String name) {
return name.equalsIgnoreCase("Content-Length") || name.equalsIgnoreCase("Transfer-Encoding"); return name.equalsIgnoreCase("Content-Length") || name.equalsIgnoreCase("Transfer-Encoding");
......
package org.springframework.cloud.netflix.ribbon;
import com.netflix.config.ConfigurationManager;
import com.netflix.config.DynamicPropertyFactory;
import com.netflix.config.DynamicStringProperty;
/**
* @author Spencer Gibb
*/
public class RibbonProperyUtils {
public static final String VALUE_NOT_SET = "__not__set__";
public static final String DEFAULT_NAMESPACE = "ribbon";
public static void setRibbonProperty(String serviceId, String suffix, String value) {
// how to set the namespace properly?
String key = getRibbonKey(serviceId, suffix);
DynamicStringProperty property = getProperty(key);
if (property.get().equals(VALUE_NOT_SET)) {
ConfigurationManager.getConfigInstance().setProperty(key, value);
}
}
public static String getRibbonKey(String serviceId, String suffix) {
return serviceId + "." + DEFAULT_NAMESPACE + "." + suffix;
}
public static DynamicStringProperty getProperty(String key) {
return DynamicPropertyFactory.getInstance().getStringProperty(key, VALUE_NOT_SET);
}
}
...@@ -16,13 +16,8 @@ ...@@ -16,13 +16,8 @@
package org.springframework.cloud.netflix.ribbon.eureka; package org.springframework.cloud.netflix.ribbon.eureka;
import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses;
import static com.netflix.client.config.CommonClientConfigKey.EnableZoneAffinity;
import javax.annotation.PostConstruct; import javax.annotation.PostConstruct;
import lombok.extern.apachecommons.CommonsLog;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
...@@ -35,14 +30,18 @@ import com.netflix.appinfo.EurekaInstanceConfig; ...@@ -35,14 +30,18 @@ import com.netflix.appinfo.EurekaInstanceConfig;
import com.netflix.client.config.IClientConfig; import com.netflix.client.config.IClientConfig;
import com.netflix.config.ConfigurationManager; import com.netflix.config.ConfigurationManager;
import com.netflix.config.DeploymentContext.ContextKey; import com.netflix.config.DeploymentContext.ContextKey;
import com.netflix.config.DynamicPropertyFactory;
import com.netflix.config.DynamicStringProperty;
import com.netflix.discovery.EurekaClientConfig; import com.netflix.discovery.EurekaClientConfig;
import com.netflix.loadbalancer.IPing; import com.netflix.loadbalancer.IPing;
import com.netflix.loadbalancer.ServerList; import com.netflix.loadbalancer.ServerList;
import com.netflix.niws.loadbalancer.DiscoveryEnabledNIWSServerList; import com.netflix.niws.loadbalancer.DiscoveryEnabledNIWSServerList;
import com.netflix.niws.loadbalancer.NIWSDiscoveryPing; import com.netflix.niws.loadbalancer.NIWSDiscoveryPing;
import lombok.extern.apachecommons.CommonsLog;
import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses;
import static com.netflix.client.config.CommonClientConfigKey.EnableZoneAffinity;
import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.setRibbonProperty;
/** /**
* Preprocessor that configures defaults for eureka-discovered ribbon clients. Such as: * Preprocessor that configures defaults for eureka-discovered ribbon clients. Such as:
* <code>@zone</code>, NIWSServerListClassName, DeploymentContextBasedVipAddresses, * <code>@zone</code>, NIWSServerListClassName, DeploymentContextBasedVipAddresses,
...@@ -62,10 +61,6 @@ public class EurekaRibbonClientConfiguration { ...@@ -62,10 +61,6 @@ public class EurekaRibbonClientConfiguration {
@Value("${ribbon.client.name}") @Value("${ribbon.client.name}")
private String serviceId = "client"; private String serviceId = "client";
protected static final String VALUE_NOT_SET = "__not__set__";
protected static final String DEFAULT_NAMESPACE = "ribbon";
@Autowired(required = false) @Autowired(required = false)
private EurekaClientConfig clientConfig; private EurekaClientConfig clientConfig;
...@@ -132,25 +127,8 @@ public class EurekaRibbonClientConfiguration { ...@@ -132,25 +127,8 @@ public class EurekaRibbonClientConfiguration {
} }
} }
} }
setProp(this.serviceId, DeploymentContextBasedVipAddresses.key(), this.serviceId); setRibbonProperty(this.serviceId, DeploymentContextBasedVipAddresses.key(), this.serviceId);
setProp(this.serviceId, EnableZoneAffinity.key(), "true"); setRibbonProperty(this.serviceId, EnableZoneAffinity.key(), "true");
}
protected void setProp(String serviceId, String suffix, String value) {
// how to set the namespace properly?
String key = getKey(serviceId, suffix);
DynamicStringProperty property = getProperty(key);
if (property.get().equals(VALUE_NOT_SET)) {
ConfigurationManager.getConfigInstance().setProperty(key, value);
}
}
protected DynamicStringProperty getProperty(String key) {
return DynamicPropertyFactory.getInstance().getStringProperty(key, VALUE_NOT_SET);
}
protected String getKey(String serviceId, String suffix) {
return serviceId + "." + DEFAULT_NAMESPACE + "." + suffix;
} }
} }
...@@ -36,7 +36,8 @@ import com.netflix.niws.loadbalancer.DiscoveryEnabledServer; ...@@ -36,7 +36,8 @@ import com.netflix.niws.loadbalancer.DiscoveryEnabledServer;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.springframework.cloud.netflix.ribbon.eureka.EurekaRibbonClientConfiguration.VALUE_NOT_SET;
import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.*;
/** /**
* @author Dave Syer * @author Dave Syer
...@@ -82,12 +83,12 @@ public class EurekaRibbonClientConfigurationTests { ...@@ -82,12 +83,12 @@ public class EurekaRibbonClientConfigurationTests {
String serviceId = "myService"; String serviceId = "myService";
String suffix = "mySuffix"; String suffix = "mySuffix";
String value = "myValue"; String value = "myValue";
DynamicStringProperty property = preprocessor.getProperty(preprocessor.getKey( DynamicStringProperty property = getProperty(getRibbonKey(
serviceId, suffix)); serviceId, suffix));
assertEquals("property doesn't have default value", VALUE_NOT_SET, property.get()); assertEquals("property doesn't have default value", VALUE_NOT_SET, property.get());
preprocessor.setProp(serviceId, suffix, value); setRibbonProperty(serviceId, suffix, value);
assertEquals("property has wrong value", value, property.get()); assertEquals("property has wrong value", value, property.get());
preprocessor.setProp(serviceId, suffix, value); setRibbonProperty(serviceId, suffix, value);
assertEquals("property has wrong value", value, property.get()); assertEquals("property has wrong value", value, property.get());
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment