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..bdf8dffde8f 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", @@ -52,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", @@ -71,14 +78,21 @@ 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.go b/pkg/controller/garbagecollector/garbagecollector.go index ef1e9bc8b5b..574391d4ae1 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" @@ -25,6 +26,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 +37,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" @@ -67,22 +73,29 @@ 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 } // 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) + absentOwnerCache := NewReferenceCache(500) gc := &GarbageCollector{ metadataClient: metadataClient, restMapper: mapper, @@ -91,6 +104,7 @@ func NewGarbageCollector( absentOwnerCache: absentOwnerCache, } gc.dependencyGraphBuilder = &GraphBuilder{ + eventRecorder: eventRecorder, metadataClient: metadataClient, informersStarted: informersStarted, restMapper: mapper, @@ -281,6 +295,10 @@ 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() @@ -294,8 +312,31 @@ 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 != 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 == 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: // 1. The reference is to an object of a custom type that has not yet been @@ -325,10 +366,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 @@ -339,6 +390,16 @@ func (gc *GarbageCollector) isDangling(reference metav1.OwnerReference, item *no if err != nil { return false, nil, err } + if !namespaced { + 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 @@ -346,7 +407,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: @@ -355,7 +416,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 @@ -400,9 +461,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() { @@ -420,10 +487,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 } @@ -431,10 +495,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 0752ec6b2a5..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,14 +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" @@ -44,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" @@ -81,7 +92,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 +213,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) } @@ -273,12 +284,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") @@ -398,7 +413,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 +474,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 +490,7 @@ func TestAbsentUIDCache(t *testing.T) { Name: "rc1", UID: "1", APIVersion: "v1", + Controller: pointer.BoolPtr(false), }, }) rc2Pod1 := getPod("rc2Pod1", []metav1.OwnerReference{ @@ -528,7 +545,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 +553,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 @@ -829,7 +846,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) } @@ -965,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.go b/pkg/controller/garbagecollector/graph.go index 2d19a150dd6..b2fd8002d5c 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() { @@ -148,6 +167,23 @@ 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, + } +} + +// 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 e392eb52f58..413395c5a92 100644 --- a/pkg/controller/garbagecollector/graph_builder.go +++ b/pkg/controller/garbagecollector/graph_builder.go @@ -24,15 +24,18 @@ 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" "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" @@ -60,6 +63,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. @@ -89,6 +94,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. @@ -101,7 +108,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{} } @@ -324,8 +331,11 @@ 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{ + virtual: true, 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}, @@ -338,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 { @@ -345,7 +359,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{}), @@ -361,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 @@ -522,6 +594,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() @@ -540,27 +624,42 @@ 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 + 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() + existingNode.identity = observedIdentity + gb.uidToNode.Write(existingNode) + } existingNode.markObserved() } 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), @@ -595,25 +694,208 @@ 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(accessor.GetUID()) - } - 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 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) + 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 } + +// 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 +} 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/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) { 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 } 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) } } 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)