diff --git a/pkg/controller/garbagecollector/dump.go b/pkg/controller/garbagecollector/dump.go index 1943a7f1fa3..884255cc413 100644 --- a/pkg/controller/garbagecollector/dump.go +++ b/pkg/controller/garbagecollector/dump.go @@ -148,9 +148,9 @@ func NewDOTVertex(node *node) *dotVertex { gvk: gv.WithKind(node.identity.Kind), namespace: node.identity.Namespace, name: node.identity.Name, - beingDeleted: node.beingDeleted, - deletingDependents: node.deletingDependents, - virtual: node.virtual, + beingDeleted: node.isBeingDeleted(), + deletingDependents: node.isDeletingDependents(), + virtual: !node.isObserved(), } } @@ -195,7 +195,7 @@ func toDOTNodesAndEdges(uidToNode map[types.UID]*node) ([]*dotVertex, []dotEdge) // add the vertices first, then edges. That avoids having to deal with missing refs. for _, node := range uidToNode { // skip adding objects that don't have owner references and aren't referred to. - if len(node.dependents) == 0 && len(node.owners) == 0 { + if node.dependentsLength() == 0 && len(node.getOwners()) == 0 { continue } vertex := NewDOTVertex(node) @@ -204,7 +204,7 @@ func toDOTNodesAndEdges(uidToNode map[types.UID]*node) ([]*dotVertex, []dotEdge) } for _, node := range uidToNode { currVertex := uidToVertex[node.identity.UID] - for _, ownerRef := range node.owners { + for _, ownerRef := range node.getOwners() { currOwnerVertex, ok := uidToVertex[ownerRef.UID] if !ok { currOwnerVertex = NewMissingdotVertex(ownerRef) @@ -253,14 +253,14 @@ func toDOTNodesAndEdgesForObj(uidToNode map[types.UID]*node, uids ...types.UID) interestingNodes[node.identity.UID] = node - for _, ownerRef := range node.owners { + for _, ownerRef := range node.getOwners() { // if we've already inspected this UID, don't add it to be inspected again if _, ok := interestingNodes[ownerRef.UID]; ok { continue } uidsToCheck = append(uidsToCheck, ownerRef.UID) } - for dependent := range node.dependents { + for _, dependent := range node.getDependents() { // if we've already inspected this UID, don't add it to be inspected again if _, ok := interestingNodes[dependent.identity.UID]; ok { continue diff --git a/pkg/controller/garbagecollector/graph.go b/pkg/controller/garbagecollector/graph.go index 77715309729..432e6537c63 100644 --- a/pkg/controller/garbagecollector/graph.go +++ b/pkg/controller/garbagecollector/graph.go @@ -79,7 +79,8 @@ type node struct { virtualLock sync.RWMutex // when processing an Update event, we need to compare the updated // ownerReferences with the owners recorded in the graph. - owners []metav1.OwnerReference + owners []metav1.OwnerReference + ownersLock sync.RWMutex } // clone() must only be called from the single-threaded GraphBuilder.processGraphChanges() @@ -138,6 +139,18 @@ func (n *node) isDeletingDependents() bool { return n.deletingDependents } +func (n *node) setOwners(owners []metav1.OwnerReference) { + n.ownersLock.Lock() + defer n.ownersLock.Unlock() + n.owners = owners +} + +func (n *node) getOwners() []metav1.OwnerReference { + n.ownersLock.RLock() + defer n.ownersLock.RUnlock() + return n.owners +} + func (n *node) addDependent(dependent *node) { n.dependentsLock.Lock() defer n.dependentsLock.Unlock() @@ -179,7 +192,7 @@ func (n *node) blockingDependents() []*node { dependents := n.getDependents() var ret []*node for _, dep := range dependents { - for _, owner := range dep.owners { + for _, owner := range dep.getOwners() { if owner.UID == n.identity.UID && owner.BlockOwnerDeletion != nil && *owner.BlockOwnerDeletion { ret = append(ret, dep) } @@ -217,6 +230,9 @@ func (n *node) String() string { n.dependentsLock.RLock() defer n.dependentsLock.RUnlock() + n.ownersLock.RLock() + defer n.ownersLock.RUnlock() + return fmt.Sprintf("%#v", n) } diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go index 7348576e291..893dabb6fe9 100644 --- a/pkg/controller/garbagecollector/graph_builder.go +++ b/pkg/controller/garbagecollector/graph_builder.go @@ -783,7 +783,7 @@ func (gb *GraphBuilder) processGraphChanges(logger klog.Logger) bool { // waiting for the deletion of their dependents. gb.addUnblockedOwnersToDeleteQueue(logger, removed, changed) // update the node itself - existingNode.owners = accessor.GetOwnerReferences() + existingNode.setOwners(accessor.GetOwnerReferences()) // Add the node to its new owners' dependent lists. gb.addDependentToOwners(logger, existingNode, added) // remove the node from the dependent list of node that are no longer in diff --git a/pkg/controller/garbagecollector/patch.go b/pkg/controller/garbagecollector/patch.go index 25d8b208513..2d69d328029 100644 --- a/pkg/controller/garbagecollector/patch.go +++ b/pkg/controller/garbagecollector/patch.go @@ -134,7 +134,7 @@ func (n *node) unblockOwnerReferencesStrategicMergePatch() ([]byte, error) { var dummy metaonly.MetadataOnlyObject var blockingRefs []metav1.OwnerReference falseVar := false - for _, owner := range n.owners { + for _, owner := range n.getOwners() { if owner.BlockOwnerDeletion != nil && *owner.BlockOwnerDeletion { ref := owner ref.BlockOwnerDeletion = &falseVar @@ -157,7 +157,7 @@ func (gc *GarbageCollector) unblockOwnerReferencesJSONMergePatch(n *node) ([]byt expectedObjectMeta.ResourceVersion = accessor.GetResourceVersion() var expectedOwners []metav1.OwnerReference falseVar := false - for _, owner := range n.owners { + for _, owner := range n.getOwners() { owner.BlockOwnerDeletion = &falseVar expectedOwners = append(expectedOwners, owner) }