From c05b99e65655ec3fd037661797257199480df105 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Mon, 29 Nov 2021 14:28:47 -0800 Subject: [PATCH 1/3] De-flake a test. --- .../builder/DefaultControllerBuilderTest.java | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/extended/src/test/java/io/kubernetes/client/extended/controller/builder/DefaultControllerBuilderTest.java b/extended/src/test/java/io/kubernetes/client/extended/controller/builder/DefaultControllerBuilderTest.java index f4b4290a41..1a336f46fd 100644 --- a/extended/src/test/java/io/kubernetes/client/extended/controller/builder/DefaultControllerBuilderTest.java +++ b/extended/src/test/java/io/kubernetes/client/extended/controller/builder/DefaultControllerBuilderTest.java @@ -37,6 +37,7 @@ import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Semaphore; import java.util.function.Function; import org.junit.After; import org.junit.Before; @@ -54,16 +55,6 @@ public class DefaultControllerBuilderTest { @Rule public WireMockRule wireMockRule = new WireMockRule(PORT); - private final int stepCooldownIntervalInMillis = 500; - - private void cooldown() { - try { - Thread.sleep(stepCooldownIntervalInMillis); - } catch (InterruptedException e) { - e.printStackTrace(); - } - } - @Before public void setUp() throws Exception { client = new ClientBuilder().setBasePath("http://localhost:" + PORT).build(); @@ -134,7 +125,7 @@ public Result reconcile(Request request) { } @Test - public void testBuildWatchEventNotificationShouldWork() { + public void testBuildWatchEventNotificationShouldWork() throws InterruptedException { V1PodList podList = new V1PodList() .metadata(new V1ListMeta().resourceVersion("0")) @@ -183,13 +174,17 @@ public void testBuildWatchEventNotificationShouldWork() { }; List controllerReceivingRequests = new ArrayList<>(); - Controller testController = + final Semaphore latch = new Semaphore(1); + latch.acquire(); + + final Controller testController = ControllerBuilder.defaultBuilder(informerFactory) .withReconciler( new Reconciler() { @Override public Result reconcile(Request request) { controllerReceivingRequests.add(request); + latch.release(); return new Result(false); } }) @@ -203,10 +198,10 @@ public Result reconcile(Request request) { controllerThead.submit(testController::run); informerFactory.startAllRegisteredInformers(); - Request expectedRequest = new Request("hostname1/test-pod1"); - - cooldown(); + // Wait for the request to be processed. + latch.acquire(1); + Request expectedRequest = new Request("hostname1/test-pod1"); assertEquals(1, keyFuncReceivingRequests.size()); assertEquals(expectedRequest, keyFuncReceivingRequests.get(0)); From 3b914d1001c454a6176ae03f9dfae3966b742c84 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Mon, 29 Nov 2021 17:03:35 -0800 Subject: [PATCH 2/3] Deflake another test. --- .../KubernetesInformerCreatorTest.java | 47 +++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerCreatorTest.java b/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerCreatorTest.java index 26ed99b677..6b0f40644a 100644 --- a/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerCreatorTest.java +++ b/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerCreatorTest.java @@ -19,10 +19,15 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import static com.github.tomakehurst.wiremock.client.WireMock.verify; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import com.github.tomakehurst.wiremock.core.Admin; +import com.github.tomakehurst.wiremock.extension.Parameters; +import com.github.tomakehurst.wiremock.extension.PostServeAction; import com.github.tomakehurst.wiremock.junit.WireMockRule; +import com.github.tomakehurst.wiremock.stubbing.ServeEvent; import com.google.gson.Gson; import io.kubernetes.client.informer.SharedIndexInformer; import io.kubernetes.client.informer.SharedInformerFactory; @@ -37,7 +42,8 @@ import io.kubernetes.client.util.ClientBuilder; import io.kubernetes.client.util.generic.GenericKubernetesApi; import java.util.Arrays; -import org.junit.Rule; +import java.util.concurrent.Semaphore; +import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -50,14 +56,30 @@ @SpringBootTest public class KubernetesInformerCreatorTest { - @Rule public WireMockRule wireMockRule = new WireMockRule(8188); + public static class CountRequestAction extends PostServeAction { + @Override + public String getName() { + return "semaphore"; + } + + @Override + public void doAction(ServeEvent serveEvent, Admin admin, Parameters parameters) { + Semaphore count = (Semaphore) parameters.get("semaphore"); + count.release(); + } + } + + @ClassRule + public static WireMockRule wireMockRule = + new WireMockRule(options().dynamicPort().extensions(new CountRequestAction())); @SpringBootApplication static class App { @Bean public ApiClient testingApiClient() { - ApiClient apiClient = new ClientBuilder().setBasePath("http://localhost:" + 8188).build(); + ApiClient apiClient = + new ClientBuilder().setBasePath("http://localhost:" + wireMockRule.port()).build(); return apiClient; } @@ -91,6 +113,13 @@ public void testInformerInjection() throws InterruptedException { assertNotNull(podInformer); assertNotNull(configMapInformer); + Semaphore getCount = new Semaphore(2); + Semaphore watchCount = new Semaphore(2); + Parameters getParams = new Parameters(); + Parameters watchParams = new Parameters(); + getParams.put("semaphore", getCount); + watchParams.put("semaphore", watchCount); + V1Pod foo1 = new V1Pod().kind("Pod").metadata(new V1ObjectMeta().namespace("default").name("foo1")); V1ConfigMap bar1 = @@ -100,6 +129,7 @@ public void testInformerInjection() throws InterruptedException { wireMockRule.stubFor( get(urlMatching("^/api/v1/pods.*")) + .withPostServeAction("semaphore", getParams) .withQueryParam("watch", equalTo("false")) .willReturn( aResponse() @@ -112,11 +142,13 @@ public void testInformerInjection() throws InterruptedException { .items(Arrays.asList(foo1)))))); wireMockRule.stubFor( get(urlMatching("^/api/v1/pods.*")) + .withPostServeAction("semaphore", watchParams) .withQueryParam("watch", equalTo("true")) .willReturn(aResponse().withStatus(200).withBody("{}"))); wireMockRule.stubFor( get(urlMatching("^/api/v1/namespaces/default/configmaps.*")) + .withPostServeAction("semaphore", getParams) .withQueryParam("watch", equalTo("false")) .willReturn( aResponse() @@ -129,12 +161,19 @@ public void testInformerInjection() throws InterruptedException { .items(Arrays.asList(bar1)))))); wireMockRule.stubFor( get(urlMatching("^/api/v1/namespaces/default/configmaps.*")) + .withPostServeAction("semaphore", watchParams) .withQueryParam("watch", equalTo("true")) .willReturn(aResponse().withStatus(200).withBody("{}"))); + // These will be released for each web call above. + getCount.acquire(2); + watchCount.acquire(2); + informerFactory.startAllRegisteredInformers(); - Thread.sleep(200); + // Wait for the GETs to complete and the watches to start. + getCount.acquire(2); + watchCount.acquire(2); verify( 1, From c9a795340f385779374cfe76c58f4cfe63686975 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Tue, 30 Nov 2021 10:24:27 -0800 Subject: [PATCH 3/3] Deflake another set of tests. --- .../workqueue/DefaultDelayingQueueTest.java | 43 ++++++++++++++++--- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/extended/src/test/java/io/kubernetes/client/extended/workqueue/DefaultDelayingQueueTest.java b/extended/src/test/java/io/kubernetes/client/extended/workqueue/DefaultDelayingQueueTest.java index e7df100d66..cbf82a52f4 100644 --- a/extended/src/test/java/io/kubernetes/client/extended/workqueue/DefaultDelayingQueueTest.java +++ b/extended/src/test/java/io/kubernetes/client/extended/workqueue/DefaultDelayingQueueTest.java @@ -52,23 +52,37 @@ public void testSimpleDelayingQueue() throws Exception { @Test public void testDeduping() throws Exception { + final Instant staticTime = Instant.now(); DefaultDelayingQueue queue = new DefaultDelayingQueue<>(); String item = "foo"; + // Hold time still + queue.injectTimeSource( + () -> { + return staticTime; + }); + queue.addAfter(item, Duration.ofMillis(50)); assertTrue(waitForWaitingQueueToFill(queue)); queue.addAfter(item, Duration.ofMillis(70)); assertTrue(waitForWaitingQueueToFill(queue)); assertTrue("should not have added", queue.length() == 0); - // step past the first block, we should receive now - Thread.sleep(60L); + // Advance time + queue.injectTimeSource( + () -> { + return staticTime.plusMillis(60); + }); assertTrue(waitForAdded(queue, 1)); item = queue.get(); queue.done(item); // step past the second add - Thread.sleep(20L); + // Advance time + queue.injectTimeSource( + () -> { + return staticTime.plusMillis(90); + }); assertTrue("should not have added", queue.length() == 0); // test again, but this time the earlier should override @@ -77,19 +91,33 @@ public void testDeduping() throws Exception { assertTrue(waitForWaitingQueueToFill(queue)); assertTrue("should not have added", queue.length() == 0); - Thread.sleep(40L); + // Advance time + queue.injectTimeSource( + () -> { + return staticTime.plusMillis(150); + }); assertTrue(waitForAdded(queue, 1)); item = queue.get(); queue.done(item); // step past the second add - Thread.sleep(1L); + // Advance time + queue.injectTimeSource( + () -> { + return staticTime.plusMillis(190); + }); assertTrue("should not have added", queue.length() == 0); } @Test public void testCopyShifting() throws Exception { + final Instant staticTime = Instant.now(); DefaultDelayingQueue queue = new DefaultDelayingQueue<>(); + queue.injectTimeSource( + () -> { + return staticTime; + }); + final String first = "foo"; final String second = "bar"; final String third = "baz"; @@ -100,7 +128,10 @@ public void testCopyShifting() throws Exception { assertTrue(waitForWaitingQueueToFill(queue)); assertTrue("should not have added", queue.length() == 0); - Thread.sleep(2000L); + queue.injectTimeSource( + () -> { + return staticTime.plusMillis(2000); + }); assertTrue(waitForAdded(queue, 3)); String actualFirst = queue.get(); assertEquals(actualFirst, third);