From 09bdf76b8a59f82a38ccf24d318e898b0e9e8716 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Thu, 12 Nov 2020 12:00:10 -0500
Subject: [PATCH 01/13] Plumb event recorder to garbage collector controller

---
 cmd/kube-controller-manager/app/core.go             |  1 +
 pkg/controller/garbagecollector/BUILD               |  4 ++++
 pkg/controller/garbagecollector/garbagecollector.go | 13 +++++++++++++
 .../garbagecollector/garbagecollector_test.go       |  6 +++---
 pkg/controller/garbagecollector/graph_builder.go    |  3 +++
 5 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go
index 832fcddac10..15dbc0586c8 100644
--- a/cmd/kube-controller-manager/app/core.go
+++ b/cmd/kube-controller-manager/app/core.go
@@ -553,6 +553,7 @@ func startGarbageCollectorController(ctx ControllerContext) (http.Handler, bool,
 		ignoredResources[schema.GroupResource{Group: r.Group, Resource: r.Resource}] = struct{}{}
 	}
 	garbageCollector, err := garbagecollector.NewGarbageCollector(
+		gcClientset,
 		metadataClient,
 		ctx.RESTMapper,
 		ignoredResources,
diff --git a/pkg/controller/garbagecollector/BUILD b/pkg/controller/garbagecollector/BUILD
index 138143ed97b..3a534936465 100644
--- a/pkg/controller/garbagecollector/BUILD
+++ b/pkg/controller/garbagecollector/BUILD
@@ -20,7 +20,9 @@ go_library(
     ],
     importpath = "k8s.io/kubernetes/pkg/controller/garbagecollector",
     deps = [
+        "//pkg/controller/apis/config/scheme:go_default_library",
         "//pkg/controller/garbagecollector/metaonly:go_default_library",
+        "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
@@ -33,8 +35,10 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//staging/src/k8s.io/client-go/discovery:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
+        "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
         "//staging/src/k8s.io/client-go/metadata:go_default_library",
         "//staging/src/k8s.io/client-go/tools/cache:go_default_library",
+        "//staging/src/k8s.io/client-go/tools/record:go_default_library",
         "//staging/src/k8s.io/client-go/util/retry:go_default_library",
         "//staging/src/k8s.io/client-go/util/workqueue:go_default_library",
         "//staging/src/k8s.io/controller-manager/pkg/informerfactory:go_default_library",
diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go
index ef1e9bc8b5b..872520fc359 100644
--- a/pkg/controller/garbagecollector/garbagecollector.go
+++ b/pkg/controller/garbagecollector/garbagecollector.go
@@ -25,6 +25,7 @@ import (
 
 	"k8s.io/klog/v2"
 
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -35,10 +36,14 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/discovery"
+	clientset "k8s.io/client-go/kubernetes"
+	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/metadata"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/controller-manager/pkg/informerfactory"
+	"k8s.io/kubernetes/pkg/controller/apis/config/scheme"
 
 	// import known versions
 	_ "k8s.io/client-go/kubernetes"
@@ -74,12 +79,19 @@ type GarbageCollector struct {
 
 // NewGarbageCollector creates a new GarbageCollector.
 func NewGarbageCollector(
+	kubeClient clientset.Interface,
 	metadataClient metadata.Interface,
 	mapper resettableRESTMapper,
 	ignoredResources map[schema.GroupResource]struct{},
 	sharedInformers informerfactory.InformerFactory,
 	informersStarted <-chan struct{},
 ) (*GarbageCollector, error) {
+
+	eventBroadcaster := record.NewBroadcaster()
+	eventBroadcaster.StartStructuredLogging(0)
+	eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")})
+	eventRecorder := eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "garbage-collector-controller"})
+
 	attemptToDelete := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_attempt_to_delete")
 	attemptToOrphan := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_attempt_to_orphan")
 	absentOwnerCache := NewUIDCache(500)
@@ -91,6 +103,7 @@ func NewGarbageCollector(
 		absentOwnerCache: absentOwnerCache,
 	}
 	gc.dependencyGraphBuilder = &GraphBuilder{
+		eventRecorder:    eventRecorder,
 		metadataClient:   metadataClient,
 		informersStarted: informersStarted,
 		restMapper:       mapper,
diff --git a/pkg/controller/garbagecollector/garbagecollector_test.go b/pkg/controller/garbagecollector/garbagecollector_test.go
index 0752ec6b2a5..f98704d9953 100644
--- a/pkg/controller/garbagecollector/garbagecollector_test.go
+++ b/pkg/controller/garbagecollector/garbagecollector_test.go
@@ -81,7 +81,7 @@ func TestGarbageCollectorConstruction(t *testing.T) {
 	// construction will not fail.
 	alwaysStarted := make(chan struct{})
 	close(alwaysStarted)
-	gc, err := NewGarbageCollector(metadataClient, rm, map[schema.GroupResource]struct{}{},
+	gc, err := NewGarbageCollector(client, metadataClient, rm, map[schema.GroupResource]struct{}{},
 		informerfactory.NewInformerFactory(sharedInformers, metadataInformers), alwaysStarted)
 	if err != nil {
 		t.Fatal(err)
@@ -202,7 +202,7 @@ func setupGC(t *testing.T, config *restclient.Config) garbageCollector {
 	sharedInformers := informers.NewSharedInformerFactory(client, 0)
 	alwaysStarted := make(chan struct{})
 	close(alwaysStarted)
-	gc, err := NewGarbageCollector(metadataClient, &testRESTMapper{testrestmapper.TestOnlyStaticRESTMapper(legacyscheme.Scheme)}, ignoredResources, sharedInformers, alwaysStarted)
+	gc, err := NewGarbageCollector(client, metadataClient, &testRESTMapper{testrestmapper.TestOnlyStaticRESTMapper(legacyscheme.Scheme)}, ignoredResources, sharedInformers, alwaysStarted)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -829,7 +829,7 @@ func TestGarbageCollectorSync(t *testing.T) {
 	sharedInformers := informers.NewSharedInformerFactory(client, 0)
 	alwaysStarted := make(chan struct{})
 	close(alwaysStarted)
-	gc, err := NewGarbageCollector(metadataClient, rm, map[schema.GroupResource]struct{}{}, sharedInformers, alwaysStarted)
+	gc, err := NewGarbageCollector(client, metadataClient, rm, map[schema.GroupResource]struct{}{}, sharedInformers, alwaysStarted)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go
index e392eb52f58..7aba0de1604 100644
--- a/pkg/controller/garbagecollector/graph_builder.go
+++ b/pkg/controller/garbagecollector/graph_builder.go
@@ -33,6 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/metadata"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/controller-manager/pkg/informerfactory"
 	"k8s.io/kubernetes/pkg/controller/garbagecollector/metaonly"
@@ -89,6 +90,8 @@ type GraphBuilder struct {
 	// it is protected by monitorLock.
 	running bool
 
+	eventRecorder record.EventRecorder
+
 	metadataClient metadata.Interface
 	// monitors are the producer of the graphChanges queue, graphBuilder alters
 	// the in-memory graph according to the changes.

From 14f7f3201fc4a998a48257032ba557372bda2ddb Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Mon, 3 Feb 2020 12:26:33 -0500
Subject: [PATCH 02/13] Add GC integration race test

---
 test/integration/garbagecollector/BUILD       |   1 +
 .../garbage_collector_test.go                 | 139 ++++++++++++++++++
 2 files changed, 140 insertions(+)

diff --git a/test/integration/garbagecollector/BUILD b/test/integration/garbagecollector/BUILD
index 9f9fb6065ca..419ee794c31 100644
--- a/test/integration/garbagecollector/BUILD
+++ b/test/integration/garbagecollector/BUILD
@@ -37,6 +37,7 @@ go_test(
         "//staging/src/k8s.io/controller-manager/pkg/informerfactory:go_default_library",
         "//test/integration:go_default_library",
         "//test/integration/framework:go_default_library",
+        "//vendor/k8s.io/utils/pointer:go_default_library",
     ],
 )
 
diff --git a/test/integration/garbagecollector/garbage_collector_test.go b/test/integration/garbagecollector/garbage_collector_test.go
index 69ffeb33f9b..0ddd1e0bf07 100644
--- a/test/integration/garbagecollector/garbage_collector_test.go
+++ b/test/integration/garbagecollector/garbage_collector_test.go
@@ -50,6 +50,7 @@ import (
 	"k8s.io/kubernetes/pkg/controller/garbagecollector"
 	"k8s.io/kubernetes/test/integration"
 	"k8s.io/kubernetes/test/integration/framework"
+	"k8s.io/utils/pointer"
 )
 
 func getForegroundOptions() metav1.DeleteOptions {
@@ -246,6 +247,7 @@ func setupWithServer(t *testing.T, result *kubeapiservertesting.TestServer, work
 	alwaysStarted := make(chan struct{})
 	close(alwaysStarted)
 	gc, err := garbagecollector.NewGarbageCollector(
+		clientSet,
 		metadataClient,
 		restMapper,
 		garbagecollector.DefaultIgnoredResources(),
@@ -314,6 +316,143 @@ func deleteNamespaceOrDie(name string, c clientset.Interface, t *testing.T) {
 	}
 }
 
+func TestCrossNamespaceReferencesWithWatchCache(t *testing.T) {
+	testCrossNamespaceReferences(t, true)
+}
+func TestCrossNamespaceReferencesWithoutWatchCache(t *testing.T) {
+	testCrossNamespaceReferences(t, false)
+}
+
+func testCrossNamespaceReferences(t *testing.T, watchCache bool) {
+	var (
+		workers            = 5
+		validChildrenCount = 10
+		namespaceB         = "b"
+		namespaceA         = "a"
+	)
+
+	// Start the server
+	testServer := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{fmt.Sprintf("--watch-cache=%v", watchCache)}, framework.SharedEtcd())
+	defer func() {
+		if testServer != nil {
+			testServer.TearDownFn()
+		}
+	}()
+	clientSet, err := clientset.NewForConfig(testServer.ClientConfig)
+	if err != nil {
+		t.Fatalf("error creating clientset: %v", err)
+	}
+
+	createNamespaceOrDie(namespaceB, clientSet, t)
+	parent, err := clientSet.CoreV1().ConfigMaps(namespaceB).Create(context.TODO(), &v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "parent"}}, metav1.CreateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	for i := 0; i < validChildrenCount; i++ {
+		_, err := clientSet.CoreV1().Secrets(namespaceB).Create(context.TODO(), &v1.Secret{ObjectMeta: metav1.ObjectMeta{GenerateName: "child-", OwnerReferences: []metav1.OwnerReference{
+			{Name: "parent", Kind: "ConfigMap", APIVersion: "v1", UID: parent.UID, Controller: pointer.BoolPtr(false)},
+		}}}, metav1.CreateOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	createNamespaceOrDie(namespaceA, clientSet, t)
+
+	// Construct invalid owner references:
+	invalidOwnerReferences := []metav1.OwnerReference{}
+	for i := 0; i < 25; i++ {
+		invalidOwnerReferences = append(invalidOwnerReferences, metav1.OwnerReference{Name: "invalid", UID: types.UID(fmt.Sprintf("invalid-%d", i)), APIVersion: "test/v1", Kind: fmt.Sprintf("invalid%d", i)})
+	}
+	invalidOwnerReferences = append(invalidOwnerReferences, metav1.OwnerReference{Name: "invalid", UID: parent.UID, APIVersion: "v1", Kind: "Pod", Controller: pointer.BoolPtr(false)})
+
+	invalidUIDs := []types.UID{}
+	for i := 0; i < workers; i++ {
+		invalidChildType1, err := clientSet.CoreV1().ConfigMaps(namespaceA).Create(context.TODO(), &v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{GenerateName: "invalid-child-", OwnerReferences: invalidOwnerReferences}}, metav1.CreateOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+		invalidChildType2, err := clientSet.CoreV1().Secrets(namespaceA).Create(context.TODO(), &v1.Secret{ObjectMeta: metav1.ObjectMeta{GenerateName: "invalid-child-a-", OwnerReferences: invalidOwnerReferences}}, metav1.CreateOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+		invalidChildType3, err := clientSet.CoreV1().Secrets(namespaceA).Create(context.TODO(), &v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Labels:          map[string]string{"single-bad-reference": "true"},
+				GenerateName:    "invalid-child-b-",
+				OwnerReferences: []metav1.OwnerReference{{Name: "invalid", UID: parent.UID, APIVersion: "v1", Kind: "Pod", Controller: pointer.BoolPtr(false)}},
+			},
+		}, metav1.CreateOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+		invalidUIDs = append(invalidUIDs, invalidChildType1.UID, invalidChildType2.UID, invalidChildType3.UID)
+	}
+
+	// start GC with existing objects in place to simulate controller-manager restart
+	ctx := setupWithServer(t, testServer, workers)
+	defer ctx.tearDown()
+	testServer = nil
+
+	// Wait for the invalid children to be garbage collected
+	if err := wait.PollImmediate(time.Second, 10*time.Second, func() (bool, error) {
+		children, err := clientSet.CoreV1().Secrets(namespaceA).List(context.TODO(), metav1.ListOptions{LabelSelector: "single-bad-reference=true"})
+		if err != nil {
+			return false, err
+		}
+		if len(children.Items) > 0 {
+			t.Logf("expected 0 invalid children, got %d, will wait and relist", len(children.Items))
+			return false, nil
+		}
+		return true, nil
+	}); err != nil && err != wait.ErrWaitTimeout {
+		t.Error(err)
+	}
+
+	// Wait for a little while to make sure they didn't trigger deletion of the valid children
+	if err := wait.Poll(time.Second, 5*time.Second, func() (bool, error) {
+		children, err := clientSet.CoreV1().Secrets(namespaceB).List(context.TODO(), metav1.ListOptions{})
+		if err != nil {
+			return false, err
+		}
+		if len(children.Items) != validChildrenCount {
+			return false, fmt.Errorf("expected %d valid children, got %d", validChildrenCount, len(children.Items))
+		}
+		return false, nil
+	}); err != nil && err != wait.ErrWaitTimeout {
+		t.Error(err)
+	}
+
+	if !ctx.gc.GraphHasUID(parent.UID) {
+		t.Errorf("valid parent UID no longer exists in the graph")
+	}
+
+	// Now that our graph has correct data in it, add a new invalid child and see if it gets deleted
+	invalidChild, err := clientSet.CoreV1().Secrets(namespaceA).Create(context.TODO(), &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName:    "invalid-child-c-",
+			OwnerReferences: []metav1.OwnerReference{{Name: "invalid", UID: parent.UID, APIVersion: "v1", Kind: "Pod", Controller: pointer.BoolPtr(false)}},
+		},
+	}, metav1.CreateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Wait for the invalid child to be garbage collected
+	if err := wait.PollImmediate(time.Second, 10*time.Second, func() (bool, error) {
+		_, err := clientSet.CoreV1().Secrets(namespaceA).Get(context.TODO(), invalidChild.Name, metav1.GetOptions{})
+		if apierrors.IsNotFound(err) {
+			return true, nil
+		}
+		if err != nil {
+			return false, err
+		}
+		t.Logf("%s remains, waiting for deletion", invalidChild.Name)
+		return false, nil
+	}); err != nil {
+		t.Fatal(err)
+	}
+}
+
 // This test simulates the cascading deletion.
 func TestCascadingDeletion(t *testing.T) {
 	ctx := setup(t, 5)

From 445f20dbdb460ba3f2c72cbb599492d0e1071bc3 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Fri, 26 Jun 2020 20:48:21 -0400
Subject: [PATCH 03/13] Switch GC absentOwnerCache to full reference

Before deleting an object based on absent owners, GC verifies absence of those owners with a live lookup.

The coordinates used to perform that live lookup are the ones specified in the ownerReference of the child.

In order to performantly delete multiple children from the same parent (e.g. 1000 pods from a replicaset),
a 404 response to a lookup is cached in absentOwnerCache.

Previously, the cache was a simple uid set. However, since children can disagree on the coordinates
that should be used to look up a given uid, the cache should record the exact coordinates verified absent.
This is a [apiVersion, kind, namespace, name, uid] tuple.
---
 pkg/controller/garbagecollector/BUILD         |  1 +
 .../garbagecollector/garbagecollector.go      | 23 +++++++++++++++----
 .../garbagecollector/garbagecollector_test.go | 15 +++++++-----
 pkg/controller/garbagecollector/graph.go      | 11 +++++++++
 .../garbagecollector/graph_builder.go         | 16 ++++++++++---
 pkg/controller/garbagecollector/uid_cache.go  | 19 ++++++++-------
 6 files changed, 61 insertions(+), 24 deletions(-)

diff --git a/pkg/controller/garbagecollector/BUILD b/pkg/controller/garbagecollector/BUILD
index 3a534936465..792ed2e1a9a 100644
--- a/pkg/controller/garbagecollector/BUILD
+++ b/pkg/controller/garbagecollector/BUILD
@@ -83,6 +83,7 @@ go_test(
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/gonum.org/v1/gonum/graph:go_default_library",
         "//vendor/gonum.org/v1/gonum/graph/simple:go_default_library",
+        "//vendor/k8s.io/utils/pointer:go_default_library",
     ],
 )
 
diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go
index 872520fc359..341bb921237 100644
--- a/pkg/controller/garbagecollector/garbagecollector.go
+++ b/pkg/controller/garbagecollector/garbagecollector.go
@@ -72,7 +72,7 @@ type GarbageCollector struct {
 	attemptToOrphan        workqueue.RateLimitingInterface
 	dependencyGraphBuilder *GraphBuilder
 	// GC caches the owners that do not exist according to the API server.
-	absentOwnerCache *UIDCache
+	absentOwnerCache *ReferenceCache
 
 	workerLock sync.RWMutex
 }
@@ -94,7 +94,7 @@ func NewGarbageCollector(
 
 	attemptToDelete := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_attempt_to_delete")
 	attemptToOrphan := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_attempt_to_orphan")
-	absentOwnerCache := NewUIDCache(500)
+	absentOwnerCache := NewReferenceCache(500)
 	gc := &GarbageCollector{
 		metadataClient:   metadataClient,
 		restMapper:       mapper,
@@ -338,10 +338,20 @@ func (gc *GarbageCollector) attemptToDeleteWorker() bool {
 // returns its latest state.
 func (gc *GarbageCollector) isDangling(reference metav1.OwnerReference, item *node) (
 	dangling bool, owner *metav1.PartialObjectMetadata, err error) {
-	if gc.absentOwnerCache.Has(reference.UID) {
+
+	// check for recorded absent cluster-scoped parent
+	absentOwnerCacheKey := objectReference{OwnerReference: ownerReferenceCoordinates(reference)}
+	if gc.absentOwnerCache.Has(absentOwnerCacheKey) {
 		klog.V(5).Infof("according to the absentOwnerCache, object %s's owner %s/%s, %s does not exist", item.identity.UID, reference.APIVersion, reference.Kind, reference.Name)
 		return true, nil, nil
 	}
+	// check for recorded absent namespaced parent
+	absentOwnerCacheKey.Namespace = item.identity.Namespace
+	if gc.absentOwnerCache.Has(absentOwnerCacheKey) {
+		klog.V(5).Infof("according to the absentOwnerCache, object %s's owner %s/%s, %s does not exist in namespace %s", item.identity.UID, reference.APIVersion, reference.Kind, reference.Name, item.identity.Namespace)
+		return true, nil, nil
+	}
+
 	// TODO: we need to verify the reference resource is supported by the
 	// system. If it's not a valid resource, the garbage collector should i)
 	// ignore the reference when decide if the object should be deleted, and
@@ -352,6 +362,9 @@ func (gc *GarbageCollector) isDangling(reference metav1.OwnerReference, item *no
 	if err != nil {
 		return false, nil, err
 	}
+	if !namespaced {
+		absentOwnerCacheKey.Namespace = ""
+	}
 
 	// TODO: It's only necessary to talk to the API server if the owner node
 	// is a "virtual" node. The local graph could lag behind the real
@@ -359,7 +372,7 @@ func (gc *GarbageCollector) isDangling(reference metav1.OwnerReference, item *no
 	owner, err = gc.metadataClient.Resource(resource).Namespace(resourceDefaultNamespace(namespaced, item.identity.Namespace)).Get(context.TODO(), reference.Name, metav1.GetOptions{})
 	switch {
 	case errors.IsNotFound(err):
-		gc.absentOwnerCache.Add(reference.UID)
+		gc.absentOwnerCache.Add(absentOwnerCacheKey)
 		klog.V(5).Infof("object %s's owner %s/%s, %s is not found", item.identity.UID, reference.APIVersion, reference.Kind, reference.Name)
 		return true, nil, nil
 	case err != nil:
@@ -368,7 +381,7 @@ func (gc *GarbageCollector) isDangling(reference metav1.OwnerReference, item *no
 
 	if owner.GetUID() != reference.UID {
 		klog.V(5).Infof("object %s's owner %s/%s, %s is not found, UID mismatch", item.identity.UID, reference.APIVersion, reference.Kind, reference.Name)
-		gc.absentOwnerCache.Add(reference.UID)
+		gc.absentOwnerCache.Add(absentOwnerCacheKey)
 		return true, nil, nil
 	}
 	return false, owner, nil
diff --git a/pkg/controller/garbagecollector/garbagecollector_test.go b/pkg/controller/garbagecollector/garbagecollector_test.go
index f98704d9953..caeb0910bd4 100644
--- a/pkg/controller/garbagecollector/garbagecollector_test.go
+++ b/pkg/controller/garbagecollector/garbagecollector_test.go
@@ -29,6 +29,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	_ "k8s.io/kubernetes/pkg/apis/core/install"
+	"k8s.io/utils/pointer"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -398,7 +399,7 @@ func TestProcessEvent(t *testing.T) {
 				uidToNode:     make(map[types.UID]*node),
 			},
 			attemptToDelete:  workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
-			absentOwnerCache: NewUIDCache(2),
+			absentOwnerCache: NewReferenceCache(2),
 		}
 		for i := 0; i < len(scenario.events); i++ {
 			dependencyGraphBuilder.graphChanges.Add(&scenario.events[i])
@@ -459,13 +460,14 @@ func podToGCNode(pod *v1.Pod) *node {
 	}
 }
 
-func TestAbsentUIDCache(t *testing.T) {
+func TestAbsentOwnerCache(t *testing.T) {
 	rc1Pod1 := getPod("rc1Pod1", []metav1.OwnerReference{
 		{
 			Kind:       "ReplicationController",
 			Name:       "rc1",
 			UID:        "1",
 			APIVersion: "v1",
+			Controller: pointer.BoolPtr(true),
 		},
 	})
 	rc1Pod2 := getPod("rc1Pod2", []metav1.OwnerReference{
@@ -474,6 +476,7 @@ func TestAbsentUIDCache(t *testing.T) {
 			Name:       "rc1",
 			UID:        "1",
 			APIVersion: "v1",
+			Controller: pointer.BoolPtr(false),
 		},
 	})
 	rc2Pod1 := getPod("rc2Pod1", []metav1.OwnerReference{
@@ -528,7 +531,7 @@ func TestAbsentUIDCache(t *testing.T) {
 	defer srv.Close()
 	gc := setupGC(t, clientConfig)
 	defer close(gc.stop)
-	gc.absentOwnerCache = NewUIDCache(2)
+	gc.absentOwnerCache = NewReferenceCache(2)
 	gc.attemptToDeleteItem(podToGCNode(rc1Pod1))
 	gc.attemptToDeleteItem(podToGCNode(rc2Pod1))
 	// rc1 should already be in the cache, no request should be sent. rc1 should be promoted in the UIDCache
@@ -536,13 +539,13 @@ func TestAbsentUIDCache(t *testing.T) {
 	// after this call, rc2 should be evicted from the UIDCache
 	gc.attemptToDeleteItem(podToGCNode(rc3Pod1))
 	// check cache
-	if !gc.absentOwnerCache.Has(types.UID("1")) {
+	if !gc.absentOwnerCache.Has(objectReference{Namespace: "ns1", OwnerReference: metav1.OwnerReference{Kind: "ReplicationController", Name: "rc1", UID: "1", APIVersion: "v1"}}) {
 		t.Errorf("expected rc1 to be in the cache")
 	}
-	if gc.absentOwnerCache.Has(types.UID("2")) {
+	if gc.absentOwnerCache.Has(objectReference{Namespace: "ns1", OwnerReference: metav1.OwnerReference{Kind: "ReplicationController", Name: "rc2", UID: "2", APIVersion: "v1"}}) {
 		t.Errorf("expected rc2 to not exist in the cache")
 	}
-	if !gc.absentOwnerCache.Has(types.UID("3")) {
+	if !gc.absentOwnerCache.Has(objectReference{Namespace: "ns1", OwnerReference: metav1.OwnerReference{Kind: "ReplicationController", Name: "rc3", UID: "3", APIVersion: "v1"}}) {
 		t.Errorf("expected rc3 to be in the cache")
 	}
 	// check the request sent to the server
diff --git a/pkg/controller/garbagecollector/graph.go b/pkg/controller/garbagecollector/graph.go
index 2d19a150dd6..252a56e98a9 100644
--- a/pkg/controller/garbagecollector/graph.go
+++ b/pkg/controller/garbagecollector/graph.go
@@ -148,6 +148,17 @@ func (n *node) blockingDependents() []*node {
 	return ret
 }
 
+// ownerReferenceCoordinates returns an owner reference containing only the coordinate fields
+// from the input reference (uid, name, kind, apiVersion)
+func ownerReferenceCoordinates(ref metav1.OwnerReference) metav1.OwnerReference {
+	return metav1.OwnerReference{
+		UID:        ref.UID,
+		Name:       ref.Name,
+		Kind:       ref.Kind,
+		APIVersion: ref.APIVersion,
+	}
+}
+
 // String renders node as a string using fmt. Acquires a read lock to ensure the
 // reflective dump of dependents doesn't race with any concurrent writes.
 func (n *node) String() string {
diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go
index 7aba0de1604..734eaea8bd7 100644
--- a/pkg/controller/garbagecollector/graph_builder.go
+++ b/pkg/controller/garbagecollector/graph_builder.go
@@ -104,7 +104,7 @@ type GraphBuilder struct {
 	attemptToOrphan workqueue.RateLimitingInterface
 	// GraphBuilder and GC share the absentOwnerCache. Objects that are known to
 	// be non-existent are added to the cached.
-	absentOwnerCache *UIDCache
+	absentOwnerCache *ReferenceCache
 	sharedInformers  informerfactory.InformerFactory
 	ignoredResources map[schema.GroupResource]struct{}
 }
@@ -327,8 +327,10 @@ func DefaultIgnoredResources() map[schema.GroupResource]struct{} {
 // enqueueVirtualDeleteEvent is used to add a virtual delete event to be processed for virtual nodes
 // once it is determined they do not have backing objects in storage
 func (gb *GraphBuilder) enqueueVirtualDeleteEvent(ref objectReference) {
+	gv, _ := schema.ParseGroupVersion(ref.APIVersion)
 	gb.graphChanges.Add(&event{
 		eventType: deleteEvent,
+		gvk:       gv.WithKind(ref.Kind),
 		obj: &metaonly.MetadataOnlyObject{
 			TypeMeta:   metav1.TypeMeta{APIVersion: ref.APIVersion, Kind: ref.Kind},
 			ObjectMeta: metav1.ObjectMeta{Namespace: ref.Namespace, UID: ref.UID, Name: ref.Name},
@@ -348,7 +350,7 @@ func (gb *GraphBuilder) addDependentToOwners(n *node, owners []metav1.OwnerRefer
 			// exist in the graph yet.
 			ownerNode = &node{
 				identity: objectReference{
-					OwnerReference: owner,
+					OwnerReference: ownerReferenceCoordinates(owner),
 					Namespace:      n.identity.Namespace,
 				},
 				dependents: make(map[*node]struct{}),
@@ -603,7 +605,15 @@ func (gb *GraphBuilder) processGraphChanges() bool {
 		existingNode.dependentsLock.RLock()
 		defer existingNode.dependentsLock.RUnlock()
 		if len(existingNode.dependents) > 0 {
-			gb.absentOwnerCache.Add(accessor.GetUID())
+			gb.absentOwnerCache.Add(objectReference{
+				OwnerReference: metav1.OwnerReference{
+					APIVersion: event.gvk.GroupVersion().String(),
+					Kind:       event.gvk.Kind,
+					Name:       accessor.GetName(),
+					UID:        accessor.GetUID(),
+				},
+				Namespace: accessor.GetNamespace(),
+			})
 		}
 		for dep := range existingNode.dependents {
 			gb.attemptToDelete.Add(dep)
diff --git a/pkg/controller/garbagecollector/uid_cache.go b/pkg/controller/garbagecollector/uid_cache.go
index 3ad40c32b0f..6206ac80738 100644
--- a/pkg/controller/garbagecollector/uid_cache.go
+++ b/pkg/controller/garbagecollector/uid_cache.go
@@ -20,33 +20,32 @@ import (
 	"sync"
 
 	"github.com/golang/groupcache/lru"
-	"k8s.io/apimachinery/pkg/types"
 )
 
-// UIDCache is an LRU cache for uid.
-type UIDCache struct {
+// ReferenceCache is an LRU cache for uid.
+type ReferenceCache struct {
 	mutex sync.Mutex
 	cache *lru.Cache
 }
 
-// NewUIDCache returns a UIDCache.
-func NewUIDCache(maxCacheEntries int) *UIDCache {
-	return &UIDCache{
+// NewReferenceCache returns a ReferenceCache.
+func NewReferenceCache(maxCacheEntries int) *ReferenceCache {
+	return &ReferenceCache{
 		cache: lru.New(maxCacheEntries),
 	}
 }
 
 // Add adds a uid to the cache.
-func (c *UIDCache) Add(uid types.UID) {
+func (c *ReferenceCache) Add(reference objectReference) {
 	c.mutex.Lock()
 	defer c.mutex.Unlock()
-	c.cache.Add(uid, nil)
+	c.cache.Add(reference, nil)
 }
 
 // Has returns if a uid is in the cache.
-func (c *UIDCache) Has(uid types.UID) bool {
+func (c *ReferenceCache) Has(reference objectReference) bool {
 	c.mutex.Lock()
 	defer c.mutex.Unlock()
-	_, found := c.cache.Get(uid)
+	_, found := c.cache.Get(reference)
 	return found
 }

From 30eb6683e6f2840a9e8272fa32c083fe189439bf Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Fri, 26 Jun 2020 21:09:54 -0400
Subject: [PATCH 04/13] Avoid marking virtual nodes as observed when they
 haven't been

Virtual nodes can be added to the GC graph in order to represent objects
which have not been observed via an informer, but are referenced via ownerReferences.

These virtual nodes are requeued into attemptToDelete until they are observed via an informer,
or successfully verified absent via a live lookup. Previously, both of those code paths
called markObserved() to stop requeuing into attemptToDelete.

Because it is useful to know whether a particular node has been observed via
a real informer event, this commit does the following:

* adds a `virtual bool` attribute to graph events so we know which ones came from a real informer
* limits the markObserved() call to the code path where a real informer event is observed
* uses an alternative mechanism to stop requeueing into attemptToDelete when a virtual node is verified absent via a live lookup
---
 .../garbagecollector/garbagecollector.go      | 26 ++++++++++++-------
 .../garbagecollector/garbagecollector_test.go |  6 ++++-
 .../garbagecollector/graph_builder.go         |  7 +++--
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go
index 341bb921237..44840c399f8 100644
--- a/pkg/controller/garbagecollector/garbagecollector.go
+++ b/pkg/controller/garbagecollector/garbagecollector.go
@@ -18,6 +18,7 @@ package garbagecollector
 
 import (
 	"context"
+	goerrors "errors"
 	"fmt"
 	"reflect"
 	"sync"
@@ -294,6 +295,8 @@ func (gc *GarbageCollector) runAttemptToDeleteWorker() {
 	}
 }
 
+var enqueuedVirtualDeleteEventErr = goerrors.New("enqueued virtual delete event")
+
 func (gc *GarbageCollector) attemptToDeleteWorker() bool {
 	item, quit := gc.attemptToDelete.Get()
 	gc.workerLock.RLock()
@@ -308,7 +311,10 @@ func (gc *GarbageCollector) attemptToDeleteWorker() bool {
 		return true
 	}
 	err := gc.attemptToDeleteItem(n)
-	if err != nil {
+	if err == enqueuedVirtualDeleteEventErr {
+		// a virtual event was produced and will be handled by processGraphChanges, no need to requeue this node
+		return true
+	} else if err != nil {
 		if _, ok := err.(*restMappingError); ok {
 			// There are at least two ways this can happen:
 			// 1. The reference is to an object of a custom type that has not yet been
@@ -426,9 +432,15 @@ func ownerRefsToUIDs(refs []metav1.OwnerReference) []types.UID {
 	return ret
 }
 
+// attemptToDeleteItem looks up the live API object associated with the node,
+// and issues a delete IFF the uid matches, the item is not blocked on deleting dependents,
+// and all owner references are dangling.
+//
+// if the API get request returns a NotFound error, or the retrieved item's uid does not match,
+// a virtual delete event for the node is enqueued and enqueuedVirtualDeleteEventErr is returned.
 func (gc *GarbageCollector) attemptToDeleteItem(item *node) error {
 	klog.V(2).InfoS("Processing object", "object", klog.KRef(item.identity.Namespace, item.identity.Name),
-		"objectUID", item.identity.UID, "kind", item.identity.Kind)
+		"objectUID", item.identity.UID, "kind", item.identity.Kind, "virtual", !item.isObserved())
 
 	// "being deleted" is an one-way trip to the final deletion. We'll just wait for the final deletion, and then process the object's dependents.
 	if item.isBeingDeleted() && !item.isDeletingDependents() {
@@ -446,10 +458,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(item *node) error {
 		// the virtual node from GraphBuilder.uidToNode.
 		klog.V(5).Infof("item %v not found, generating a virtual delete event", item.identity)
 		gc.dependencyGraphBuilder.enqueueVirtualDeleteEvent(item.identity)
-		// since we're manually inserting a delete event to remove this node,
-		// we don't need to keep tracking it as a virtual node and requeueing in attemptToDelete
-		item.markObserved()
-		return nil
+		return enqueuedVirtualDeleteEventErr
 	case err != nil:
 		return err
 	}
@@ -457,10 +466,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(item *node) error {
 	if latest.GetUID() != item.identity.UID {
 		klog.V(5).Infof("UID doesn't match, item %v not found, generating a virtual delete event", item.identity)
 		gc.dependencyGraphBuilder.enqueueVirtualDeleteEvent(item.identity)
-		// since we're manually inserting a delete event to remove this node,
-		// we don't need to keep tracking it as a virtual node and requeueing in attemptToDelete
-		item.markObserved()
-		return nil
+		return enqueuedVirtualDeleteEventErr
 	}
 
 	// TODO: attemptToOrphanWorker() routine is similar. Consider merging
diff --git a/pkg/controller/garbagecollector/garbagecollector_test.go b/pkg/controller/garbagecollector/garbagecollector_test.go
index caeb0910bd4..c5972088a63 100644
--- a/pkg/controller/garbagecollector/garbagecollector_test.go
+++ b/pkg/controller/garbagecollector/garbagecollector_test.go
@@ -274,12 +274,16 @@ func TestAttemptToDeleteItem(t *testing.T) {
 			Namespace: pod.Namespace,
 		},
 		// owners are intentionally left empty. The attemptToDeleteItem routine should get the latest item from the server.
-		owners: nil,
+		owners:  nil,
+		virtual: true,
 	}
 	err := gc.attemptToDeleteItem(item)
 	if err != nil {
 		t.Errorf("Unexpected Error: %v", err)
 	}
+	if !item.virtual {
+		t.Errorf("attemptToDeleteItem changed virtual to false unexpectedly")
+	}
 	expectedActionSet := sets.NewString()
 	expectedActionSet.Insert("GET=/api/v1/namespaces/ns1/replicationcontrollers/owner1")
 	expectedActionSet.Insert("DELETE=/api/v1/namespaces/ns1/pods/ToBeDeletedPod")
diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go
index 734eaea8bd7..bc1c4b1dc8a 100644
--- a/pkg/controller/garbagecollector/graph_builder.go
+++ b/pkg/controller/garbagecollector/graph_builder.go
@@ -61,6 +61,8 @@ const (
 )
 
 type event struct {
+	// virtual indicates this event did not come from an informer, but was constructed artificially
+	virtual   bool
 	eventType eventType
 	obj       interface{}
 	// the update event comes with an old object, but it's not used by the garbage collector.
@@ -329,6 +331,7 @@ func DefaultIgnoredResources() map[schema.GroupResource]struct{} {
 func (gb *GraphBuilder) enqueueVirtualDeleteEvent(ref objectReference) {
 	gv, _ := schema.ParseGroupVersion(ref.APIVersion)
 	gb.graphChanges.Add(&event{
+		virtual:   true,
 		eventType: deleteEvent,
 		gvk:       gv.WithKind(ref.Kind),
 		obj: &metaonly.MetadataOnlyObject{
@@ -545,10 +548,10 @@ func (gb *GraphBuilder) processGraphChanges() bool {
 		utilruntime.HandleError(fmt.Errorf("cannot access obj: %v", err))
 		return true
 	}
-	klog.V(5).Infof("GraphBuilder process object: %s/%s, namespace %s, name %s, uid %s, event type %v", event.gvk.GroupVersion().String(), event.gvk.Kind, accessor.GetNamespace(), accessor.GetName(), string(accessor.GetUID()), event.eventType)
+	klog.V(5).Infof("GraphBuilder process object: %s/%s, namespace %s, name %s, uid %s, event type %v, virtual=%v", event.gvk.GroupVersion().String(), event.gvk.Kind, accessor.GetNamespace(), accessor.GetName(), string(accessor.GetUID()), event.eventType, event.virtual)
 	// Check if the node already exists
 	existingNode, found := gb.uidToNode.Read(accessor.GetUID())
-	if found {
+	if found && !event.virtual && !existingNode.isObserved() {
 		// this marks the node as having been observed via an informer event
 		// 1. this depends on graphChanges only containing add/update events from the actual informer
 		// 2. this allows things tracking virtual nodes' existence to stop polling and rely on informer events

From cb7b9ed5327bd63b63d05e87a70fbcb288f67ccc Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Wed, 8 Jul 2020 00:48:38 -0400
Subject: [PATCH 05/13] Refactor identityFromEvent

---
 .../garbagecollector/graph_builder.go         | 32 ++++++++-----------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go
index bc1c4b1dc8a..ddf4037abc9 100644
--- a/pkg/controller/garbagecollector/graph_builder.go
+++ b/pkg/controller/garbagecollector/graph_builder.go
@@ -530,6 +530,18 @@ func (gb *GraphBuilder) runProcessGraphChanges() {
 	}
 }
 
+func identityFromEvent(event *event, accessor metav1.Object) objectReference {
+	return objectReference{
+		OwnerReference: metav1.OwnerReference{
+			APIVersion: event.gvk.GroupVersion().String(),
+			Kind:       event.gvk.Kind,
+			UID:        accessor.GetUID(),
+			Name:       accessor.GetName(),
+		},
+		Namespace: accessor.GetNamespace(),
+	}
+}
+
 // Dequeueing an event from graphChanges, updating graph, populating dirty_queue.
 func (gb *GraphBuilder) processGraphChanges() bool {
 	item, quit := gb.graphChanges.Get()
@@ -560,15 +572,7 @@ func (gb *GraphBuilder) processGraphChanges() bool {
 	switch {
 	case (event.eventType == addEvent || event.eventType == updateEvent) && !found:
 		newNode := &node{
-			identity: objectReference{
-				OwnerReference: metav1.OwnerReference{
-					APIVersion: event.gvk.GroupVersion().String(),
-					Kind:       event.gvk.Kind,
-					UID:        accessor.GetUID(),
-					Name:       accessor.GetName(),
-				},
-				Namespace: accessor.GetNamespace(),
-			},
+			identity:           identityFromEvent(event, accessor),
 			dependents:         make(map[*node]struct{}),
 			owners:             accessor.GetOwnerReferences(),
 			deletingDependents: beingDeleted(accessor) && hasDeleteDependentsFinalizer(accessor),
@@ -608,15 +612,7 @@ func (gb *GraphBuilder) processGraphChanges() bool {
 		existingNode.dependentsLock.RLock()
 		defer existingNode.dependentsLock.RUnlock()
 		if len(existingNode.dependents) > 0 {
-			gb.absentOwnerCache.Add(objectReference{
-				OwnerReference: metav1.OwnerReference{
-					APIVersion: event.gvk.GroupVersion().String(),
-					Kind:       event.gvk.Kind,
-					Name:       accessor.GetName(),
-					UID:        accessor.GetUID(),
-				},
-				Namespace: accessor.GetNamespace(),
-			})
+			gb.absentOwnerCache.Add(identityFromEvent(event, accessor))
 		}
 		for dep := range existingNode.dependents {
 			gb.attemptToDelete.Add(dep)

From cae56bea0a7192dfae234ef30c4ac19a6249dd7f Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Fri, 26 Jun 2020 23:51:24 -0400
Subject: [PATCH 06/13] Replace virtual node with observed node if identity
 differs

If the graph contains a virtual node (because some child object referenced it in an OwnerRef),
and a real informer event is observed for that uid at different coordinates,
we want to fix the coordinates of the node in the graph to match the actual coordinates.

The safe way to do this is to clone the node, replace the identity in the clone,
then replace the node with the clone.

Modifying the identity directly is not safe because it is accessed lock-free from many code paths.

Replacing the node in the graph from processGraphChanges is safe because it is the only graph writer.
---
 pkg/controller/garbagecollector/graph.go      | 19 +++++++++++++++++++
 .../garbagecollector/graph_builder.go         |  8 ++++++++
 2 files changed, 27 insertions(+)

diff --git a/pkg/controller/garbagecollector/graph.go b/pkg/controller/garbagecollector/graph.go
index 252a56e98a9..d98b83b7d54 100644
--- a/pkg/controller/garbagecollector/graph.go
+++ b/pkg/controller/garbagecollector/graph.go
@@ -61,6 +61,25 @@ type node struct {
 	owners []metav1.OwnerReference
 }
 
+// clone() must only be called from the single-threaded GraphBuilder.processGraphChanges()
+func (n *node) clone() *node {
+	c := &node{
+		identity:           n.identity,
+		dependents:         make(map[*node]struct{}, len(n.dependents)),
+		deletingDependents: n.deletingDependents,
+		beingDeleted:       n.beingDeleted,
+		virtual:            n.virtual,
+		owners:             make([]metav1.OwnerReference, 0, len(n.owners)),
+	}
+	for dep := range n.dependents {
+		c.dependents[dep] = struct{}{}
+	}
+	for _, owner := range n.owners {
+		c.owners = append(c.owners, owner)
+	}
+	return c
+}
+
 // An object is on a one way trip to its final deletion if it starts being
 // deleted, so we only provide a function to set beingDeleted to true.
 func (n *node) markBeingDeleted() {
diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go
index ddf4037abc9..1de1685f165 100644
--- a/pkg/controller/garbagecollector/graph_builder.go
+++ b/pkg/controller/garbagecollector/graph_builder.go
@@ -567,6 +567,14 @@ func (gb *GraphBuilder) processGraphChanges() bool {
 		// this marks the node as having been observed via an informer event
 		// 1. this depends on graphChanges only containing add/update events from the actual informer
 		// 2. this allows things tracking virtual nodes' existence to stop polling and rely on informer events
+		observedIdentity := identityFromEvent(event, accessor)
+		if observedIdentity != existingNode.identity {
+			// make a copy (so we don't modify the existing node in place), store the observed identity, and replace the virtual node
+			klog.V(2).Infof("replacing virtual node %s with observed node %s", existingNode.identity, observedIdentity)
+			existingNode = existingNode.clone()
+			existingNode.identity = observedIdentity
+			gb.uidToNode.Write(existingNode)
+		}
 		existingNode.markObserved()
 	}
 	switch {

From 78317edb8b8e0b0ec32442554fef31f3c2e0703a Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Thu, 8 Oct 2020 00:15:42 -0400
Subject: [PATCH 07/13] Short-circuit attemptToDelete loop for virtual nodes
 that are removed or observed

Virtual nodes are added to the attemptToDelete queue, and continue getting requeued
until they are successfully verified absent or are observed via informer.

In the meantime, if the real object associated with that UID is observed via informer,
or is observed to be deleted via informer, the graph node for that UID can be removed
or marked as observed. In that case, we should stop retrying to get the virtual node coordinates.
---
 .../garbagecollector/garbagecollector.go        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go
index 44840c399f8..537913ed7fd 100644
--- a/pkg/controller/garbagecollector/garbagecollector.go
+++ b/pkg/controller/garbagecollector/garbagecollector.go
@@ -310,6 +310,23 @@ func (gc *GarbageCollector) attemptToDeleteWorker() bool {
 		utilruntime.HandleError(fmt.Errorf("expect *node, got %#v", item))
 		return true
 	}
+
+	if !n.isObserved() {
+		nodeFromGraph, existsInGraph := gc.dependencyGraphBuilder.uidToNode.Read(n.identity.UID)
+		if !existsInGraph {
+			// this can happen if attemptToDelete loops on a requeued virtual node because attemptToDeleteItem returned an error,
+			// and in the meantime a deletion of the real object associated with that uid was observed
+			klog.V(5).Infof("item %s no longer in the graph, skipping attemptToDeleteItem", n)
+			return true
+		}
+		if nodeFromGraph.isObserved() {
+			// this can happen if attemptToDelete loops on a requeued virtual node because attemptToDeleteItem returned an error,
+			// and in the meantime the real object associated with that uid was observed
+			klog.V(5).Infof("item %s no longer virtual in the graph, skipping attemptToDeleteItem on virtual node", n)
+			return true
+		}
+	}
+
 	err := gc.attemptToDeleteItem(n)
 	if err == enqueuedVirtualDeleteEventErr {
 		// a virtual event was produced and will be handled by processGraphChanges, no need to requeue this node

From ac8d419b4c7aa44273573cf05effab79feff482d Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Thu, 2 Jul 2020 01:04:13 -0400
Subject: [PATCH 08/13] Enqueue dependents for deletion when their
 ownerReference does not match observed parent coordinates

When adding a dependent to the graph, we ensure there is a node representing each owner reference,
and add the dependent to each parent node.

If the parent node already exists, and the dependent's ownerReference
coordinates disagree with the verified coordinates, add the dependent to the attemptToDelete queue.

This queue will check the dependent's ownerReferences using the coordinates specified by the dependent.
If all of the owners can be verified absent, the dependent will be deleted.
If some are still present, or if there are errors looking them up, the dependent will not be deleted.

If the parent node has been observed via informer event (so we know the coordinates are accurate),
and the verified owner is namespaced, and the dependent is not in the same namespace,
an event will be recorded for user visibility, since cross-namespace ownerReferences are not supported.
---
 pkg/controller/garbagecollector/graph.go      |  6 ++
 .../garbagecollector/graph_builder.go         | 64 +++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/pkg/controller/garbagecollector/graph.go b/pkg/controller/garbagecollector/graph.go
index d98b83b7d54..b2fd8002d5c 100644
--- a/pkg/controller/garbagecollector/graph.go
+++ b/pkg/controller/garbagecollector/graph.go
@@ -178,6 +178,12 @@ func ownerReferenceCoordinates(ref metav1.OwnerReference) metav1.OwnerReference
 	}
 }
 
+// ownerReferenceMatchesCoordinates returns true if all of the coordinate fields match
+// between the two references (uid, name, kind, apiVersion)
+func ownerReferenceMatchesCoordinates(a, b metav1.OwnerReference) bool {
+	return a.UID == b.UID && a.Name == b.Name && a.Kind == b.Kind && a.APIVersion == b.APIVersion
+}
+
 // String renders node as a string using fmt. Acquires a read lock to ensure the
 // reflective dump of dependents doesn't race with any concurrent writes.
 func (n *node) String() string {
diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go
index 1de1685f165..dfaf2806c6c 100644
--- a/pkg/controller/garbagecollector/graph_builder.go
+++ b/pkg/controller/garbagecollector/graph_builder.go
@@ -24,9 +24,11 @@ import (
 
 	"k8s.io/klog/v2"
 
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -346,6 +348,10 @@ func (gb *GraphBuilder) enqueueVirtualDeleteEvent(ref objectReference) {
 // the owner. The "virtual" node will be enqueued to the attemptToDelete, so that
 // attemptToDeleteItem() will verify if the owner exists according to the API server.
 func (gb *GraphBuilder) addDependentToOwners(n *node, owners []metav1.OwnerReference) {
+	// track if some of the referenced owners already exist in the graph and have been observed,
+	// and the dependent's ownerRef does not match their observed coordinates
+	hasPotentiallyInvalidOwnerReference := false
+
 	for _, owner := range owners {
 		ownerNode, ok := gb.uidToNode.Read(owner.UID)
 		if !ok {
@@ -369,8 +375,66 @@ func (gb *GraphBuilder) addDependentToOwners(n *node, owners []metav1.OwnerRefer
 			// event to delete it from the graph if API server confirms this
 			// owner doesn't exist.
 			gb.attemptToDelete.Add(ownerNode)
+		} else if !hasPotentiallyInvalidOwnerReference {
+			ownerIsNamespaced := len(ownerNode.identity.Namespace) > 0
+			if ownerIsNamespaced && ownerNode.identity.Namespace != n.identity.Namespace {
+				if ownerNode.isObserved() {
+					// The owner node has been observed via an informer
+					// the dependent's namespace doesn't match the observed owner's namespace, this is definitely wrong.
+					// cluster-scoped owners can be referenced as an owner from any namespace or cluster-scoped object.
+					klog.V(2).Infof("node %s references an owner %s but does not match namespaces", n.identity, ownerNode.identity)
+					gb.reportInvalidNamespaceOwnerRef(n, owner.UID)
+				}
+				hasPotentiallyInvalidOwnerReference = true
+			} else if !ownerReferenceMatchesCoordinates(owner, ownerNode.identity.OwnerReference) {
+				if ownerNode.isObserved() {
+					// The owner node has been observed via an informer
+					// n's owner reference doesn't match the observed identity, this might be wrong.
+					klog.V(2).Infof("node %s references an owner %s with coordinates that do not match the observed identity", n.identity, ownerNode.identity)
+				}
+				hasPotentiallyInvalidOwnerReference = true
+			}
 		}
 	}
+
+	if hasPotentiallyInvalidOwnerReference {
+		// Enqueue the potentially invalid dependent node into attemptToDelete.
+		// The garbage processor will verify whether the owner references are dangling
+		// and delete the dependent if all owner references are confirmed absent.
+		gb.attemptToDelete.Add(n)
+	}
+}
+
+func (gb *GraphBuilder) reportInvalidNamespaceOwnerRef(n *node, invalidOwnerUID types.UID) {
+	var invalidOwnerRef metav1.OwnerReference
+	var found = false
+	for _, ownerRef := range n.owners {
+		if ownerRef.UID == invalidOwnerUID {
+			invalidOwnerRef = ownerRef
+			found = true
+			break
+		}
+	}
+	if !found {
+		return
+	}
+	ref := &v1.ObjectReference{
+		Kind:       n.identity.Kind,
+		APIVersion: n.identity.APIVersion,
+		Namespace:  n.identity.Namespace,
+		Name:       n.identity.Name,
+		UID:        n.identity.UID,
+	}
+	invalidIdentity := objectReference{
+		OwnerReference: metav1.OwnerReference{
+			Kind:       invalidOwnerRef.Kind,
+			APIVersion: invalidOwnerRef.APIVersion,
+			Name:       invalidOwnerRef.Name,
+			UID:        invalidOwnerRef.UID,
+		},
+		Namespace: n.identity.Namespace,
+	}
+	gb.eventRecorder.Eventf(ref, v1.EventTypeWarning, "OwnerRefInvalidNamespace", "ownerRef %s does not exist in namespace %q", invalidIdentity, n.identity.Namespace)
 }
 
 // insertNode insert the node to gb.uidToNode; then it finds all owners as listed

From b8d7ecf73b65f95f6683d2cd32ac1c3fd7f14b93 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Wed, 8 Jul 2020 01:54:03 -0400
Subject: [PATCH 09/13] Make node removal conditional in processGraphChanges

---
 .../garbagecollector/graph_builder.go         | 39 +++++++++++--------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go
index dfaf2806c6c..69fc4a44221 100644
--- a/pkg/controller/garbagecollector/graph_builder.go
+++ b/pkg/controller/garbagecollector/graph_builder.go
@@ -679,24 +679,29 @@ func (gb *GraphBuilder) processGraphChanges() bool {
 			klog.V(5).Infof("%v doesn't exist in the graph, this shouldn't happen", accessor.GetUID())
 			return true
 		}
-		// removeNode updates the graph
-		gb.removeNode(existingNode)
-		existingNode.dependentsLock.RLock()
-		defer existingNode.dependentsLock.RUnlock()
-		if len(existingNode.dependents) > 0 {
-			gb.absentOwnerCache.Add(identityFromEvent(event, accessor))
-		}
-		for dep := range existingNode.dependents {
-			gb.attemptToDelete.Add(dep)
-		}
-		for _, owner := range existingNode.owners {
-			ownerNode, found := gb.uidToNode.Read(owner.UID)
-			if !found || !ownerNode.isDeletingDependents() {
-				continue
+
+		removeExistingNode := true
+
+		if removeExistingNode {
+			// removeNode updates the graph
+			gb.removeNode(existingNode)
+			existingNode.dependentsLock.RLock()
+			defer existingNode.dependentsLock.RUnlock()
+			if len(existingNode.dependents) > 0 {
+				gb.absentOwnerCache.Add(identityFromEvent(event, accessor))
+			}
+			for dep := range existingNode.dependents {
+				gb.attemptToDelete.Add(dep)
+			}
+			for _, owner := range existingNode.owners {
+				ownerNode, found := gb.uidToNode.Read(owner.UID)
+				if !found || !ownerNode.isDeletingDependents() {
+					continue
+				}
+				// this is to let attempToDeleteItem check if all the owner's
+				// dependents are deleted, if so, the owner will be deleted.
+				gb.attemptToDelete.Add(ownerNode)
 			}
-			// this is to let attempToDeleteItem check if all the owner's
-			// dependents are deleted, if so, the owner will be deleted.
-			gb.attemptToDelete.Add(ownerNode)
 		}
 	}
 	return true

From b655f225095a089dbbc45bb6b143789668e63990 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Wed, 8 Jul 2020 01:46:50 -0400
Subject: [PATCH 10/13] Handle virtual delete events when children don't agree
 on owner coordinates

If a virtual delete event is received for a node whose dependents disagree on the parent's coordinates:
1. propagate the delete to children that matched the verified absent coordinates
2. if the existing node is virtual, select a new set of coordinates from the remaining dependents
3. do not delete the parent node from the graph if the parent node is non-virtual,
   or if there are dependents that do not agree with the virtual delete event coordinates
---
 .../garbagecollector/graph_builder.go         | 178 ++++++++++++++++++
 1 file changed, 178 insertions(+)

diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go
index 69fc4a44221..3cec0af87bd 100644
--- a/pkg/controller/garbagecollector/graph_builder.go
+++ b/pkg/controller/garbagecollector/graph_builder.go
@@ -682,6 +682,60 @@ func (gb *GraphBuilder) processGraphChanges() bool {
 
 		removeExistingNode := true
 
+		if event.virtual {
+			// this is a virtual delete event, not one observed from an informer
+			deletedIdentity := identityFromEvent(event, accessor)
+			if existingNode.virtual {
+
+				// our existing node is also virtual, we're not sure of its coordinates.
+				// see if any dependents reference this owner with coordinates other than the one we got a virtual delete event for.
+				if matchingDependents, nonmatchingDependents := partitionDependents(existingNode.getDependents(), deletedIdentity); len(nonmatchingDependents) > 0 {
+
+					// some of our dependents disagree on our coordinates, so do not remove the existing virtual node from the graph
+					removeExistingNode = false
+
+					if len(matchingDependents) > 0 {
+						// mark the observed deleted identity as absent
+						gb.absentOwnerCache.Add(deletedIdentity)
+						// attempt to delete dependents that do match the verified deleted identity
+						for _, dep := range matchingDependents {
+							gb.attemptToDelete.Add(dep)
+						}
+					}
+
+					// if the delete event verified existingNode.identity doesn't exist...
+					if existingNode.identity == deletedIdentity {
+						// find an alternative identity our nonmatching dependents refer to us by
+						replacementIdentity := getAlternateOwnerIdentity(nonmatchingDependents, deletedIdentity)
+						if replacementIdentity != nil {
+							// replace the existing virtual node with a new one with one of our other potential identities
+							replacementNode := existingNode.clone()
+							replacementNode.identity = *replacementIdentity
+							gb.uidToNode.Write(replacementNode)
+							// and add the new virtual node back to the attemptToDelete queue
+							gb.attemptToDelete.AddRateLimited(replacementNode)
+						}
+					}
+				}
+
+			} else if existingNode.identity != deletedIdentity {
+				// do not remove the existing real node from the graph based on a virtual delete event
+				removeExistingNode = false
+
+				// our existing node which was observed via informer disagrees with the virtual delete event's coordinates
+				matchingDependents, _ := partitionDependents(existingNode.getDependents(), deletedIdentity)
+
+				if len(matchingDependents) > 0 {
+					// mark the observed deleted identity as absent
+					gb.absentOwnerCache.Add(deletedIdentity)
+					// attempt to delete dependents that do match the verified deleted identity
+					for _, dep := range matchingDependents {
+						gb.attemptToDelete.Add(dep)
+					}
+				}
+			}
+		}
+
 		if removeExistingNode {
 			// removeNode updates the graph
 			gb.removeNode(existingNode)
@@ -706,3 +760,127 @@ func (gb *GraphBuilder) processGraphChanges() bool {
 	}
 	return true
 }
+
+// partitionDependents divides the provided dependents into a list which have an ownerReference matching the provided identity,
+// and ones which have an ownerReference for the given uid that do not match the provided identity.
+// Note that a dependent with multiple ownerReferences for the target uid can end up in both lists.
+func partitionDependents(dependents []*node, matchOwnerIdentity objectReference) (matching, nonmatching []*node) {
+	ownerIsNamespaced := len(matchOwnerIdentity.Namespace) > 0
+	for i := range dependents {
+		dep := dependents[i]
+		foundMatch := false
+		foundMismatch := false
+		// if the dep namespace matches or the owner is cluster scoped ...
+		if ownerIsNamespaced && matchOwnerIdentity.Namespace != dep.identity.Namespace {
+			// all references to the parent do not match, since the dependent namespace does not match the owner
+			foundMismatch = true
+		} else {
+			for _, ownerRef := range dep.owners {
+				// ... find the ownerRef with a matching uid ...
+				if ownerRef.UID == matchOwnerIdentity.UID {
+					// ... and check if it matches all coordinates
+					if ownerReferenceMatchesCoordinates(ownerRef, matchOwnerIdentity.OwnerReference) {
+						foundMatch = true
+					} else {
+						foundMismatch = true
+					}
+				}
+			}
+		}
+
+		if foundMatch {
+			matching = append(matching, dep)
+		}
+		if foundMismatch {
+			nonmatching = append(nonmatching, dep)
+		}
+	}
+	return matching, nonmatching
+}
+
+func referenceLessThan(a, b objectReference) bool {
+	// kind/apiVersion are more significant than namespace,
+	// so that we get coherent ordering between kinds
+	// regardless of whether they are cluster-scoped or namespaced
+	if a.Kind != b.Kind {
+		return a.Kind < b.Kind
+	}
+	if a.APIVersion != b.APIVersion {
+		return a.APIVersion < b.APIVersion
+	}
+	// namespace is more significant than name
+	if a.Namespace != b.Namespace {
+		return a.Namespace < b.Namespace
+	}
+	// name is more significant than uid
+	if a.Name != b.Name {
+		return a.Name < b.Name
+	}
+	// uid is included for completeness, but is expected to be identical
+	// when getting alternate identities for an owner since they are keyed by uid
+	if a.UID != b.UID {
+		return a.UID < b.UID
+	}
+	return false
+}
+
+// getAlternateOwnerIdentity searches deps for owner references which match
+// verifiedAbsentIdentity.UID but differ in apiVersion/kind/name or namespace.
+// The first that follows verifiedAbsentIdentity (according to referenceLessThan) is returned.
+// If none follow verifiedAbsentIdentity, the first (according to referenceLessThan) is returned.
+// If no alternate identities are found, nil is returned.
+func getAlternateOwnerIdentity(deps []*node, verifiedAbsentIdentity objectReference) *objectReference {
+	absentIdentityIsClusterScoped := len(verifiedAbsentIdentity.Namespace) == 0
+
+	seenAlternates := map[objectReference]bool{verifiedAbsentIdentity: true}
+
+	// keep track of the first alternate reference (according to referenceLessThan)
+	var first *objectReference
+	// keep track of the first reference following verifiedAbsentIdentity (according to referenceLessThan)
+	var firstFollowing *objectReference
+
+	for _, dep := range deps {
+		for _, ownerRef := range dep.owners {
+			if ownerRef.UID != verifiedAbsentIdentity.UID {
+				// skip references that aren't the uid we care about
+				continue
+			}
+
+			if ownerReferenceMatchesCoordinates(ownerRef, verifiedAbsentIdentity.OwnerReference) {
+				if absentIdentityIsClusterScoped || verifiedAbsentIdentity.Namespace == dep.identity.Namespace {
+					// skip references that exactly match verifiedAbsentIdentity
+					continue
+				}
+			}
+
+			ref := objectReference{OwnerReference: ownerReferenceCoordinates(ownerRef), Namespace: dep.identity.Namespace}
+			if absentIdentityIsClusterScoped && ref.APIVersion == verifiedAbsentIdentity.APIVersion && ref.Kind == verifiedAbsentIdentity.Kind {
+				// we know this apiVersion/kind is cluster-scoped because of verifiedAbsentIdentity,
+				// so clear the namespace from the alternate identity
+				ref.Namespace = ""
+			}
+
+			if seenAlternates[ref] {
+				// skip references we've already seen
+				continue
+			}
+			seenAlternates[ref] = true
+
+			if first == nil || referenceLessThan(ref, *first) {
+				// this alternate comes first lexically
+				first = &ref
+			}
+			if referenceLessThan(verifiedAbsentIdentity, ref) && (firstFollowing == nil || referenceLessThan(ref, *firstFollowing)) {
+				// this alternate is the first following verifiedAbsentIdentity lexically
+				firstFollowing = &ref
+			}
+		}
+	}
+
+	// return the first alternate identity following the verified absent identity, if there is one
+	if firstFollowing != nil {
+		return firstFollowing
+	}
+	// otherwise return the first alternate identity
+	return first
+}

From 221e4aa2c2366a6ca06e9dd070531cd06c62bad7 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Tue, 3 Nov 2020 16:51:54 -0500
Subject: [PATCH 11/13] Queue non-matching children for deletion when a virtual
 node is marked as observed

When we observe valid coordinates for a previously virtual node,
if there are dependents that do not agree with those coordinates,
add them to the attemptToDelete queue.

This queue will check the dependent's ownerReferences using the coordinates specified by the dependent.
If all of the owners can be verified absent, the dependent will be deleted.
If some are still present, or if there are errors looking them up, the dependent will not be deleted.

If the verified owner is namespaced, and the dependent is not in the same namespace,
an event will be recorded for user visibility, since cross-namespace ownerReferences are not supported.
---
 pkg/controller/garbagecollector/graph_builder.go | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go
index 3cec0af87bd..413395c5a92 100644
--- a/pkg/controller/garbagecollector/graph_builder.go
+++ b/pkg/controller/garbagecollector/graph_builder.go
@@ -633,6 +633,21 @@ func (gb *GraphBuilder) processGraphChanges() bool {
 		// 2. this allows things tracking virtual nodes' existence to stop polling and rely on informer events
 		observedIdentity := identityFromEvent(event, accessor)
 		if observedIdentity != existingNode.identity {
+			// find dependents that don't match the identity we observed
+			_, potentiallyInvalidDependents := partitionDependents(existingNode.getDependents(), observedIdentity)
+			// add those potentially invalid dependents to the attemptToDelete queue.
+			// if their owners are still solid the attemptToDelete will be a no-op.
+			// this covers the bad child -> good parent observation sequence.
+			// the good parent -> bad child observation sequence is handled in addDependentToOwners
+			for _, dep := range potentiallyInvalidDependents {
+				if len(observedIdentity.Namespace) > 0 && dep.identity.Namespace != observedIdentity.Namespace {
+					// Namespace mismatch, this is definitely wrong
+					klog.V(2).Infof("node %s references an owner %s but does not match namespaces", dep.identity, observedIdentity)
+					gb.reportInvalidNamespaceOwnerRef(dep, observedIdentity.UID)
+				}
+				gb.attemptToDelete.Add(dep)
+			}
+
 			// make a copy (so we don't modify the existing node in place), store the observed identity, and replace the virtual node
 			klog.V(2).Infof("replacing virtual node %s with observed node %s", existingNode.identity, observedIdentity)
 			existingNode = existingNode.clone()

From 603a0b016ec9417aa47dd894fe5f9ccf0cd9be58 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Thu, 12 Nov 2020 12:24:41 -0500
Subject: [PATCH 12/13] Log cluster-scoped owners referencing namespaced
 owners, avoid retrying lookups forever

If a cluster-scoped dependent references a namespace-scoped owner,
this is an invalid relationship, and the lookup will never succeed in attemptToDelete.

Short-circuit requeueing in attemptToDelete and log.
---
 pkg/controller/garbagecollector/garbagecollector.go | 12 ++++++++++++
 pkg/controller/garbagecollector/operations.go       |  8 +++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go
index 537913ed7fd..574391d4ae1 100644
--- a/pkg/controller/garbagecollector/garbagecollector.go
+++ b/pkg/controller/garbagecollector/garbagecollector.go
@@ -297,6 +297,8 @@ func (gc *GarbageCollector) runAttemptToDeleteWorker() {
 
 var enqueuedVirtualDeleteEventErr = goerrors.New("enqueued virtual delete event")
 
+var namespacedOwnerOfClusterScopedObjectErr = goerrors.New("cluster-scoped objects cannot refer to namespaced owners")
+
 func (gc *GarbageCollector) attemptToDeleteWorker() bool {
 	item, quit := gc.attemptToDelete.Get()
 	gc.workerLock.RLock()
@@ -331,6 +333,9 @@ func (gc *GarbageCollector) attemptToDeleteWorker() bool {
 	if err == enqueuedVirtualDeleteEventErr {
 		// a virtual event was produced and will be handled by processGraphChanges, no need to requeue this node
 		return true
+	} else if err == namespacedOwnerOfClusterScopedObjectErr {
+		// a cluster-scoped object referring to a namespaced owner is an error that will not resolve on retry, no need to requeue this node
+		return true
 	} else if err != nil {
 		if _, ok := err.(*restMappingError); ok {
 			// There are at least two ways this can happen:
@@ -389,6 +394,13 @@ func (gc *GarbageCollector) isDangling(reference metav1.OwnerReference, item *no
 		absentOwnerCacheKey.Namespace = ""
 	}
 
+	if len(item.identity.Namespace) == 0 && namespaced {
+		// item is a cluster-scoped object referring to a namespace-scoped owner, which is not valid.
+		// return a marker error, rather than retrying on the lookup failure forever.
+		klog.V(2).Infof("object %s is cluster-scoped, but refers to a namespaced owner of type %s/%s", item.identity, reference.APIVersion, reference.Kind)
+		return false, nil, namespacedOwnerOfClusterScopedObjectErr
+	}
+
 	// TODO: It's only necessary to talk to the API server if the owner node
 	// is a "virtual" node. The local graph could lag behind the real
 	// status, but in practice, the difference is small.
diff --git a/pkg/controller/garbagecollector/operations.go b/pkg/controller/garbagecollector/operations.go
index ab64a3baceb..de4ae5e722c 100644
--- a/pkg/controller/garbagecollector/operations.go
+++ b/pkg/controller/garbagecollector/operations.go
@@ -65,7 +65,13 @@ func (gc *GarbageCollector) getObject(item objectReference) (*metav1.PartialObje
 	if err != nil {
 		return nil, err
 	}
-	return gc.metadataClient.Resource(resource).Namespace(resourceDefaultNamespace(namespaced, item.Namespace)).Get(context.TODO(), item.Name, metav1.GetOptions{})
+	namespace := resourceDefaultNamespace(namespaced, item.Namespace)
+	if namespaced && len(namespace) == 0 {
+		// the type is namespaced, but we have no namespace coordinate.
+		// the only way this can happen is if a cluster-scoped object referenced this type as an owner.
+		return nil, namespacedOwnerOfClusterScopedObjectErr
+	}
+	return gc.metadataClient.Resource(resource).Namespace(namespace).Get(context.TODO(), item.Name, metav1.GetOptions{})
 }
 
 func (gc *GarbageCollector) patchObject(item objectReference, patch []byte, pt types.PatchType) (*metav1.PartialObjectMetadata, error) {

From e491c3bc7056530d82590d95f0af0e8c4d8dded5 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Fri, 9 Oct 2020 17:50:52 -0400
Subject: [PATCH 13/13] Add GC unit tests

Adds unit tests covering the problematic scenarios identified
around conflicting data in child owner references

                      Before   After
package level         51%      68%
garbagecollector.go   60%      75%
graph_builder.go      50%      81%
graph.go              50%      68%

Added/improved coverage of key functions that had lacking unit test coverage:

* attemptToDeleteWorker
* attemptToDeleteItem
* processGraphChanges (added coverage of all added code)
---
 pkg/controller/garbagecollector/BUILD         |    9 +
 .../garbagecollector/garbagecollector_test.go | 1558 +++++++++++++++++
 .../garbagecollector/graph_builder_test.go    |  212 +++
 .../src/k8s.io/client-go/tools/record/fake.go |   16 +-
 4 files changed, 1793 insertions(+), 2 deletions(-)
 create mode 100644 pkg/controller/garbagecollector/graph_builder_test.go

diff --git a/pkg/controller/garbagecollector/BUILD b/pkg/controller/garbagecollector/BUILD
index 792ed2e1a9a..bdf8dffde8f 100644
--- a/pkg/controller/garbagecollector/BUILD
+++ b/pkg/controller/garbagecollector/BUILD
@@ -56,15 +56,18 @@ go_test(
     srcs = [
         "dump_test.go",
         "garbagecollector_test.go",
+        "graph_builder_test.go",
     ],
     embed = [":go_default_library"],
     deps = [
         "//pkg/api/legacyscheme:go_default_library",
         "//pkg/apis/core/install:go_default_library",
+        "//pkg/controller/garbagecollector/metaonly:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta/testrestmapper:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/json:go_default_library",
@@ -75,12 +78,18 @@ go_test(
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
         "//staging/src/k8s.io/client-go/metadata:go_default_library",
+        "//staging/src/k8s.io/client-go/metadata/fake:go_default_library",
         "//staging/src/k8s.io/client-go/metadata/metadatainformer:go_default_library",
         "//staging/src/k8s.io/client-go/rest:go_default_library",
+        "//staging/src/k8s.io/client-go/testing:go_default_library",
+        "//staging/src/k8s.io/client-go/tools/record:go_default_library",
         "//staging/src/k8s.io/client-go/util/workqueue:go_default_library",
         "//staging/src/k8s.io/controller-manager/pkg/informerfactory:go_default_library",
         "//vendor/github.com/davecgh/go-spew/spew:go_default_library",
+        "//vendor/github.com/golang/groupcache/lru:go_default_library",
+        "//vendor/github.com/google/go-cmp/cmp:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
+        "//vendor/golang.org/x/time/rate:go_default_library",
         "//vendor/gonum.org/v1/gonum/graph:go_default_library",
         "//vendor/gonum.org/v1/gonum/graph/simple:go_default_library",
         "//vendor/k8s.io/utils/pointer:go_default_library",
diff --git a/pkg/controller/garbagecollector/garbagecollector_test.go b/pkg/controller/garbagecollector/garbagecollector_test.go
index c5972088a63..1ad9101aaee 100644
--- a/pkg/controller/garbagecollector/garbagecollector_test.go
+++ b/pkg/controller/garbagecollector/garbagecollector_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package garbagecollector
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -26,15 +27,21 @@ import (
 	"testing"
 	"time"
 
+	"golang.org/x/time/rate"
+
+	"github.com/golang/groupcache/lru"
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 
 	_ "k8s.io/kubernetes/pkg/apis/core/install"
+	"k8s.io/kubernetes/pkg/controller/garbagecollector/metaonly"
 	"k8s.io/utils/pointer"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/api/meta/testrestmapper"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/json"
@@ -45,8 +52,11 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/metadata"
+	fakemetadata "k8s.io/client-go/metadata/fake"
 	"k8s.io/client-go/metadata/metadatainformer"
 	restclient "k8s.io/client-go/rest"
+	clientgotesting "k8s.io/client-go/testing"
+	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/controller-manager/pkg/informerfactory"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
@@ -972,3 +982,1551 @@ func (f *fakeServerResources) getInterfaceUsedCount() int {
 func (*fakeServerResources) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) {
 	return nil, nil
 }
+
+func TestConflictingData(t *testing.T) {
+	pod1ns1 := makeID("v1", "Pod", "ns1", "podname1", "poduid1")
+	pod2ns1 := makeID("v1", "Pod", "ns1", "podname2", "poduid2")
+	pod2ns2 := makeID("v1", "Pod", "ns2", "podname2", "poduid2")
+	node1 := makeID("v1", "Node", "", "nodename", "nodeuid1")
+
+	role1v1beta1 := makeID("rbac.authorization.k8s.io/v1beta1", "Role", "ns1", "role1", "roleuid1")
+	role1v1 := makeID("rbac.authorization.k8s.io/v1", "Role", "ns1", "role1", "roleuid1")
+
+	deployment1apps := makeID("apps/v1", "Deployment", "ns1", "deployment1", "deploymentuid1")
+	deployment1extensions := makeID("extensions/v1beta1", "Deployment", "ns1", "deployment1", "deploymentuid1") // not served, still referenced
+
+	// when a reference is made to node1 from a namespaced resource, the virtual node inserted has namespace coordinates
+	node1WithNamespace := makeID("v1", "Node", "ns1", "nodename", "nodeuid1")
+
+	// when a reference is made to pod1 from a cluster-scoped resource, the virtual node inserted has no namespace
+	pod1nonamespace := makeID("v1", "Pod", "", "podname1", "poduid1")
+
+	badSecretReferenceWithDeploymentUID := makeID("v1", "Secret", "ns1", "secretname", string(deployment1apps.UID))
+	badChildPod := makeID("v1", "Pod", "ns1", "badpod", "badpoduid")
+	goodChildPod := makeID("v1", "Pod", "ns1", "goodpod", "goodpoduid")
+
+	var testScenarios = []struct {
+		name           string
+		initialObjects []runtime.Object
+		steps          []step
+	}{
+		{
+			name: "good child in ns1 -> cluster-scoped owner",
+			steps: []step{
+				// setup
+				createObjectInClient("", "v1", "nodes", "", makeMetadataObj(node1)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1, node1)),
+				// observe namespaced child with not-yet-observed cluster-scoped parent
+				processEvent(makeAddEvent(pod1ns1, node1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(node1)), makeNode(node1WithNamespace, virtual)}, // virtual node1 (matching child namespace)
+					pendingAttemptToDelete: []*node{makeNode(node1WithNamespace, virtual)},                                       // virtual node1 queued for attempted delete
+				}),
+				// handle queued delete of virtual node
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions:          []string{"get /v1, Resource=nodes name=nodename"},
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(node1)), makeNode(node1WithNamespace, virtual)}, // virtual node1 (matching child namespace)
+					pendingAttemptToDelete: []*node{makeNode(node1WithNamespace, virtual)},                                       // virtual node1 still not observed, got requeued
+				}),
+				// observe cluster-scoped parent
+				processEvent(makeAddEvent(node1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(node1)), makeNode(node1)}, // node1 switched to observed, fixed namespace coordinate
+					pendingAttemptToDelete: []*node{makeNode(node1WithNamespace, virtual)},                 // virtual node1 queued for attempted delete
+				}),
+				// handle queued delete of virtual node
+				// final state: child and parent present in graph, no queued actions
+				processAttemptToDelete(1),
+				assertState(state{
+					graphNodes: []*node{makeNode(pod1ns1, withOwners(node1)), makeNode(node1)},
+				}),
+			},
+		},
+		// child in namespace A with owner reference to namespaced type in namespace B
+		// * should be deleted immediately
+		// * event should be logged in namespace A with involvedObject of bad-child indicating the error
+		{
+			name: "bad child in ns1 -> owner in ns2 (child first)",
+			steps: []step{
+				// 0,1: setup
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1, pod2ns1)),
+				createObjectInClient("", "v1", "pods", "ns2", makeMetadataObj(pod2ns2)),
+				// 2,3: observe namespaced child with not-yet-observed namespace-scoped parent
+				processEvent(makeAddEvent(pod1ns1, pod2ns2)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(pod2ns2)), makeNode(pod2ns1, virtual)}, // virtual pod2 (matching child namespace)
+					pendingAttemptToDelete: []*node{makeNode(pod2ns1, virtual)},                                         // virtual pod2 queued for attempted delete
+				}),
+				// 4,5: observe parent
+				processEvent(makeAddEvent(pod2ns2)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(pod2ns2)), makeNode(pod2ns2)}, // pod2 is no longer virtual, namespace coordinate is corrected
+					pendingAttemptToDelete: []*node{makeNode(pod2ns1, virtual), makeNode(pod1ns1)},             // virtual pod2 still queued for attempted delete, bad child pod1 queued because it disagreed with observed parent
+					events:                 []string{`Warning OwnerRefInvalidNamespace ownerRef [v1/Pod, namespace: ns1, name: podname2, uid: poduid2] does not exist in namespace "ns1" involvedObject{kind=Pod,apiVersion=v1}`},
+				}),
+				// 6,7: handle queued delete of virtual parent
+				processAttemptToDelete(1),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(pod2ns2)), makeNode(pod2ns2)},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1)}, // bad child pod1 queued because it disagreed with observed parent
+				}),
+				// 8,9: handle queued delete of bad child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1",    // lookup of pod1 pre-delete
+						"get /v1, Resource=pods ns=ns1 name=podname2",    // verification bad parent reference is absent
+						"delete /v1, Resource=pods ns=ns1 name=podname1", // pod1 delete
+					},
+					graphNodes:       []*node{makeNode(pod1ns1, withOwners(pod2ns2)), makeNode(pod2ns2)},
+					absentOwnerCache: []objectReference{pod2ns1}, // cached absence of bad parent
+				}),
+				// 10,11: observe delete issued in step 8
+				// final state: parent present in graph, no queued actions
+				processEvent(makeDeleteEvent(pod1ns1)),
+				assertState(state{
+					graphNodes:       []*node{makeNode(pod2ns2)}, // only good parent remains
+					absentOwnerCache: []objectReference{pod2ns1}, // cached absence of bad parent
+				}),
+			},
+		},
+		{
+			name: "bad child in ns1 -> owner in ns2 (owner first)",
+			steps: []step{
+				// 0,1: setup
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1, pod2ns1)),
+				createObjectInClient("", "v1", "pods", "ns2", makeMetadataObj(pod2ns2)),
+				// 2,3: observe parent
+				processEvent(makeAddEvent(pod2ns2)),
+				assertState(state{
+					graphNodes: []*node{makeNode(pod2ns2)},
+				}),
+				// 4,5: observe namespaced child with invalid cross-namespace reference to parent
+				processEvent(makeAddEvent(pod1ns1, pod2ns1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(pod2ns1)), makeNode(pod2ns2)},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1)}, // bad child queued for attempted delete
+					events:                 []string{`Warning OwnerRefInvalidNamespace ownerRef [v1/Pod, namespace: ns1, name: podname2, uid: poduid2] does not exist in namespace "ns1" involvedObject{kind=Pod,apiVersion=v1}`},
+				}),
+				// 6,7: handle queued delete of bad child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1",    // lookup of pod1 pre-delete
+						"get /v1, Resource=pods ns=ns1 name=podname2",    // verification bad parent reference is absent
+						"delete /v1, Resource=pods ns=ns1 name=podname1", // pod1 delete
+					},
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(pod2ns1)), makeNode(pod2ns2)},
+					pendingAttemptToDelete: []*node{},
+					absentOwnerCache:       []objectReference{pod2ns1}, // cached absence of bad parent
+				}),
+				// 8,9: observe delete issued in step 6
+				// final state: parent present in graph, no queued actions
+				processEvent(makeDeleteEvent(pod1ns1)),
+				assertState(state{
+					graphNodes:       []*node{makeNode(pod2ns2)}, // only good parent remains
+					absentOwnerCache: []objectReference{pod2ns1}, // cached absence of bad parent
+				}),
+			},
+		},
+		// child that is cluster-scoped with owner reference to namespaced type in namespace B
+		// * should not be deleted
+		// * event should be logged in namespace kube-system with involvedObject of bad-child indicating the error
+		{
+			name: "bad cluster-scoped child -> owner in ns1 (child first)",
+			steps: []step{
+				// setup
+				createObjectInClient("", "v1", "nodes", "", makeMetadataObj(node1, pod1ns1)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1)),
+				// 2,3: observe cluster-scoped child with not-yet-observed namespaced parent
+				processEvent(makeAddEvent(node1, pod1ns1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(node1, withOwners(pod1nonamespace)), makeNode(pod1nonamespace, virtual)}, // virtual pod1 (with no namespace)
+					pendingAttemptToDelete: []*node{makeNode(pod1nonamespace, virtual)},                                               // virtual pod1 queued for attempted delete
+				}),
+				// 4,5: handle queued delete of virtual pod1
+				processAttemptToDelete(1),
+				assertState(state{
+					graphNodes:             []*node{makeNode(node1, withOwners(pod1nonamespace)), makeNode(pod1nonamespace, virtual)}, // virtual pod1 (with no namespace)
+					pendingAttemptToDelete: []*node{},                                                                                 // namespace-scoped virtual object without a namespace coordinate not re-queued
+				}),
+				// 6,7: observe namespace-scoped parent
+				processEvent(makeAddEvent(pod1ns1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(node1, withOwners(pod1nonamespace)), makeNode(pod1ns1)}, // pod1 namespace coordinate corrected, made non-virtual
+					events:                 []string{`Warning OwnerRefInvalidNamespace ownerRef [v1/Pod, namespace: , name: podname1, uid: poduid1] does not exist in namespace "" involvedObject{kind=Node,apiVersion=v1}`},
+					pendingAttemptToDelete: []*node{makeNode(node1, withOwners(pod1ns1))}, // bad cluster-scoped child added to attemptToDelete queue
+				}),
+				// 8,9: handle queued attempted delete of bad cluster-scoped child
+				// final state: parent and child present in graph, no queued actions
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=nodes name=nodename", // lookup of node pre-delete
+					},
+					graphNodes: []*node{makeNode(node1, withOwners(pod1nonamespace)), makeNode(pod1ns1)},
+				}),
+			},
+		},
+		{
+			name: "bad cluster-scoped child -> owner in ns1 (owner first)",
+			steps: []step{
+				// setup
+				createObjectInClient("", "v1", "nodes", "", makeMetadataObj(node1, pod1ns1)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1)),
+				// 2,3: observe namespace-scoped parent
+				processEvent(makeAddEvent(pod1ns1)),
+				assertState(state{
+					graphNodes: []*node{makeNode(pod1ns1)},
+				}),
+				// 4,5: observe cluster-scoped child
+				processEvent(makeAddEvent(node1, pod1ns1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(node1, withOwners(pod1nonamespace)), makeNode(pod1ns1)},
+					events:                 []string{`Warning OwnerRefInvalidNamespace ownerRef [v1/Pod, namespace: , name: podname1, uid: poduid1] does not exist in namespace "" involvedObject{kind=Node,apiVersion=v1}`},
+					pendingAttemptToDelete: []*node{makeNode(node1, withOwners(pod1ns1))}, // bad cluster-scoped child added to attemptToDelete queue
+				}),
+				// 6,7: handle queued attempted delete of bad cluster-scoped child
+				// final state: parent and child present in graph, no queued actions
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=nodes name=nodename", // lookup of node pre-delete
+					},
+					graphNodes: []*node{makeNode(node1, withOwners(pod1nonamespace)), makeNode(pod1ns1)},
+				}),
+			},
+		},
+		// child pointing at non-preferred still-served apiVersion of parent object (e.g. rbac/v1beta1)
+		// * should not be deleted prematurely
+		// * should not repeatedly poll attemptToDelete while waiting
+		// * should be deleted when the actual parent is deleted
+		{
+			name: "good child -> existing owner with non-preferred accessible API version",
+			steps: []step{
+				// setup
+				createObjectInClient("rbac.authorization.k8s.io", "v1", "roles", "ns1", makeMetadataObj(role1v1)),
+				createObjectInClient("rbac.authorization.k8s.io", "v1beta1", "roles", "ns1", makeMetadataObj(role1v1beta1)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1, role1v1beta1)),
+				// 3,4: observe child
+				processEvent(makeAddEvent(pod1ns1, role1v1beta1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(role1v1beta1)), makeNode(role1v1beta1, virtual)},
+					pendingAttemptToDelete: []*node{makeNode(role1v1beta1, virtual)}, // virtual parent enqueued for delete attempt
+				}),
+				// 5,6: handle queued attempted delete of virtual parent
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get rbac.authorization.k8s.io/v1beta1, Resource=roles ns=ns1 name=role1", // lookup of node pre-delete
+					},
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(role1v1beta1)), makeNode(role1v1beta1, virtual)},
+					pendingAttemptToDelete: []*node{makeNode(role1v1beta1, virtual)}, // not yet observed, still in the attemptToDelete queue
+				}),
+				// 7,8: observe parent via v1
+				processEvent(makeAddEvent(role1v1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(role1v1beta1)), makeNode(role1v1)},               // parent version/virtual state gets corrected
+					pendingAttemptToDelete: []*node{makeNode(role1v1beta1, virtual), makeNode(pod1ns1, withOwners(role1v1beta1))}, // virtual parent and mismatched child enqueued for delete attempt
+				}),
+				// 9,10: process attemptToDelete
+				// virtual node dropped from attemptToDelete with no further action because the real node has been observed now
+				processAttemptToDelete(1),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(role1v1beta1)), makeNode(role1v1)},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(role1v1beta1))}, // mismatched child enqueued for delete attempt
+				}),
+				// 11,12: process attemptToDelete for mismatched parent
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1",                             // lookup of child pre-delete
+						"get rbac.authorization.k8s.io/v1beta1, Resource=roles ns=ns1 name=role1", // verifying parent is solid
+					},
+					graphNodes: []*node{makeNode(pod1ns1, withOwners(role1v1beta1)), makeNode(role1v1)},
+				}),
+				// 13,14: teardown
+				deleteObjectFromClient("rbac.authorization.k8s.io", "v1", "roles", "ns1", "role1"),
+				deleteObjectFromClient("rbac.authorization.k8s.io", "v1beta1", "roles", "ns1", "role1"),
+				// 15,16: observe delete via v1
+				processEvent(makeDeleteEvent(role1v1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(role1v1beta1))}, // only child remains
+					absentOwnerCache:       []objectReference{role1v1},                           // cached absence of parent via v1
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(role1v1beta1))},
+				}),
+				// 17,18: process attemptToDelete for child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1",                             // lookup of child pre-delete
+						"get rbac.authorization.k8s.io/v1beta1, Resource=roles ns=ns1 name=role1", // verifying parent is solid
+						"delete /v1, Resource=pods ns=ns1 name=podname1",
+					},
+					absentOwnerCache: []objectReference{role1v1, role1v1beta1}, // cached absence of v1beta1 role
+					graphNodes:       []*node{makeNode(pod1ns1, withOwners(role1v1beta1))},
+				}),
+				// 19,20: observe delete issued in step 17
+				// final state: empty graph, no queued actions
+				processEvent(makeDeleteEvent(pod1ns1)),
+				assertState(state{
+					absentOwnerCache: []objectReference{role1v1, role1v1beta1},
+				}),
+			},
+		},
+		// child pointing at no-longer-served apiVersion of still-existing parent object (e.g. extensions/v1beta1 deployment)
+		// * should not be deleted (this is indistinguishable from referencing an unknown kind/version)
+		// * virtual parent should not repeatedly poll attemptToDelete once real parent is observed
+		{
+			name: "child -> existing owner with inaccessible API version (child first)",
+			steps: []step{
+				// setup
+				createObjectInClient("apps", "v1", "deployments", "ns1", makeMetadataObj(deployment1apps)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1, deployment1extensions)),
+				// 2,3: observe child
+				processEvent(makeAddEvent(pod1ns1, deployment1extensions)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1extensions, virtual)},
+					pendingAttemptToDelete: []*node{makeNode(deployment1extensions, virtual)}, // virtual parent enqueued for delete attempt
+				}),
+				// 4,5: handle queued attempted delete of virtual parent
+				processAttemptToDelete(1),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1extensions, virtual)},
+					pendingAttemptToDelete: []*node{makeNode(deployment1extensions, virtual)}, // requeued on restmapper error
+				}),
+				// 6,7: observe parent via v1
+				processEvent(makeAddEvent(deployment1apps)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1apps)},                // parent version/virtual state gets corrected
+					pendingAttemptToDelete: []*node{makeNode(deployment1extensions, virtual), makeNode(pod1ns1, withOwners(deployment1extensions))}, // virtual parent and mismatched child enqueued for delete attempt
+				}),
+				// 8,9: process attemptToDelete
+				// virtual node dropped from attemptToDelete with no further action because the real node has been observed now
+				processAttemptToDelete(1),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1apps)},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // mismatched child enqueued for delete attempt
+				}),
+				// 10,11: process attemptToDelete for mismatched child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1", // lookup of child pre-delete
+					},
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1apps)},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // mismatched child still enqueued - restmapper error
+				}),
+				// 12: teardown
+				deleteObjectFromClient("apps", "v1", "deployments", "ns1", "deployment1"),
+				// 13,14: observe delete via v1
+				processEvent(makeDeleteEvent(deployment1apps)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // only child remains
+					absentOwnerCache:       []objectReference{deployment1apps},                            // cached absence of parent via v1
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))},
+				}),
+				// 17,18: process attemptToDelete for child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1", // lookup of child pre-delete
+					},
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // only child remains
+					absentOwnerCache:       []objectReference{deployment1apps},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // mismatched child still enqueued - restmapper error
+				}),
+			},
+		},
+		{
+			name: "child -> existing owner with inaccessible API version (owner first)",
+			steps: []step{
+				// setup
+				createObjectInClient("apps", "v1", "deployments", "ns1", makeMetadataObj(deployment1apps)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1, deployment1extensions)),
+				// 2,3: observe parent via v1
+				processEvent(makeAddEvent(deployment1apps)),
+				assertState(state{
+					graphNodes: []*node{makeNode(deployment1apps)},
+				}),
+				// 4,5: observe child
+				processEvent(makeAddEvent(pod1ns1, deployment1extensions)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1apps)},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // mismatched child enqueued for delete attempt
+				}),
+				// 6,7: process attemptToDelete for mismatched child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1", // lookup of child pre-delete
+					},
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1apps)},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // mismatched child still enqueued - restmapper error
+				}),
+				// 8: teardown
+				deleteObjectFromClient("apps", "v1", "deployments", "ns1", "deployment1"),
+				// 9,10: observe delete via v1
+				processEvent(makeDeleteEvent(deployment1apps)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // only child remains
+					absentOwnerCache:       []objectReference{deployment1apps},                            // cached absence of parent via v1
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))},
+				}),
+				// 11,12: process attemptToDelete for child
+				// final state: child with unresolveable ownerRef remains, queued in pendingAttemptToDelete
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1", // lookup of child pre-delete
+					},
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // only child remains
+					absentOwnerCache:       []objectReference{deployment1apps},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // mismatched child still enqueued - restmapper error
+				}),
+			},
+		},
+		// child pointing at no-longer-served apiVersion of no-longer-existing parent object (e.g. extensions/v1beta1 deployment)
+		// * should not be deleted (this is indistinguishable from referencing an unknown kind/version)
+		// * should repeatedly poll attemptToDelete
+		// * should not block deletion of legitimate children of missing deployment
+		{
+			name: "child -> non-existent owner with inaccessible API version (inaccessible parent apiVersion first)",
+			steps: []step{
+				// setup
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1, deployment1extensions)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod2ns1, deployment1apps)),
+				// 2,3: observe child pointing at no-longer-served apiVersion
+				processEvent(makeAddEvent(pod1ns1, deployment1extensions)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1extensions, virtual)},
+					pendingAttemptToDelete: []*node{makeNode(deployment1extensions, virtual)}, // virtual parent enqueued for delete attempt
+				}),
+				// 4,5: observe child pointing at served apiVersion where owner does not exist
+				processEvent(makeAddEvent(pod2ns1, deployment1apps)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1extensions, virtual), makeNode(pod2ns1, withOwners(deployment1apps))},
+					pendingAttemptToDelete: []*node{makeNode(deployment1extensions, virtual), makeNode(pod2ns1, withOwners(deployment1apps))}, // mismatched child enqueued for delete attempt
+				}),
+				// 6,7: handle attempt to delete virtual parent for inaccessible apiVersion
+				processAttemptToDelete(1),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1extensions, virtual), makeNode(pod2ns1, withOwners(deployment1apps))},
+					pendingAttemptToDelete: []*node{makeNode(pod2ns1, withOwners(deployment1apps)), makeNode(deployment1extensions, virtual)}, // inaccessible parent requeued to end
+				}),
+				// 8,9: handle attempt to delete mismatched child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname2",               // lookup of child pre-delete
+						"get apps/v1, Resource=deployments ns=ns1 name=deployment1", // lookup of parent
+						"delete /v1, Resource=pods ns=ns1 name=podname2",            // delete child
+					},
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1extensions, virtual), makeNode(pod2ns1, withOwners(deployment1apps))},
+					absentOwnerCache:       []objectReference{deployment1apps},                // verifiably absent parent remembered
+					pendingAttemptToDelete: []*node{makeNode(deployment1extensions, virtual)}, // mismatched child with verifiably absent parent deleted
+				}),
+				// 10,11: observe delete issued in step 8
+				processEvent(makeDeleteEvent(pod2ns1)),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1extensions, virtual)},
+					absentOwnerCache:       []objectReference{deployment1apps},
+					pendingAttemptToDelete: []*node{makeNode(deployment1extensions, virtual)},
+				}),
+				// 12,13: final state: inaccessible parent requeued in attemptToDelete
+				processAttemptToDelete(1),
+				assertState(state{
+					graphNodes:             []*node{makeNode(pod1ns1, withOwners(deployment1extensions)), makeNode(deployment1extensions, virtual)},
+					absentOwnerCache:       []objectReference{deployment1apps},
+					pendingAttemptToDelete: []*node{makeNode(deployment1extensions, virtual)},
+				}),
+			},
+		},
+
+		{
+			name: "child -> non-existent owner with inaccessible API version (accessible parent apiVersion first)",
+			steps: []step{
+				// setup
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1, deployment1extensions)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod2ns1, deployment1apps)),
+				// 2,3: observe child pointing at served apiVersion where owner does not exist
+				processEvent(makeAddEvent(pod2ns1, deployment1apps)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(pod2ns1, withOwners(deployment1apps)),
+						makeNode(deployment1apps, virtual)},
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps, virtual)}, // virtual parent enqueued for delete attempt
+				}),
+				// 4,5: observe child pointing at no-longer-served apiVersion
+				processEvent(makeAddEvent(pod1ns1, deployment1extensions)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(pod2ns1, withOwners(deployment1apps)),
+						makeNode(deployment1apps, virtual),
+						makeNode(pod1ns1, withOwners(deployment1extensions))},
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps, virtual),
+						makeNode(pod1ns1, withOwners(deployment1extensions))}, // mismatched child enqueued for delete attempt
+				}),
+				// 6,7: handle attempt to delete virtual parent for accessible apiVersion
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get apps/v1, Resource=deployments ns=ns1 name=deployment1", // lookup of parent, gets 404
+					},
+					pendingGraphChanges: []*event{makeVirtualDeleteEvent(deployment1apps)}, // virtual parent not found, queued virtual delete event
+					graphNodes: []*node{
+						makeNode(pod2ns1, withOwners(deployment1apps)),
+						makeNode(deployment1apps, virtual),
+						makeNode(pod1ns1, withOwners(deployment1extensions))},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))},
+				}),
+				// 8,9: handle attempt to delete mismatched child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1", // lookup of child pre-delete
+					},
+					pendingGraphChanges: []*event{makeVirtualDeleteEvent(deployment1apps)},
+					graphNodes: []*node{
+						makeNode(pod2ns1, withOwners(deployment1apps)),
+						makeNode(deployment1apps, virtual),
+						makeNode(pod1ns1, withOwners(deployment1extensions))},
+					pendingAttemptToDelete: []*node{makeNode(pod1ns1, withOwners(deployment1extensions))}, // restmapper on inaccessible parent, requeued
+				}),
+				// 10,11: handle queued virtual delete event
+				processPendingGraphChanges(1),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(pod2ns1, withOwners(deployment1apps)),
+						makeNode(deployment1extensions, virtual), // deployment node changed identity to alternative virtual identity
+						makeNode(pod1ns1, withOwners(deployment1extensions)),
+					},
+					absentOwnerCache: []objectReference{deployment1apps}, // absent apps/v1 parent remembered
+					pendingAttemptToDelete: []*node{
+						makeNode(pod1ns1, withOwners(deployment1extensions)), // child referencing inaccessible apiVersion
+						makeNode(pod2ns1, withOwners(deployment1apps)),       // children of absent apps/v1 parent queued for delete attempt
+						makeNode(deployment1extensions, virtual),             // new virtual parent queued for delete attempt
+					},
+				}),
+
+				// 12,13: handle attempt to delete child referencing inaccessible apiVersion
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname1", // lookup of child pre-delete
+					},
+					graphNodes: []*node{
+						makeNode(pod2ns1, withOwners(deployment1apps)),
+						makeNode(deployment1extensions, virtual),
+						makeNode(pod1ns1, withOwners(deployment1extensions))},
+					absentOwnerCache: []objectReference{deployment1apps},
+					pendingAttemptToDelete: []*node{
+						makeNode(pod2ns1, withOwners(deployment1apps)),       // children of absent apps/v1 parent queued for delete attempt
+						makeNode(deployment1extensions, virtual),             // new virtual parent queued for delete attempt
+						makeNode(pod1ns1, withOwners(deployment1extensions)), // child referencing inaccessible apiVersion - requeued to end
+					},
+				}),
+
+				// 14,15: handle attempt to delete child referencing accessible apiVersion
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=podname2",    // lookup of child pre-delete
+						"delete /v1, Resource=pods ns=ns1 name=podname2", // parent absent, delete
+					},
+					graphNodes: []*node{
+						makeNode(pod2ns1, withOwners(deployment1apps)),
+						makeNode(deployment1extensions, virtual),
+						makeNode(pod1ns1, withOwners(deployment1extensions))},
+					absentOwnerCache: []objectReference{deployment1apps},
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1extensions, virtual),             // new virtual parent queued for delete attempt
+						makeNode(pod1ns1, withOwners(deployment1extensions)), // child referencing inaccessible apiVersion
+					},
+				}),
+
+				// 16,17: handle attempt to delete virtual parent in inaccessible apiVersion
+				processAttemptToDelete(1),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(pod2ns1, withOwners(deployment1apps)),
+						makeNode(deployment1extensions, virtual),
+						makeNode(pod1ns1, withOwners(deployment1extensions))},
+					absentOwnerCache: []objectReference{deployment1apps},
+					pendingAttemptToDelete: []*node{
+						makeNode(pod1ns1, withOwners(deployment1extensions)), // child referencing inaccessible apiVersion
+						makeNode(deployment1extensions, virtual),             // virtual parent with inaccessible apiVersion - requeued to end
+					},
+				}),
+
+				// 18,19: observe delete of pod2 from step 14
+				// final state: virtual parent for inaccessible apiVersion and child of that parent remain in graph, queued for delete attempts with backoff
+				processEvent(makeDeleteEvent(pod2ns1)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(deployment1extensions, virtual),
+						makeNode(pod1ns1, withOwners(deployment1extensions))},
+					absentOwnerCache: []objectReference{deployment1apps},
+					pendingAttemptToDelete: []*node{
+						makeNode(pod1ns1, withOwners(deployment1extensions)), // child referencing inaccessible apiVersion
+						makeNode(deployment1extensions, virtual),             // virtual parent with inaccessible apiVersion
+					},
+				}),
+			},
+		},
+		// child pointing at incorrect apiVersion/kind of still-existing parent object (e.g. core/v1 Secret with uid=123, where an apps/v1 Deployment with uid=123 exists)
+		// * should be deleted immediately
+		// * should not trigger deletion of legitimate children of parent
+		{
+			name: "bad child -> existing owner with incorrect API version (bad child, good child, bad parent delete, good parent)",
+			steps: []step{
+				// setup
+				createObjectInClient("apps", "v1", "deployments", "ns1", makeMetadataObj(deployment1apps)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(badChildPod, badSecretReferenceWithDeploymentUID)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(goodChildPod, deployment1apps)),
+				// 3,4: observe bad child
+				processEvent(makeAddEvent(badChildPod, badSecretReferenceWithDeploymentUID)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)},
+					pendingAttemptToDelete: []*node{
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)}, // virtual parent enqueued for delete attempt
+				}),
+
+				// 5,6: observe good child
+				processEvent(makeAddEvent(goodChildPod, deployment1apps)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)), // good child added
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)},
+					pendingAttemptToDelete: []*node{
+						makeNode(badSecretReferenceWithDeploymentUID, virtual), // virtual parent enqueued for delete attempt
+						makeNode(goodChildPod, withOwners(deployment1apps)),    // good child enqueued for delete attempt
+					},
+				}),
+
+				// 7,8: process pending delete of virtual parent
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=secrets ns=ns1 name=secretname", // lookup of bad parent reference
+					},
+					pendingGraphChanges: []*event{makeVirtualDeleteEvent(badSecretReferenceWithDeploymentUID)}, // bad virtual parent not found, queued virtual delete event
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)},
+					pendingAttemptToDelete: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)), // good child enqueued for delete attempt
+					},
+				}),
+
+				// 9,10: process pending delete of good child, gets 200, remains
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=goodpod",                // lookup of child pre-delete
+						"get apps/v1, Resource=deployments ns=ns1 name=deployment1", // lookup of good parent reference, returns 200
+					},
+					pendingGraphChanges: []*event{makeVirtualDeleteEvent(badSecretReferenceWithDeploymentUID)}, // bad virtual parent not found, queued virtual delete event
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)},
+				}),
+
+				// 11,12: process virtual delete event of bad parent reference
+				processPendingGraphChanges(1),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(deployment1apps, virtual)}, // parent node switched to alternate identity, still virtual
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID}, // remember absence of bad parent coordinates
+					pendingAttemptToDelete: []*node{
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)), // child of bad parent coordinates enqueued for delete attempt
+						makeNode(deployment1apps, virtual),                                     // new alternate virtual parent identity queued for delete attempt
+					},
+				}),
+
+				// 13,14: process pending delete of bad child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=badpod",    // lookup of child pre-delete
+						"delete /v1, Resource=pods ns=ns1 name=badpod", // delete of bad child (absence of bad parent is cached)
+					},
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(deployment1apps, virtual)}, // parent node switched to alternate identity, still virtual
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps, virtual), // new alternate virtual parent identity queued for delete attempt
+					},
+				}),
+
+				// 15,16: process pending delete of new virtual parent
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get apps/v1, Resource=deployments ns=ns1 name=deployment1", // lookup of virtual parent, returns 200
+					},
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(deployment1apps, virtual)}, // parent node switched to alternate identity, still virtual
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps, virtual), // requeued, not yet observed
+					},
+				}),
+
+				// 17,18: observe good parent
+				processEvent(makeAddEvent(deployment1apps)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(deployment1apps)}, // parent node made non-virtual
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps), // still queued, no longer virtual
+					},
+				}),
+
+				// 19,20: observe delete of bad child from step 13
+				processEvent(makeDeleteEvent(badChildPod, badSecretReferenceWithDeploymentUID)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						// bad child node removed
+						makeNode(deployment1apps)},
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps), // still queued, no longer virtual
+					},
+				}),
+
+				// 21,22: process pending delete of good parent
+				// final state: good parent in graph with correct coordinates, good children remain, no pending deletions
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get apps/v1, Resource=deployments ns=ns1 name=deployment1", // lookup of good parent, returns 200
+					},
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(deployment1apps)},
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+				}),
+			},
+		},
+		{
+			name: "bad child -> existing owner with incorrect API version (bad child, good child, good parent, bad parent delete)",
+			steps: []step{
+				// setup
+				createObjectInClient("apps", "v1", "deployments", "ns1", makeMetadataObj(deployment1apps)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(badChildPod, badSecretReferenceWithDeploymentUID)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(goodChildPod, deployment1apps)),
+				// 3,4: observe bad child
+				processEvent(makeAddEvent(badChildPod, badSecretReferenceWithDeploymentUID)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)},
+					pendingAttemptToDelete: []*node{
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)}, // virtual parent enqueued for delete attempt
+				}),
+
+				// 5,6: observe good child
+				processEvent(makeAddEvent(goodChildPod, deployment1apps)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)), // good child added
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)},
+					pendingAttemptToDelete: []*node{
+						makeNode(badSecretReferenceWithDeploymentUID, virtual), // virtual parent enqueued for delete attempt
+						makeNode(goodChildPod, withOwners(deployment1apps)),    // good child enqueued for delete attempt
+					},
+				}),
+
+				// 7,8: process pending delete of virtual parent
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=secrets ns=ns1 name=secretname", // lookup of bad parent reference
+					},
+					pendingGraphChanges: []*event{makeVirtualDeleteEvent(badSecretReferenceWithDeploymentUID)}, // bad virtual parent not found, queued virtual delete event
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)},
+					pendingAttemptToDelete: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)), // good child enqueued for delete attempt
+					},
+				}),
+
+				// 9,10: process pending delete of good child, gets 200, remains
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=goodpod",                // lookup of child pre-delete
+						"get apps/v1, Resource=deployments ns=ns1 name=deployment1", // lookup of good parent reference, returns 200
+					},
+					pendingGraphChanges: []*event{makeVirtualDeleteEvent(badSecretReferenceWithDeploymentUID)}, // bad virtual parent not found, queued virtual delete event
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)},
+				}),
+
+				// 11,12: good parent add event
+				insertEvent(makeAddEvent(deployment1apps)),
+				assertState(state{
+					pendingGraphChanges: []*event{
+						makeAddEvent(deployment1apps),                                // good parent observation sneaked in
+						makeVirtualDeleteEvent(badSecretReferenceWithDeploymentUID)}, // bad virtual parent not found, queued virtual delete event
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(badSecretReferenceWithDeploymentUID, virtual)},
+				}),
+
+				// 13,14: process good parent add
+				processPendingGraphChanges(1),
+				assertState(state{
+					pendingGraphChanges: []*event{
+						makeVirtualDeleteEvent(badSecretReferenceWithDeploymentUID)}, // bad virtual parent still queued virtual delete event
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(deployment1apps)}, // parent node gets fixed, no longer virtual
+					pendingAttemptToDelete: []*node{
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID))}, // child of bad parent coordinates enqueued for delete attempt
+				}),
+
+				// 15,16: process virtual delete event of bad parent reference
+				processPendingGraphChanges(1),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(deployment1apps)},
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID}, // remember absence of bad parent coordinates
+					pendingAttemptToDelete: []*node{
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)), // child of bad parent coordinates enqueued for delete attempt
+					},
+				}),
+
+				// 17,18: process pending delete of bad child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=badpod",    // lookup of child pre-delete
+						"delete /v1, Resource=pods ns=ns1 name=badpod", // delete of bad child (absence of bad parent is cached)
+					},
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)),
+						makeNode(deployment1apps)},
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+				}),
+
+				// 19,20: observe delete of bad child from step 17
+				// final state: good parent in graph with correct coordinates, good children remain, no pending deletions
+				processEvent(makeDeleteEvent(badChildPod, badSecretReferenceWithDeploymentUID)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						// bad child node removed
+						makeNode(deployment1apps)},
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+				}),
+			},
+		},
+		{
+			name: "bad child -> existing owner with incorrect API version (good child, bad child, good parent)",
+			steps: []step{
+				// setup
+				createObjectInClient("apps", "v1", "deployments", "ns1", makeMetadataObj(deployment1apps)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(badChildPod, badSecretReferenceWithDeploymentUID)),
+				createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(goodChildPod, deployment1apps)),
+				// 3,4: observe good child
+				processEvent(makeAddEvent(goodChildPod, deployment1apps)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)), // good child added
+						makeNode(deployment1apps, virtual)},                 // virtual parent added
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps, virtual), // virtual parent enqueued for delete attempt
+					},
+				}),
+
+				// 5,6: observe bad child
+				processEvent(makeAddEvent(badChildPod, badSecretReferenceWithDeploymentUID)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(deployment1apps, virtual),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID))}, // bad child added
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps, virtual),                                     // virtual parent enqueued for delete attempt
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)), // bad child enqueued for delete attempt
+					},
+				}),
+
+				// 7,8: process pending delete of virtual parent
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get apps/v1, Resource=deployments ns=ns1 name=deployment1", // lookup of good parent reference, returns 200
+					},
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(deployment1apps, virtual),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID))},
+					pendingAttemptToDelete: []*node{
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID)), // bad child enqueued for delete attempt
+						makeNode(deployment1apps, virtual),                                     // virtual parent requeued to end, still virtual
+					},
+				}),
+
+				// 9,10: process pending delete of bad child
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get /v1, Resource=pods ns=ns1 name=badpod",        // lookup of child pre-delete
+						"get /v1, Resource=secrets ns=ns1 name=secretname", // lookup of bad parent reference, returns 404
+						"delete /v1, Resource=pods ns=ns1 name=badpod",     // delete of bad child
+					},
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(deployment1apps, virtual),
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID))},
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID}, // remember absence of bad parent
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps, virtual), // virtual parent requeued to end, still virtual
+					},
+				}),
+
+				// 11,12: observe good parent
+				processEvent(makeAddEvent(deployment1apps)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(deployment1apps), // good parent no longer virtual
+						makeNode(badChildPod, withOwners(badSecretReferenceWithDeploymentUID))},
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps), // parent requeued to end, no longer virtual
+					},
+				}),
+
+				// 13,14: observe delete of bad child from step 9
+				processEvent(makeDeleteEvent(badChildPod, badSecretReferenceWithDeploymentUID)),
+				assertState(state{
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						// bad child node removed
+						makeNode(deployment1apps)},
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+					pendingAttemptToDelete: []*node{
+						makeNode(deployment1apps), // parent requeued to end, no longer virtual
+					},
+				}),
+
+				// 15,16: process pending delete of good parent
+				// final state: good parent in graph with correct coordinates, good children remain, no pending deletions
+				processAttemptToDelete(1),
+				assertState(state{
+					clientActions: []string{
+						"get apps/v1, Resource=deployments ns=ns1 name=deployment1", // lookup of good parent, returns 200
+					},
+					graphNodes: []*node{
+						makeNode(goodChildPod, withOwners(deployment1apps)),
+						makeNode(deployment1apps)},
+					absentOwnerCache: []objectReference{badSecretReferenceWithDeploymentUID},
+				}),
+			},
+		},
+	}
+
+	alwaysStarted := make(chan struct{})
+	close(alwaysStarted)
+	for _, scenario := range testScenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+
+			absentOwnerCache := NewReferenceCache(100)
+
+			eventRecorder := record.NewFakeRecorder(100)
+			eventRecorder.IncludeObject = true
+
+			metadataClient := fakemetadata.NewSimpleMetadataClient(legacyscheme.Scheme)
+
+			tweakableRM := meta.NewDefaultRESTMapper(nil)
+			tweakableRM.AddSpecific(
+				schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "Role"},
+				schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "roles"},
+				schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "role"},
+				meta.RESTScopeNamespace,
+			)
+			tweakableRM.AddSpecific(
+				schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1beta1", Kind: "Role"},
+				schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1beta1", Resource: "roles"},
+				schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1beta1", Resource: "role"},
+				meta.RESTScopeNamespace,
+			)
+			tweakableRM.AddSpecific(
+				schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
+				schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
+				schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployment"},
+				meta.RESTScopeNamespace,
+			)
+			restMapper := &testRESTMapper{meta.MultiRESTMapper{tweakableRM, testrestmapper.TestOnlyStaticRESTMapper(legacyscheme.Scheme)}}
+
+			// set up our workqueues
+			attemptToDelete := newTrackingWorkqueue()
+			attemptToOrphan := newTrackingWorkqueue()
+			graphChanges := newTrackingWorkqueue()
+
+			gc := &GarbageCollector{
+				metadataClient:   metadataClient,
+				restMapper:       restMapper,
+				attemptToDelete:  attemptToDelete,
+				attemptToOrphan:  attemptToOrphan,
+				absentOwnerCache: absentOwnerCache,
+				dependencyGraphBuilder: &GraphBuilder{
+					eventRecorder:    eventRecorder,
+					metadataClient:   metadataClient,
+					informersStarted: alwaysStarted,
+					graphChanges:     graphChanges,
+					uidToNode: &concurrentUIDToNode{
+						uidToNodeLock: sync.RWMutex{},
+						uidToNode:     make(map[types.UID]*node),
+					},
+					attemptToDelete:  attemptToDelete,
+					absentOwnerCache: absentOwnerCache,
+				},
+			}
+
+			ctx := stepContext{
+				t:               t,
+				gc:              gc,
+				eventRecorder:   eventRecorder,
+				metadataClient:  metadataClient,
+				attemptToDelete: attemptToDelete,
+				attemptToOrphan: attemptToOrphan,
+				graphChanges:    graphChanges,
+			}
+			for i, s := range scenario.steps {
+				ctx.t.Logf("%d: %s", i, s.name)
+				s.check(ctx)
+				if ctx.t.Failed() {
+					return
+				}
+				verifyGraphInvariants(fmt.Sprintf("after step %d", i), gc.dependencyGraphBuilder.uidToNode.uidToNode, t)
+				if ctx.t.Failed() {
+					return
+				}
+			}
+		})
+	}
+}
+
+func makeID(groupVersion string, kind string, namespace, name, uid string) objectReference {
+	return objectReference{
+		OwnerReference: metav1.OwnerReference{APIVersion: groupVersion, Kind: kind, Name: name, UID: types.UID(uid)},
+		Namespace:      namespace,
+	}
+}
+
+type nodeTweak func(*node) *node
+
+func virtual(n *node) *node {
+	n.virtual = true
+	return n
+}
+func withOwners(ownerReferences ...objectReference) nodeTweak {
+	return func(n *node) *node {
+		var owners []metav1.OwnerReference
+		for _, o := range ownerReferences {
+			owners = append(owners, o.OwnerReference)
+		}
+		n.owners = owners
+		return n
+	}
+}
+
+func makeNode(identity objectReference, tweaks ...nodeTweak) *node {
+	n := &node{identity: identity}
+	for _, tweak := range tweaks {
+		n = tweak(n)
+	}
+	return n
+}
+
+func makeAddEvent(identity objectReference, owners ...objectReference) *event {
+	gv, err := schema.ParseGroupVersion(identity.APIVersion)
+	if err != nil {
+		panic(err)
+	}
+	return &event{
+		eventType: addEvent,
+		gvk:       gv.WithKind(identity.Kind),
+		obj:       makeObj(identity, owners...),
+	}
+}
+
+func makeVirtualDeleteEvent(identity objectReference, owners ...objectReference) *event {
+	e := makeDeleteEvent(identity, owners...)
+	e.virtual = true
+	return e
+}
+
+func makeDeleteEvent(identity objectReference, owners ...objectReference) *event {
+	gv, err := schema.ParseGroupVersion(identity.APIVersion)
+	if err != nil {
+		panic(err)
+	}
+	return &event{
+		eventType: deleteEvent,
+		gvk:       gv.WithKind(identity.Kind),
+		obj:       makeObj(identity, owners...),
+	}
+}
+
+func makeObj(identity objectReference, owners ...objectReference) *metaonly.MetadataOnlyObject {
+	obj := &metaonly.MetadataOnlyObject{
+		TypeMeta:   metav1.TypeMeta{APIVersion: identity.APIVersion, Kind: identity.Kind},
+		ObjectMeta: metav1.ObjectMeta{Namespace: identity.Namespace, UID: identity.UID, Name: identity.Name},
+	}
+	for _, owner := range owners {
+		obj.ObjectMeta.OwnerReferences = append(obj.ObjectMeta.OwnerReferences, owner.OwnerReference)
+	}
+	return obj
+}
+
+func makeMetadataObj(identity objectReference, owners ...objectReference) *metav1.PartialObjectMetadata {
+	obj := &metav1.PartialObjectMetadata{
+		TypeMeta:   metav1.TypeMeta{APIVersion: identity.APIVersion, Kind: identity.Kind},
+		ObjectMeta: metav1.ObjectMeta{Namespace: identity.Namespace, UID: identity.UID, Name: identity.Name},
+	}
+	for _, owner := range owners {
+		obj.ObjectMeta.OwnerReferences = append(obj.ObjectMeta.OwnerReferences, owner.OwnerReference)
+	}
+	return obj
+}
+
+type stepContext struct {
+	t               *testing.T
+	gc              *GarbageCollector
+	eventRecorder   *record.FakeRecorder
+	metadataClient  *fakemetadata.FakeMetadataClient
+	attemptToDelete *trackingWorkqueue
+	attemptToOrphan *trackingWorkqueue
+	graphChanges    *trackingWorkqueue
+}
+
+type step struct {
+	name  string
+	check func(stepContext)
+}
+
+func processPendingGraphChanges(count int) step {
+	return step{
+		name: "processPendingGraphChanges",
+		check: func(ctx stepContext) {
+			ctx.t.Helper()
+			if count <= 0 {
+				// process all
+				for ctx.gc.dependencyGraphBuilder.graphChanges.Len() != 0 {
+					ctx.gc.dependencyGraphBuilder.processGraphChanges()
+				}
+			} else {
+				for i := 0; i < count; i++ {
+					if ctx.gc.dependencyGraphBuilder.graphChanges.Len() == 0 {
+						ctx.t.Errorf("expected at least %d pending changes, got %d", count, i+1)
+						return
+					}
+					ctx.gc.dependencyGraphBuilder.processGraphChanges()
+				}
+			}
+		},
+	}
+}
+
+func processAttemptToDelete(count int) step {
+	return step{
+		name: "processAttemptToDelete",
+		check: func(ctx stepContext) {
+			ctx.t.Helper()
+			if count <= 0 {
+				// process all
+				for ctx.gc.dependencyGraphBuilder.attemptToDelete.Len() != 0 {
+					ctx.gc.attemptToDeleteWorker()
+				}
+			} else {
+				for i := 0; i < count; i++ {
+					if ctx.gc.dependencyGraphBuilder.attemptToDelete.Len() == 0 {
+						ctx.t.Errorf("expected at least %d pending changes, got %d", count, i+1)
+						return
+					}
+					ctx.gc.attemptToDeleteWorker()
+				}
+			}
+		},
+	}
+}
+
+func insertEvent(e *event) step {
+	return step{
+		name: "insertEvent",
+		check: func(ctx stepContext) {
+			ctx.t.Helper()
+			// drain queue into items
+			var items []interface{}
+			for ctx.gc.dependencyGraphBuilder.graphChanges.Len() > 0 {
+				item, _ := ctx.gc.dependencyGraphBuilder.graphChanges.Get()
+				ctx.gc.dependencyGraphBuilder.graphChanges.Done(item)
+				items = append(items, item)
+			}
+
+			// add the new event
+			ctx.gc.dependencyGraphBuilder.graphChanges.Add(e)
+
+			// reappend the items
+			for _, item := range items {
+				ctx.gc.dependencyGraphBuilder.graphChanges.Add(item)
+			}
+		},
+	}
+}
+
+func processEvent(e *event) step {
+	return step{
+		name: "processEvent",
+		check: func(ctx stepContext) {
+			ctx.t.Helper()
+			if ctx.gc.dependencyGraphBuilder.graphChanges.Len() != 0 {
+				ctx.t.Fatalf("events present in graphChanges, must process pending graphChanges before calling processEvent")
+			}
+			ctx.gc.dependencyGraphBuilder.graphChanges.Add(e)
+			ctx.gc.dependencyGraphBuilder.processGraphChanges()
+		},
+	}
+}
+
+func createObjectInClient(group, version, resource, namespace string, obj *metav1.PartialObjectMetadata) step {
+	return step{
+		name: "createObjectInClient",
+		check: func(ctx stepContext) {
+			ctx.t.Helper()
+			if len(ctx.metadataClient.Actions()) > 0 {
+				ctx.t.Fatal("cannot call createObjectInClient with pending client actions, call assertClientActions to check and clear first")
+			}
+			gvr := schema.GroupVersionResource{Group: group, Version: version, Resource: resource}
+			var c fakemetadata.MetadataClient
+			if namespace == "" {
+				c = ctx.metadataClient.Resource(gvr).(fakemetadata.MetadataClient)
+			} else {
+				c = ctx.metadataClient.Resource(gvr).Namespace(namespace).(fakemetadata.MetadataClient)
+			}
+			if _, err := c.CreateFake(obj, metav1.CreateOptions{}); err != nil {
+				ctx.t.Fatal(err)
+			}
+			ctx.metadataClient.ClearActions()
+		},
+	}
+}
+
+func deleteObjectFromClient(group, version, resource, namespace, name string) step {
+	return step{
+		name: "deleteObjectFromClient",
+		check: func(ctx stepContext) {
+			ctx.t.Helper()
+			if len(ctx.metadataClient.Actions()) > 0 {
+				ctx.t.Fatal("cannot call deleteObjectFromClient with pending client actions, call assertClientActions to check and clear first")
+			}
+			gvr := schema.GroupVersionResource{Group: group, Version: version, Resource: resource}
+			var c fakemetadata.MetadataClient
+			if namespace == "" {
+				c = ctx.metadataClient.Resource(gvr).(fakemetadata.MetadataClient)
+			} else {
+				c = ctx.metadataClient.Resource(gvr).Namespace(namespace).(fakemetadata.MetadataClient)
+			}
+			if err := c.Delete(context.TODO(), name, metav1.DeleteOptions{}); err != nil {
+				ctx.t.Fatal(err)
+			}
+			ctx.metadataClient.ClearActions()
+		},
+	}
+}
+
+type state struct {
+	events                 []string
+	clientActions          []string
+	graphNodes             []*node
+	pendingGraphChanges    []*event
+	pendingAttemptToDelete []*node
+	pendingAttemptToOrphan []*node
+	absentOwnerCache       []objectReference
+}
+
+func assertState(s state) step {
+	return step{
+		name: "assertState",
+		check: func(ctx stepContext) {
+			ctx.t.Helper()
+
+			{
+				for _, absent := range s.absentOwnerCache {
+					if !ctx.gc.absentOwnerCache.Has(absent) {
+						ctx.t.Errorf("expected absent owner %s was not in the absentOwnerCache", absent)
+					}
+				}
+				if len(s.absentOwnerCache) != ctx.gc.absentOwnerCache.cache.Len() {
+					// only way to inspect is to drain them all, but that's ok because we're failing the test anyway
+					ctx.gc.absentOwnerCache.cache.OnEvicted = func(key lru.Key, item interface{}) {
+						found := false
+						for _, absent := range s.absentOwnerCache {
+							if absent == key {
+								found = true
+								break
+							}
+						}
+						if !found {
+							ctx.t.Errorf("unexpected item in absent owner cache: %s", key)
+						}
+					}
+					ctx.gc.absentOwnerCache.cache.Clear()
+					ctx.t.Error("unexpected items in absent owner cache")
+				}
+			}
+
+			{
+				var actualEvents []string
+				// drain sent events
+			loop:
+				for {
+					select {
+					case event := <-ctx.eventRecorder.Events:
+						actualEvents = append(actualEvents, event)
+					default:
+						break loop
+					}
+				}
+				if !reflect.DeepEqual(actualEvents, s.events) {
+					ctx.t.Logf("expected:\n%s", strings.Join(s.events, "\n"))
+					ctx.t.Logf("actual:\n%s", strings.Join(actualEvents, "\n"))
+					ctx.t.Fatalf("did not get expected events")
+				}
+			}
+
+			{
+				var actualClientActions []string
+				for _, action := range ctx.metadataClient.Actions() {
+					s := fmt.Sprintf("%s %s", action.GetVerb(), action.GetResource())
+					if action.GetNamespace() != "" {
+						s += " ns=" + action.GetNamespace()
+					}
+					if get, ok := action.(clientgotesting.GetAction); ok && get.GetName() != "" {
+						s += " name=" + get.GetName()
+					}
+					actualClientActions = append(actualClientActions, s)
+				}
+				if (len(s.clientActions) > 0 || len(actualClientActions) > 0) && !reflect.DeepEqual(s.clientActions, actualClientActions) {
+					ctx.t.Logf("expected:\n%s", strings.Join(s.clientActions, "\n"))
+					ctx.t.Logf("actual:\n%s", strings.Join(actualClientActions, "\n"))
+					ctx.t.Fatalf("did not get expected client actions")
+				}
+				ctx.metadataClient.ClearActions()
+			}
+
+			{
+				if l := len(ctx.gc.dependencyGraphBuilder.uidToNode.uidToNode); l != len(s.graphNodes) {
+					ctx.t.Errorf("expected %d nodes, got %d", len(s.graphNodes), l)
+				}
+				for _, n := range s.graphNodes {
+					graphNode, ok := ctx.gc.dependencyGraphBuilder.uidToNode.Read(n.identity.UID)
+					if !ok {
+						ctx.t.Errorf("%s: no node in graph with uid=%s", n.identity.UID, n.identity.UID)
+						continue
+					}
+					if graphNode.identity != n.identity {
+						ctx.t.Errorf("%s: expected identity %v, got %v", n.identity.UID, n.identity, graphNode.identity)
+					}
+					if graphNode.virtual != n.virtual {
+						ctx.t.Errorf("%s: expected virtual %v, got %v", n.identity.UID, n.virtual, graphNode.virtual)
+					}
+					if (len(graphNode.owners) > 0 || len(n.owners) > 0) && !reflect.DeepEqual(graphNode.owners, n.owners) {
+						expectedJSON, _ := json.Marshal(n.owners)
+						actualJSON, _ := json.Marshal(graphNode.owners)
+						ctx.t.Errorf("%s: expected owners %s, got %s", n.identity.UID, expectedJSON, actualJSON)
+					}
+				}
+			}
+
+			{
+				for i := range s.pendingGraphChanges {
+					e := s.pendingGraphChanges[i]
+					if len(ctx.graphChanges.pendingList) < i+1 {
+						ctx.t.Errorf("graphChanges: expected %d events, got %d", len(s.pendingGraphChanges), ctx.graphChanges.Len())
+						break
+					}
+
+					a := ctx.graphChanges.pendingList[i].(*event)
+					if !reflect.DeepEqual(e, a) {
+						objectDiff := ""
+						if !reflect.DeepEqual(e.obj, a.obj) {
+							objectDiff = "\nobjectDiff:\n" + cmp.Diff(e.obj, a.obj)
+						}
+						oldObjectDiff := ""
+						if !reflect.DeepEqual(e.oldObj, a.oldObj) {
+							oldObjectDiff = "\noldObjectDiff:\n" + cmp.Diff(e.oldObj, a.oldObj)
+						}
+						ctx.t.Errorf("graphChanges[%d]: expected\n%#v\ngot\n%#v%s%s", i, e, a, objectDiff, oldObjectDiff)
+					}
+				}
+				if ctx.graphChanges.Len() > len(s.pendingGraphChanges) {
+					for i, a := range ctx.graphChanges.pendingList[len(s.pendingGraphChanges):] {
+						ctx.t.Errorf("graphChanges[%d]: unexpected event: %v", len(s.pendingGraphChanges)+i, a)
+					}
+				}
+			}
+
+			{
+				for i := range s.pendingAttemptToDelete {
+					e := s.pendingAttemptToDelete[i].identity
+					e_virtual := s.pendingAttemptToDelete[i].virtual
+					if ctx.attemptToDelete.Len() < i+1 {
+						ctx.t.Errorf("attemptToDelete: expected %d events, got %d", len(s.pendingAttemptToDelete), ctx.attemptToDelete.Len())
+						break
+					}
+					a := ctx.attemptToDelete.pendingList[i].(*node).identity
+					a_virtual := ctx.attemptToDelete.pendingList[i].(*node).virtual
+					if !reflect.DeepEqual(e, a) {
+						ctx.t.Errorf("attemptToDelete[%d]: expected %v, got %v", i, e, a)
+					}
+					if e_virtual != a_virtual {
+						ctx.t.Errorf("attemptToDelete[%d]: expected virtual node %v, got non-virtual node %v", i, e, a)
+					}
+				}
+				if ctx.attemptToDelete.Len() > len(s.pendingAttemptToDelete) {
+					for i, a := range ctx.attemptToDelete.pendingList[len(s.pendingAttemptToDelete):] {
+						ctx.t.Errorf("attemptToDelete[%d]: unexpected node: %v", len(s.pendingAttemptToDelete)+i, a.(*node).identity)
+					}
+				}
+			}
+
+			{
+				for i := range s.pendingAttemptToOrphan {
+					e := s.pendingAttemptToOrphan[i].identity
+					if ctx.attemptToOrphan.Len() < i+1 {
+						ctx.t.Errorf("attemptToOrphan: expected %d events, got %d", len(s.pendingAttemptToOrphan), ctx.attemptToOrphan.Len())
+						break
+					}
+					a := ctx.attemptToOrphan.pendingList[i].(*node).identity
+					if !reflect.DeepEqual(e, a) {
+						ctx.t.Errorf("attemptToOrphan[%d]: expected %v, got %v", i, e, a)
+					}
+				}
+				if ctx.attemptToOrphan.Len() > len(s.pendingAttemptToOrphan) {
+					for i, a := range ctx.attemptToOrphan.pendingList[len(s.pendingAttemptToOrphan):] {
+						ctx.t.Errorf("attemptToOrphan[%d]: unexpected node: %v", len(s.pendingAttemptToOrphan)+i, a.(*node).identity)
+					}
+				}
+			}
+		},
+	}
+
+}
+
+// trackingWorkqueue implements RateLimitingInterface,
+// allows introspection of the items in the queue,
+// and treats AddAfter and AddRateLimited the same as Add
+// so they are always synchronous.
+type trackingWorkqueue struct {
+	limiter     workqueue.RateLimitingInterface
+	pendingList []interface{}
+	pendingMap  map[interface{}]struct{}
+}
+
+var _ = workqueue.RateLimitingInterface(&trackingWorkqueue{})
+
+func newTrackingWorkqueue() *trackingWorkqueue {
+	return &trackingWorkqueue{
+		limiter:    workqueue.NewRateLimitingQueue(&workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Inf, 100)}),
+		pendingMap: map[interface{}]struct{}{},
+	}
+}
+
+func (t *trackingWorkqueue) Add(item interface{}) {
+	t.queue(item)
+	t.limiter.Add(item)
+}
+func (t *trackingWorkqueue) AddAfter(item interface{}, duration time.Duration) {
+	t.Add(item)
+}
+func (t *trackingWorkqueue) AddRateLimited(item interface{}) {
+	t.Add(item)
+}
+func (t *trackingWorkqueue) Get() (interface{}, bool) {
+	item, shutdown := t.limiter.Get()
+	t.dequeue(item)
+	return item, shutdown
+}
+func (t *trackingWorkqueue) Done(item interface{}) {
+	t.limiter.Done(item)
+}
+func (t *trackingWorkqueue) Forget(item interface{}) {
+	t.limiter.Forget(item)
+}
+func (t *trackingWorkqueue) NumRequeues(item interface{}) int {
+	return 0
+}
+func (t *trackingWorkqueue) Len() int {
+	if e, a := len(t.pendingList), len(t.pendingMap); e != a {
+		panic(fmt.Errorf("pendingList != pendingMap: %d / %d", e, a))
+	}
+	if e, a := len(t.pendingList), t.limiter.Len(); e != a {
+		panic(fmt.Errorf("pendingList != limiter.Len(): %d / %d", e, a))
+	}
+	return len(t.pendingList)
+}
+func (t *trackingWorkqueue) ShutDown() {
+	t.limiter.ShutDown()
+}
+func (t *trackingWorkqueue) ShuttingDown() bool {
+	return t.limiter.ShuttingDown()
+}
+
+func (t *trackingWorkqueue) queue(item interface{}) {
+	if _, queued := t.pendingMap[item]; queued {
+		// fmt.Printf("already queued: %#v\n", item)
+		return
+	}
+	t.pendingMap[item] = struct{}{}
+	t.pendingList = append(t.pendingList, item)
+}
+func (t *trackingWorkqueue) dequeue(item interface{}) {
+	if _, queued := t.pendingMap[item]; !queued {
+		// fmt.Printf("not queued: %#v\n", item)
+		return
+	}
+	delete(t.pendingMap, item)
+	newPendingList := []interface{}{}
+	for _, p := range t.pendingList {
+		if p == item {
+			continue
+		}
+		newPendingList = append(newPendingList, p)
+	}
+	t.pendingList = newPendingList
+}
diff --git a/pkg/controller/garbagecollector/graph_builder_test.go b/pkg/controller/garbagecollector/graph_builder_test.go
new file mode 100644
index 00000000000..d52d223500b
--- /dev/null
+++ b/pkg/controller/garbagecollector/graph_builder_test.go
@@ -0,0 +1,212 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package garbagecollector
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestGetAlternateOwnerIdentity(t *testing.T) {
+	ns1child1 := makeID("v1", "Child", "ns1", "child1", "childuid11")
+	ns1child2 := makeID("v1", "Child", "ns1", "child2", "childuid12")
+
+	ns2child1 := makeID("v1", "Child", "ns2", "child1", "childuid21")
+
+	clusterchild1 := makeID("v1", "Child", "", "child1", "childuidc1")
+
+	var (
+		nsabsentparentns1 = makeID("v1", "NSParent", "ns1", "parentname", "parentuid")
+		nsabsentparentns2 = makeID("v1", "NSParent", "ns2", "parentname", "parentuid")
+
+		nsabsentparent_version = makeID("xx", "NSParent", "ns1", "parentname", "parentuid")
+		nsabsentparent_kind    = makeID("v1", "xxxxxxxx", "ns1", "parentname", "parentuid")
+		nsabsentparent_name    = makeID("v1", "NSParent", "ns1", "xxxxxxxxxx", "parentuid")
+
+		clusterabsentparent         = makeID("v1", "ClusterParent", "", "parentname", "parentuid")
+		clusterabsentparent_version = makeID("xx", "ClusterParent", "", "parentname", "parentuid")
+		clusterabsentparent_kind    = makeID("v1", "xxxxxxxxxxxxx", "", "parentname", "parentuid")
+		clusterabsentparent_name    = makeID("v1", "ClusterParent", "", "xxxxxxxxxx", "parentuid")
+
+		clusterabsentparent_ns1_version = makeID("xx", "ClusterParent", "ns1", "parentname", "parentuid")
+		clusterabsentparent_ns1_kind    = makeID("v1", "xxxxxxxxxxxxx", "ns1", "parentname", "parentuid")
+	)
+
+	orderedNamespacedReferences := []objectReference{
+		makeID("v1", "kind", "ns1", "name", "uid"),
+		makeID("v2", "kind", "ns1", "name", "uid"),
+		makeID("v3", "kind", "ns1", "name", "uid"),
+		makeID("v4", "kind", "ns1", "name", "uid"),
+		makeID("v5", "kind", "ns1", "name", "uid"),
+	}
+	orderedClusterReferences := []objectReference{
+		makeID("v1", "kind", "", "name", "uid"),
+		makeID("v2", "kind", "", "name", "uid"),
+		makeID("v3", "kind", "", "name", "uid"),
+		makeID("v4", "kind", "", "name", "uid"),
+		makeID("v5", "kind", "", "name", "uid"),
+	}
+
+	testcases := []struct {
+		name              string
+		deps              []*node
+		verifiedAbsent    objectReference
+		expectedAlternate *objectReference
+	}{
+		{
+			name: "namespaced alternate version",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(nsabsentparentns1)),
+				makeNode(ns1child2, withOwners(nsabsentparent_version)),
+			},
+			verifiedAbsent:    nsabsentparentns1,
+			expectedAlternate: &nsabsentparent_version, // switch to alternate version
+		},
+		{
+			name: "namespaced alternate kind",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(nsabsentparentns1)),
+				makeNode(ns1child2, withOwners(nsabsentparent_kind)),
+			},
+			verifiedAbsent:    nsabsentparentns1,
+			expectedAlternate: &nsabsentparent_kind, // switch to alternate kind
+		},
+		{
+			name: "namespaced alternate namespace",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(nsabsentparentns1)),
+				makeNode(ns2child1, withOwners(nsabsentparentns2)),
+			},
+			verifiedAbsent:    nsabsentparentns1,
+			expectedAlternate: &nsabsentparentns2, // switch to alternate namespace
+		},
+		{
+			name: "namespaced alternate name",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(nsabsentparentns1)),
+				makeNode(ns1child1, withOwners(nsabsentparent_name)),
+			},
+			verifiedAbsent:    nsabsentparentns1,
+			expectedAlternate: &nsabsentparent_name, // switch to alternate name
+		},
+
+		{
+			name: "cluster alternate version",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(clusterabsentparent)),
+				makeNode(ns1child2, withOwners(clusterabsentparent_version)),
+			},
+			verifiedAbsent:    clusterabsentparent,
+			expectedAlternate: &clusterabsentparent_ns1_version, // switch to alternate version, namespaced to new dependent since we don't know the version is cluster-scoped
+		},
+		{
+			name: "cluster alternate kind",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(clusterabsentparent)),
+				makeNode(ns1child2, withOwners(clusterabsentparent_kind)),
+			},
+			verifiedAbsent:    clusterabsentparent,
+			expectedAlternate: &clusterabsentparent_ns1_kind, // switch to alternate kind, namespaced to new dependent since we don't know the new kind is cluster-scoped
+		},
+		{
+			name: "cluster alternate namespace",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(clusterabsentparent)),
+				makeNode(ns2child1, withOwners(clusterabsentparent)),
+			},
+			verifiedAbsent:    clusterabsentparent,
+			expectedAlternate: nil, // apiVersion/kind verified cluster-scoped, namespace delta ignored, no alternates found
+		},
+		{
+			name: "cluster alternate name",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(clusterabsentparent)),
+				makeNode(ns1child1, withOwners(clusterabsentparent_name)),
+			},
+			verifiedAbsent:    clusterabsentparent,
+			expectedAlternate: &clusterabsentparent_name, // switch to alternate name, apiVersion/kind verified cluster-scoped, namespace dropped
+		},
+
+		{
+			name: "namespaced ref from namespaced child returns first if absent is sorted last",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(orderedNamespacedReferences...)),
+			},
+			verifiedAbsent:    orderedNamespacedReferences[len(orderedNamespacedReferences)-1],
+			expectedAlternate: &orderedNamespacedReferences[0],
+		},
+		{
+			name: "namespaced ref from namespaced child returns next after absent",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(orderedNamespacedReferences...)),
+			},
+			verifiedAbsent:    orderedNamespacedReferences[len(orderedNamespacedReferences)-2],
+			expectedAlternate: &orderedNamespacedReferences[len(orderedNamespacedReferences)-1],
+		},
+
+		{
+			name: "cluster ref from cluster child returns first if absent is sorted last",
+			deps: []*node{
+				makeNode(clusterchild1, withOwners(orderedClusterReferences...)),
+			},
+			verifiedAbsent:    orderedClusterReferences[len(orderedClusterReferences)-1],
+			expectedAlternate: &orderedClusterReferences[0],
+		},
+		{
+			name: "cluster ref from cluster child returns next after absent",
+			deps: []*node{
+				makeNode(clusterchild1, withOwners(orderedClusterReferences...)),
+			},
+			verifiedAbsent:    orderedClusterReferences[len(orderedClusterReferences)-2],
+			expectedAlternate: &orderedClusterReferences[len(orderedClusterReferences)-1],
+		},
+
+		{
+			name: "ignore unrelated",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(clusterabsentparent, makeID("v1", "Parent", "ns1", "name", "anotheruid"))),
+			},
+			verifiedAbsent:    clusterabsentparent,
+			expectedAlternate: nil,
+		},
+		{
+			name: "ignore matches",
+			deps: []*node{
+				makeNode(ns1child1, withOwners(clusterabsentparent, clusterabsentparent)),
+			},
+			verifiedAbsent:    clusterabsentparent,
+			expectedAlternate: nil,
+		},
+		{
+			name: "collapse duplicates",
+			deps: []*node{
+				makeNode(clusterchild1, withOwners(clusterabsentparent, clusterabsentparent_kind, clusterabsentparent_kind)),
+			},
+			verifiedAbsent:    clusterabsentparent,
+			expectedAlternate: &clusterabsentparent_kind,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			alternate := getAlternateOwnerIdentity(tc.deps, tc.verifiedAbsent)
+			if !reflect.DeepEqual(alternate, tc.expectedAlternate) {
+				t.Errorf("expected\n%#v\ngot\n%#v", tc.expectedAlternate, alternate)
+			}
+		})
+	}
+}
diff --git a/staging/src/k8s.io/client-go/tools/record/fake.go b/staging/src/k8s.io/client-go/tools/record/fake.go
index 2ff444ea887..0b3f344a977 100644
--- a/staging/src/k8s.io/client-go/tools/record/fake.go
+++ b/staging/src/k8s.io/client-go/tools/record/fake.go
@@ -27,17 +27,29 @@ import (
 // thrown away in this case.
 type FakeRecorder struct {
 	Events chan string
+
+	IncludeObject bool
+}
+
+func objectString(object runtime.Object, includeObject bool) string {
+	if !includeObject {
+		return ""
+	}
+	return fmt.Sprintf(" involvedObject{kind=%s,apiVersion=%s}",
+		object.GetObjectKind().GroupVersionKind().Kind,
+		object.GetObjectKind().GroupVersionKind().GroupVersion(),
+	)
 }
 
 func (f *FakeRecorder) Event(object runtime.Object, eventtype, reason, message string) {
 	if f.Events != nil {
-		f.Events <- fmt.Sprintf("%s %s %s", eventtype, reason, message)
+		f.Events <- fmt.Sprintf("%s %s %s%s", eventtype, reason, message, objectString(object, f.IncludeObject))
 	}
 }
 
 func (f *FakeRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) {
 	if f.Events != nil {
-		f.Events <- fmt.Sprintf(eventtype+" "+reason+" "+messageFmt, args...)
+		f.Events <- fmt.Sprintf(eventtype+" "+reason+" "+messageFmt, args...) + objectString(object, f.IncludeObject)
 	}
 }