From fc4a29da2ca5a47cb5c2a1aa5eddc3ad55d0c70b Mon Sep 17 00:00:00 2001 From: Geonju Kim Date: Thu, 11 Feb 2021 16:37:07 +0900 Subject: [PATCH 1/3] kubelet: Make the test fail if (*FakeRuntime).Assert fails --- pkg/kubelet/container/testing/fake_runtime.go | 20 +++++++++-------- pkg/kubelet/images/image_manager_test.go | 6 ++--- pkg/kubelet/kubelet_test.go | 22 ++++++++++--------- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/pkg/kubelet/container/testing/fake_runtime.go b/pkg/kubelet/container/testing/fake_runtime.go index 6598c727920..4e50b2c53c7 100644 --- a/pkg/kubelet/container/testing/fake_runtime.go +++ b/pkg/kubelet/container/testing/fake_runtime.go @@ -18,11 +18,11 @@ package testing import ( "context" - "fmt" "io" "net/url" "reflect" "sync" + "testing" "time" v1 "k8s.io/api/core/v1" @@ -58,6 +58,7 @@ type FakeRuntime struct { Err error InspectErr error StatusErr error + T *testing.T } const FakeHost = "localhost:12345" @@ -135,39 +136,40 @@ func (f *FakeRuntime) UpdatePodCIDR(c string) error { return nil } -func (f *FakeRuntime) assertList(expect []string, test []string) error { +func (f *FakeRuntime) assertList(expect []string, test []string) bool { if !reflect.DeepEqual(expect, test) { - return fmt.Errorf("expected %#v, got %#v", expect, test) + f.T.Errorf("AssertList: expected %#v, got %#v", expect, test) + return false } - return nil + return true } // AssertCalls test if the invoked functions are as expected. -func (f *FakeRuntime) AssertCalls(calls []string) error { +func (f *FakeRuntime) AssertCalls(calls []string) bool { f.Lock() defer f.Unlock() return f.assertList(calls, f.CalledFunctions) } -func (f *FakeRuntime) AssertStartedPods(pods []string) error { +func (f *FakeRuntime) AssertStartedPods(pods []string) bool { f.Lock() defer f.Unlock() return f.assertList(pods, f.StartedPods) } -func (f *FakeRuntime) AssertKilledPods(pods []string) error { +func (f *FakeRuntime) AssertKilledPods(pods []string) bool { f.Lock() defer f.Unlock() return f.assertList(pods, f.KilledPods) } -func (f *FakeRuntime) AssertStartedContainers(containers []string) error { +func (f *FakeRuntime) AssertStartedContainers(containers []string) bool { f.Lock() defer f.Unlock() return f.assertList(containers, f.StartedContainers) } -func (f *FakeRuntime) AssertKilledContainers(containers []string) error { +func (f *FakeRuntime) AssertKilledContainers(containers []string) bool { f.Lock() defer f.Unlock() return f.assertList(containers, f.KilledContainers) diff --git a/pkg/kubelet/images/image_manager_test.go b/pkg/kubelet/images/image_manager_test.go index 9ce3dcf3ef4..b5b926e0399 100644 --- a/pkg/kubelet/images/image_manager_test.go +++ b/pkg/kubelet/images/image_manager_test.go @@ -201,7 +201,7 @@ func TestParallelPuller(t *testing.T) { fakeRuntime.CalledFunctions = nil fakeClock.Step(time.Second) _, _, err := puller.EnsureImageExists(pod, container, nil, nil) - assert.NoError(t, fakeRuntime.AssertCalls(expected.calls)) + fakeRuntime.AssertCalls(expected.calls) assert.Equal(t, expected.err, err) } }) @@ -229,7 +229,7 @@ func TestSerializedPuller(t *testing.T) { fakeRuntime.CalledFunctions = nil fakeClock.Step(time.Second) _, _, err := puller.EnsureImageExists(pod, container, nil, nil) - assert.NoError(t, fakeRuntime.AssertCalls(expected.calls)) + fakeRuntime.AssertCalls(expected.calls) assert.Equal(t, expected.err, err) } }) @@ -287,7 +287,7 @@ func TestPullAndListImageWithPodAnnotations(t *testing.T) { t.Run(c.testName, func(t *testing.T) { _, _, err := puller.EnsureImageExists(pod, container, nil, nil) - assert.NoError(t, fakeRuntime.AssertCalls(c.expected[0].calls), "tick=%d", 0) + fakeRuntime.AssertCalls(c.expected[0].calls) assert.Equal(t, c.expected[0].err, err, "tick=%d", 0) images, _ := fakeRuntime.ListImages() diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index aaf74b400d0..7bec7e23564 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -144,16 +144,18 @@ func newTestKubeletWithImageList( imageList []kubecontainer.Image, controllerAttachDetachEnabled bool, initFakeVolumePlugin bool) *TestKubelet { - fakeRuntime := &containertest.FakeRuntime{} - fakeRuntime.RuntimeType = "test" - fakeRuntime.VersionInfo = "1.5.0" - fakeRuntime.ImageList = imageList - // Set ready conditions by default. - fakeRuntime.RuntimeStatus = &kubecontainer.RuntimeStatus{ - Conditions: []kubecontainer.RuntimeCondition{ - {Type: "RuntimeReady", Status: true}, - {Type: "NetworkReady", Status: true}, + fakeRuntime := &containertest.FakeRuntime{ + ImageList: imageList, + // Set ready conditions by default. + RuntimeStatus: &kubecontainer.RuntimeStatus{ + Conditions: []kubecontainer.RuntimeCondition{ + {Type: "RuntimeReady", Status: true}, + {Type: "NetworkReady", Status: true}, + }, }, + VersionInfo: "1.5.0", + RuntimeType: "test", + T: t, } fakeRecorder := &record.FakeRecorder{} @@ -658,7 +660,7 @@ func TestKillPodFollwedByIsPodPendingTermination(t *testing.T) { RunningPod: pod, }) - if !(kl.podKiller.IsPodPendingTerminationByUID(pod.ID) || fakeRuntime.AssertKilledPods([]string{"12345678"}) == nil) { + if !(kl.podKiller.IsPodPendingTerminationByUID(pod.ID) || fakeRuntime.AssertKilledPods([]string{"12345678"})) { t.Fatal("Race condition: When KillPod is complete, the pod should be pending termination or be killed") } } From 256447a349209a26839a190020268940b97a9711 Mon Sep 17 00:00:00 2001 From: Geonju Kim Date: Thu, 11 Feb 2021 17:20:48 +0900 Subject: [PATCH 2/3] kubelet_test: Fix TestHandlePodCleanups --- pkg/kubelet/kubelet_test.go | 43 +++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 7bec7e23564..43424aa4f0d 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -409,7 +409,7 @@ func TestSyncPodsStartPod(t *testing.T) { fakeRuntime.AssertStartedPods([]string{string(pods[0].UID)}) } -func TestSyncPodsDeletesWhenSourcesAreReadyPerQOS(t *testing.T) { +func TestHandlePodCleanupsPerQOS(t *testing.T) { testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) go testKubelet.kubelet.podKiller.PerformPodKillingWork() defer testKubelet.Cleanup() @@ -432,7 +432,6 @@ 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 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 @@ -602,33 +601,31 @@ func TestDispatchWorkOfActivePod(t *testing.T) { } } -func TestSyncPodsDeletesWhenSourcesAreReady(t *testing.T) { - ready := false - +func TestHandlePodCleanups(t *testing.T) { testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) + go testKubelet.kubelet.podKiller.PerformPodKillingWork() defer testKubelet.Cleanup() - fakeRuntime := testKubelet.fakeRuntime - kubelet := testKubelet.kubelet - kubelet.sourcesReady = config.NewSourcesReady(func(_ sets.String) bool { return ready }) + defer testKubelet.kubelet.podKiller.Close() - fakeRuntime.PodList = []*containertest.FakePod{ - {Pod: &kubecontainer.Pod{ - ID: "12345678", - Name: "foo", - Namespace: "new", - Containers: []*kubecontainer.Container{ - {Name: "bar"}, - }, - }}, + pod := &kubecontainer.Pod{ + ID: "12345678", + Name: "foo", + Namespace: "new", + Containers: []*kubecontainer.Container{ + {Name: "bar"}, + }, } - kubelet.HandlePodCleanups() - // Sources are not ready yet. Don't remove any pods. - fakeRuntime.AssertKilledPods([]string{}) - ready = true - kubelet.HandlePodCleanups() + fakeRuntime := testKubelet.fakeRuntime + fakeRuntime.PodList = []*containertest.FakePod{ + {Pod: pod}, + } + kubelet := testKubelet.kubelet - // Sources are ready. Remove unwanted pods. + kubelet.HandlePodCleanups() + time.Sleep(2 * time.Second) + + // assert that unwanted pods were killed fakeRuntime.AssertKilledPods([]string{"12345678"}) } From b4b7cea413758b7c28bd124050fb7d35b58727d6 Mon Sep 17 00:00:00 2001 From: Geonju Kim Date: Thu, 11 Feb 2021 17:21:24 +0900 Subject: [PATCH 3/3] kubelet_test: Add TestHandlePodRemovesWhenSourcesAreReady --- pkg/kubelet/kubelet_test.go | 43 +++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 43424aa4f0d..b6272dc8046 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -353,6 +353,7 @@ func newTestKubeletWithImageList( kubelet.AddPodSyncLoopHandler(activeDeadlineHandler) kubelet.AddPodSyncHandler(activeDeadlineHandler) + kubelet.lastContainerStartedTime = newTimeCache() return &TestKubelet{kubelet, fakeRuntime, fakeContainerManager, fakeKubeClient, fakeMirrorClient, fakeClock, nil, plug} } @@ -629,6 +630,48 @@ func TestHandlePodCleanups(t *testing.T) { fakeRuntime.AssertKilledPods([]string{"12345678"}) } +func TestHandlePodRemovesWhenSourcesAreReady(t *testing.T) { + ready := false + + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) + defer testKubelet.Cleanup() + go testKubelet.kubelet.podKiller.PerformPodKillingWork() + defer testKubelet.kubelet.podKiller.Close() + + fakePod := &kubecontainer.Pod{ + ID: "1", + Name: "foo", + Namespace: "new", + Containers: []*kubecontainer.Container{ + {Name: "bar"}, + }, + } + + pods := []*v1.Pod{ + podWithUIDNameNs("1", "foo", "new"), + } + + fakeRuntime := testKubelet.fakeRuntime + fakeRuntime.PodList = []*containertest.FakePod{ + {Pod: fakePod}, + } + kubelet := testKubelet.kubelet + kubelet.sourcesReady = config.NewSourcesReady(func(_ sets.String) bool { return ready }) + + kubelet.HandlePodRemoves(pods) + time.Sleep(2 * time.Second) + + // Sources are not ready yet. Don't remove any pods. + fakeRuntime.AssertKilledPods(nil) + + ready = true + kubelet.HandlePodRemoves(pods) + time.Sleep(2 * time.Second) + + // Sources are ready. Remove unwanted pods. + fakeRuntime.AssertKilledPods([]string{"1"}) +} + func TestKillPodFollwedByIsPodPendingTermination(t *testing.T) { testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup()