From 31d0869f834ea118bb9c9988d96e155b5ce2fa0c Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Sun, 25 Jun 2017 21:41:10 -0700 Subject: [PATCH 1/2] revert 45764 --- pkg/controller/controller_ref_manager.go | 14 +++----------- pkg/controller/deployment/sync.go | 1 - 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/pkg/controller/controller_ref_manager.go b/pkg/controller/controller_ref_manager.go index 3e02d83475c..3ef7ade5c7d 100644 --- a/pkg/controller/controller_ref_manager.go +++ b/pkg/controller/controller_ref_manager.go @@ -341,9 +341,8 @@ func (m *ReplicaSetControllerRefManager) ClaimReplicaSets(sets []*extensions.Rep return claimed, utilerrors.NewAggregate(errlist) } -// AdoptReplicaSet sends a patch to take control of the ReplicaSet and also -// sets the finalizers to foregroundDeletion. It returns the error if -// the patching fails. +// AdoptReplicaSet sends a patch to take control of the ReplicaSet. It returns +// the error if the patching fails. func (m *ReplicaSetControllerRefManager) AdoptReplicaSet(rs *extensions.ReplicaSet) error { if err := m.canAdopt(); err != nil { return fmt.Errorf("can't adopt ReplicaSet %v/%v (%v): %v", rs.Namespace, rs.Name, rs.UID, err) @@ -351,14 +350,7 @@ func (m *ReplicaSetControllerRefManager) AdoptReplicaSet(rs *extensions.ReplicaS // Note that ValidateOwnerReferences() will reject this patch if another // OwnerReference exists with controller=true. addControllerPatch := fmt.Sprintf( - `{ - "metadata": - { - "ownerReferences": [{"apiVersion":"%s","kind":"%s","name":"%s","uid":"%s","controller":true,"blockOwnerDeletion":true}], - "uid":"%s", - "finalizers": ["foregroundDeletion"] - } - }`, + `{"metadata":{"ownerReferences":[{"apiVersion":"%s","kind":"%s","name":"%s","uid":"%s","controller":true,"blockOwnerDeletion":true}],"uid":"%s"}}`, m.controllerKind.GroupVersion(), m.controllerKind.Kind, m.controller.GetName(), m.controller.GetUID(), rs.UID) return m.rsControl.PatchReplicaSet(rs.Namespace, rs.Name, []byte(addControllerPatch)) diff --git a/pkg/controller/deployment/sync.go b/pkg/controller/deployment/sync.go index 788c91b842d..8b0408bb433 100644 --- a/pkg/controller/deployment/sync.go +++ b/pkg/controller/deployment/sync.go @@ -300,7 +300,6 @@ func (dc *DeploymentController) getNewReplicaSet(d *extensions.Deployment, rsLis Name: d.Name + "-" + podTemplateSpecHash, Namespace: d.Namespace, OwnerReferences: []metav1.OwnerReference{*newControllerRef(d)}, - Finalizers: []string{metav1.FinalizerDeleteDependents}, }, Spec: extensions.ReplicaSetSpec{ Replicas: new(int32), From 229ae59e73ce17bf9c7471a4e3836164247fc7ce Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Sun, 25 Jun 2017 21:41:33 -0700 Subject: [PATCH 2/2] garbage collector controller propagates DeletePropagationForeground policy if the object doesn't already have finalizers. --- .../garbagecollector/garbagecollector.go | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go index 30cf5e43c79..785bcf3792f 100644 --- a/pkg/controller/garbagecollector/garbagecollector.go +++ b/pkg/controller/garbagecollector/garbagecollector.go @@ -364,7 +364,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(item *node) error { break } } - glog.V(2).Infof("at least one owner of object %s has FinalizerDeletingDependents, and the object itself has dependents, so it is going to be deleted with Foreground", item.identity) + glog.V(2).Infof("at least one owner of object %s has FinalizerDeletingDependents, and the object itself has dependents, so it is going to be deleted in Foreground", item.identity) // the deletion event will be observed by the graphBuilder, so the item // will be processed again in processDeletingDependentsItem. If it // doesn't have dependents, the function will remove the @@ -375,9 +375,21 @@ func (gc *GarbageCollector) attemptToDeleteItem(item *node) error { default: // item doesn't have any solid owner, so it needs to be garbage // collected. Also, none of item's owners is waiting for the deletion of - // the dependents, so GC deletes item with Default. - glog.V(2).Infof("delete object %s with Default", item.identity) - return gc.deleteObject(item.identity, nil) + // the dependents, so set propagationPolicy based on existing finalizers. + var policy metav1.DeletionPropagation + switch { + case hasOrphanFinalizer(latest): + // if an existing orphan finalizer is already on the object, honor it. + policy = metav1.DeletePropagationOrphan + case hasDeleteDependentsFinalizer(latest): + // if an existing foreground finalizer is already on the object, honor it. + policy = metav1.DeletePropagationForeground + default: + // otherwise, default to background. + policy = metav1.DeletePropagationBackground + } + glog.V(2).Infof("delete object %s with propagation policy %s", item.identity, policy) + return gc.deleteObject(item.identity, &policy) } }