Repository: kubernetes
Issue: use foreground cascading deletion will be stuck when garbagecontroller lost the firset update event(add deletetimestamp and finalizer) after delete
Labels: kind/bug,sig/api-machinery,triage/accepted

### What happened?

in larget k8s cluster. i have encountered a problem sometimes as follows:

when i delete a deploy in foreground  cascading deletion。i found:

(1) the deploy has deletetimeStamp and foreground finalizer

(2) the rs has deletetimeStamp and foreground finalizer

(3) the pod is running, has not deletetimeStamp !!!

(4) i can delete pod with kubectl。but when i delete pod, the rs and deploy is still exist ! 

the step 3 and step 4 is strange!!

<br>
after i checkout the kube-controller-manage log。and  I realized that  there are some problems in garbagecontroller。 so i use the garbagecontroller debug, and get this message:

i found the rs.deletingDependents is not right。 it should be true, but it is false. This is explian why the step 3 and step 4 above；
```
strict digraph full {
  // Node definitions.
  0 [
    kind="pod"
    namespace="xx"
    name="xx"
    uid="xx"
    missing="false"
    beingDeleted="false"
    deletingDependents="false"  
    virtual="false"
  ];
  1 [
    group="apps"
    version="v1"
    kind="ReplicaSet"
    namespace="xx"
    name="xx"
    uid="xx"
    missing="false"
    beingDeleted="true"
    deletingDependents="false"  //this is not normal
    virtual="false"
  ];
  2 [
    group="apps"
    version="v1"
    kind="Deployment"
    namespace="xx"
    name="xxx"
    uid="xx"
    missing="false"
    beingDeleted="true"
    deletingDependents="true"
    virtual="false"
  ];

  // Edge definitions.
  0 -> 1;
  1 -> 2;
}
```

i go on to find why deletingDependents is false?  

and i found a bug in garbagecontroller whe the first update event (add deletetimestamp and finalizer) after delete comand.

deletingDependents is true only when deletionStartsWithFinalizer return true, but deletionStartsWithFinalizer return true only

* the old object has not deletetimeStamp and has not finalizer
* The new object has deletetimeStamp and finalizer

https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/garbagecollector/graph_builder.go#L504
```
func deletionStartsWithFinalizer(oldObj interface{}, newAccessor metav1.Object, matchingFinalizer string) bool {
	// if the new object isn't being deleted, or doesn't have the finalizer we're interested in, return false
	if !beingDeleted(newAccessor) || !hasFinalizer(newAccessor, matchingFinalizer) {
		return false
	}

	// if the old object is nil, or wasn't being deleted, or didn't have the finalizer, return true
	if oldObj == nil {
		return true
	}
	oldAccessor, err := meta.Accessor(oldObj)
	if err != nil {
		utilruntime.HandleError(fmt.Errorf("cannot access oldObj: %v", err))
		return false
	}
	return !beingDeleted(oldAccessor) || !hasFinalizer(oldAccessor, matchingFinalizer)
}
```
<br>

when we delete object  in foreground. the apiserver will add deletetimestamp and finalizer for this object；

and the gc controller will handler this update event,  and the set deletingDependents=true

https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/garbagecollector/graph_builder.go#L147
```
UpdateFunc: func(oldObj, newObj interface{}) {
			// TODO: check if there are differences in the ownerRefs,
			// finalizers, and DeletionTimestamp; if not, ignore the update.
			event := &event{
				eventType: updateEvent,
				obj:       newObj,
				oldObj:    oldObj,
				gvk:       kind,
			}
			gb.graphChanges.Add(event)
```
**but if  gc controller  lose the event。deletingDependents is always false。because of deletionStartsWithFinalizer!!**

**so i think gc controller should Considering the circumstances。**

### What did you expect to happen?

 i think gc controller should Considering the circumstances. And deploy can be deleted!

### How can we reproduce it (as minimally and precisely as possible)?

it is hard to reproduce lost the update event lose, because it is a low probability event。but you can hardcord it like this:

https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/garbagecollector/graph_builder.go#L147
```
UpdateFunc: func(oldObj, newObj interface{}) {
			// TODO: check if there are differences in the ownerRefs,
			// finalizers, and DeletionTimestamp; if not, ignore the update.
+			newRs, ok := newObj.(*apps.ReplicaSet)
+			if ok {
+				if newRs.Namespace == "test-test" {
+					oldRs, _ := oldObj.(apps.ReplicaSet)
+					if (oldRs.DeletionTimestamp ==nil && len(oldRs.Finalizers)==0) && (newRs.DeletionTimestamp!=nil && len(newRs.Finalizers)==1){
+						klog.V(4).Infof("skip first delete event for %s", newRs.Name, kind.String())
+						return
+					}
+				}
+			}
			event := &event{
				eventType: updateEvent,
				obj:       newObj,
				oldObj:    oldObj,
				gvk:       kind,
			}
			gb.graphChanges.Add(event)
		},
```
after modify the UpdateFunc to Simulate。when you delete the deploy in test-test namespace in foreground cascading deletion。you will see:

(1) the deploy has deletetimeStamp and foreground finalizer

(2) the rs has deletetimeStamp and foreground finalizer

(3) the pod is running, has not deletetimeStamp!

### Anything else we need to know?

_No response_

### Kubernetes version

v1.17.4
but  i test the lastest kube-controller-manager version, it still exist the problem。


### Cloud provider

in all Cloud provider. because it related to kube-controller-manager garbagecollector runtime logic。



### OS version

_No response_

### Install tools

_No response_

### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

_No response_