Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add UseIPAddrForServer configuration to not require DNS for 1.x #163

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public enum CommonClientConfigKey implements IClientConfigKey {
VipAddressResolverClassName("VipAddressResolverClassName"),
TargetRegion("TargetRegion"),
RulePredicateClasses("RulePredicateClasses"),
UseIPAddrForServer("UseIPAddrForServer"),

// serialization
DefaultSerializationFactoryClassName("DefaultSerializationClassName");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class DiscoveryEnabledNIWSServerList extends AbstractServerList<Discovery

int overridePort = DefaultClientConfigImpl.DEFAULT_PORT;
boolean shouldUseOverridePort = false;
boolean shouldUseIpAddr = false;

@Override
public void initWithNiwsConfig(IClientConfig clientConfig) {
Expand All @@ -69,6 +70,8 @@ public void initWithNiwsConfig(IClientConfig clientConfig) {
prioritizeVipAddressBasedServers = Boolean.parseBoolean(""+clientConfig.getProperty(CommonClientConfigKey.PrioritizeVipAddressBasedServers, prioritizeVipAddressBasedServers));
datacenter = ConfigurationManager.getDeploymentContext().getDeploymentDatacenter();
targetRegion = (String) clientConfig.getProperty(CommonClientConfigKey.TargetRegion);

shouldUseIpAddr = clientConfig.getPropertyAsBoolean(CommonClientConfigKey.UseIPAddrForServer, false);

// override client configuration and use client-defined port
if(clientConfig.getPropertyAsBoolean(CommonClientConfigKey.ForceClientPortConfiguration, false)){
Expand Down Expand Up @@ -142,7 +145,7 @@ private List<DiscoveryEnabledServer> obtainServersViaDiscovery() {
}
}

DiscoveryEnabledServer des = new DiscoveryEnabledServer(ii, isSecure);
DiscoveryEnabledServer des = new DiscoveryEnabledServer(ii, isSecure, shouldUseIpAddr);
des.setZone(DiscoveryClient.getZone(ii));
serverList.add(des);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<InstanceInfo> dummyII = getDummyInstanceInfo("dummy", "http://www.host.com", 8001);
List<InstanceInfo> secureDummyII = getDummyInstanceInfo("secureDummy", "http://www.host.com", 8002);
List<InstanceInfo> dummyII = getDummyInstanceInfo("dummy", "http://www.host.com", "1.1.1.1", 8001);
List<InstanceInfo> secureDummyII = getDummyInstanceInfo("secureDummy", "http://www.host.com", "1.1.1.1", 8002);


PowerMock.mockStatic(DiscoveryManager.class);
Expand Down Expand Up @@ -264,12 +265,13 @@ public void testTwoInstancesDontStepOnEachOther() throws Exception{



protected static List<InstanceInfo> getDummyInstanceInfo(String appName, String host, int port){
protected static List<InstanceInfo> getDummyInstanceInfo(String appName, String host, String ipAddr, int port){

List<InstanceInfo> list = new ArrayList<InstanceInfo>();

InstanceInfo info = InstanceInfo.Builder.newBuilder().setAppName(appName)
.setHostName(host)
.setIPAddr(ipAddr)
.setPort(port)
.build();

Expand Down
Original file line number Diff line number Diff line change
@@ -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<InstanceInfo> servers = DiscoveryEnabledLoadBalancerSupportsPortOverrideTest.getDummyInstanceInfo("dummy", HOST1, IP1, 8080);
List<InstanceInfo> 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<Server> 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<DiscoveryEnabledServer> serverList = deList.getInitialListOfServers();

Assert.assertEquals(2, serverList.size());
List<Server> servers = new ArrayList<Server>();
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<Server> 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<Server> 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<Server> 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<Server> 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<Server> 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);
}
}