Merge pull request #443 from aaronlevy/pr-387

Ensure labels are preserved

Author: aaronlevy

cmd/checkpoint/main.go

@@ -404,14 +404,26 @@ func isPodCheckpointer(pod *v1.Pod) bool {
 }
 
 func sanitizeCheckpointPod(cp *v1.Pod) (*v1.Pod, error) {
-	// Clear ObjectMeta except for name/namespace
-	// NOTE(aaron): If we want to keep labels, we need to add a new label so the static pod
-	//              will not be adopted by higher-level parent (e.g. daemonset/deployment).
-	//              Otherwise you end up in situations where parent tries deleting mirror pods.
+	trueVar := true
+
+	// Keep same name, namespace, and labels as parent.
 	cp.ObjectMeta = metav1.ObjectMeta{
 		Name:        cp.Name,
 		Namespace:   cp.Namespace,
 		Annotations: make(map[string]string),
+		Labels:      cp.Labels,
+		// Set the ownerRef to the parent pod. We do this because:
+		// If the ownerRef stays the same (e.g. the original deployment), then the deployment controller will try to manage the static/mirror pod.
+		// If we clear the ownerRef, then a higher-level object will adopt this pod based on the label selector (e.g. the original deployment).
+		OwnerReferences: []metav1.OwnerReference{
+			{
+				APIVersion: cp.APIVersion,
+				Kind:       cp.Kind,
+				Name:       cp.Name,
+				UID:        cp.UID,
+				Controller: &trueVar,
+			},
+		},
 	}
 
 	// Track this checkpoint's parent pod

cmd/checkpoint/main_test.go

@@ -286,10 +286,11 @@ func TestSanitizeCheckpointPod(t *testing.T) {
 		pod      *v1.Pod
 		expected *v1.Pod
 	}
+	trueVar := true
 
 	cases := []testCase{
 		{
-			desc: "Pod name and namespace are preserved & checkpoint annotation added",
+			desc: "Pod name and namespace are preserved, checkpoint annotation added, owner points to parent",
 			pod: &v1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "podname",
@@ -298,14 +299,15 @@ func TestSanitizeCheckpointPod(t *testing.T) {
 			},
 			expected: &v1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:        "podname",
-					Namespace:   "podnamespace",
-					Annotations: map[string]string{checkpointParentAnnotation: "podname"},
+					Name:            "podname",
+					Namespace:       "podnamespace",
+					Annotations:     map[string]string{checkpointParentAnnotation: "podname"},
+					OwnerReferences: []metav1.OwnerReference{{Name: "podname", Controller: &trueVar}},
 				},
 			},
 		},
 		{
-			desc: "Existing annotations are removed, checkpoint annotation added",
+			desc: "Existing annotations are removed, checkpoint annotation added, owner points to parent",
 			pod: &v1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:        "podname",
@@ -315,9 +317,10 @@ func TestSanitizeCheckpointPod(t *testing.T) {
 			},
 			expected: &v1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:        "podname",
-					Namespace:   "podnamespace",
-					Annotations: map[string]string{checkpointParentAnnotation: "podname"},
+					Name:            "podname",
+					Namespace:       "podnamespace",
+					Annotations:     map[string]string{checkpointParentAnnotation: "podname"},
+					OwnerReferences: []metav1.OwnerReference{{Name: "podname", Controller: &trueVar}},
 				},
 			},
 		},
@@ -332,9 +335,10 @@ func TestSanitizeCheckpointPod(t *testing.T) {
 			},
 			expected: &v1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:        "podname",
-					Namespace:   "podnamespace",
-					Annotations: map[string]string{checkpointParentAnnotation: "podname"},
+					Name:            "podname",
+					Namespace:       "podnamespace",
+					Annotations:     map[string]string{checkpointParentAnnotation: "podname"},
+					OwnerReferences: []metav1.OwnerReference{{Name: "podname", Controller: &trueVar}},
 				},
 			},
 		},
@@ -348,10 +352,61 @@ func TestSanitizeCheckpointPod(t *testing.T) {
 				Spec: v1.PodSpec{ServiceAccountName: "foo", DeprecatedServiceAccount: "bar"},
 			},
 			expected: &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "podname",
+					Namespace:       "podnamespace",
+					Annotations:     map[string]string{checkpointParentAnnotation: "podname"},
+					OwnerReferences: []metav1.OwnerReference{{Name: "podname", Controller: &trueVar}},
+				},
+			},
+		},
+		{
+			desc: "Labels are preserved",
+			pod: &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "podname",
+					Namespace: "podnamespace",
+					Labels:    map[string]string{"foo": "bar"},
+				},
+			},
+			expected: &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "podname",
+					Namespace:       "podnamespace",
+					Labels:          map[string]string{"foo": "bar"},
+					Annotations:     map[string]string{checkpointParentAnnotation: "podname"},
+					OwnerReferences: []metav1.OwnerReference{{Name: "podname", Controller: &trueVar}},
+				},
+			},
+		},
+		{
+			desc: "OwnerReference of checkpoint points to parent pod",
+			pod: &v1.Pod{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Pod",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "podname",
+					Namespace: "podnamespace",
+					UID:       "pod-uid",
+					OwnerReferences: []metav1.OwnerReference{
+						{APIVersion: "v1", Kind: "Daemonset", Name: "daemonname", UID: "daemon-uid"},
+					},
+				},
+			},
+			expected: &v1.Pod{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Pod",
+				},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:        "podname",
 					Namespace:   "podnamespace",
 					Annotations: map[string]string{checkpointParentAnnotation: "podname"},
+					OwnerReferences: []metav1.OwnerReference{
+						{APIVersion: "v1", Kind: "Pod", Name: "podname", UID: "pod-uid", Controller: &trueVar},
+					},
 				},
 			},
 		},
@@ -360,12 +415,13 @@ func TestSanitizeCheckpointPod(t *testing.T) {
 	for _, tc := range cases {
 		got, err := sanitizeCheckpointPod(tc.pod)
 		if err != nil {
-			t.Errorf("Unexpected error: %v", err)
+			t.Errorf("\nUnexpected error: %v\n", err)
 		}
 		if !api.Semantic.DeepEqual(tc.expected, got) {
-			t.Errorf("For Test: %s\n\nExpected:\n%+v\nGot:\n%+v\n", tc.desc, tc.expected, got)
+			t.Errorf("\nFor Test: %s\n\nExpected:\n%#v\nGot:\n%#v\n", tc.desc, tc.expected, got)
 		}
 	}
+
 }
 
 func TestPodListToParentPods(t *testing.T) {
@@ -603,7 +659,7 @@ func TestCopyPod(t *testing.T) {
 		t.Errorf("Unexpected error: %v", err)
 	}
 	if !api.Semantic.DeepEqual(pod, *got) {
-		t.Errorf("Expected:\n%+v\nGot:\n%+v", pod, got)
+		t.Errorf("Expected:\n%#v\nGot:\n%#v", pod, got)
 	}
 }