From 1e71a28490774ac93e5b57f169017cd767ad9fdf Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 14 Jan 2021 11:29:31 -0500 Subject: [PATCH] Clean up namespaced children of missing virtual parents with incorrectly cluster-scoped nodes --- .../garbagecollector/garbagecollector_test.go | 105 +++++++++++++++++- .../garbagecollector/graph_builder.go | 5 + 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/pkg/controller/garbagecollector/garbagecollector_test.go b/pkg/controller/garbagecollector/garbagecollector_test.go index 398138b0995..6548274e2f2 100644 --- a/pkg/controller/garbagecollector/garbagecollector_test.go +++ b/pkg/controller/garbagecollector/garbagecollector_test.go @@ -1982,7 +1982,9 @@ func TestConflictingData(t *testing.T) { makeNode(pod2ns1, withOwners(pod1ns1)), makeNode(pod1nonamespace, virtual)}, pendingAttemptToDelete: []*node{ - makeNode(pod1nonamespace, virtual)}, // virtual parent queued for deletion + makeNode(pod1nonamespace, virtual), // virtual parent queued for deletion + makeNode(pod2ns1, withOwners(pod1ns1)), // mismatched child queued for deletion + }, }), // 6,7: process attemptToDelete of bad virtual parent coordinates @@ -1992,9 +1994,28 @@ func TestConflictingData(t *testing.T) { makeNode(node1, withOwners(pod1nonamespace)), makeNode(pod2ns1, withOwners(pod1ns1)), makeNode(pod1nonamespace, virtual)}, + pendingAttemptToDelete: []*node{ + makeNode(pod2ns1, withOwners(pod1ns1))}, // mismatched child queued for deletion }), + // 8,9: process attemptToDelete of good child + processAttemptToDelete(1), + assertState(state{ + clientActions: []string{ + "get /v1, Resource=pods ns=ns1 name=podname2", // get good child, returns 200 + "get /v1, Resource=pods ns=ns1 name=podname1", // get missing parent, returns 404 + "delete /v1, Resource=pods ns=ns1 name=podname2", // delete good child + }, + graphNodes: []*node{ + makeNode(node1, withOwners(pod1nonamespace)), + makeNode(pod2ns1, withOwners(pod1ns1)), + makeNode(pod1nonamespace, virtual)}, + absentOwnerCache: []objectReference{pod1ns1}, // missing parent cached + }), + + // 10,11: observe deletion of good child // steady-state is bad cluster child and bad virtual parent coordinates, with no retries + processEvent(makeDeleteEvent(pod2ns1, pod1ns1)), assertState(state{ graphNodes: []*node{ makeNode(node1, withOwners(pod1nonamespace)), @@ -2003,6 +2024,88 @@ func TestConflictingData(t *testing.T) { }), }, }, + { + // https://github.com/kubernetes/kubernetes/issues/98040 + name: "cluster-scoped bad child, namespaced good child, late observed parent", + steps: []step{ + // setup + createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod1ns1)), // good parent + createObjectInClient("", "v1", "pods", "ns1", makeMetadataObj(pod2ns1, pod1ns1)), // good child + createObjectInClient("", "v1", "nodes", "", makeMetadataObj(node1, pod1nonamespace)), // bad child + + // 3,4: observe bad child + processEvent(makeAddEvent(node1, pod1nonamespace)), + assertState(state{ + graphNodes: []*node{ + makeNode(node1, withOwners(pod1nonamespace)), + makeNode(pod1nonamespace, virtual)}, + pendingAttemptToDelete: []*node{ + makeNode(pod1nonamespace, virtual)}, // virtual parent queued for deletion + }), + + // 5,6: observe good child + processEvent(makeAddEvent(pod2ns1, pod1ns1)), + assertState(state{ + graphNodes: []*node{ + makeNode(node1, withOwners(pod1nonamespace)), + makeNode(pod2ns1, withOwners(pod1ns1)), + makeNode(pod1nonamespace, virtual)}, + pendingAttemptToDelete: []*node{ + makeNode(pod1nonamespace, virtual), // virtual parent queued for deletion + makeNode(pod2ns1, withOwners(pod1ns1))}, // mismatched child queued for deletion + }), + + // 7,8: process attemptToDelete of bad virtual parent coordinates + processAttemptToDelete(1), + assertState(state{ + graphNodes: []*node{ + makeNode(node1, withOwners(pod1nonamespace)), + makeNode(pod2ns1, withOwners(pod1ns1)), + makeNode(pod1nonamespace, virtual)}, + pendingAttemptToDelete: []*node{ + makeNode(pod2ns1, withOwners(pod1ns1))}, // mismatched child queued for deletion + }), + + // 9,10: process attemptToDelete of good child + processAttemptToDelete(1), + assertState(state{ + clientActions: []string{ + "get /v1, Resource=pods ns=ns1 name=podname2", // get good child, returns 200 + "get /v1, Resource=pods ns=ns1 name=podname1", // get late-observed parent, returns 200 + }, + graphNodes: []*node{ + makeNode(node1, withOwners(pod1nonamespace)), + makeNode(pod2ns1, withOwners(pod1ns1)), + makeNode(pod1nonamespace, virtual)}, + }), + + // 11,12: late observe good parent + processEvent(makeAddEvent(pod1ns1)), + assertState(state{ + graphNodes: []*node{ + makeNode(node1, withOwners(pod1nonamespace)), + makeNode(pod2ns1, withOwners(pod1ns1)), + makeNode(pod1ns1)}, + // warn about bad node reference + 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(pod1nonamespace))}, // queue bad cluster-scoped child for delete attempt + }), + + // 13,14: process attemptToDelete of bad child + // steady state is bad cluster-scoped child remaining with no retries, good parent and good child in graph + processAttemptToDelete(1), + assertState(state{ + clientActions: []string{ + "get /v1, Resource=nodes name=nodename", // get bad child, returns 200 + }, + graphNodes: []*node{ + makeNode(node1, withOwners(pod1nonamespace)), + makeNode(pod2ns1, withOwners(pod1ns1)), + makeNode(pod1ns1)}, + }), + }, + }, { // https://github.com/kubernetes/kubernetes/issues/98040 name: "namespaced good child, cluster-scoped bad child, missing parent", diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go index 413395c5a92..94da41a8a7f 100644 --- a/pkg/controller/garbagecollector/graph_builder.go +++ b/pkg/controller/garbagecollector/graph_builder.go @@ -393,6 +393,11 @@ func (gb *GraphBuilder) addDependentToOwners(n *node, owners []metav1.OwnerRefer 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 + } else if !ownerIsNamespaced && ownerNode.identity.Namespace != n.identity.Namespace && !ownerNode.isObserved() { + // the ownerNode is cluster-scoped and virtual, and does not match the child node's namespace. + // the owner could be a missing instance of a namespaced type incorrectly referenced by a cluster-scoped child (issue #98040). + // enqueue this child to attemptToDelete to verify parent references. + hasPotentiallyInvalidOwnerReference = true } } }