From f4ee44eb390a10761d5d0fdac11f0f2e09f2c88c Mon Sep 17 00:00:00 2001 From: Anthony Yeh Date: Tue, 7 Mar 2017 10:40:20 -0800 Subject: [PATCH] RC/RS: Check that ControllerRef UID matches found controller. Otherwise, we may confuse a former controller by that name with a new one that has the same name. --- pkg/controller/replicaset/replica_set.go | 56 ++++++++++--------- .../replication/replication_controller.go | 56 ++++++++++--------- 2 files changed, 62 insertions(+), 50 deletions(-) diff --git a/pkg/controller/replicaset/replica_set.go b/pkg/controller/replicaset/replica_set.go index 98970d7840c..0a31df9122f 100644 --- a/pkg/controller/replicaset/replica_set.go +++ b/pkg/controller/replicaset/replica_set.go @@ -181,6 +181,27 @@ func (rsc *ReplicaSetController) getPodReplicaSets(pod *v1.Pod) []*extensions.Re return rss } +// resolveControllerRef returns the controller referenced by a ControllerRef, +// or nil if the ControllerRef could not be resolved to a matching controller +// of the corrrect Kind. +func (rsc *ReplicaSetController) resolveControllerRef(namespace string, controllerRef *metav1.OwnerReference) *extensions.ReplicaSet { + // We can't look up by UID, so look up by Name and then verify UID. + // Don't even try to look up by Name if it's the wrong Kind. + if controllerRef.Kind != controllerKind.Kind { + return nil + } + rs, err := rsc.rsLister.ReplicaSets(namespace).Get(controllerRef.Name) + if err != nil { + return nil + } + if rs.UID != controllerRef.UID { + // The controller we found with this Name is not the same one that the + // ControllerRef points to. + return nil + } + return rs +} + // callback when RS is updated func (rsc *ReplicaSetController) updateRS(old, cur interface{}) { oldRS := old.(*extensions.ReplicaSet) @@ -217,19 +238,15 @@ func (rsc *ReplicaSetController) addPod(obj interface{}) { // If it has a ControllerRef, that's all that matters. if controllerRef := controller.GetControllerOf(pod); controllerRef != nil { - if controllerRef.Kind != controllerKind.Kind { - // It's controlled by a different type of controller. - return - } - glog.V(4).Infof("Pod %s created: %#v.", pod.Name, pod) - rs, err := rsc.rsLister.ReplicaSets(pod.Namespace).Get(controllerRef.Name) - if err != nil { + rs := rsc.resolveControllerRef(pod.Namespace, controllerRef) + if rs == nil { return } rsKey, err := controller.KeyFunc(rs) if err != nil { return } + glog.V(4).Infof("Pod %s created: %#v.", pod.Name, pod) rsc.expectations.CreationObserved(rsKey) rsc.enqueueReplicaSet(rs) return @@ -279,26 +296,20 @@ func (rsc *ReplicaSetController) updatePod(old, cur interface{}) { curControllerRef := controller.GetControllerOf(curPod) oldControllerRef := controller.GetControllerOf(oldPod) controllerRefChanged := !reflect.DeepEqual(curControllerRef, oldControllerRef) - if controllerRefChanged && - oldControllerRef != nil && oldControllerRef.Kind == controllerKind.Kind { + if controllerRefChanged && oldControllerRef != nil { // The ControllerRef was changed. Sync the old controller, if any. - rs, err := rsc.rsLister.ReplicaSets(oldPod.Namespace).Get(oldControllerRef.Name) - if err == nil { + if rs := rsc.resolveControllerRef(oldPod.Namespace, oldControllerRef); rs != nil { rsc.enqueueReplicaSet(rs) } } // If it has a ControllerRef, that's all that matters. if curControllerRef != nil { - if curControllerRef.Kind != controllerKind.Kind { - // It's controlled by a different type of controller. + rs := rsc.resolveControllerRef(curPod.Namespace, curControllerRef) + if rs == nil { return } glog.V(4).Infof("Pod %s updated, objectMeta %+v -> %+v.", curPod.Name, oldPod.ObjectMeta, curPod.ObjectMeta) - rs, err := rsc.rsLister.ReplicaSets(curPod.Namespace).Get(curControllerRef.Name) - if err != nil { - return - } rsc.enqueueReplicaSet(rs) // TODO: MinReadySeconds in the Pod will generate an Available condition to be added in // the Pod status which in turn will trigger a requeue of the owning replica set thus @@ -355,20 +366,15 @@ func (rsc *ReplicaSetController) deletePod(obj interface{}) { // No controller should care about orphans being deleted. return } - if controllerRef.Kind != controllerKind.Kind { - // It's controlled by a different type of controller. - return - } - glog.V(4).Infof("Pod %s/%s deleted through %v, timestamp %+v: %#v.", pod.Namespace, pod.Name, utilruntime.GetCaller(), pod.DeletionTimestamp, pod) - - rs, err := rsc.rsLister.ReplicaSets(pod.Namespace).Get(controllerRef.Name) - if err != nil { + rs := rsc.resolveControllerRef(pod.Namespace, controllerRef) + if rs == nil { return } rsKey, err := controller.KeyFunc(rs) if err != nil { return } + glog.V(4).Infof("Pod %s/%s deleted through %v, timestamp %+v: %#v.", pod.Namespace, pod.Name, utilruntime.GetCaller(), pod.DeletionTimestamp, pod) rsc.expectations.DeletionObserved(rsKey, controller.PodKey(pod)) rsc.enqueueReplicaSet(rs) } diff --git a/pkg/controller/replication/replication_controller.go b/pkg/controller/replication/replication_controller.go index 872acb0656c..332cf1d85f2 100644 --- a/pkg/controller/replication/replication_controller.go +++ b/pkg/controller/replication/replication_controller.go @@ -176,6 +176,27 @@ func (rm *ReplicationManager) getPodControllers(pod *v1.Pod) []*v1.ReplicationCo return rcs } +// resolveControllerRef returns the controller referenced by a ControllerRef, +// or nil if the ControllerRef could not be resolved to a matching controller +// of the corrrect Kind. +func (rm *ReplicationManager) resolveControllerRef(namespace string, controllerRef *metav1.OwnerReference) *v1.ReplicationController { + // We can't look up by UID, so look up by Name and then verify UID. + // Don't even try to look up by Name if it's the wrong Kind. + if controllerRef.Kind != controllerKind.Kind { + return nil + } + rc, err := rm.rcLister.ReplicationControllers(namespace).Get(controllerRef.Name) + if err != nil { + return nil + } + if rc.UID != controllerRef.UID { + // The controller we found with this Name is not the same one that the + // ControllerRef points to. + return nil + } + return rc +} + // callback when RC is updated func (rm *ReplicationManager) updateRC(old, cur interface{}) { oldRC := old.(*v1.ReplicationController) @@ -216,19 +237,15 @@ func (rm *ReplicationManager) addPod(obj interface{}) { // If it has a ControllerRef, that's all that matters. if controllerRef := controller.GetControllerOf(pod); controllerRef != nil { - if controllerRef.Kind != controllerKind.Kind { - // It's controlled by a different type of controller. - return - } - glog.V(4).Infof("Pod %s created: %#v.", pod.Name, pod) - rc, err := rm.rcLister.ReplicationControllers(pod.Namespace).Get(controllerRef.Name) - if err != nil { + rc := rm.resolveControllerRef(pod.Namespace, controllerRef) + if rc == nil { return } rsKey, err := controller.KeyFunc(rc) if err != nil { return } + glog.V(4).Infof("Pod %s created: %#v.", pod.Name, pod) rm.expectations.CreationObserved(rsKey) rm.enqueueController(rc) return @@ -278,26 +295,20 @@ func (rm *ReplicationManager) updatePod(old, cur interface{}) { curControllerRef := controller.GetControllerOf(curPod) oldControllerRef := controller.GetControllerOf(oldPod) controllerRefChanged := !reflect.DeepEqual(curControllerRef, oldControllerRef) - if controllerRefChanged && - oldControllerRef != nil && oldControllerRef.Kind == controllerKind.Kind { + if controllerRefChanged && oldControllerRef != nil { // The ControllerRef was changed. Sync the old controller, if any. - rc, err := rm.rcLister.ReplicationControllers(oldPod.Namespace).Get(oldControllerRef.Name) - if err == nil { + if rc := rm.resolveControllerRef(oldPod.Namespace, oldControllerRef); rc != nil { rm.enqueueController(rc) } } // If it has a ControllerRef, that's all that matters. if curControllerRef != nil { - if curControllerRef.Kind != controllerKind.Kind { - // It's controlled by a different type of controller. + rc := rm.resolveControllerRef(curPod.Namespace, curControllerRef) + if rc == nil { return } glog.V(4).Infof("Pod %s updated, objectMeta %+v -> %+v.", curPod.Name, oldPod.ObjectMeta, curPod.ObjectMeta) - rc, err := rm.rcLister.ReplicationControllers(curPod.Namespace).Get(curControllerRef.Name) - if err != nil { - return - } rm.enqueueController(rc) // TODO: MinReadySeconds in the Pod will generate an Available condition to be added in // the Pod status which in turn will trigger a requeue of the owning ReplicationController thus @@ -354,20 +365,15 @@ func (rm *ReplicationManager) deletePod(obj interface{}) { // No controller should care about orphans being deleted. return } - if controllerRef.Kind != controllerKind.Kind { - // It's controlled by a different type of controller. - return - } - glog.V(4).Infof("Pod %s/%s deleted through %v, timestamp %+v: %#v.", pod.Namespace, pod.Name, utilruntime.GetCaller(), pod.DeletionTimestamp, pod) - - rc, err := rm.rcLister.ReplicationControllers(pod.Namespace).Get(controllerRef.Name) - if err != nil { + rc := rm.resolveControllerRef(pod.Namespace, controllerRef) + if rc == nil { return } rsKey, err := controller.KeyFunc(rc) if err != nil { return } + glog.V(4).Infof("Pod %s/%s deleted through %v, timestamp %+v: %#v.", pod.Namespace, pod.Name, utilruntime.GetCaller(), pod.DeletionTimestamp, pod) rm.expectations.DeletionObserved(rsKey, controller.PodKey(pod)) rm.enqueueController(rc) }