From db1bba857a44d13db6df913b20767c2834da29aa Mon Sep 17 00:00:00 2001
From: Inigo Goiri <inigoiri@apache.org>
Date: Tue, 17 Apr 2018 13:08:01 -0700
Subject: [PATCH] YARN-8096. Wrong condition in AmIpFilter#getProxyAddresses()
 to update the proxy IP list. Contributed by Oleksandr Shevchenko.

---
 .../server/webproxy/amfilter/AmIpFilter.java  | 13 ++++--
 .../webproxy/amfilter/TestAmFilter.java       | 40 +++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/main/java/org/apache/hadoop/yarn/server/webproxy/amfilter/AmIpFilter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/main/java/org/apache/hadoop/yarn/server/webproxy/amfilter/AmIpFilter.java
index ac4afa845c..bd425a7290 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/main/java/org/apache/hadoop/yarn/server/webproxy/amfilter/AmIpFilter.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/main/java/org/apache/hadoop/yarn/server/webproxy/amfilter/AmIpFilter.java
@@ -20,6 +20,7 @@ package org.apache.hadoop.yarn.server.webproxy.amfilter;
 
 import com.google.common.annotations.VisibleForTesting;
 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.WebAppProxyServlet;
 import org.slf4j.Logger;
@@ -44,6 +45,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 @Public
 public class AmIpFilter implements Filter {
@@ -59,7 +61,7 @@ public class AmIpFilter implements Filter {
   public static final String PROXY_URI_BASES_DELIMITER = ",";
   private static final String PROXY_PATH = "/proxy";
   //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 Set<String> proxyAddresses = null;
@@ -99,9 +101,9 @@ public class AmIpFilter implements Filter {
   }
 
   protected Set<String> getProxyAddresses() throws ServletException {
-    long now = System.currentTimeMillis();
+    long now = Time.monotonicNow();
     synchronized(this) {
-      if (proxyAddresses == null || (lastUpdate + UPDATE_INTERVAL) >= now) {
+      if (proxyAddresses == null || (lastUpdate + updateInterval) <= now) {
         proxyAddresses = new HashSet<>();
         for (String proxyHost : proxyHosts) {
           try {
@@ -226,4 +228,9 @@ public class AmIpFilter implements Filter {
     }
     return isValid;
   }
+
+  @VisibleForTesting
+  protected static void setUpdateInterval(long updateInterval) {
+    AmIpFilter.updateInterval = updateInterval;
+  }
 }
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/test/java/org/apache/hadoop/yarn/server/webproxy/amfilter/TestAmFilter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/test/java/org/apache/hadoop/yarn/server/webproxy/amfilter/TestAmFilter.java
index f775c6d9c4..6eb4d4844b 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/test/java/org/apache/hadoop/yarn/server/webproxy/amfilter/TestAmFilter.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy/src/test/java/org/apache/hadoop/yarn/server/webproxy/amfilter/TestAmFilter.java
@@ -49,7 +49,9 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
+import com.google.common.base.Supplier;
 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.WebAppProxyServlet;
 import org.eclipse.jetty.server.Server;
@@ -179,6 +181,44 @@ public class TestAmFilter {
     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
    */