From edc333c01c8d8b00ac9b07cd59324928a5b6506f Mon Sep 17 00:00:00 2001 From: Ryan Phillips Date: Wed, 10 Feb 2021 10:15:25 -0600 Subject: [PATCH] UPSTREAM: 98939: fixes race in TestSyncPodsDeletesWhenSourcesAreReadyPerQOS --- pkg/kubelet/kubelet_test.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index f10859a22ef3a..bc9c66ae38536 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -405,8 +405,6 @@ func TestSyncPodsStartPod(t *testing.T) { } func TestSyncPodsDeletesWhenSourcesAreReadyPerQOS(t *testing.T) { - ready := false // sources will not be ready initially, enabled later - testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) go testKubelet.kubelet.podKiller.PerformPodKillingWork() defer testKubelet.Cleanup() @@ -429,7 +427,7 @@ func TestSyncPodsDeletesWhenSourcesAreReadyPerQOS(t *testing.T) { } kubelet := testKubelet.kubelet kubelet.cgroupsPerQOS = true // enable cgroupsPerQOS to turn on the cgroups cleanup - kubelet.sourcesReady = config.NewSourcesReady(func(_ sets.String) bool { return ready }) + kubelet.sourcesReady = config.NewSourcesReady(func(_ sets.String) bool { return true }) // HandlePodCleanups gets called every 2 seconds within the Kubelet's // housekeeping routine. This test registers the pod, removes the unwanted pod, then calls into @@ -437,11 +435,6 @@ func TestSyncPodsDeletesWhenSourcesAreReadyPerQOS(t *testing.T) { // within a goroutine so a two second delay should be enough time to // mark the pod as killed (within this test case). - kubelet.HandlePodCleanups() - time.Sleep(2 * time.Second) - fakeRuntime.AssertKilledPods([]string{}) // Sources are not ready yet. Don't remove any pods. - - ready = true // mark sources as ready kubelet.HandlePodCleanups() time.Sleep(2 * time.Second) @@ -453,18 +446,25 @@ func TestSyncPodsDeletesWhenSourcesAreReadyPerQOS(t *testing.T) { kubelet.HandlePodCleanups() time.Sleep(2 * time.Second) - fakeContainerManager.PodContainerManager.Lock() - defer fakeContainerManager.PodContainerManager.Unlock() - calledFunctionCount := len(fakeContainerManager.PodContainerManager.CalledFunctions) destroyCount := 0 - for _, functionName := range fakeContainerManager.PodContainerManager.CalledFunctions { - if functionName == "Destroy" { - destroyCount = destroyCount + 1 + err := wait.Poll(100*time.Millisecond, 10*time.Second, func() (bool, error) { + fakeContainerManager.PodContainerManager.Lock() + defer fakeContainerManager.PodContainerManager.Unlock() + destroyCount = 0 + for _, functionName := range fakeContainerManager.PodContainerManager.CalledFunctions { + if functionName == "Destroy" { + destroyCount = destroyCount + 1 + } } - } + return destroyCount >= 1, nil + }) - assert.Equal(t, 1, destroyCount, "Expect only 1 destroy") - assert.True(t, calledFunctionCount > 2, "expect more than two PodContainerManager calls") + assert.NoError(t, err, "wait should not return error") + // housekeeping can get called multiple times. The cgroup Destroy() is + // done within a goroutine and can get called multiple times, so the + // Destroy() count in not deterministic on the actual number. + // https://github.com/kubernetes/kubernetes/blob/29fdbb065b5e0d195299eb2d260b975cbc554673/pkg/kubelet/kubelet_pods.go#L2006 + assert.True(t, destroyCount >= 1, "Expect 1 or more destroys") } func TestSyncPodsDeletesWhenSourcesAreReady(t *testing.T) {