HADOOP-10823. TestReloadingX509TrustManager is flaky. Contributed by Mingliang Liu.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/62558595 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/62558595 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/62558595 Branch: refs/heads/YARN-3368 Commit: 625585950a15461eb032e5e7ed8fdf4e1113b2bb Parents: 37d939a Author: Jitendra Pandey <[email protected]> Authored: Mon Aug 8 11:00:19 2016 -0700 Committer: Jitendra Pandey <[email protected]> Committed: Mon Aug 8 11:00:19 2016 -0700 ---------------------------------------------------------------------- .../security/ssl/ReloadingX509TrustManager.java | 12 ++-- .../ssl/TestReloadingX509TrustManager.java | 63 ++++++++++++++------ 2 files changed, 53 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/62558595/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/ReloadingX509TrustManager.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/ReloadingX509TrustManager.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/ReloadingX509TrustManager.java index 1b24940..bb90a61 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/ReloadingX509TrustManager.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/ReloadingX509TrustManager.java @@ -23,6 +23,8 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; +import com.google.common.annotations.VisibleForTesting; + import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509TrustManager; @@ -44,8 +46,11 @@ import java.util.concurrent.atomic.AtomicReference; public final class ReloadingX509TrustManager implements X509TrustManager, Runnable { - private static final Log LOG = - LogFactory.getLog(ReloadingX509TrustManager.class); + @VisibleForTesting + static final Log LOG = LogFactory.getLog(ReloadingX509TrustManager.class); + @VisibleForTesting + static final String RELOAD_ERROR_MESSAGE = + "Could not load truststore (keep using existing one) : "; private String type; private File file; @@ -194,8 +199,7 @@ public final class ReloadingX509TrustManager try { trustManagerRef.set(loadTrustManager()); } catch (Exception ex) { - LOG.warn("Could not load truststore (keep using existing one) : " + - ex.toString(), ex); + LOG.warn(RELOAD_ERROR_MESSAGE + ex.toString(), ex); } } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/62558595/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/ssl/TestReloadingX509TrustManager.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/ssl/TestReloadingX509TrustManager.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/ssl/TestReloadingX509TrustManager.java index 9375da8..bf058cd 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/ssl/TestReloadingX509TrustManager.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/ssl/TestReloadingX509TrustManager.java @@ -19,6 +19,10 @@ package org.apache.hadoop.security.ssl; import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.test.GenericTestUtils; +import org.apache.hadoop.test.GenericTestUtils.LogCapturer; + +import com.google.common.base.Supplier; + import org.junit.BeforeClass; import org.junit.Test; @@ -30,11 +34,13 @@ import java.security.KeyPair; import java.security.cert.X509Certificate; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.TimeoutException; import static org.junit.Assert.assertEquals; import static org.apache.hadoop.security.ssl.KeyStoreTestUtil.createTrustStore; import static org.apache.hadoop.security.ssl.KeyStoreTestUtil.generateCertificate; import static org.apache.hadoop.security.ssl.KeyStoreTestUtil.generateKeyPair; +import static org.junit.Assert.assertFalse; public class TestReloadingX509TrustManager { @@ -43,6 +49,8 @@ public class TestReloadingX509TrustManager { private X509Certificate cert1; private X509Certificate cert2; + private final LogCapturer reloaderLog = LogCapturer.captureLogs( + ReloadingX509TrustManager.LOG); @BeforeClass public static void setUp() throws Exception { @@ -80,7 +88,7 @@ public class TestReloadingX509TrustManager { } } - @Test + @Test (timeout = 30000) public void testReload() throws Exception { KeyPair kp = generateKeyPair("RSA"); cert1 = generateCertificate("CN=Cert1", kp, 30, "SHA1withRSA"); @@ -88,7 +96,7 @@ public class TestReloadingX509TrustManager { String truststoreLocation = BASEDIR + "/testreload.jks"; createTrustStore(truststoreLocation, "password", "cert1", cert1); - ReloadingX509TrustManager tm = + final ReloadingX509TrustManager tm = new ReloadingX509TrustManager("jks", truststoreLocation, "password", 10); try { tm.init(); @@ -103,19 +111,18 @@ public class TestReloadingX509TrustManager { certs.put("cert2", cert2); createTrustStore(truststoreLocation, "password", certs); - // and wait to be sure reload has taken place - assertEquals(10, tm.getReloadInterval()); - - // Wait so that the file modification time is different - Thread.sleep((tm.getReloadInterval() + 200)); - - assertEquals(2, tm.getAcceptedIssuers().length); + GenericTestUtils.waitFor(new Supplier<Boolean>() { + @Override + public Boolean get() { + return tm.getAcceptedIssuers().length == 2; + } + }, (int) tm.getReloadInterval(), 10000); } finally { tm.destroy(); } } - @Test + @Test (timeout = 30000) public void testReloadMissingTrustStore() throws Exception { KeyPair kp = generateKeyPair("RSA"); cert1 = generateCertificate("CN=Cert1", kp, 30, "SHA1withRSA"); @@ -129,19 +136,22 @@ public class TestReloadingX509TrustManager { tm.init(); assertEquals(1, tm.getAcceptedIssuers().length); X509Certificate cert = tm.getAcceptedIssuers()[0]; + + assertFalse(reloaderLog.getOutput().contains( + ReloadingX509TrustManager.RELOAD_ERROR_MESSAGE)); new File(truststoreLocation).delete(); - // Wait so that the file modification time is different - Thread.sleep((tm.getReloadInterval() + 200)); + waitForFailedReloadAtLeastOnce((int) tm.getReloadInterval()); assertEquals(1, tm.getAcceptedIssuers().length); assertEquals(cert, tm.getAcceptedIssuers()[0]); } finally { + reloaderLog.stopCapturing(); tm.destroy(); } } - @Test + @Test (timeout = 30000) public void testReloadCorruptTrustStore() throws Exception { KeyPair kp = generateKeyPair("RSA"); cert1 = generateCertificate("CN=Cert1", kp, 30, "SHA1withRSA"); @@ -154,22 +164,39 @@ public class TestReloadingX509TrustManager { try { tm.init(); assertEquals(1, tm.getAcceptedIssuers().length); - X509Certificate cert = tm.getAcceptedIssuers()[0]; + final X509Certificate cert = tm.getAcceptedIssuers()[0]; + + // Wait so that the file modification time is different + Thread.sleep((tm.getReloadInterval() + 1000)); + assertFalse(reloaderLog.getOutput().contains( + ReloadingX509TrustManager.RELOAD_ERROR_MESSAGE)); OutputStream os = new FileOutputStream(truststoreLocation); os.write(1); os.close(); - new File(truststoreLocation).setLastModified(System.currentTimeMillis() - - 1000); - // Wait so that the file modification time is different - Thread.sleep((tm.getReloadInterval() + 200)); + waitForFailedReloadAtLeastOnce((int) tm.getReloadInterval()); assertEquals(1, tm.getAcceptedIssuers().length); assertEquals(cert, tm.getAcceptedIssuers()[0]); } finally { + reloaderLog.stopCapturing(); tm.destroy(); } } + /**Wait for the reloader thread to load the configurations at least once + * by probing the log of the thread if the reload fails. + */ + private void waitForFailedReloadAtLeastOnce(int reloadInterval) + throws InterruptedException, TimeoutException { + GenericTestUtils.waitFor(new Supplier<Boolean>() { + @Override + public Boolean get() { + return reloaderLog.getOutput().contains( + ReloadingX509TrustManager.RELOAD_ERROR_MESSAGE); + } + }, reloadInterval, 10 * 1000); + } + } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
