Bugfix prevent daemon controller to adopt controller revisions of other namespaces

pkg/controller/controller_ref_manager.go

@@ -107,6 +107,12 @@ func (m *BaseControllerRefManager) ClaimObject(ctx context.Context, obj metav1.O
 		// Ignore if the object is being deleted
 		return false, nil
 	}
+
+	if m.Controller.GetNamespace() != obj.GetNamespace() {
+		// Ignore if namespace not match
+		return false, nil
+	}
+
 	// Selector matches. Try to adopt.
 	if err := adopt(ctx, obj); err != nil {
 		// If the pod no longer exists, ignore the error.

pkg/controller/controller_ref_manager_test.go

@@ -68,19 +68,24 @@ func TestClaimPods(t *testing.T) {
 		patches int
 	}
 	var tests = []test{
-		{
-			name: "Claim pods with correct label",
-			manager: NewPodControllerRefManager(&FakePodControl{},
-				&v1.ReplicationController{},
-				productionLabelSelector,
-				controllerKind,
-				func(ctx context.Context) error { return nil }),
-			pods:    []*v1.Pod{newPod("pod1", productionLabel, nil), newPod("pod2", testLabel, nil)},
-			claimed: []*v1.Pod{newPod("pod1", productionLabel, nil)},
-			patches: 1,
-		},
 		func() test {
 			controller := v1.ReplicationController{}
+			controller.Namespace = metav1.NamespaceDefault
+			return test{
+				name: "Claim pods with correct label",
+				manager: NewPodControllerRefManager(&FakePodControl{},
+					&controller,
+					productionLabelSelector,
+					controllerKind,
+					func(ctx context.Context) error { return nil }),
+				pods:    []*v1.Pod{newPod("pod1", productionLabel, nil), newPod("pod2", testLabel, nil)},
+				claimed: []*v1.Pod{newPod("pod1", productionLabel, nil)},
+				patches: 1,
+			}
+		}(),
+		func() test {
+			controller := v1.ReplicationController{}
+			controller.Namespace = metav1.NamespaceDefault
 			controller.UID = types.UID(controllerUID)
 			now := metav1.Now()
 			controller.DeletionTimestamp = &now
@@ -97,6 +102,7 @@ func TestClaimPods(t *testing.T) {
 		}(),
 		func() test {
 			controller := v1.ReplicationController{}
+			controller.Namespace = metav1.NamespaceDefault
 			controller.UID = types.UID(controllerUID)
 			now := metav1.Now()
 			controller.DeletionTimestamp = &now
@@ -115,7 +121,9 @@ func TestClaimPods(t *testing.T) {
 			controller := v1.ReplicationController{}
 			controller2 := v1.ReplicationController{}
 			controller.UID = types.UID(controllerUID)
+			controller.Namespace = metav1.NamespaceDefault
 			controller2.UID = types.UID("AAAAA")
+			controller2.Namespace = metav1.NamespaceDefault
 			return test{
 				name: "Controller can not claim pods owned by another controller",
 				manager: NewPodControllerRefManager(&FakePodControl{},
@@ -129,6 +137,7 @@ func TestClaimPods(t *testing.T) {
 		}(),
 		func() test {
 			controller := v1.ReplicationController{}
+			controller.Namespace = metav1.NamespaceDefault
 			controller.UID = types.UID(controllerUID)
 			return test{
 				name: "Controller releases claimed pods when selector doesn't match",
@@ -144,6 +153,7 @@ func TestClaimPods(t *testing.T) {
 		}(),
 		func() test {
 			controller := v1.ReplicationController{}
+			controller.Namespace = metav1.NamespaceDefault
 			controller.UID = types.UID(controllerUID)
 			podToDelete1 := newPod("pod1", productionLabel, &controller)
 			podToDelete2 := newPod("pod2", productionLabel, nil)
@@ -164,6 +174,7 @@ func TestClaimPods(t *testing.T) {
 		}(),
 		func() test {
 			controller := v1.ReplicationController{}
+			controller.Namespace = metav1.NamespaceDefault
 			controller.UID = types.UID(controllerUID)
 			return test{
 				name: "Controller claims or release pods according to selector with finalizers",
@@ -178,6 +189,25 @@ func TestClaimPods(t *testing.T) {
 				patches: 2,
 			}
 		}(),
+		func() test {
+			controller := v1.ReplicationController{}
+			controller.Namespace = metav1.NamespaceDefault
+			controller.UID = types.UID(controllerUID)
+			pod1 := newPod("pod1", productionLabel, nil)
+			pod2 := newPod("pod2", productionLabel, nil)
+			pod2.Namespace = "fakens"
+			return test{
+				name: "Controller does not claim pods of different namespace",
+				manager: NewPodControllerRefManager(&FakePodControl{},
+					&controller,
+					productionLabelSelector,
+					controllerKind,
+					func(ctx context.Context) error { return nil }),
+				pods:    []*v1.Pod{pod1, pod2},
+				claimed: []*v1.Pod{pod1},
+				patches: 1,
+			}
+		}(),
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {

pkg/controller/daemon/daemon_controller_test.go

@@ -225,6 +225,19 @@ func addFailedPods(podStore cache.Store, nodeName string, label map[string]strin
 	}
 }
 
+func newControllerRevision(name string, namespace string, label map[string]string,
+	ownerReferences []metav1.OwnerReference) *apps.ControllerRevision {
+	return &apps.ControllerRevision{
+		TypeMeta: metav1.TypeMeta{APIVersion: "apps/v1"},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:            name,
+			Labels:          label,
+			Namespace:       namespace,
+			OwnerReferences: ownerReferences,
+		},
+	}
+}
+
 type fakePodControl struct {
 	sync.Mutex
 	*controller.FakePodControl

pkg/controller/daemon/update.go

@@ -406,7 +406,7 @@ func (dsc *DaemonSetsController) controlledHistories(ctx context.Context, ds *ap
 
 	// List all histories to include those that don't match the selector anymore
 	// but have a ControllerRef pointing to the controller.
-	histories, err := dsc.historyLister.List(labels.Everything())
+	histories, err := dsc.historyLister.ControllerRevisions(ds.Namespace).List(labels.Everything())
 	if err != nil {
 		return nil, err
 	}

pkg/controller/daemon/update_test.go

@@ -18,6 +18,7 @@ package daemon
 
 import (
 	"context"
+	"reflect"
 	"testing"
 	"time"
 
@@ -635,3 +636,73 @@ func TestGetUnavailableNumbers(t *testing.T) {
 		})
 	}
 }
+
+func TestControlledHistories(t *testing.T) {
+	ds1 := newDaemonSet("ds1")
+	crOfDs1 := newControllerRevision(ds1.GetName()+"-x1", ds1.GetNamespace(), ds1.Spec.Template.Labels,
+		[]metav1.OwnerReference{*metav1.NewControllerRef(ds1, controllerKind)})
+	orphanCrInSameNsWithDs1 := newControllerRevision(ds1.GetName()+"-x2", ds1.GetNamespace(), ds1.Spec.Template.Labels, nil)
+	orphanCrNotInSameNsWithDs1 := newControllerRevision(ds1.GetName()+"-x3", ds1.GetNamespace()+"-other", ds1.Spec.Template.Labels, nil)
+	cases := []struct {
+		name                      string
+		manager                   *daemonSetsController
+		historyCRAll              []*apps.ControllerRevision
+		expectControllerRevisions []*apps.ControllerRevision
+	}{
+		{
+			name: "controller revision in the same namespace",
+			manager: func() *daemonSetsController {
+				manager, _, _, err := newTestController(ds1, crOfDs1, orphanCrInSameNsWithDs1)
+				if err != nil {
+					t.Fatalf("error creating DaemonSets controller: %v", err)
+				}
+				manager.dsStore.Add(ds1)
+				return manager
+			}(),
+			historyCRAll:              []*apps.ControllerRevision{crOfDs1, orphanCrInSameNsWithDs1},
+			expectControllerRevisions: []*apps.ControllerRevision{crOfDs1, orphanCrInSameNsWithDs1},
+		},
+		{
+			name: "Skip adopting the controller revision in namespace other than the one in which DS lives",
+			manager: func() *daemonSetsController {
+				manager, _, _, err := newTestController(ds1, orphanCrNotInSameNsWithDs1)
+				if err != nil {
+					t.Fatalf("error creating DaemonSets controller: %v", err)
+				}
+				manager.dsStore.Add(ds1)
+				return manager
+			}(),
+			historyCRAll:              []*apps.ControllerRevision{orphanCrNotInSameNsWithDs1},
+			expectControllerRevisions: []*apps.ControllerRevision{},
+		},
+	}
+	for _, c := range cases {
+		for _, h := range c.historyCRAll {
+			c.manager.historyStore.Add(h)
+		}
+		crList, err := c.manager.controlledHistories(context.TODO(), ds1)
+		if err != nil {
+			t.Fatalf("Test case: %s. Unexpected error: %v", c.name, err)
+		}
+		if len(crList) != len(c.expectControllerRevisions) {
+			t.Errorf("Test case: %s, expect controllerrevision count %d but got:%d",
+				c.name, len(c.expectControllerRevisions), len(crList))
+		} else {
+			// check controller revisions match
+			for _, cr := range crList {
+				found := false
+				for _, expectCr := range c.expectControllerRevisions {
+					if reflect.DeepEqual(cr, expectCr) {
+						found = true
+						break
+					}
+				}
+				if !found {
+					t.Errorf("Test case: %s, controllerrevision %v not expected",
+						c.name, cr)
+				}
+			}
+			t.Logf("Test case: %s done", c.name)
+		}
+	}
+}