Use ribbon to set port if defined.
Otherwise, use a configurable default. fixes gh-15
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2013-2015 the original author or authors.
|
||||
* Copyright 2013-2018 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.
|
||||
@@ -35,6 +35,11 @@ public class CloudFoundryDiscoveryProperties {
|
||||
*/
|
||||
private long heartbeatFrequency = 5000;
|
||||
|
||||
/**
|
||||
* Port to use when no port is defined by ribbon.
|
||||
*/
|
||||
private int defaultServerPort = 80;
|
||||
|
||||
public boolean isEnabled() {
|
||||
return enabled;
|
||||
}
|
||||
@@ -50,4 +55,21 @@ public class CloudFoundryDiscoveryProperties {
|
||||
public void setHeartbeatFrequency(long heartbeatFrequency) {
|
||||
this.heartbeatFrequency = heartbeatFrequency;
|
||||
}
|
||||
}
|
||||
|
||||
public int getDefaultServerPort() {
|
||||
return defaultServerPort;
|
||||
}
|
||||
|
||||
public void setDefaultServerPort(int defaultServerPort) {
|
||||
this.defaultServerPort = defaultServerPort;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return "CloudFoundryDiscoveryProperties{" +
|
||||
"enabled=" + enabled +
|
||||
", heartbeatFrequency=" + heartbeatFrequency +
|
||||
", defaultServerPort=" + defaultServerPort +
|
||||
'}';
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2013-2015 the original author or authors.
|
||||
* Copyright 2013-2018 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.
|
||||
@@ -16,19 +16,17 @@
|
||||
|
||||
package org.springframework.cloud.cloudfoundry.discovery;
|
||||
|
||||
import com.netflix.client.config.CommonClientConfigKey;
|
||||
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.ServerList;
|
||||
import org.springframework.beans.factory.annotation.Value;
|
||||
import javax.annotation.PostConstruct;
|
||||
|
||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
|
||||
import org.springframework.cloud.cloudfoundry.CloudFoundryService;
|
||||
import org.springframework.cloud.netflix.ribbon.RibbonClientName;
|
||||
import org.springframework.cloud.netflix.ribbon.RibbonUtils;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
|
||||
import javax.annotation.PostConstruct;
|
||||
import com.netflix.client.config.IClientConfig;
|
||||
import com.netflix.loadbalancer.ServerList;
|
||||
|
||||
/**
|
||||
* @author Josh Long
|
||||
@@ -36,10 +34,7 @@ import javax.annotation.PostConstruct;
|
||||
@Configuration
|
||||
public class CloudFoundryRibbonClientConfiguration {
|
||||
|
||||
protected static final String DEFAULT_NAMESPACE = "ribbon";
|
||||
protected static final String VALUE_NOT_SET = "__not__set__";
|
||||
|
||||
@Value("${ribbon.client.name}")
|
||||
@RibbonClientName
|
||||
private String serviceId;
|
||||
|
||||
public CloudFoundryRibbonClientConfiguration() {
|
||||
@@ -51,36 +46,16 @@ public class CloudFoundryRibbonClientConfiguration {
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
public ServerList<?> ribbonServerList(CloudFoundryService svc,
|
||||
IClientConfig config) {
|
||||
CloudFoundryServerList cloudFoundryServerList = new CloudFoundryServerList(svc);
|
||||
public ServerList<?> ribbonServerList(CloudFoundryService svc, IClientConfig config,
|
||||
CloudFoundryDiscoveryProperties properties) {
|
||||
CloudFoundryServerList cloudFoundryServerList = new CloudFoundryServerList(svc, properties);
|
||||
cloudFoundryServerList.initWithNiwsConfig(config);
|
||||
return cloudFoundryServerList;
|
||||
}
|
||||
|
||||
@PostConstruct
|
||||
public void postConstruct() {
|
||||
// FIXME: what should this be?
|
||||
setProp(this.serviceId,
|
||||
CommonClientConfigKey.DeploymentContextBasedVipAddresses.key(),
|
||||
this.serviceId);
|
||||
setProp(this.serviceId, CommonClientConfigKey.EnableZoneAffinity.key(), "true");
|
||||
RibbonUtils.initializeRibbonDefaults(this.serviceId);
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2013-2015 the original author or authors.
|
||||
* Copyright 2013-2018 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.
|
||||
@@ -16,13 +16,17 @@
|
||||
|
||||
package org.springframework.cloud.cloudfoundry.discovery;
|
||||
|
||||
import com.netflix.client.config.IClientConfig;
|
||||
import com.netflix.loadbalancer.AbstractServerList;
|
||||
import org.springframework.cloud.cloudfoundry.CloudFoundryService;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import org.springframework.cloud.cloudfoundry.CloudFoundryService;
|
||||
import org.springframework.cloud.netflix.ribbon.RibbonUtils;
|
||||
import org.springframework.cloud.netflix.ribbon.RibbonUtils.RibbonProperties;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
import com.netflix.client.config.IClientConfig;
|
||||
import com.netflix.loadbalancer.AbstractServerList;
|
||||
|
||||
/**
|
||||
* @author Josh Long
|
||||
*/
|
||||
@@ -31,14 +35,18 @@ public class CloudFoundryServerList extends AbstractServerList<CloudFoundryServe
|
||||
private String serviceId;
|
||||
|
||||
private final CloudFoundryService cloudFoundryService;
|
||||
private final CloudFoundryDiscoveryProperties properties;
|
||||
private IClientConfig clientConfig;
|
||||
|
||||
CloudFoundryServerList(CloudFoundryService svc) {
|
||||
CloudFoundryServerList(CloudFoundryService svc, CloudFoundryDiscoveryProperties properties) {
|
||||
this.cloudFoundryService = svc;
|
||||
this.properties = properties;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void initWithNiwsConfig(IClientConfig iClientConfig) {
|
||||
this.serviceId = iClientConfig.getClientName();
|
||||
public void initWithNiwsConfig(IClientConfig clientConfig) {
|
||||
this.clientConfig = clientConfig;
|
||||
this.serviceId = this.clientConfig.getClientName();
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -52,11 +60,32 @@ public class CloudFoundryServerList extends AbstractServerList<CloudFoundryServe
|
||||
}
|
||||
|
||||
private List<CloudFoundryServer> cloudFoundryServers() {
|
||||
Assert.notNull(this.clientConfig, "clientConfig may not be null");
|
||||
|
||||
RibbonProperties ribbon = RibbonUtils.from(clientConfig);
|
||||
|
||||
Boolean secure = ribbon.getSecure();
|
||||
Integer securePort = ribbon.getSecurePort();
|
||||
Integer nonSecurePort = ribbon.getPort();
|
||||
|
||||
final int port;
|
||||
if (secure != null && secure && securePort != null) {
|
||||
port = securePort;
|
||||
} else if (nonSecurePort != null) {
|
||||
port = nonSecurePort;
|
||||
} else {
|
||||
port = this.properties.getDefaultServerPort();
|
||||
}
|
||||
|
||||
return cloudFoundryService
|
||||
.getApplicationInstances(this.serviceId)
|
||||
.map(tpl -> new CloudFoundryServer(tpl.getT1().getName(), tpl.getT1().getUrls().get(0), 80))
|
||||
.map(tpl -> new CloudFoundryServer(tpl.getT1().getName(), tpl.getT1().getUrls().get(0), port))
|
||||
.collectList()
|
||||
.blockOptional()
|
||||
.orElse(new ArrayList<>());
|
||||
}
|
||||
|
||||
/** for testing */ String getServiceId() {
|
||||
return serviceId;
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2013-2015 the original author or authors.
|
||||
* Copyright 2013-2018 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.
|
||||
@@ -16,20 +16,18 @@
|
||||
|
||||
package org.springframework.cloud.cloudfoundry.discovery;
|
||||
|
||||
import java.lang.reflect.Field;
|
||||
import java.util.List;
|
||||
|
||||
import org.cloudfoundry.operations.applications.ApplicationDetail;
|
||||
import org.cloudfoundry.operations.applications.InstanceDetail;
|
||||
import org.junit.Assert;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.mockito.Mockito;
|
||||
import org.springframework.cloud.cloudfoundry.CloudFoundryService;
|
||||
import org.springframework.util.ReflectionUtils;
|
||||
|
||||
import com.netflix.client.config.CommonClientConfigKey;
|
||||
import com.netflix.client.config.IClientConfig;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
@@ -47,11 +45,22 @@ public class CloudFoundryServerListTest {
|
||||
|
||||
@Before
|
||||
public void setUp() {
|
||||
IClientConfig iClientConfig = IClientConfig.Builder.newBuilder(this.serviceId)
|
||||
.withSecure(true)
|
||||
.build();
|
||||
iClientConfig.set(CommonClientConfigKey.SecurePort, 443);
|
||||
|
||||
IClientConfig iClientConfig = mock(IClientConfig.class);
|
||||
when(iClientConfig.getClientName()).thenReturn(this.serviceId);
|
||||
Tuple2<ApplicationDetail, InstanceDetail> tuple2 = getInstanceDetail();
|
||||
|
||||
CloudFoundryService cfs = mock(CloudFoundryService.class);
|
||||
when(cfs.getApplicationInstances(this.serviceId)).thenReturn(Flux.just(tuple2));
|
||||
|
||||
this.cloudFoundryServerList = new CloudFoundryServerList(cfs, new CloudFoundryDiscoveryProperties());
|
||||
this.cloudFoundryServerList.initWithNiwsConfig(iClientConfig);
|
||||
}
|
||||
|
||||
private Tuple2<ApplicationDetail, InstanceDetail> getInstanceDetail() {
|
||||
// @formatter:off
|
||||
ApplicationDetail applicationDetail = ApplicationDetail
|
||||
.builder()
|
||||
.instances(2)
|
||||
@@ -61,7 +70,7 @@ public class CloudFoundryServerListTest {
|
||||
.id("id")
|
||||
.requestedState("requestedState")
|
||||
.runningInstances(2)
|
||||
.url("http://my-app.cfapps.io")
|
||||
.url("https://my-app.cfapps.io")
|
||||
.diskQuota(20)
|
||||
.build();
|
||||
|
||||
@@ -69,28 +78,43 @@ public class CloudFoundryServerListTest {
|
||||
.builder()
|
||||
.index("0")
|
||||
.build();
|
||||
// @formatter:on
|
||||
|
||||
Tuple2<ApplicationDetail, InstanceDetail> tuple2 = Tuples.of(applicationDetail, instanceDetail);
|
||||
Mockito.when(cfs.getApplicationInstances(this.serviceId)).thenReturn(Flux.just(tuple2));
|
||||
|
||||
this.cloudFoundryServerList = new CloudFoundryServerList(cfs);
|
||||
this.cloudFoundryServerList.initWithNiwsConfig(iClientConfig);
|
||||
return Tuples.of(applicationDetail, instanceDetail);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testListOfServers() {
|
||||
List<CloudFoundryServer> initialListOfServers = this.cloudFoundryServerList.getInitialListOfServers();
|
||||
List<CloudFoundryServer> updatedListOfServers = this.cloudFoundryServerList.getUpdatedListOfServers();
|
||||
Assert.assertEquals(updatedListOfServers, initialListOfServers);
|
||||
Assert.assertTrue(initialListOfServers.size() == 1);
|
||||
assertThat(initialListOfServers)
|
||||
.containsExactly(updatedListOfServers.toArray(new CloudFoundryServer[0]))
|
||||
.hasSize(1);
|
||||
|
||||
CloudFoundryServer server = initialListOfServers.get(0);
|
||||
assertThat(server.getPort()).isEqualTo(443);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInit() throws Exception {
|
||||
Field field = ReflectionUtils.findField(this.cloudFoundryServerList.getClass(), "serviceId");
|
||||
assert field != null;
|
||||
ReflectionUtils.makeAccessible(field);
|
||||
Assert.assertEquals(String.class.cast(field.get(this.cloudFoundryServerList)), this.serviceId);
|
||||
public void testDefaultServerPort() {
|
||||
IClientConfig iClientConfig = mock(IClientConfig.class);
|
||||
when(iClientConfig.getClientName()).thenReturn(this.serviceId);
|
||||
|
||||
Tuple2<ApplicationDetail, InstanceDetail> tuple2 = getInstanceDetail();
|
||||
|
||||
CloudFoundryService cfs = mock(CloudFoundryService.class);
|
||||
when(cfs.getApplicationInstances(this.serviceId)).thenReturn(Flux.just(tuple2));
|
||||
|
||||
CloudFoundryServerList serverList = new CloudFoundryServerList(cfs, new CloudFoundryDiscoveryProperties());
|
||||
serverList.initWithNiwsConfig(iClientConfig);
|
||||
|
||||
CloudFoundryServer server = serverList.getInitialListOfServers().get(0);
|
||||
assertThat(server.getPort()).isEqualTo(80);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInit() {
|
||||
assertThat(this.cloudFoundryServerList.getServiceId()).isEqualTo(this.serviceId);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user