Merge pull request #19690 from Random-Liu/fix-bug-in-status-test

Fix bug in status/manager_test.go

pkg/kubelet/status/manager_test.go

@@ -36,12 +36,15 @@ import (
 	"k8s.io/kubernetes/pkg/runtime"
 )
 
-var testPod *api.Pod = &api.Pod{
+// Generate new instance of test pod with the same initial value.
+func getTestPod() *api.Pod {
+	return &api.Pod{
 		ObjectMeta: api.ObjectMeta{
 			UID:       "12345678",
 			Name:      "foo",
 			Namespace: "new",
 		},
+	}
 }
 
 // After adding reconciliation, if status in pod manager is different from the cached status, a reconciliation
@@ -64,7 +67,7 @@ func (m *manager) testSyncBatch() {
 
 func newTestManager(kubeClient client.Interface) *manager {
 	podManager := kubepod.NewBasicPodManager(kubepod.NewFakeMirrorClient())
-	podManager.AddPod(testPod)
+	podManager.AddPod(getTestPod())
 	return NewManager(kubeClient, podManager).(*manager)
 }
 
@@ -117,6 +120,7 @@ func verifyUpdates(t *testing.T, manager *manager, expectedUpdates int) {
 
 func TestNewStatus(t *testing.T) {
 	syncer := newTestManager(&testclient.Fake{})
+	testPod := getTestPod()
 	syncer.SetPodStatus(testPod, getRandomPodStatus())
 	verifyUpdates(t, syncer, 1)
 
@@ -180,6 +184,7 @@ func TestNewStatusSetsReadyTransitionTime(t *testing.T) {
 
 func TestChangedStatus(t *testing.T) {
 	syncer := newTestManager(&testclient.Fake{})
+	testPod := getTestPod()
 	syncer.SetPodStatus(testPod, getRandomPodStatus())
 	syncer.SetPodStatus(testPod, getRandomPodStatus())
 	verifyUpdates(t, syncer, 2)
@@ -187,6 +192,7 @@ func TestChangedStatus(t *testing.T) {
 
 func TestChangedStatusKeepsStartTime(t *testing.T) {
 	syncer := newTestManager(&testclient.Fake{})
+	testPod := getTestPod()
 	now := unversioned.Now()
 	firstStatus := getRandomPodStatus()
 	firstStatus.StartTime = &now
@@ -234,6 +240,7 @@ func TestChangedStatusUpdatesLastTransitionTime(t *testing.T) {
 
 func TestUnchangedStatus(t *testing.T) {
 	syncer := newTestManager(&testclient.Fake{})
+	testPod := getTestPod()
 	podStatus := getRandomPodStatus()
 	syncer.SetPodStatus(testPod, podStatus)
 	syncer.SetPodStatus(testPod, podStatus)
@@ -276,7 +283,7 @@ func TestSyncBatchIgnoresNotFound(t *testing.T) {
 	client.AddReactor("get", "pods", func(action testclient.Action) (bool, runtime.Object, error) {
 		return true, nil, errors.NewNotFound(api.Resource("pods"), "test-pod")
 	})
-	syncer.SetPodStatus(testPod, getRandomPodStatus())
+	syncer.SetPodStatus(getTestPod(), getRandomPodStatus())
 	syncer.testSyncBatch()
 
 	verifyActions(t, syncer.kubeClient, []testclient.Action{
@@ -286,6 +293,7 @@ func TestSyncBatchIgnoresNotFound(t *testing.T) {
 
 func TestSyncBatch(t *testing.T) {
 	syncer := newTestManager(&testclient.Fake{})
+	testPod := getTestPod()
 	syncer.kubeClient = testclient.NewSimpleFake(testPod)
 	syncer.SetPodStatus(testPod, getRandomPodStatus())
 	syncer.testSyncBatch()
@@ -298,14 +306,14 @@ func TestSyncBatch(t *testing.T) {
 
 func TestSyncBatchChecksMismatchedUID(t *testing.T) {
 	syncer := newTestManager(&testclient.Fake{})
-	pod := *testPod
+	pod := getTestPod()
 	pod.UID = "first"
-	syncer.podManager.AddPod(&pod)
+	syncer.podManager.AddPod(pod)
-	differentPod := *testPod
+	differentPod := getTestPod()
 	differentPod.UID = "second"
-	syncer.podManager.AddPod(&differentPod)
+	syncer.podManager.AddPod(differentPod)
-	syncer.kubeClient = testclient.NewSimpleFake(&pod)
+	syncer.kubeClient = testclient.NewSimpleFake(pod)
-	syncer.SetPodStatus(&differentPod, getRandomPodStatus())
+	syncer.SetPodStatus(differentPod, getRandomPodStatus())
 	syncer.testSyncBatch()
 	verifyActions(t, syncer.kubeClient, []testclient.Action{
 		testclient.GetActionImpl{ActionImpl: testclient.ActionImpl{Verb: "get", Resource: "pods"}},
@@ -315,6 +323,7 @@ func TestSyncBatchChecksMismatchedUID(t *testing.T) {
 func TestSyncBatchNoDeadlock(t *testing.T) {
 	client := &testclient.Fake{}
 	m := newTestManager(client)
+	pod := getTestPod()
 
 	// Setup fake client.
 	var ret api.Pod
@@ -322,17 +331,15 @@ func TestSyncBatchNoDeadlock(t *testing.T) {
 	client.AddReactor("*", "pods", func(action testclient.Action) (bool, runtime.Object, error) {
 		switch action := action.(type) {
 		case testclient.GetAction:
-			assert.Equal(t, testPod.Name, action.GetName(), "Unexpeted GetAction: %+v", action)
+			assert.Equal(t, pod.Name, action.GetName(), "Unexpeted GetAction: %+v", action)
 		case testclient.UpdateAction:
-			assert.Equal(t, testPod.Name, action.GetObject().(*api.Pod).Name, "Unexpeted UpdateAction: %+v", action)
+			assert.Equal(t, pod.Name, action.GetObject().(*api.Pod).Name, "Unexpeted UpdateAction: %+v", action)
 		default:
 			assert.Fail(t, "Unexpected Action: %+v", action)
 		}
 		return true, &ret, err
 	})
 
-	pod := new(api.Pod)
-	*pod = *testPod
 	pod.Status.ContainerStatuses = []api.ContainerStatus{{State: api.ContainerState{Running: &api.ContainerStateRunning{}}}}
 
 	getAction := testclient.GetActionImpl{ActionImpl: testclient.ActionImpl{Verb: "get", Resource: "pods"}}
@@ -385,16 +392,16 @@ func TestSyncBatchNoDeadlock(t *testing.T) {
 }
 
 func TestStaleUpdates(t *testing.T) {
-	pod := *testPod
+	pod := getTestPod()
-	client := testclient.NewSimpleFake(&pod)
+	client := testclient.NewSimpleFake(pod)
 	m := newTestManager(client)
 
 	status := api.PodStatus{Message: "initial status"}
-	m.SetPodStatus(&pod, status)
+	m.SetPodStatus(pod, status)
 	status.Message = "first version bump"
-	m.SetPodStatus(&pod, status)
+	m.SetPodStatus(pod, status)
 	status.Message = "second version bump"
-	m.SetPodStatus(&pod, status)
+	m.SetPodStatus(pod, status)
 	verifyUpdates(t, m, 3)
 
 	t.Logf("First sync pushes latest status.")
@@ -412,13 +419,13 @@ func TestStaleUpdates(t *testing.T) {
 	}
 
 	t.Log("Unchanged status should not send an update.")
-	m.SetPodStatus(&pod, status)
+	m.SetPodStatus(pod, status)
 	verifyUpdates(t, m, 0)
 
 	t.Log("... unless it's stale.")
 	m.apiStatusVersions[pod.UID] = m.apiStatusVersions[pod.UID] - 1
 
-	m.SetPodStatus(&pod, status)
+	m.SetPodStatus(pod, status)
 	m.testSyncBatch()
 	verifyActions(t, m.kubeClient, []testclient.Action{
 		testclient.GetActionImpl{ActionImpl: testclient.ActionImpl{Verb: "get", Resource: "pods"}},
@@ -462,29 +469,29 @@ func TestStatusEquality(t *testing.T) {
 }
 
 func TestStaticPodStatus(t *testing.T) {
-	staticPod := *testPod
+	staticPod := getTestPod()
 	staticPod.Annotations = map[string]string{kubetypes.ConfigSourceAnnotationKey: "file"}
-	mirrorPod := *testPod
+	mirrorPod := getTestPod()
 	mirrorPod.UID = "mirror-12345678"
 	mirrorPod.Annotations = map[string]string{
 		kubetypes.ConfigSourceAnnotationKey: "api",
 		kubetypes.ConfigMirrorAnnotationKey: "mirror",
 	}
-	client := testclient.NewSimpleFake(&mirrorPod)
+	client := testclient.NewSimpleFake(mirrorPod)
 	m := newTestManager(client)
-	m.podManager.AddPod(&staticPod)
+	m.podManager.AddPod(staticPod)
-	m.podManager.AddPod(&mirrorPod)
+	m.podManager.AddPod(mirrorPod)
 	// Verify setup.
-	assert.True(t, kubepod.IsStaticPod(&staticPod), "SetUp error: staticPod")
+	assert.True(t, kubepod.IsStaticPod(staticPod), "SetUp error: staticPod")
-	assert.True(t, kubepod.IsMirrorPod(&mirrorPod), "SetUp error: mirrorPod")
+	assert.True(t, kubepod.IsMirrorPod(mirrorPod), "SetUp error: mirrorPod")
 	assert.Equal(t, m.podManager.TranslatePodUID(mirrorPod.UID), staticPod.UID)
 
 	status := getRandomPodStatus()
 	now := unversioned.Now()
 	status.StartTime = &now
 
-	m.SetPodStatus(&staticPod, status)
+	m.SetPodStatus(staticPod, status)
-	retrievedStatus := expectPodStatus(t, m, &staticPod)
+	retrievedStatus := expectPodStatus(t, m, staticPod)
 	assert.True(t, isStatusEqual(&status, &retrievedStatus), "Expected: %+v, Got: %+v", status, retrievedStatus)
 	retrievedStatus, _ = m.GetPodStatus(mirrorPod.UID)
 	assert.True(t, isStatusEqual(&status, &retrievedStatus), "Expected: %+v, Got: %+v", status, retrievedStatus)
@@ -505,10 +512,10 @@ func TestStaticPodStatus(t *testing.T) {
 	verifyActions(t, m.kubeClient, []testclient.Action{})
 
 	// Mirror pod identity changes.
-	m.podManager.DeletePod(&mirrorPod)
+	m.podManager.DeletePod(mirrorPod)
 	mirrorPod.UID = "new-mirror-pod"
 	mirrorPod.Status = api.PodStatus{}
-	m.podManager.AddPod(&mirrorPod)
+	m.podManager.AddPod(mirrorPod)
 	// Expect update to new mirrorPod.
 	m.testSyncBatch()
 	verifyActions(t, m.kubeClient, []testclient.Action{
@@ -542,8 +549,7 @@ func TestSetContainerReadiness(t *testing.T) {
 			Status: api.ConditionFalse,
 		}},
 	}
-	pod := new(api.Pod)
+	pod := getTestPod()
-	*pod = *testPod
 	pod.Spec.Containers = []api.Container{{Name: "c1"}, {Name: "c2"}}
 
 	// Verify expected readiness of containers & pod.
@@ -611,7 +617,8 @@ func TestSetContainerReadiness(t *testing.T) {
 
 func TestSyncBatchCleanupVersions(t *testing.T) {
 	m := newTestManager(&testclient.Fake{})
-	mirrorPod := *testPod
+	testPod := getTestPod()
+	mirrorPod := getTestPod()
 	mirrorPod.UID = "mirror-uid"
 	mirrorPod.Name = "mirror_pod"
 	mirrorPod.Annotations = map[string]string{
@@ -632,11 +639,11 @@ func TestSyncBatchCleanupVersions(t *testing.T) {
 
 	// Non-orphaned pods should not be removed.
 	m.SetPodStatus(testPod, getRandomPodStatus())
-	m.podManager.AddPod(&mirrorPod)
+	m.podManager.AddPod(mirrorPod)
 	staticPod := mirrorPod
 	staticPod.UID = "static-uid"
 	staticPod.Annotations = map[string]string{kubetypes.ConfigSourceAnnotationKey: "file"}
-	m.podManager.AddPod(&staticPod)
+	m.podManager.AddPod(staticPod)
 	m.apiStatusVersions[testPod.UID] = 100
 	m.apiStatusVersions[mirrorPod.UID] = 200
 	m.testSyncBatch()
@@ -649,13 +656,13 @@ func TestSyncBatchCleanupVersions(t *testing.T) {
 }
 
 func TestReconcilePodStatus(t *testing.T) {
+	testPod := getTestPod()
 	client := testclient.NewSimpleFake(testPod)
 	syncer := newTestManager(client)
 	syncer.SetPodStatus(testPod, getRandomPodStatus())
 	// Call syncBatch directly to test reconcile
 	syncer.syncBatch() // The apiStatusVersions should be set now
 
-	originalStatus := testPod.Status
 	podStatus, ok := syncer.GetPodStatus(testPod.UID)
 	if !ok {
 		t.Fatal("Should find pod status for pod: %+v", testPod)
@@ -698,9 +705,6 @@ func TestReconcilePodStatus(t *testing.T) {
 		testclient.GetActionImpl{ActionImpl: testclient.ActionImpl{Verb: "get", Resource: "pods"}},
 		testclient.UpdateActionImpl{ActionImpl: testclient.ActionImpl{Verb: "update", Resource: "pods", Subresource: "status"}},
 	})
-
-	// Just in case that testPod is shared among different test functions, set it back.
-	testPod.Status = originalStatus
 }
 
 func expectPodStatus(t *testing.T, m *manager, pod *api.Pod) api.PodStatus {