github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #101250 from evertrain/master

Browse Source 
Bugfix: prevent daemon controller to adopt controller revisions of ot…
This commit is contained in:
Kubernetes Prow Robot 2021-11-10 09:19:26 -08:00 committed by GitHub
commit 91ff1f9840
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 152 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
pkg/controller/controller_ref_manager.go
View File

@ -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 len(m.Controller.GetNamespace()) > 0 && 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.

72
pkg/controller/controller_ref_manager_test.go
View File

@ -69,19 +69,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
@ -98,6 +103,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
@ -116,7 +122,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{},
@ -130,6 +138,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",
@ -145,6 +154,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)
@ -165,6 +175,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",
@ -179,6 +190,45 @@ 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,
}
}(),
func() test {
// act as a cluster-scoped controller
controller := v1.ReplicationController{}
controller.Namespace = ""
controller.UID = types.UID(controllerUID)
pod1 := newPod("pod1", productionLabel, nil)
pod2 := newPod("pod2", productionLabel, nil)
pod2.Namespace = "fakens"
return test{
name: "Cluster scoped controller claims pods of specified namespace",
manager: NewPodControllerRefManager(&FakePodControl{},
&controller,
productionLabelSelector,
controllerKind,
func(ctx context.Context) error { return nil }),
pods: []*v1.Pod{pod1, pod2},
claimed: []*v1.Pod{pod1, pod2},
patches: 2,
}
}(),
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {

13
pkg/controller/daemon/daemon_controller_test.go
View File

@ -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

2
pkg/controller/daemon/update.go
View File

@ -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
}

71
pkg/controller/daemon/update_test.go
View File

@ -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)
}
}
}