Skip to content

K
k3s
Commit 6a28658c authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub
Browse files
Options

Merge pull request #44058 from caesarxuchao/background-cascading

Automatic merge from submit-queue (batch tested with PRs 44058, 48085, 48077, 48076, 47823) Make background garbage collection cascading Fix #44046, fix #47843 where user reported that the garbage collector didn't delete pods when a deployment was deleted with PropagationPolicy=Background. The cause is that when propagating background garbage collection request, the garbage collector deletes dependents with DeleteOptions.PropagationPolicy=nil, which means the default GC policy of a resource (defined by its REST strategy) and the existing GC-related finalizers will decide how the delete request is propagated further. Unfortunately, the default GC policy for RS is orphaning, so the pods are behind when a deployment is deleted. This PR changes the garbage collector to delete dependents with DeleteOptions.PropagationPolicy=Background when the owner is deleted in background. This means the dependent's existing GC finalizers will be overridden, making orphaning less flexible (see this made-up [case](https://github.com/kubernetes/kubeadm/issues/149#issuecomment-278942012)). I think sacrificing the flexibility of orphaning is worthwhile, because making the behavior of background garbage collection matching users' expectation is more important. cc @lavalamp @kargakis @krmayankk @enisoc ```release-note The garbage collector now cascades deletion properly when deleting an object with propagationPolicy="background". This resolves issue [#44046](https://github.com/kubernetes/kubernetes/issues/44046), so that when a deployment is deleted with propagationPolicy="background", the garbage collector ensures dependent pods are deleted as well. ```
parents ec1fcbed 229ae59e
Hide whitespace changes
Inline Side-by-side
Showing with 19 additions and 16 deletions
pkg/controller/controller_ref_manager.go
View file @ 6a28658c
...@@ -341,9 +341,8 @@ func (m *ReplicaSetControllerRefManager) ClaimReplicaSets(sets []*extensions.Rep ...@@ -341,9 +341,8 @@ func (m *ReplicaSetControllerRefManager) ClaimReplicaSets(sets []*extensions.Rep
return claimed, utilerrors.NewAggregate(errlist) return claimed, utilerrors.NewAggregate(errlist)
} }
// AdoptReplicaSet sends a patch to take control of the ReplicaSet and also // AdoptReplicaSet sends a patch to take control of the ReplicaSet. It returns
// sets the finalizers to foregroundDeletion. It returns the error if // the error if the patching fails.
// the patching fails.
func (m *ReplicaSetControllerRefManager) AdoptReplicaSet(rs *extensions.ReplicaSet) error { func (m *ReplicaSetControllerRefManager) AdoptReplicaSet(rs *extensions.ReplicaSet) error {
if err := m.canAdopt(); err != nil { if err := m.canAdopt(); err != nil {
return fmt.Errorf("can't adopt ReplicaSet %v/%v (%v): %v", rs.Namespace, rs.Name, rs.UID, err) 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 ...@@ -351,14 +350,7 @@ func (m *ReplicaSetControllerRefManager) AdoptReplicaSet(rs *extensions.ReplicaS
// Note that ValidateOwnerReferences() will reject this patch if another // Note that ValidateOwnerReferences() will reject this patch if another
// OwnerReference exists with controller=true. // OwnerReference exists with controller=true.
addControllerPatch := fmt.Sprintf( addControllerPatch := fmt.Sprintf(
`{ `{"metadata":{"ownerReferences":[{"apiVersion":"%s","kind":"%s","name":"%s","uid":"%s","controller":true,"blockOwnerDeletion":true}],"uid":"%s"}}`,
"metadata":
{
"ownerReferences": [{"apiVersion":"%s","kind":"%s","name":"%s","uid":"%s","controller":true,"blockOwnerDeletion":true}],
"uid":"%s",
"finalizers": ["foregroundDeletion"]
}
}`,
m.controllerKind.GroupVersion(), m.controllerKind.Kind, m.controllerKind.GroupVersion(), m.controllerKind.Kind,
m.controller.GetName(), m.controller.GetUID(), rs.UID) m.controller.GetName(), m.controller.GetUID(), rs.UID)
return m.rsControl.PatchReplicaSet(rs.Namespace, rs.Name, []byte(addControllerPatch)) return m.rsControl.PatchReplicaSet(rs.Namespace, rs.Name, []byte(addControllerPatch))
......
pkg/controller/deployment/sync.go
View file @ 6a28658c
...@@ -300,7 +300,6 @@ func (dc *DeploymentController) getNewReplicaSet(d *extensions.Deployment, rsLis ...@@ -300,7 +300,6 @@ func (dc *DeploymentController) getNewReplicaSet(d *extensions.Deployment, rsLis
Name: d.Name + "-" + podTemplateSpecHash, Name: d.Name + "-" + podTemplateSpecHash,
Namespace: d.Namespace, Namespace: d.Namespace,
OwnerReferences: []metav1.OwnerReference{*newControllerRef(d)}, OwnerReferences: []metav1.OwnerReference{*newControllerRef(d)},
Finalizers: []string{metav1.FinalizerDeleteDependents},
}, },
Spec: extensions.ReplicaSetSpec{ Spec: extensions.ReplicaSetSpec{
Replicas: new(int32), Replicas: new(int32),
......
pkg/controller/garbagecollector/garbagecollector.go
View file @ 6a28658c
...@@ -364,7 +364,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(item *node) error { ...@@ -364,7 +364,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(item *node) error {
break 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 // the deletion event will be observed by the graphBuilder, so the item
// will be processed again in processDeletingDependentsItem. If it // will be processed again in processDeletingDependentsItem. If it
// doesn't have dependents, the function will remove the // doesn't have dependents, the function will remove the
...@@ -375,9 +375,21 @@ func (gc *GarbageCollector) attemptToDeleteItem(item *node) error { ...@@ -375,9 +375,21 @@ func (gc *GarbageCollector) attemptToDeleteItem(item *node) error {
default: default:
// item doesn't have any solid owner, so it needs to be garbage // 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 // collected. Also, none of item's owners is waiting for the deletion of
// the dependents, so GC deletes item with Default. // the dependents, so set propagationPolicy based on existing finalizers.
glog.V(2).Infof("delete object %s with Default", item.identity) var policy metav1.DeletionPropagation
return gc.deleteObject(item.identity, nil) 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)
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment