YARN-8096. Wrong condition in AmIpFilter#getProxyAddresses() to update the proxy IP list. Contributed by Oleksandr Shevchenko.

This commit is contained in:
Inigo Goiri 2018-04-17 13:08:01 -07:00
parent a68b04370f
commit db1bba857a
2 changed files with 50 additions and 3 deletions

View File

@ -20,6 +20,7 @@ package org.apache.hadoop.yarn.server.webproxy.amfilter;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import org.apache.hadoop.classification.InterfaceAudience.Public; import org.apache.hadoop.classification.InterfaceAudience.Public;
import org.apache.hadoop.util.Time;
import org.apache.hadoop.yarn.server.webproxy.ProxyUtils; import org.apache.hadoop.yarn.server.webproxy.ProxyUtils;
import org.apache.hadoop.yarn.server.webproxy.WebAppProxyServlet; import org.apache.hadoop.yarn.server.webproxy.WebAppProxyServlet;
import org.slf4j.Logger; import org.slf4j.Logger;
@ -44,6 +45,7 @@ import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.concurrent.TimeUnit;
@Public @Public
public class AmIpFilter implements Filter { public class AmIpFilter implements Filter {
@ -59,7 +61,7 @@ public class AmIpFilter implements Filter {
public static final String PROXY_URI_BASES_DELIMITER = ","; public static final String PROXY_URI_BASES_DELIMITER = ",";
private static final String PROXY_PATH = "/proxy"; private static final String PROXY_PATH = "/proxy";
//update the proxy IP list about every 5 min //update the proxy IP list about every 5 min
private static final long UPDATE_INTERVAL = 5 * 60 * 1000; private static long updateInterval = TimeUnit.MINUTES.toMillis(5);
private String[] proxyHosts; private String[] proxyHosts;
private Set<String> proxyAddresses = null; private Set<String> proxyAddresses = null;
@ -99,9 +101,9 @@ public class AmIpFilter implements Filter {
} }
protected Set<String> getProxyAddresses() throws ServletException { protected Set<String> getProxyAddresses() throws ServletException {
long now = System.currentTimeMillis(); long now = Time.monotonicNow();
synchronized(this) { synchronized(this) {
if (proxyAddresses == null || (lastUpdate + UPDATE_INTERVAL) >= now) { if (proxyAddresses == null || (lastUpdate + updateInterval) <= now) {
proxyAddresses = new HashSet<>(); proxyAddresses = new HashSet<>();
for (String proxyHost : proxyHosts) { for (String proxyHost : proxyHosts) {
try { try {
@ -226,4 +228,9 @@ public class AmIpFilter implements Filter {
} }
return isValid; return isValid;
} }
@VisibleForTesting
protected static void setUpdateInterval(long updateInterval) {
AmIpFilter.updateInterval = updateInterval;
}
} }

View File

@ -49,7 +49,9 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import com.google.common.base.Supplier;
import org.apache.hadoop.http.TestHttpServer; import org.apache.hadoop.http.TestHttpServer;
import org.apache.hadoop.test.GenericTestUtils;
import org.apache.hadoop.yarn.server.webproxy.ProxyUtils; import org.apache.hadoop.yarn.server.webproxy.ProxyUtils;
import org.apache.hadoop.yarn.server.webproxy.WebAppProxyServlet; import org.apache.hadoop.yarn.server.webproxy.WebAppProxyServlet;
import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.Server;
@ -179,6 +181,44 @@ public class TestAmFilter {
return server.getURI().toString() + servletPath; return server.getURI().toString() + servletPath;
} }
@Test(timeout = 2000)
public void testProxyUpdate() throws Exception {
Map<String, String> params = new HashMap<>();
params.put(AmIpFilter.PROXY_HOSTS, proxyHost);
params.put(AmIpFilter.PROXY_URI_BASES, proxyUri);
FilterConfig conf = new DummyFilterConfig(params);
AmIpFilter filter = new AmIpFilter();
int updateInterval = 1000;
AmIpFilter.setUpdateInterval(updateInterval);
filter.init(conf);
filter.getProxyAddresses();
// check that the configuration was applied
assertTrue(filter.getProxyAddresses().contains("127.0.0.1"));
// change proxy configurations
params = new HashMap<>();
params.put(AmIpFilter.PROXY_HOSTS, "unknownhost");
params.put(AmIpFilter.PROXY_URI_BASES, proxyUri);
conf = new DummyFilterConfig(params);
filter.init(conf);
// configurations shouldn't be updated now
assertFalse(filter.getProxyAddresses().isEmpty());
// waiting for configuration update
GenericTestUtils.waitFor(new Supplier<Boolean>() {
@Override
public Boolean get() {
try {
return filter.getProxyAddresses().isEmpty();
} catch (ServletException e) {
return true;
}
}
}, 500, updateInterval);
}
/** /**
* Test AmIpFilter * Test AmIpFilter
*/ */