diff --git a/hack/kcp/garbage_collector_patch.go b/hack/kcp/garbage_collector_patch.go new file mode 100644 index 0000000000000..522d7100a156f --- /dev/null +++ b/hack/kcp/garbage_collector_patch.go @@ -0,0 +1,109 @@ +/* +Copyright 2022 The KCP 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 main + +import ( + "bufio" + "bytes" + "fmt" + "go/ast" + "go/format" + "go/parser" + "go/token" + "log" + "strings" +) + +/* +Process: + +1. go run ./hack/kcp/garbage_collector_patch.go > pkg/controller/garbagecollector/garbagecollector_patch.go +(you may need to add -mod=readonly) + +2. goimports -w pkg/controller/garbagecollector/garbagecollector_patch.go + +3. reapply patch for kcp to pkg/controller/garbagecollector/garbagecollector_patch.go +*/ + +func main() { + fileSet := token.NewFileSet() + + file, err := parser.ParseFile(fileSet, "pkg/controller/garbagecollector/garbagecollector.go", nil, parser.ParseComments) + if err != nil { + log.Fatal(err) + } + + // n stores a reference to the node for the function declaration for Sync + var n ast.Node + + ast.Inspect(file, func(node ast.Node) bool { + switch x := node.(type) { + case *ast.FuncDecl: + if x.Name.Name == "Sync" { + // Store the reference + n = node + // Stop further inspection + return false + } + } + + // Continue recursing + return true + }) + + startLine := fileSet.Position(n.Pos()).Line + endLine := fileSet.Position(n.End()).Line + + // To preserve the comments from within the function body itself, we have to write out the entire file to a buffer, + // then extract only the lines we care about (the function body). + var buf bytes.Buffer + if err := format.Node(&buf, fileSet, file); err != nil { + log.Fatal(err) + } + + // Convert the buffer to a slice of lines, so we can grab the portion we want + var lines []string + scanner := bufio.NewScanner(&buf) + for scanner.Scan() { + lines = append(lines, scanner.Text()) + } + if err := scanner.Err(); err != nil { + log.Fatal(err) + } + + fmt.Println(`/* +Copyright 2022 The KCP 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 +`) + + // Finally, print the line range we need + fmt.Println(strings.Join(lines[startLine-1:endLine], "\n")) +} diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go index 3fc0fda7ae256..8668a37222c8e 100644 --- a/pkg/controller/garbagecollector/garbagecollector.go +++ b/pkg/controller/garbagecollector/garbagecollector.go @@ -24,6 +24,8 @@ import ( "sync" "time" + "github.com/kcp-dev/logicalcluster/v2" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -78,6 +80,9 @@ type GarbageCollector struct { absentOwnerCache *ReferenceCache workerLock sync.RWMutex + + // kcp + clusterName logicalcluster.Name } var _ controller.Interface = (*GarbageCollector)(nil) @@ -146,16 +151,16 @@ func (gc *GarbageCollector) Run(ctx context.Context, workers int) { defer gc.attemptToOrphan.ShutDown() defer gc.dependencyGraphBuilder.graphChanges.ShutDown() - klog.Infof("Starting garbage collector controller") - defer klog.Infof("Shutting down garbage collector controller") + klog.Infof("%s: Starting garbage collector controller", gc.clusterName) + defer klog.Infof("%s: Shutting down garbage collector controller", gc.clusterName) go gc.dependencyGraphBuilder.Run(ctx.Done()) - if !cache.WaitForNamedCacheSync("garbage collector", ctx.Done(), gc.dependencyGraphBuilder.IsSynced) { + if !cache.WaitForNamedCacheSync(fmt.Sprintf("%s: garbage collector", gc.clusterName), ctx.Done(), gc.dependencyGraphBuilder.IsSynced) { return } - klog.Infof("Garbage collector: all resource monitors have synced. Proceeding to collect garbage") + klog.Infof("%s: Garbage collector: all resource monitors have synced. Proceeding to collect garbage", gc.clusterName) // gc workers for i := 0; i < workers; i++ { @@ -177,18 +182,18 @@ func (gc *GarbageCollector) Sync(discoveryClient discovery.ServerResourcesInterf oldResources := make(map[schema.GroupVersionResource]struct{}) wait.Until(func() { // Get the current resource list from discovery. - newResources := GetDeletableResources(discoveryClient) + newResources := gc.GetDeletableResources(discoveryClient) // This can occur if there is an internal error in GetDeletableResources. if len(newResources) == 0 { - klog.V(2).Infof("no resources reported by discovery, skipping garbage collector sync") + klog.V(2).Infof("%s: no resources reported by discovery, skipping garbage collector sync", gc.clusterName) metrics.GarbageCollectorResourcesSyncError.Inc() return } // Decide whether discovery has reported a change. if reflect.DeepEqual(oldResources, newResources) { - klog.V(5).Infof("no resource updates from discovery, skipping garbage collector sync") + klog.V(5).Infof("%s: no resource updates from discovery, skipping garbage collector sync", gc.clusterName) return } @@ -204,21 +209,21 @@ func (gc *GarbageCollector) Sync(discoveryClient discovery.ServerResourcesInterf // On a reattempt, check if available resources have changed if attempt > 1 { - newResources = GetDeletableResources(discoveryClient) + newResources = gc.GetDeletableResources(discoveryClient) if len(newResources) == 0 { - klog.V(2).Infof("no resources reported by discovery (attempt %d)", attempt) + klog.V(2).Infof("%s: no resources reported by discovery (attempt %d)", gc.clusterName, attempt) metrics.GarbageCollectorResourcesSyncError.Inc() return false, nil } } - klog.V(2).Infof("syncing garbage collector with updated resources from discovery (attempt %d): %s", attempt, printDiff(oldResources, newResources)) + klog.V(2).Infof("%s: syncing garbage collector with updated resources from discovery (attempt %d): %s", gc.clusterName, attempt, printDiff(oldResources, newResources)) // Resetting the REST mapper will also invalidate the underlying discovery // client. This is a leaky abstraction and assumes behavior about the REST // mapper, but we'll deal with it for now. gc.restMapper.Reset() - klog.V(4).Infof("reset restmapper") + klog.V(4).Infof("%s: reset restmapper", gc.clusterName) // Perform the monitor resync and wait for controllers to report cache sync. // @@ -230,19 +235,19 @@ func (gc *GarbageCollector) Sync(discoveryClient discovery.ServerResourcesInterf // case, the restMapper will fail to map some of newResources until the next // attempt. if err := gc.resyncMonitors(newResources); err != nil { - utilruntime.HandleError(fmt.Errorf("failed to sync resource monitors (attempt %d): %v", attempt, err)) + utilruntime.HandleError(fmt.Errorf("%s: failed to sync resource monitors (attempt %d): %v", gc.clusterName, attempt, err)) metrics.GarbageCollectorResourcesSyncError.Inc() return false, nil } - klog.V(4).Infof("resynced monitors") + klog.V(4).Infof("%s: resynced monitors", gc.clusterName) // wait for caches to fill for a while (our sync period) before attempting to rediscover resources and retry syncing. // this protects us from deadlocks where available resources changed and one of our informer caches will never fill. // informers keep attempting to sync in the background, so retrying doesn't interrupt them. // the call to resyncMonitors on the reattempt will no-op for resources that still exist. // note that workers stay paused until we successfully resync. - if !cache.WaitForNamedCacheSync("garbage collector", waitForStopOrTimeout(stopCh, period), gc.dependencyGraphBuilder.IsSynced) { - utilruntime.HandleError(fmt.Errorf("timed out waiting for dependency graph builder sync during GC sync (attempt %d)", attempt)) + if !cache.WaitForNamedCacheSync(fmt.Sprintf("%s: garbage collector", gc.clusterName), waitForStopOrTimeout(stopCh, period), gc.dependencyGraphBuilder.IsSynced) { + utilruntime.HandleError(fmt.Errorf("%s: timed out waiting for dependency graph builder sync during GC sync (attempt %d)", gc.clusterName, attempt)) metrics.GarbageCollectorResourcesSyncError.Inc() return false, nil } @@ -255,7 +260,7 @@ func (gc *GarbageCollector) Sync(discoveryClient discovery.ServerResourcesInterf // have succeeded to ensure we'll retry on subsequent syncs if an error // occurred. oldResources = newResources - klog.V(2).Infof("synced garbage collector") + klog.V(2).Infof("%s: synced garbage collector", gc.clusterName) }, period, stopCh) } @@ -333,7 +338,7 @@ const ( func (gc *GarbageCollector) attemptToDeleteWorker(ctx context.Context, item interface{}) workQueueItemAction { n, ok := item.(*node) if !ok { - utilruntime.HandleError(fmt.Errorf("expect *node, got %#v", item)) + utilruntime.HandleError(fmt.Errorf("%s: expect *node, got %#v", gc.clusterName, item)) return forgetItem } @@ -342,13 +347,13 @@ func (gc *GarbageCollector) attemptToDeleteWorker(ctx context.Context, item inte 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) + klog.V(5).Infof("%s: item %s no longer in the graph, skipping attemptToDeleteItem", gc.clusterName, n) return forgetItem } 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) + klog.V(5).Infof("%s: item %s no longer virtual in the graph, skipping attemptToDeleteItem on virtual node", gc.clusterName, n) return forgetItem } } @@ -369,9 +374,9 @@ func (gc *GarbageCollector) attemptToDeleteWorker(ctx context.Context, item inte // have a way to distinguish this from a valid type we will recognize // after the next discovery sync. // For now, record the error and retry. - klog.V(5).Infof("error syncing item %s: %v", n, err) + klog.V(5).Infof("%s: error syncing item %s: %v", gc.clusterName, n, err) } else { - utilruntime.HandleError(fmt.Errorf("error syncing item %s: %v", n, err)) + utilruntime.HandleError(fmt.Errorf("%s: error syncing item %s: %v", gc.clusterName, n, err)) } // retry if garbage collection of an object failed. return requeueItem @@ -379,7 +384,7 @@ func (gc *GarbageCollector) attemptToDeleteWorker(ctx context.Context, item inte // requeue if item hasn't been observed via an informer event yet. // otherwise a virtual node for an item added AND removed during watch reestablishment can get stuck in the graph and never removed. // see https://issue.k8s.io/56121 - klog.V(5).Infof("item %s hasn't been observed via informer yet", n.identity) + klog.V(5).Infof("%s: item %s hasn't been observed via informer yet", gc.clusterName, n.identity) return requeueItem } @@ -395,13 +400,13 @@ func (gc *GarbageCollector) isDangling(ctx context.Context, reference metav1.Own // 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) + klog.V(5).Infof("%s: according to the absentOwnerCache, object %s's owner %s/%s, %s does not exist", gc.clusterName, 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) + klog.V(5).Infof("%s: according to the absentOwnerCache, object %s's owner %s/%s, %s does not exist in namespace %s", gc.clusterName, item.identity.UID, reference.APIVersion, reference.Kind, reference.Name, item.identity.Namespace) return true, nil, nil } @@ -422,7 +427,7 @@ func (gc *GarbageCollector) isDangling(ctx context.Context, reference metav1.Own 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) + klog.V(2).Infof("%s: object %s is cluster-scoped, but refers to a namespaced owner of type %s/%s", gc.clusterName, item.identity, reference.APIVersion, reference.Kind) return false, nil, namespacedOwnerOfClusterScopedObjectErr } @@ -433,14 +438,14 @@ func (gc *GarbageCollector) isDangling(ctx context.Context, reference metav1.Own switch { case errors.IsNotFound(err): 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) + klog.V(5).Infof("%s: object %s's owner %s/%s, %s is not found", gc.clusterName, item.identity.UID, reference.APIVersion, reference.Kind, reference.Name) return true, nil, nil case err != nil: return false, nil, err } 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) + klog.V(5).Infof("%s: object %s's owner %s/%s, %s is not found, UID mismatch", gc.clusterName, item.identity.UID, reference.APIVersion, reference.Kind, reference.Name) gc.absentOwnerCache.Add(absentOwnerCacheKey) return true, nil, nil } @@ -493,12 +498,12 @@ func ownerRefsToUIDs(refs []metav1.OwnerReference) []types.UID { // 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(ctx context.Context, item *node) error { - klog.V(2).InfoS("Processing object", "object", klog.KRef(item.identity.Namespace, item.identity.Name), + klog.V(2).InfoS("Processing object", "cluster", gc.clusterName, "object", klog.KRef(item.identity.Namespace, item.identity.Name), "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() { - klog.V(5).Infof("processing item %s returned at once, because its DeletionTimestamp is non-nil", item.identity) + klog.V(5).Infof("%s: processing item %s returned at once, because its DeletionTimestamp is non-nil", gc.clusterName, item.identity) return nil } // TODO: It's only necessary to talk to the API server if this is a @@ -510,7 +515,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(ctx context.Context, item *node) // the GraphBuilder can add "virtual" node for an owner that doesn't // exist yet, so we need to enqueue a virtual Delete event to remove // the virtual node from GraphBuilder.uidToNode. - klog.V(5).Infof("item %v not found, generating a virtual delete event", item.identity) + klog.V(5).Infof("%s: item %v not found, generating a virtual delete event", gc.clusterName, item.identity) gc.dependencyGraphBuilder.enqueueVirtualDeleteEvent(item.identity) return enqueuedVirtualDeleteEventErr case err != nil: @@ -518,7 +523,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(ctx context.Context, item *node) } 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) + klog.V(5).Infof("%s: UID doesn't match, item %v not found, generating a virtual delete event", gc.clusterName, item.identity) gc.dependencyGraphBuilder.enqueueVirtualDeleteEvent(item.identity) return enqueuedVirtualDeleteEventErr } @@ -532,7 +537,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(ctx context.Context, item *node) // compute if we should delete the item ownerReferences := latest.GetOwnerReferences() if len(ownerReferences) == 0 { - klog.V(2).Infof("object %s's doesn't have an owner, continue on next item", item.identity) + klog.V(2).Infof("%s: object %s's doesn't have an owner, continue on next item", gc.clusterName, item.identity) return nil } @@ -540,15 +545,15 @@ func (gc *GarbageCollector) attemptToDeleteItem(ctx context.Context, item *node) if err != nil { return err } - klog.V(5).Infof("classify references of %s.\nsolid: %#v\ndangling: %#v\nwaitingForDependentsDeletion: %#v\n", item.identity, solid, dangling, waitingForDependentsDeletion) + klog.V(5).Infof("%s: classify references of %s.\nsolid: %#v\ndangling: %#v\nwaitingForDependentsDeletion: %#v\n", gc.clusterName, item.identity, solid, dangling, waitingForDependentsDeletion) switch { case len(solid) != 0: - klog.V(2).Infof("object %#v has at least one existing owner: %#v, will not garbage collect", item.identity, solid) + klog.V(2).Infof("%s: object %#v has at least one existing owner: %#v, will not garbage collect", gc.clusterName, item.identity, solid) if len(dangling) == 0 && len(waitingForDependentsDeletion) == 0 { return nil } - klog.V(2).Infof("remove dangling references %#v and waiting references %#v for object %s", dangling, waitingForDependentsDeletion, item.identity) + klog.V(2).Infof("%s: remove dangling references %#v and waiting references %#v for object %s", gc.clusterName, dangling, waitingForDependentsDeletion, item.identity) // waitingForDependentsDeletion needs to be deleted from the // ownerReferences, otherwise the referenced objects will be stuck with // the FinalizerDeletingDependents and never get deleted. @@ -570,7 +575,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(ctx context.Context, item *node) // problem. // there are multiple workers run attemptToDeleteItem in // parallel, the circle detection can fail in a race condition. - klog.V(2).Infof("processing object %s, some of its owners and its dependent [%s] have FinalizerDeletingDependents, to prevent potential cycle, its ownerReferences are going to be modified to be non-blocking, then the object is going to be deleted with Foreground", item.identity, dep.identity) + klog.V(2).Infof("%s: processing object %s, some of its owners and its dependent [%s] have FinalizerDeletingDependents, to prevent potential cycle, its ownerReferences are going to be modified to be non-blocking, then the object is going to be deleted with Foreground", gc.clusterName, item.identity, dep.identity) patch, err := item.unblockOwnerReferencesStrategicMergePatch() if err != nil { return err @@ -581,7 +586,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(ctx context.Context, item *node) break } } - klog.V(2).Infof("at least one owner of object %s has FinalizerDeletingDependents, and the object itself has dependents, so it is going to be deleted in Foreground", item.identity) + klog.V(2).Infof("%s: at least one owner of object %s has FinalizerDeletingDependents, and the object itself has dependents, so it is going to be deleted in Foreground", gc.clusterName, item.identity) // the deletion event will be observed by the graphBuilder, so the item // will be processed again in processDeletingDependentsItem. If it // doesn't have dependents, the function will remove the @@ -605,7 +610,7 @@ func (gc *GarbageCollector) attemptToDeleteItem(ctx context.Context, item *node) // otherwise, default to background. policy = metav1.DeletePropagationBackground } - klog.V(2).InfoS("Deleting object", "object", klog.KRef(item.identity.Namespace, item.identity.Name), + klog.V(2).InfoS("Deleting object", "cluster", gc.clusterName, "object", klog.KRef(item.identity.Namespace, item.identity.Name), "objectUID", item.identity.UID, "kind", item.identity.Kind, "propagationPolicy", policy) return gc.deleteObject(item.identity, &policy) } @@ -615,12 +620,12 @@ func (gc *GarbageCollector) attemptToDeleteItem(ctx context.Context, item *node) func (gc *GarbageCollector) processDeletingDependentsItem(item *node) error { blockingDependents := item.blockingDependents() if len(blockingDependents) == 0 { - klog.V(2).Infof("remove DeleteDependents finalizer for item %s", item.identity) + klog.V(2).Infof("%s: remove DeleteDependents finalizer for item %s", gc.clusterName, item.identity) return gc.removeFinalizer(item, metav1.FinalizerDeleteDependents) } for _, dep := range blockingDependents { if !dep.isDeletingDependents() { - klog.V(2).Infof("adding %s to attemptToDelete, because its owner %s is deletingDependents", dep.identity, item.identity) + klog.V(2).Infof("%s: adding %s to attemptToDelete, because its owner %s is deletingDependents", gc.clusterName, dep.identity, item.identity) gc.attemptToDelete.Add(dep) } } @@ -638,7 +643,7 @@ func (gc *GarbageCollector) orphanDependents(owner objectReference, dependents [ // the dependent.identity.UID is used as precondition p, err := c.GenerateDeleteOwnerRefStrategicMergeBytes(dependent.identity.UID, []types.UID{owner.UID}) if err != nil { - errCh <- fmt.Errorf("orphaning %s failed, %v", dependent.identity, err) + errCh <- fmt.Errorf("%s: orphaning %s failed, %v", gc.clusterName, dependent.identity, err) return } _, err = gc.patch(dependent, p, func(n *node) ([]byte, error) { @@ -647,7 +652,7 @@ func (gc *GarbageCollector) orphanDependents(owner objectReference, dependents [ // note that if the target ownerReference doesn't exist in the // dependent, strategic merge patch will NOT return an error. if err != nil && !errors.IsNotFound(err) { - errCh <- fmt.Errorf("orphaning %s failed, %v", dependent.identity, err) + errCh <- fmt.Errorf("%s: orphaning %s failed, %v", gc.clusterName, dependent.identity, err) } }(dependents[i]) } @@ -660,9 +665,9 @@ func (gc *GarbageCollector) orphanDependents(owner objectReference, dependents [ } if len(errorsSlice) != 0 { - return fmt.Errorf("failed to orphan dependents of owner %s, got errors: %s", owner, utilerrors.NewAggregate(errorsSlice).Error()) + return fmt.Errorf("%s: failed to orphan dependents of owner %s, got errors: %s", gc.clusterName, owner, utilerrors.NewAggregate(errorsSlice).Error()) } - klog.V(5).Infof("successfully updated all dependents of owner %s", owner) + klog.V(5).Infof("%s: successfully updated all dependents of owner %s", gc.clusterName, owner) return nil } @@ -699,7 +704,7 @@ func (gc *GarbageCollector) processAttemptToOrphanWorker() bool { func (gc *GarbageCollector) attemptToOrphanWorker(item interface{}) workQueueItemAction { owner, ok := item.(*node) if !ok { - utilruntime.HandleError(fmt.Errorf("expect *node, got %#v", item)) + utilruntime.HandleError(fmt.Errorf("%s: expect *node, got %#v", gc.clusterName, item)) return forgetItem } // we don't need to lock each element, because they never get updated @@ -712,13 +717,13 @@ func (gc *GarbageCollector) attemptToOrphanWorker(item interface{}) workQueueIte err := gc.orphanDependents(owner.identity, dependents) if err != nil { - utilruntime.HandleError(fmt.Errorf("orphanDependents for %s failed with %v", owner.identity, err)) + utilruntime.HandleError(fmt.Errorf("%s: orphanDependents for %s failed with %v", gc.clusterName, owner.identity, err)) return requeueItem } // update the owner, remove "orphaningFinalizer" from its finalizers list err = gc.removeFinalizer(owner, metav1.FinalizerOrphanDependents) if err != nil { - utilruntime.HandleError(fmt.Errorf("removeOrphanFinalizer for %s failed with %v", owner.identity, err)) + utilruntime.HandleError(fmt.Errorf("%s: removeOrphanFinalizer for %s failed with %v", gc.clusterName, owner.identity, err)) return requeueItem } return forgetItem @@ -740,13 +745,13 @@ func (gc *GarbageCollector) GraphHasUID(u types.UID) bool { // All discovery errors are considered temporary. Upon encountering any error, // GetDeletableResources will log and return any discovered resources it was // able to process (which may be none). -func GetDeletableResources(discoveryClient discovery.ServerResourcesInterface) map[schema.GroupVersionResource]struct{} { +func (gc *GarbageCollector) GetDeletableResources(discoveryClient discovery.ServerResourcesInterface) map[schema.GroupVersionResource]struct{} { preferredResources, err := discoveryClient.ServerPreferredResources() if err != nil { if discovery.IsGroupDiscoveryFailedError(err) { - klog.Warningf("failed to discover some groups: %v", err.(*discovery.ErrGroupDiscoveryFailed).Groups) + klog.Warningf("%s: failed to discover some groups: %v", gc.clusterName, err.(*discovery.ErrGroupDiscoveryFailed).Groups) } else { - klog.Warningf("failed to discover preferred resources: %v", err) + klog.Warningf("%s: failed to discover preferred resources: %v", gc.clusterName, err) } } if preferredResources == nil { @@ -760,7 +765,7 @@ func GetDeletableResources(discoveryClient discovery.ServerResourcesInterface) m for _, rl := range deletableResources { gv, err := schema.ParseGroupVersion(rl.GroupVersion) if err != nil { - klog.Warningf("ignoring invalid discovered resource %q: %v", rl.GroupVersion, err) + klog.Warningf("%s: ignoring invalid discovered resource %q: %v", gc.clusterName, rl.GroupVersion, err) continue } for i := range rl.APIResources { diff --git a/pkg/controller/garbagecollector/garbagecollector_patch.go b/pkg/controller/garbagecollector/garbagecollector_patch.go new file mode 100644 index 0000000000000..c60d41fb8b291 --- /dev/null +++ b/pkg/controller/garbagecollector/garbagecollector_patch.go @@ -0,0 +1,143 @@ +/* +Copyright 2022 The KCP 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 ( + "context" + "fmt" + "reflect" + "time" + + "github.com/kcp-dev/logicalcluster/v2" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/discovery" + clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/metadata" + "k8s.io/client-go/tools/cache" + "k8s.io/controller-manager/pkg/informerfactory" + "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/controller/garbagecollector/metrics" +) + +// NewClusterAwareGarbageCollector creates a new GarbageCollector for the provided logical cluster. +func NewClusterAwareGarbageCollector( + kubeClient clientset.Interface, + metadataClient metadata.Interface, + mapper meta.ResettableRESTMapper, + ignoredResources map[schema.GroupResource]struct{}, + sharedInformers informerfactory.InformerFactory, + informersStarted <-chan struct{}, + clusterName logicalcluster.Name, +) (*GarbageCollector, error) { + gc, err := NewGarbageCollector(kubeClient, metadataClient, mapper, ignoredResources, sharedInformers, informersStarted) + if err != nil { + return nil, err + } + gc.clusterName = clusterName + gc.dependencyGraphBuilder.clusterName = clusterName + return gc, nil +} + +func (gc *GarbageCollector) ResyncMonitors(ctx context.Context, discoveryClient discovery.ServerResourcesInterface) { + oldResources := make(map[schema.GroupVersionResource]struct{}) + func() { + // Get the current resource list from discovery. + newResources := gc.GetDeletableResources(discoveryClient) + + // This can occur if there is an internal error in GetDeletableResources. + if len(newResources) == 0 { + klog.V(2).Infof("%s: no resources reported by discovery, skipping garbage collector sync", gc.clusterName) + metrics.GarbageCollectorResourcesSyncError.Inc() + return + } + + // Decide whether discovery has reported a change. + if reflect.DeepEqual(oldResources, newResources) { + klog.V(5).Infof("%s: no resource updates from discovery, skipping garbage collector sync", gc.clusterName) + return + } + + // Ensure workers are paused to avoid processing events before informers + // have resynced. + gc.workerLock.Lock() + defer gc.workerLock.Unlock() + + // Once we get here, we should not unpause workers until we've successfully synced + attempt := 0 + wait.PollImmediateUntil(100*time.Millisecond, func() (bool, error) { + attempt++ + + // On a reattempt, check if available resources have changed + if attempt > 1 { + newResources = gc.GetDeletableResources(discoveryClient) + if len(newResources) == 0 { + klog.V(2).Infof("%s: no resources reported by discovery (attempt %d)", gc.clusterName, attempt) + metrics.GarbageCollectorResourcesSyncError.Inc() + return false, nil + } + } + + klog.V(2).Infof("%s: syncing garbage collector with updated resources from discovery (attempt %d): %s", gc.clusterName, attempt, printDiff(oldResources, newResources)) + + // Resetting the REST mapper will also invalidate the underlying discovery + // client. This is a leaky abstraction and assumes behavior about the REST + // mapper, but we'll deal with it for now. + gc.restMapper.Reset() + klog.V(4).Infof("%s: reset restmapper", gc.clusterName) + + // Perform the monitor resync and wait for controllers to report cache sync. + // + // NOTE: It's possible that newResources will diverge from the resources + // discovered by restMapper during the call to Reset, since they are + // distinct discovery clients invalidated at different times. For example, + // newResources may contain resources not returned in the restMapper's + // discovery call if the resources appeared in-between the calls. In that + // case, the restMapper will fail to map some of newResources until the next + // attempt. + if err := gc.resyncMonitors(newResources); err != nil { + utilruntime.HandleError(fmt.Errorf("%s: failed to sync resource monitors (attempt %d): %v", gc.clusterName, attempt, err)) + metrics.GarbageCollectorResourcesSyncError.Inc() + return false, nil + } + klog.V(4).Infof("%s: resynced monitors", gc.clusterName) + + // wait for caches to fill for a while (our sync period) before attempting to rediscover resources and retry syncing. + // this protects us from deadlocks where available resources changed and one of our informer caches will never fill. + // informers keep attempting to sync in the background, so retrying doesn't interrupt them. + // the call to resyncMonitors on the reattempt will no-op for resources that still exist. + // note that workers stay paused until we successfully resync. + if !cache.WaitForNamedCacheSync(fmt.Sprintf("%s: garbage collector", gc.clusterName), ctx.Done(), gc.dependencyGraphBuilder.IsSynced) { + utilruntime.HandleError(fmt.Errorf("%s: timed out waiting for dependency graph builder sync during GC sync (attempt %d)", gc.clusterName, attempt)) + metrics.GarbageCollectorResourcesSyncError.Inc() + return false, nil + } + + // success, break out of the loop + return true, nil + }, ctx.Done()) + + // Finally, keep track of our new state. Do this after all preceding steps + // have succeeded to ensure we'll retry on subsequent syncs if an error + // occurred. + oldResources = newResources + klog.V(2).Infof("%s: synced garbage collector", gc.clusterName) + }() +} diff --git a/pkg/controller/garbagecollector/graph_builder.go b/pkg/controller/garbagecollector/graph_builder.go index 94da41a8a7fab..a4cef307cdf25 100644 --- a/pkg/controller/garbagecollector/graph_builder.go +++ b/pkg/controller/garbagecollector/graph_builder.go @@ -22,6 +22,8 @@ import ( "sync" "time" + "github.com/kcp-dev/logicalcluster/v2" + "k8s.io/klog/v2" v1 "k8s.io/api/core/v1" @@ -111,6 +113,9 @@ type GraphBuilder struct { absentOwnerCache *ReferenceCache sharedInformers informerfactory.InformerFactory ignoredResources map[schema.GroupResource]struct{} + + // kcp + clusterName logicalcluster.Name } // monitor runs a Controller with a local stop channel. @@ -168,10 +173,10 @@ func (gb *GraphBuilder) controllerFor(resource schema.GroupVersionResource, kind } shared, err := gb.sharedInformers.ForResource(resource) if err != nil { - klog.V(4).Infof("unable to use a shared informer for resource %q, kind %q: %v", resource.String(), kind.String(), err) + klog.V(4).Infof("%s: unable to use a shared informer for resource %q, kind %q: %v", gb.clusterName, resource.String(), kind.String(), err) return nil, nil, err } - klog.V(4).Infof("using a shared informer for resource %q, kind %q", resource.String(), kind.String()) + klog.V(4).Infof("%s: using a shared informer for resource %q, kind %q", gb.clusterName, resource.String(), kind.String()) // need to clone because it's from a shared cache shared.Informer().AddEventHandlerWithResyncPeriod(handlers, ResourceResyncTime) return shared.Informer().GetController(), shared.Informer().GetStore(), nil @@ -207,12 +212,12 @@ func (gb *GraphBuilder) syncMonitors(resources map[schema.GroupVersionResource]s } kind, err := gb.restMapper.KindFor(resource) if err != nil { - errs = append(errs, fmt.Errorf("couldn't look up resource %q: %v", resource, err)) + errs = append(errs, fmt.Errorf("%s: couldn't look up resource %q: %v", gb.clusterName, resource, err)) continue } c, s, err := gb.controllerFor(resource, kind) if err != nil { - errs = append(errs, fmt.Errorf("couldn't start monitor for resource %q: %v", resource, err)) + errs = append(errs, fmt.Errorf("%s: couldn't start monitor for resource %q: %v", gb.clusterName, resource, err)) continue } current[resource] = &monitor{store: s, controller: c} @@ -226,7 +231,7 @@ func (gb *GraphBuilder) syncMonitors(resources map[schema.GroupVersionResource]s } } - klog.V(4).Infof("synced monitors; added %d, kept %d, removed %d", added, kept, len(toRemove)) + klog.V(4).Infof("%s: synced monitors; added %d, kept %d, removed %d", gb.clusterName, added, kept, len(toRemove)) // NewAggregate returns nil if errs is 0-length return utilerrors.NewAggregate(errs) } @@ -258,7 +263,7 @@ func (gb *GraphBuilder) startMonitors() { started++ } } - klog.V(4).Infof("started %d new monitors, %d currently running", started, len(monitors)) + klog.V(4).Infof("%s: started %d new monitors, %d currently running", gb.clusterName, started, len(monitors)) } // IsSynced returns true if any monitors exist AND all those monitors' @@ -270,13 +275,13 @@ func (gb *GraphBuilder) IsSynced() bool { defer gb.monitorLock.Unlock() if len(gb.monitors) == 0 { - klog.V(4).Info("garbage controller monitor not synced: no monitors") + klog.V(4).Infof("%s: garbage controller monitor not synced: no monitors", gb.clusterName) return false } for resource, monitor := range gb.monitors { if !monitor.controller.HasSynced() { - klog.V(4).Infof("garbage controller monitor not yet synced: %+v", resource) + klog.V(4).Infof("%s: garbage controller monitor not yet synced: %+v", gb.clusterName, resource) return false } } @@ -286,8 +291,8 @@ func (gb *GraphBuilder) IsSynced() bool { // Run sets the stop channel and starts monitor execution until stopCh is // closed. Any running monitors will be stopped before Run returns. func (gb *GraphBuilder) Run(stopCh <-chan struct{}) { - klog.Infof("GraphBuilder running") - defer klog.Infof("GraphBuilder stopping") + klog.Infof("%s: GraphBuilder running", gb.clusterName) + defer klog.Infof("%s: GraphBuilder stopping", gb.clusterName) // Set up the stop channel. gb.monitorLock.Lock() @@ -314,7 +319,7 @@ func (gb *GraphBuilder) Run(stopCh <-chan struct{}) { // reset monitors so that the graph builder can be safely re-run/synced. gb.monitors = nil - klog.Infof("stopped %d of %d monitors", stopped, len(monitors)) + klog.Infof("%s: stopped %d of %d monitors", gb.clusterName, stopped, len(monitors)) } var ignoredResources = map[schema.GroupResource]struct{}{ @@ -365,7 +370,7 @@ func (gb *GraphBuilder) addDependentToOwners(n *node, owners []metav1.OwnerRefer dependents: make(map[*node]struct{}), virtual: true, } - klog.V(5).Infof("add virtual node.identity: %s\n\n", ownerNode.identity) + klog.V(5).Infof("%s: add virtual node.identity: %s\n\n", gb.clusterName, ownerNode.identity) gb.uidToNode.Write(ownerNode) } ownerNode.addDependent(n) @@ -382,7 +387,7 @@ func (gb *GraphBuilder) addDependentToOwners(n *node, owners []metav1.OwnerRefer // 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) + klog.V(2).Infof("%s: node %s references an owner %s but does not match namespaces", gb.clusterName, n.identity, ownerNode.identity) gb.reportInvalidNamespaceOwnerRef(n, owner.UID) } hasPotentiallyInvalidOwnerReference = true @@ -390,7 +395,7 @@ func (gb *GraphBuilder) addDependentToOwners(n *node, owners []metav1.OwnerRefer 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) + klog.V(2).Infof("%s: node %s references an owner %s with coordinates that do not match the observed identity", gb.clusterName, n.identity, ownerNode.identity) } hasPotentiallyInvalidOwnerReference = true } else if !ownerIsNamespaced && ownerNode.identity.Namespace != n.identity.Namespace && !ownerNode.isObserved() { @@ -498,7 +503,7 @@ func referencesDiffs(old []metav1.OwnerReference, new []metav1.OwnerReference) ( return added, removed, changed } -func deletionStartsWithFinalizer(oldObj interface{}, newAccessor metav1.Object, matchingFinalizer string) bool { +func (gb *GraphBuilder) deletionStartsWithFinalizer(oldObj interface{}, newAccessor metav1.Object, matchingFinalizer string) bool { // if the new object isn't being deleted, or doesn't have the finalizer we're interested in, return false if !beingDeleted(newAccessor) || !hasFinalizer(newAccessor, matchingFinalizer) { return false @@ -510,7 +515,7 @@ func deletionStartsWithFinalizer(oldObj interface{}, newAccessor metav1.Object, } oldAccessor, err := meta.Accessor(oldObj) if err != nil { - utilruntime.HandleError(fmt.Errorf("cannot access oldObj: %v", err)) + utilruntime.HandleError(fmt.Errorf("%s: cannot access oldObj: %v", gb.clusterName, err)) return false } return !beingDeleted(oldAccessor) || !hasFinalizer(oldAccessor, matchingFinalizer) @@ -540,14 +545,14 @@ func hasFinalizer(accessor metav1.Object, matchingFinalizer string) bool { // this function takes newAccessor directly because the caller already // instantiates an accessor for the newObj. -func startsWaitingForDependentsDeleted(oldObj interface{}, newAccessor metav1.Object) bool { - return deletionStartsWithFinalizer(oldObj, newAccessor, metav1.FinalizerDeleteDependents) +func (gb *GraphBuilder) startsWaitingForDependentsDeleted(oldObj interface{}, newAccessor metav1.Object) bool { + return gb.deletionStartsWithFinalizer(oldObj, newAccessor, metav1.FinalizerDeleteDependents) } // this function takes newAccessor directly because the caller already // instantiates an accessor for the newObj. -func startsWaitingForDependentsOrphaned(oldObj interface{}, newAccessor metav1.Object) bool { - return deletionStartsWithFinalizer(oldObj, newAccessor, metav1.FinalizerOrphanDependents) +func (gb *GraphBuilder) startsWaitingForDependentsOrphaned(oldObj interface{}, newAccessor metav1.Object) bool { + return gb.deletionStartsWithFinalizer(oldObj, newAccessor, metav1.FinalizerOrphanDependents) } // if an blocking ownerReference points to an object gets removed, or gets set to @@ -557,7 +562,7 @@ func (gb *GraphBuilder) addUnblockedOwnersToDeleteQueue(removed []metav1.OwnerRe if ref.BlockOwnerDeletion != nil && *ref.BlockOwnerDeletion { node, found := gb.uidToNode.Read(ref.UID) if !found { - klog.V(5).Infof("cannot find %s in uidToNode", ref.UID) + klog.V(5).Infof("%s: cannot find %s in uidToNode", gb.clusterName, ref.UID) continue } gb.attemptToDelete.Add(node) @@ -569,7 +574,7 @@ func (gb *GraphBuilder) addUnblockedOwnersToDeleteQueue(removed []metav1.OwnerRe if wasBlocked && isUnblocked { node, found := gb.uidToNode.Read(c.newRef.UID) if !found { - klog.V(5).Infof("cannot find %s in uidToNode", c.newRef.UID) + klog.V(5).Infof("%s: cannot find %s in uidToNode", gb.clusterName, c.newRef.UID) continue } gb.attemptToDelete.Add(node) @@ -578,13 +583,13 @@ func (gb *GraphBuilder) addUnblockedOwnersToDeleteQueue(removed []metav1.OwnerRe } func (gb *GraphBuilder) processTransitions(oldObj interface{}, newAccessor metav1.Object, n *node) { - if startsWaitingForDependentsOrphaned(oldObj, newAccessor) { - klog.V(5).Infof("add %s to the attemptToOrphan", n.identity) + if gb.startsWaitingForDependentsOrphaned(oldObj, newAccessor) { + klog.V(5).Infof("%s: add %s to the attemptToOrphan", gb.clusterName, n.identity) gb.attemptToOrphan.Add(n) return } - if startsWaitingForDependentsDeleted(oldObj, newAccessor) { - klog.V(2).Infof("add %s to the attemptToDelete, because it's waiting for its dependents to be deleted", n.identity) + if gb.startsWaitingForDependentsDeleted(oldObj, newAccessor) { + klog.V(2).Infof("%s: add %s to the attemptToDelete, because it's waiting for its dependents to be deleted", gb.clusterName, n.identity) // if the n is added as a "virtual" node, its deletingDependents field is not properly set, so always set it here. n.markDeletingDependents() for dep := range n.dependents { @@ -620,16 +625,16 @@ func (gb *GraphBuilder) processGraphChanges() bool { defer gb.graphChanges.Done(item) event, ok := item.(*event) if !ok { - utilruntime.HandleError(fmt.Errorf("expect a *event, got %v", item)) + utilruntime.HandleError(fmt.Errorf("%s: expect a *event, got %v", gb.clusterName, item)) return true } obj := event.obj accessor, err := meta.Accessor(obj) if err != nil { - utilruntime.HandleError(fmt.Errorf("cannot access obj: %v", err)) + utilruntime.HandleError(fmt.Errorf("%s: cannot access obj: %v", gb.clusterName, err)) return true } - 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) + klog.V(5).Infof("%s: GraphBuilder process object: %s/%s, namespace %s, name %s, uid %s, event type %v, virtual=%v", gb.clusterName, 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 && !event.virtual && !existingNode.isObserved() { @@ -647,14 +652,14 @@ func (gb *GraphBuilder) processGraphChanges() bool { 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) + klog.V(2).Infof("%s: node %s references an owner %s but does not match namespaces", gb.clusterName, 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) + klog.V(2).Infof("%s: replacing virtual node %s with observed node %s", gb.clusterName, existingNode.identity, observedIdentity) existingNode = existingNode.clone() existingNode.identity = observedIdentity gb.uidToNode.Write(existingNode) @@ -696,7 +701,7 @@ func (gb *GraphBuilder) processGraphChanges() bool { gb.processTransitions(event.oldObj, accessor, existingNode) case event.eventType == deleteEvent: if !found { - klog.V(5).Infof("%v doesn't exist in the graph, this shouldn't happen", accessor.GetUID()) + klog.V(5).Infof("%s: %v doesn't exist in the graph, this shouldn't happen", gb.clusterName, accessor.GetUID()) return true } diff --git a/pkg/controller/garbagecollector/operations.go b/pkg/controller/garbagecollector/operations.go index de4ae5e722c65..9fb6eb343b5f1 100644 --- a/pkg/controller/garbagecollector/operations.go +++ b/pkg/controller/garbagecollector/operations.go @@ -89,11 +89,11 @@ func (gc *GarbageCollector) removeFinalizer(owner *node, targetFinalizer string) return nil } if err != nil { - return fmt.Errorf("cannot finalize owner %s, because cannot get it: %v. The garbage collector will retry later", owner.identity, err) + return fmt.Errorf("%s: cannot finalize owner %s, because cannot get it: %v. The garbage collector will retry later", gc.clusterName, owner.identity, err) } accessor, err := meta.Accessor(ownerObject) if err != nil { - return fmt.Errorf("cannot access the owner object %v: %v. The garbage collector will retry later", ownerObject, err) + return fmt.Errorf("%s: cannot access the owner object %v: %v. The garbage collector will retry later", gc.clusterName, ownerObject, err) } finalizers := accessor.GetFinalizers() var newFinalizers []string @@ -106,7 +106,7 @@ func (gc *GarbageCollector) removeFinalizer(owner *node, targetFinalizer string) newFinalizers = append(newFinalizers, f) } if !found { - klog.V(5).Infof("the %s finalizer is already removed from object %s", targetFinalizer, owner.identity) + klog.V(5).Infof("%s: the %s finalizer is already removed from object %s", gc.clusterName, targetFinalizer, owner.identity) return nil } @@ -118,13 +118,13 @@ func (gc *GarbageCollector) removeFinalizer(owner *node, targetFinalizer string) }, }) if err != nil { - return fmt.Errorf("unable to finalize %s due to an error serializing patch: %v", owner.identity, err) + return fmt.Errorf("%s: unable to finalize %s due to an error serializing patch: %v", gc.clusterName, owner.identity, err) } _, err = gc.patchObject(owner.identity, patch, types.MergePatchType) return err }) if errors.IsConflict(err) { - return fmt.Errorf("updateMaxRetries(%d) has reached. The garbage collector will retry later for owner %v", retry.DefaultBackoff.Steps, owner.identity) + return fmt.Errorf("%s: updateMaxRetries(%d) has reached. The garbage collector will retry later for owner %v", gc.clusterName, retry.DefaultBackoff.Steps, owner.identity) } return err } diff --git a/pkg/controller/garbagecollector/patch.go b/pkg/controller/garbagecollector/patch.go index 25d8b2085132c..25eb9fbd085e0 100644 --- a/pkg/controller/garbagecollector/patch.go +++ b/pkg/controller/garbagecollector/patch.go @@ -57,7 +57,7 @@ func (gc *GarbageCollector) getMetadata(apiVersion, kind, namespace, name string } obj, ok := raw.(runtime.Object) if !ok { - return nil, fmt.Errorf("expect a runtime.Object, got %v", raw) + return nil, fmt.Errorf("%s: expect a runtime.Object, got %v", gc.clusterName, raw) } return meta.Accessor(obj) }