Merge pull request #98986 from gjkim42/fix-runtime-assert

kubelet: Make the test fail if (*FakeRuntime).Assert fails

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)

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()

pkg/kubelet/kubelet_test.go

@@ -144,16 +144,18 @@ func newTestKubeletWithImageList(
 	imageList []kubecontainer.Image,
 	controllerAttachDetachEnabled bool,
 	initFakeVolumePlugin bool) *TestKubelet {
-	fakeRuntime := &containertest.FakeRuntime{}
+	fakeRuntime := &containertest.FakeRuntime{
-	fakeRuntime.RuntimeType = "test"
+		ImageList: imageList,
-	fakeRuntime.VersionInfo = "1.5.0"
+		// Set ready conditions by default.
-	fakeRuntime.ImageList = imageList
+		RuntimeStatus: &kubecontainer.RuntimeStatus{
-	// Set ready conditions by default.
+			Conditions: []kubecontainer.RuntimeCondition{
-	fakeRuntime.RuntimeStatus = &kubecontainer.RuntimeStatus{
+				{Type: "RuntimeReady", Status: true},
-		Conditions: []kubecontainer.RuntimeCondition{
+				{Type: "NetworkReady", Status: true},
-			{Type: "RuntimeReady", Status: true},
+			},
-			{Type: "NetworkReady", Status: true},
 		},
+		VersionInfo: "1.5.0",
+		RuntimeType: "test",
+		T:           t,
 	}
 
 	fakeRecorder := &record.FakeRecorder{}
@@ -351,6 +353,7 @@ func newTestKubeletWithImageList(
 
 	kubelet.AddPodSyncLoopHandler(activeDeadlineHandler)
 	kubelet.AddPodSyncHandler(activeDeadlineHandler)
+	kubelet.lastContainerStartedTime = newTimeCache()
 	return &TestKubelet{kubelet, fakeRuntime, fakeContainerManager, fakeKubeClient, fakeMirrorClient, fakeClock, nil, plug}
 }
 
@@ -407,7 +410,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()
@@ -430,7 +433,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
@@ -600,34 +602,74 @@ func TestDispatchWorkOfActivePod(t *testing.T) {
 	}
 }
 
-func TestSyncPodsDeletesWhenSourcesAreReady(t *testing.T) {
+func TestHandlePodCleanups(t *testing.T) {
+	testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */)
+	go testKubelet.kubelet.podKiller.PerformPodKillingWork()
+	defer testKubelet.Cleanup()
+	defer testKubelet.kubelet.podKiller.Close()
+
+	pod := &kubecontainer.Pod{
+		ID:        "12345678",
+		Name:      "foo",
+		Namespace: "new",
+		Containers: []*kubecontainer.Container{
+			{Name: "bar"},
+		},
+	}
+
+	fakeRuntime := testKubelet.fakeRuntime
+	fakeRuntime.PodList = []*containertest.FakePod{
+		{Pod: pod},
+	}
+	kubelet := testKubelet.kubelet
+
+	kubelet.HandlePodCleanups()
+	time.Sleep(2 * time.Second)
+
+	// assert that unwanted pods were killed
+	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 })
 
-	fakeRuntime.PodList = []*containertest.FakePod{
+	kubelet.HandlePodRemoves(pods)
-		{Pod: &kubecontainer.Pod{
+	time.Sleep(2 * time.Second)
-			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{})
+	fakeRuntime.AssertKilledPods(nil)
 
 	ready = true
-	kubelet.HandlePodCleanups()
+	kubelet.HandlePodRemoves(pods)
+	time.Sleep(2 * time.Second)
 
 	// Sources are ready. Remove unwanted pods.
-	fakeRuntime.AssertKilledPods([]string{"12345678"})
+	fakeRuntime.AssertKilledPods([]string{"1"})
 }
 
 func TestKillPodFollwedByIsPodPendingTermination(t *testing.T) {
@@ -658,7 +700,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")
 	}
 }