diff --git a/ribbon-core/src/main/java/com/netflix/client/config/CommonClientConfigKey.java b/ribbon-core/src/main/java/com/netflix/client/config/CommonClientConfigKey.java index cfde8986..05184be7 100644 --- a/ribbon-core/src/main/java/com/netflix/client/config/CommonClientConfigKey.java +++ b/ribbon-core/src/main/java/com/netflix/client/config/CommonClientConfigKey.java @@ -94,6 +94,7 @@ public enum CommonClientConfigKey implements IClientConfigKey { VipAddressResolverClassName("VipAddressResolverClassName"), TargetRegion("TargetRegion"), RulePredicateClasses("RulePredicateClasses"), + UseIPAddrForServer("UseIPAddrForServer"), // serialization DefaultSerializationFactoryClassName("DefaultSerializationClassName"); diff --git a/ribbon-core/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java b/ribbon-core/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java index 7aa0d883..832d3510 100644 --- a/ribbon-core/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java +++ b/ribbon-core/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java @@ -87,6 +87,8 @@ public class DefaultClientConfigImpl implements IClientConfig { public static final String DEFAULT_NFLOADBALANCER_RULE_CLASSNAME = "com.netflix.loadbalancer.AvailabilityFilteringRule"; public static final String DEFAULT_NFLOADBALANCER_CLASSNAME = "com.netflix.loadbalancer.ZoneAwareLoadBalancer"; + + public static final boolean DEFAULT_USEIPADDRESS_FOR_SERVER = Boolean.FALSE; public static final String DEFAULT_CLIENT_CLASSNAME = "com.netflix.niws.client.http.RestClient"; @@ -188,6 +190,10 @@ public String getDefaultNfloadbalancerRuleClassname() { public String getDefaultNfloadbalancerClassname() { return DEFAULT_NFLOADBALANCER_CLASSNAME; } + + public boolean getDefaultUseIpAddressForServer() { + return DEFAULT_USEIPADDRESS_FOR_SERVER; + } public String getDefaultClientClassname() { return DEFAULT_CLIENT_CLASSNAME; @@ -402,6 +408,7 @@ protected void loadDefaultValues() { putDefaultStringProperty(CommonClientConfigKey.NIWSServerListClassName, getDefaultSeverListClass()); putDefaultStringProperty(CommonClientConfigKey.VipAddressResolverClassName, getDefaultVipaddressResolverClassname()); putDefaultBooleanProperty(CommonClientConfigKey.IsClientAuthRequired, getDefaultIsClientAuthRequired()); + putDefaultBooleanProperty(CommonClientConfigKey.UseIPAddrForServer, getDefaultUseIpAddressForServer()); } protected void setPropertyInternal(IClientConfigKey propName, Object value) { diff --git a/ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledNIWSServerList.java b/ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledNIWSServerList.java index 872732c9..3cbdcd34 100644 --- a/ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledNIWSServerList.java +++ b/ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledNIWSServerList.java @@ -56,6 +56,7 @@ public class DiscoveryEnabledNIWSServerList extends AbstractServerList obtainServersViaDiscovery() { } } - DiscoveryEnabledServer des = new DiscoveryEnabledServer(ii, isSecure); + DiscoveryEnabledServer des = new DiscoveryEnabledServer(ii, isSecure, shouldUseIpAddr); des.setZone(DiscoveryClient.getZone(ii)); serverList.add(des); } diff --git a/ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledServer.java b/ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledServer.java index 1d932ba4..d63bc829 100644 --- a/ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledServer.java +++ b/ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledServer.java @@ -33,8 +33,8 @@ public class DiscoveryEnabledServer extends Server{ InstanceInfo instanceInfo; - public DiscoveryEnabledServer(InstanceInfo instanceInfo, boolean useSecurePort) { - super(instanceInfo.getHostName(), instanceInfo.getPort()); + public DiscoveryEnabledServer(InstanceInfo instanceInfo, boolean useSecurePort, boolean useIpAddr) { + super(useIpAddr ? instanceInfo.getIPAddr() : instanceInfo.getHostName(), instanceInfo.getPort()); if(useSecurePort && instanceInfo.isPortEnabled(PortType.SECURE)) super.setPort(instanceInfo.getSecurePort()); this.instanceInfo = instanceInfo; diff --git a/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DefaultNIWSServerListFilterTest.java b/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DefaultNIWSServerListFilterTest.java index 457ef751..1ed5cdaf 100644 --- a/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DefaultNIWSServerListFilterTest.java +++ b/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DefaultNIWSServerListFilterTest.java @@ -56,7 +56,7 @@ private DiscoveryEnabledServer createServer(String host, int port, String zone) .setHostName(host) .setPort(port) .build(); - DiscoveryEnabledServer server = new DiscoveryEnabledServer(info, false); + DiscoveryEnabledServer server = new DiscoveryEnabledServer(info, false, false); server.setZone(zone); return server; } diff --git a/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DiscoveryEnabledLoadBalancerSupportsPortOverrideTest.java b/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DiscoveryEnabledLoadBalancerSupportsPortOverrideTest.java index ce6d232c..a30adf40 100644 --- a/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DiscoveryEnabledLoadBalancerSupportsPortOverrideTest.java +++ b/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DiscoveryEnabledLoadBalancerSupportsPortOverrideTest.java @@ -62,14 +62,15 @@ @RunWith(PowerMockRunner.class) @PrepareForTest( {DiscoveryManager.class, DiscoveryClient.class} ) @PowerMockIgnore("javax.management.*") +@SuppressWarnings("PMD.AvoidUsingHardCodedIP") public class DiscoveryEnabledLoadBalancerSupportsPortOverrideTest { @Before public void setupMock(){ - List dummyII = getDummyInstanceInfo("dummy", "http://www.host.com", 8001); - List secureDummyII = getDummyInstanceInfo("secureDummy", "http://www.host.com", 8002); + List dummyII = getDummyInstanceInfo("dummy", "http://www.host.com", "1.1.1.1", 8001); + List secureDummyII = getDummyInstanceInfo("secureDummy", "http://www.host.com", "1.1.1.1", 8002); PowerMock.mockStatic(DiscoveryManager.class); @@ -264,12 +265,13 @@ public void testTwoInstancesDontStepOnEachOther() throws Exception{ - protected static List getDummyInstanceInfo(String appName, String host, int port){ + protected static List getDummyInstanceInfo(String appName, String host, String ipAddr, int port){ List list = new ArrayList(); InstanceInfo info = InstanceInfo.Builder.newBuilder().setAppName(appName) .setHostName(host) + .setIPAddr(ipAddr) .setPort(port) .build(); diff --git a/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DiscoveryEnabledLoadBalancerSupportsUseIpAddr.java b/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DiscoveryEnabledLoadBalancerSupportsUseIpAddr.java new file mode 100644 index 00000000..5d6b8ad1 --- /dev/null +++ b/ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DiscoveryEnabledLoadBalancerSupportsUseIpAddr.java @@ -0,0 +1,195 @@ +/* +* +* Copyright 2014 Netflix, Inc. +* +* 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 com.netflix.niws.loadbalancer; + +import static org.easymock.EasyMock.expect; +import static org.powermock.api.easymock.PowerMock.createMock; +import static org.powermock.api.easymock.PowerMock.replay; +import static org.powermock.api.easymock.PowerMock.verify; + +import java.util.ArrayList; +import java.util.List; + +import org.easymock.EasyMock; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.powermock.api.easymock.PowerMock; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import com.netflix.appinfo.InstanceInfo; +import com.netflix.client.config.DefaultClientConfigImpl; +import com.netflix.config.ConfigurationManager; +import com.netflix.discovery.DiscoveryClient; +import com.netflix.discovery.DiscoveryManager; +import com.netflix.loadbalancer.Server; + +/** + * Verify behavior of the override flag DiscoveryEnabledNIWSServerList.useIpAddr + * + * Currently only works with the DiscoveryEnabledNIWSServerList since this is where the current limitation is applicable + * + * See also: https://groups.google.com/forum/#!topic/eureka_netflix/7M28bK-SCZU + * + * Created by aspyker on 8/21/14. + */ +@RunWith(PowerMockRunner.class) +@PrepareForTest( {DiscoveryManager.class, DiscoveryClient.class} ) +@PowerMockIgnore("javax.management.*") +@SuppressWarnings("PMD.AvoidUsingHardCodedIP") +public class DiscoveryEnabledLoadBalancerSupportsUseIpAddr { + static final String IP1 = "1.1.1.1"; + static final String HOST1 = "server1.app.host.com"; + static final String IP2 = "1.1.1.2"; + static final String HOST2 = "server1.app.host.com"; + + @Before + public void setupMock(){ + + List servers = DiscoveryEnabledLoadBalancerSupportsPortOverrideTest.getDummyInstanceInfo("dummy", HOST1, IP1, 8080); + List servers2 = DiscoveryEnabledLoadBalancerSupportsPortOverrideTest.getDummyInstanceInfo("dummy", HOST2, IP2, 8080); + servers.addAll(servers2); + + + PowerMock.mockStatic(DiscoveryManager.class); + PowerMock.mockStatic(DiscoveryClient.class); + + DiscoveryClient mockedDiscoveryClient = createMock(DiscoveryClient.class); + DiscoveryManager mockedDiscoveryManager = createMock(DiscoveryManager.class); + + expect(DiscoveryClient.getZone((InstanceInfo) EasyMock.anyObject())).andReturn("dummyZone").anyTimes(); + expect(DiscoveryManager.getInstance()).andReturn(mockedDiscoveryManager).anyTimes(); + expect(mockedDiscoveryManager.getDiscoveryClient()).andReturn(mockedDiscoveryClient).anyTimes(); + + + expect(mockedDiscoveryClient.getInstancesByVipAddress("dummy", false, "region")).andReturn(servers).anyTimes(); + + replay(DiscoveryManager.class); + replay(DiscoveryClient.class); + replay(mockedDiscoveryManager); + replay(mockedDiscoveryClient); + } + + /** + * Generic method to help with various tests + * @param globalspecified if false, will clear property DiscoveryEnabledNIWSServerList.useIpAddr + * @param global value of DiscoveryEnabledNIWSServerList.useIpAddr + * @param clientspecified if false, will not set property on client config + * @param client value of client.namespace.ribbon.UseIPAddrForServer + */ + private List testUsesIpAddr(boolean globalSpecified, boolean global, boolean clientSpecified, boolean client) throws Exception{ + if (globalSpecified) { + ConfigurationManager.getConfigInstance().setProperty("ribbon.UseIPAddrForServer", global); + } + else { + ConfigurationManager.getConfigInstance().clearProperty("ribbon.UseIPAddrForServer"); + } + if (clientSpecified) { + ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testUsesIpAddr.ribbon.UseIPAddrForServer", client); + } + else { + ConfigurationManager.getConfigInstance().clearProperty("DiscoveryEnabled.testUsesIpAddr.ribbon.UseIPAddrForServer"); + } + System.out.println("r = " + ConfigurationManager.getConfigInstance().getProperty("ribbon.UseIPAddrForServer")); + System.out.println("d = " + ConfigurationManager.getConfigInstance().getProperty("DiscoveryEnabled.testUsesIpAddr.ribbon.UseIPAddrForServer")); + + ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testUsesIpAddr.ribbon.NIWSServerListClassName", DiscoveryEnabledNIWSServerList.class.getName()); + ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testUsesIpAddr.ribbon.DeploymentContextBasedVipAddresses", "dummy"); + ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testUsesIpAddr.ribbon.TargetRegion", "region"); + + DiscoveryEnabledNIWSServerList deList = new DiscoveryEnabledNIWSServerList(); + + DefaultClientConfigImpl clientConfig = DefaultClientConfigImpl.class.newInstance(); + clientConfig.loadProperties("DiscoveryEnabled.testUsesIpAddr"); + deList.initWithNiwsConfig(clientConfig); + + List serverList = deList.getInitialListOfServers(); + + Assert.assertEquals(2, serverList.size()); + List servers = new ArrayList(); + for (DiscoveryEnabledServer server : serverList) { + servers.add((Server)server); + } + return servers; + } + + /** + * Test the case where the property has been used specific to client with true + * @throws Exception + */ + @Test + public void testUsesIpAddrByWhenClientTrueOnly() throws Exception{ + List servers = testUsesIpAddr(false, false, true, true); + Assert.assertEquals(servers.get(0).getHost(), IP1); + Assert.assertEquals(servers.get(1).getHost(), IP2); + } + + /** + * Test the case where the property has been used specific to client with false + * @throws Exception + */ + @Test + public void testUsesIpAddrByWhenClientFalseOnly() throws Exception{ + List servers = testUsesIpAddr(false, false, true, false); + Assert.assertEquals(servers.get(0).getHost(), HOST1); + Assert.assertEquals(servers.get(1).getHost(), HOST2); + } + + @Test + /** + * Test the case where the property has been used globally with true + * @throws Exception + */ + public void testUsesIpAddrByWhenGlobalTrueOnly() throws Exception{ + List servers = testUsesIpAddr(true, true, false, false); + Assert.assertEquals(servers.get(0).getHost(), IP1); + Assert.assertEquals(servers.get(1).getHost(), IP2); + } + + @Test + /** + * Test the case where the property has been used globally with false + * @throws Exception + */ + public void testUsesIpAddrByWhenGlobalFalseOnly() throws Exception{ + List servers = testUsesIpAddr(true, false, false, false); + Assert.assertEquals(servers.get(0).getHost(), HOST1); + Assert.assertEquals(servers.get(1).getHost(), HOST2); + } + + @Test + /** + * Test the case where the property hasn't been used at the global or client level + * @throws Exception + */ + public void testUsesHostnameByDefault() throws Exception{ + List servers = testUsesIpAddr(false, false, false, false); + Assert.assertEquals(servers.get(0).getHost(), HOST1); + Assert.assertEquals(servers.get(1).getHost(), HOST2); + } + + @After + public void afterMock(){ + verify(DiscoveryManager.class); + verify(DiscoveryClient.class); + } +}