From 54715cb2129b35137c371eae16398336707d5078 Mon Sep 17 00:00:00 2001 From: dislbenn Date: Fri, 4 Sep 2026 15:11:22 -0400 Subject: [PATCH 1/2] [ACM-40355] Clean up component resources removed from Helm charts Adds per-component resource tracking so that resources removed from a component's Helm chart templates (e.g. a ServiceMonitor deleted in a newer release) are cleaned up automatically during upgrades, instead of being silently orphaned. Mirrors the equivalent fix applied to multiclusterhub-operator. - InternalEngineComponentSpec now tracks ManagedResources (APIVersion, Kind, Name, Namespace) for the resources currently rendered by a component's chart. The list is refreshed on every reconcile, but the InternalEngineComponent CR is only patched when it actually changes. - Each of the 18 toggleable components that render Helm chart templates (all except local-cluster, which has no chart templates) now diffs its previously tracked resource list against the newly rendered list on every reconcile, and deletes anything no longer present via the existing deleteTemplate() ownership-check logic (backplaneconfig.name label applied by utils.AddBackplaneConfigLabels to every rendered template), so manually recreated resources are left untouched. maestro's disable path removes its whole namespace directly rather than deleting individual templates, so only its enable path needed instrumenting. - A small legacyManagedResources bridge list handles the specific ACM-40355 regression: InternalEngineComponent CRs created before this change have no resource history to diff against, so the console-mce component's legacy "console-mce-monitor" ServiceMonitor (removed in #3062) is checked and cleaned up unconditionally until all upgrade paths have passed through a release with resource tracking enabled. - Updated both CRD copies (config/crd/bases and pkg/templates/crds/internal, the one actually applied at runtime) to include the new managedResources field so it isn't pruned by the API server's structural schema validation. Scope notes: - local-cluster is intentionally excluded: it doesn't render chart templates or use InternalEngineComponent tracking at all. - Components with resources outside their rendered chart templates (Hive's HiveConfig, ClusterManager's ClusterManager CR/TLS ConfigMaps, HyperShift's addon removal wait, maestro's gRPC ConfigMap/Route) only get tracking for their chart-rendered templates, consistent with the multiclusterhub-operator fix's scope. Fixes stale ServiceMonitor resources causing TargetDown alerts after upgrading with console-mce enabled and PR #3062 removing the legacy metrics ServiceMonitor. --- api/v1/multiclusterengine_types.go | 29 +- api/v1/zz_generated.deepcopy.go | 22 +- ...openshift.io_internalenginecomponents.yaml | 34 ++ controllers/managed_resources.go | 196 ++++++ controllers/managed_resources_test.go | 386 ++++++++++++ controllers/toggle_components.go | 561 ++++++++++++++++++ .../internal/internal-engine-component.yaml | 28 + 7 files changed, 1254 insertions(+), 2 deletions(-) create mode 100644 controllers/managed_resources.go create mode 100644 controllers/managed_resources_test.go diff --git a/api/v1/multiclusterengine_types.go b/api/v1/multiclusterengine_types.go index 47e779006..cdcf0d385 100644 --- a/api/v1/multiclusterengine_types.go +++ b/api/v1/multiclusterengine_types.go @@ -308,7 +308,34 @@ type InternalEngineComponentList struct { Items []InternalEngineComponent `json:"items"` } -type InternalEngineComponentSpec struct{} +type InternalEngineComponentSpec struct { + // ManagedResources tracks the resources currently rendered and applied for this component. + // It is refreshed on every reconcile and is used to detect resources that were deployed by a + // previous version of the component's chart but are no longer part of its rendered templates + // (for example, a resource removed from a Helm chart), so they can be safely cleaned up during + // upgrades. Resources are only removed if they still carry the backplaneconfig.name ownership + // label applied by this operator (see utils.AddBackplaneConfigLabels), so manually recreated + // resources are left untouched. + // +optional + ManagedResources []ManagedResource `json:"managedResources,omitempty"` +} + +// ManagedResource identifies a resource that was rendered and applied as part of a component's +// Helm chart templates. +type ManagedResource struct { + // APIVersion of the resource (e.g. "monitoring.coreos.com/v1"). + APIVersion string `json:"apiVersion"` + + // Kind of the resource (e.g. "ServiceMonitor"). + Kind string `json:"kind"` + + // Name of the resource. + Name string `json:"name"` + + // Namespace of the resource. Empty for cluster-scoped resources. + // +optional + Namespace string `json:"namespace,omitempty"` +} func init() { SchemeBuilder.Register(&MultiClusterEngine{}, &MultiClusterEngineList{}) diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 79190b72d..0fd38f7d8 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -170,7 +170,7 @@ func (in *InternalEngineComponent) DeepCopyInto(out *InternalEngineComponent) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InternalEngineComponent. @@ -226,6 +226,11 @@ func (in *InternalEngineComponentList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InternalEngineComponentSpec) DeepCopyInto(out *InternalEngineComponentSpec) { *out = *in + if in.ManagedResources != nil { + in, out := &in.ManagedResources, &out.ManagedResources + *out = make([]ManagedResource, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InternalEngineComponentSpec. @@ -238,6 +243,21 @@ func (in *InternalEngineComponentSpec) DeepCopy() *InternalEngineComponentSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ManagedResource) DeepCopyInto(out *ManagedResource) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ManagedResource. +func (in *ManagedResource) DeepCopy() *ManagedResource { + if in == nil { + return nil + } + out := new(ManagedResource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MultiClusterEngine) DeepCopyInto(out *MultiClusterEngine) { *out = *in diff --git a/config/crd/bases/multicluster.openshift.io_internalenginecomponents.yaml b/config/crd/bases/multicluster.openshift.io_internalenginecomponents.yaml index 84f594a25..29e324962 100644 --- a/config/crd/bases/multicluster.openshift.io_internalenginecomponents.yaml +++ b/config/crd/bases/multicluster.openshift.io_internalenginecomponents.yaml @@ -36,6 +36,40 @@ spec: metadata: type: object spec: + properties: + managedResources: + description: |- + ManagedResources tracks the resources currently rendered and applied for this component. + It is refreshed on every reconcile and is used to detect resources that were deployed by a + previous version of the component's chart but are no longer part of its rendered templates + (for example, a resource removed from a Helm chart), so they can be safely cleaned up during + upgrades. Resources are only removed if they still carry the backplaneconfig.name ownership + label applied by this operator (see utils.AddBackplaneConfigLabels), so manually recreated + resources are left untouched. + items: + description: |- + ManagedResource identifies a resource that was rendered and applied as part of a component's + Helm chart templates. + properties: + apiVersion: + description: APIVersion of the resource (e.g. "monitoring.coreos.com/v1"). + type: string + kind: + description: Kind of the resource (e.g. "ServiceMonitor"). + type: string + name: + description: Name of the resource. + type: string + namespace: + description: Namespace of the resource. Empty for cluster-scoped + resources. + type: string + required: + - apiVersion + - kind + - name + type: object + type: array type: object type: object served: true diff --git a/controllers/managed_resources.go b/controllers/managed_resources.go new file mode 100644 index 000000000..956d21819 --- /dev/null +++ b/controllers/managed_resources.go @@ -0,0 +1,196 @@ +// Copyright Contributors to the Open Cluster Management project + +package controllers + +import ( + "context" + "fmt" + + backplanev1 "github.com/stolostron/backplane-operator/api/v1" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" +) + +/* +legacyManagedResources lists resources that were removed from a component's Helm chart before +per-component resource tracking (InternalEngineComponentSpec.ManagedResources) existed. Because +InternalEngineComponent CRs created by older operator versions have no recorded resource history, +the generic drift-detection in cleanupOrphanedManagedResources cannot infer that these resources +should be deleted on the first reconcile after upgrading to an operator version that includes this +list. Entries here are checked unconditionally (in addition to the generic tracked-resource diff) +so that already-orphaned resources from prior releases are still cleaned up. + +This is a temporary bridge: once a component has reconciled at least once with resource tracking +enabled, InternalEngineComponent.Spec.ManagedResources will accurately reflect its previously +deployed resources, and future drift will be caught generically. Entries below can be removed once +all supported upgrade paths have passed through a release that populates ManagedResources. + +See ACM-40355 / stolostron/backplane-operator#3062: the console-mce component's legacy +ServiceMonitor ("console-mce-monitor") was removed from the chart, but upgrades that kept +console-mce enabled never rendered/deleted it, leaving customers with stale ServiceMonitors and +TargetDown alerts. +*/ +var legacyManagedResources = map[string][]backplanev1.ManagedResource{ + backplanev1.ConsoleMCE: { + { + APIVersion: "monitoring.coreos.com/v1", + Kind: "ServiceMonitor", + Name: "console-mce-monitor", + // Namespace is set to the MultiClusterEngine target namespace at cleanup time, since + // the legacy resource was always deployed alongside MCE (not necessarily the + // operator's own namespace, in cases where those differ). + }, + }, +} + +// managedResourceKey returns a string uniquely identifying a ManagedResource for set comparisons. +func managedResourceKey(resource backplanev1.ManagedResource) string { + return fmt.Sprintf("%s/%s/%s/%s", resource.APIVersion, resource.Kind, resource.Namespace, resource.Name) +} + +// extractManagedResources builds the list of resources represented by the given rendered +// templates. NetworkPolicy resources are intentionally excluded because they are managed +// separately by ensureNetworkPolicies using a create-once pattern. +func extractManagedResources(templates []*unstructured.Unstructured) []backplanev1.ManagedResource { + resources := make([]backplanev1.ManagedResource, 0, len(templates)) + for _, template := range templates { + if template.GetKind() == "NetworkPolicy" { + continue + } + + resources = append(resources, backplanev1.ManagedResource{ + APIVersion: template.GetAPIVersion(), + Kind: template.GetKind(), + Name: template.GetName(), + Namespace: template.GetNamespace(), + }) + } + return resources +} + +// managedResourcesEqual reports whether two ManagedResource lists represent the same set of +// resources, regardless of order. +func managedResourcesEqual(a, b []backplanev1.ManagedResource) bool { + if len(a) != len(b) { + return false + } + + seen := make(map[string]struct{}, len(a)) + for _, resource := range a { + seen[managedResourceKey(resource)] = struct{}{} + } + + for _, resource := range b { + if _, ok := seen[managedResourceKey(resource)]; !ok { + return false + } + } + return true +} + +// getManagedResources returns the resources currently recorded on the component's +// InternalEngineComponent CR. It returns nil (without error) if the CR does not exist, since +// callers treat "no tracked resources" as an empty diff baseline rather than a failure. +func (r *MultiClusterEngineReconciler) getManagedResources(ctx context.Context, mce *backplanev1.MultiClusterEngine, + component string) []backplanev1.ManagedResource { + + iec := &backplanev1.InternalEngineComponent{} + if err := r.Client.Get(ctx, types.NamespacedName{Name: component, Namespace: mce.Spec.TargetNamespace}, + iec); err != nil { + if !apierrors.IsNotFound(err) { + log.Error(err, "failed to get InternalEngineComponent while reading managed resources", + "Component", component, "Namespace", mce.Spec.TargetNamespace) + } + return nil + } + + return iec.Spec.ManagedResources +} + +// updateManagedResources patches the component's InternalEngineComponent CR with the current list +// of managed resources, but only if the list has actually changed, to avoid unnecessary writes on +// every reconcile. +func (r *MultiClusterEngineReconciler) updateManagedResources(ctx context.Context, mce *backplanev1.MultiClusterEngine, + component string, resources []backplanev1.ManagedResource) error { + + iec := &backplanev1.InternalEngineComponent{} + if err := r.Client.Get(ctx, types.NamespacedName{Name: component, Namespace: mce.Spec.TargetNamespace}, + iec); err != nil { + if apierrors.IsNotFound(err) { + // The InternalEngineComponent CR is created by ensureInternalEngineComponent; if it's + // missing here there's nothing to update. + return nil + } + return fmt.Errorf("failed to get InternalEngineComponent %s/%s: %v", mce.Spec.TargetNamespace, component, err) + } + + if managedResourcesEqual(iec.Spec.ManagedResources, resources) { + return nil + } + + iec.Spec.ManagedResources = resources + if err := r.Client.Update(ctx, iec); err != nil { + return fmt.Errorf("failed to update InternalEngineComponent %s/%s managed resources: %v", + mce.Spec.TargetNamespace, component, err) + } + + return nil +} + +// cleanupOrphanedManagedResources deletes resources that are present in oldResources but absent +// from newResources - i.e. resources that were deployed by a previous version of the component's +// templates but are no longer rendered. Deletion is delegated to deleteTemplate, which only +// removes resources that still carry this operator's backplaneconfig.name ownership label, so +// resources that were manually recreated (and therefore lack that label) are left untouched. +func (r *MultiClusterEngineReconciler) cleanupOrphanedManagedResources(ctx context.Context, + mce *backplanev1.MultiClusterEngine, component string, oldResources, + newResources []backplanev1.ManagedResource) (ctrl.Result, error) { + + current := make(map[string]struct{}, len(newResources)) + for _, resource := range newResources { + current[managedResourceKey(resource)] = struct{}{} + } + + // Merge in any known legacy resources for this component that predate resource tracking (see + // legacyManagedResources doc comment). Namespace defaults to the MultiClusterEngine target + // namespace when unset, matching how these resources were originally deployed. + orphanCandidates := append([]backplanev1.ManagedResource{}, oldResources...) + for _, legacy := range legacyManagedResources[component] { + if legacy.Namespace == "" { + legacy.Namespace = mce.Spec.TargetNamespace + } + orphanCandidates = append(orphanCandidates, legacy) + } + + seenCandidates := make(map[string]struct{}, len(orphanCandidates)) + for _, resource := range orphanCandidates { + key := managedResourceKey(resource) + if _, alreadyHandled := seenCandidates[key]; alreadyHandled { + continue + } + seenCandidates[key] = struct{}{} + + if _, stillRendered := current[key]; stillRendered { + continue + } + + stub := &unstructured.Unstructured{} + stub.SetAPIVersion(resource.APIVersion) + stub.SetKind(resource.Kind) + stub.SetName(resource.Name) + stub.SetNamespace(resource.Namespace) + + log.Info("Cleaning up resource no longer present in component templates", + "Component", component, "APIVersion", resource.APIVersion, "Kind", resource.Kind, + "Name", resource.Name, "Namespace", resource.Namespace) + + if result, err := r.deleteTemplate(ctx, mce, stub); result != (ctrl.Result{}) || err != nil { + return result, err + } + } + + return ctrl.Result{}, nil +} diff --git a/controllers/managed_resources_test.go b/controllers/managed_resources_test.go new file mode 100644 index 000000000..f37ddc1d4 --- /dev/null +++ b/controllers/managed_resources_test.go @@ -0,0 +1,386 @@ +// Copyright Contributors to the Open Cluster Management project + +package controllers + +import ( + "context" + "testing" + + promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + backplanev1 "github.com/stolostron/backplane-operator/api/v1" + + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func newManagedResourcesTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + s := runtime.NewScheme() + if err := appsv1.AddToScheme(s); err != nil { + t.Fatalf("failed to add appsv1 to scheme: %v", err) + } + if err := backplanev1.AddToScheme(s); err != nil { + t.Fatalf("failed to add backplanev1 to scheme: %v", err) + } + if err := promv1.AddToScheme(s); err != nil { + t.Fatalf("failed to add promv1 to scheme: %v", err) + } + return s +} + +func newManagedResource(apiVersion, kind, name, namespace string) backplanev1.ManagedResource { + return backplanev1.ManagedResource{ + APIVersion: apiVersion, + Kind: kind, + Name: name, + Namespace: namespace, + } +} + +func TestManagedResourcesEqual(t *testing.T) { + tests := []struct { + name string + a []backplanev1.ManagedResource + b []backplanev1.ManagedResource + want bool + }{ + { + name: "both empty", + a: nil, + b: []backplanev1.ManagedResource{}, + want: true, + }, + { + name: "identical order", + a: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "a", "ns"), + newManagedResource("v1", "Service", "b", "ns"), + }, + b: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "a", "ns"), + newManagedResource("v1", "Service", "b", "ns"), + }, + want: true, + }, + { + name: "same set, different order", + a: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "a", "ns"), + newManagedResource("v1", "Service", "b", "ns"), + }, + b: []backplanev1.ManagedResource{ + newManagedResource("v1", "Service", "b", "ns"), + newManagedResource("apps/v1", "Deployment", "a", "ns"), + }, + want: true, + }, + { + name: "different lengths", + a: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "a", "ns"), + }, + b: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "a", "ns"), + newManagedResource("v1", "Service", "b", "ns"), + }, + want: false, + }, + { + name: "same length, different contents", + a: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "a", "ns"), + }, + b: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "c", "ns"), + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := managedResourcesEqual(tt.a, tt.b); got != tt.want { + t.Errorf("managedResourcesEqual() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestExtractManagedResources(t *testing.T) { + deployment := &unstructured.Unstructured{} + deployment.SetAPIVersion("apps/v1") + deployment.SetKind("Deployment") + deployment.SetName("my-deploy") + deployment.SetNamespace("test-ns") + + networkPolicy := &unstructured.Unstructured{} + networkPolicy.SetAPIVersion("networking.k8s.io/v1") + networkPolicy.SetKind("NetworkPolicy") + networkPolicy.SetName("my-np") + networkPolicy.SetNamespace("test-ns") + + clusterRole := &unstructured.Unstructured{} + clusterRole.SetAPIVersion("rbac.authorization.k8s.io/v1") + clusterRole.SetKind("ClusterRole") + clusterRole.SetName("my-cr") + + resources := extractManagedResources([]*unstructured.Unstructured{deployment, networkPolicy, clusterRole}) + + want := []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "my-deploy", "test-ns"), + newManagedResource("rbac.authorization.k8s.io/v1", "ClusterRole", "my-cr", ""), + } + + if !managedResourcesEqual(resources, want) { + t.Errorf("extractManagedResources() = %v, want %v (NetworkPolicy should be excluded)", resources, want) + } + if len(resources) != 2 { + t.Errorf("extractManagedResources() returned %d resources, want 2 (NetworkPolicy should be skipped)", + len(resources)) + } +} + +func TestGetAndUpdateManagedResources(t *testing.T) { + s := newManagedResourcesTestScheme(t) + + mce := &backplanev1.MultiClusterEngine{ + ObjectMeta: metav1.ObjectMeta{Name: "mce"}, + Spec: backplanev1.MultiClusterEngineSpec{TargetNamespace: "test-ns"}, + } + + fakeClient := fake.NewClientBuilder().WithScheme(s).Build() + r := &MultiClusterEngineReconciler{Client: fakeClient} + + // No InternalEngineComponent exists yet - should return nil without error. + if got := r.getManagedResources(context.TODO(), mce, "console-mce"); got != nil { + t.Errorf("getManagedResources() with no CR = %v, want nil", got) + } + + // updateManagedResources should be a no-op (not an error) when the CR doesn't exist yet. + if err := r.updateManagedResources(context.TODO(), mce, "console-mce", + []backplanev1.ManagedResource{newManagedResource("v1", "ConfigMap", "cm", "test-ns")}); err != nil { + t.Errorf("updateManagedResources() with no CR returned error: %v", err) + } + + // Create the InternalEngineComponent CR (mirrors ensureInternalEngineComponent). + iec := &backplanev1.InternalEngineComponent{ + ObjectMeta: metav1.ObjectMeta{Name: "console-mce", Namespace: "test-ns"}, + } + if err := fakeClient.Create(context.TODO(), iec); err != nil { + t.Fatalf("failed to create InternalEngineComponent: %v", err) + } + + initial := []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "console-mce-console", "test-ns"), + newManagedResource("monitoring.coreos.com/v1", "ServiceMonitor", "console-mce-monitor", "test-ns"), + } + if err := r.updateManagedResources(context.TODO(), mce, "console-mce", initial); err != nil { + t.Fatalf("updateManagedResources() returned error: %v", err) + } + + got := r.getManagedResources(context.TODO(), mce, "console-mce") + if !managedResourcesEqual(got, initial) { + t.Errorf("getManagedResources() = %v, want %v", got, initial) + } + + // Updating with the same set (different order) should be a no-op but not error. + reordered := []backplanev1.ManagedResource{initial[1], initial[0]} + if err := r.updateManagedResources(context.TODO(), mce, "console-mce", reordered); err != nil { + t.Fatalf("updateManagedResources() with reordered list returned error: %v", err) + } + + // Updating with a smaller set (ServiceMonitor removed from chart) should persist the new list. + updated := []backplanev1.ManagedResource{initial[0]} + if err := r.updateManagedResources(context.TODO(), mce, "console-mce", updated); err != nil { + t.Fatalf("updateManagedResources() returned error: %v", err) + } + + got = r.getManagedResources(context.TODO(), mce, "console-mce") + if !managedResourcesEqual(got, updated) { + t.Errorf("getManagedResources() after update = %v, want %v", got, updated) + } +} + +func TestCleanupOrphanedManagedResources(t *testing.T) { + s := newManagedResourcesTestScheme(t) + + mce := &backplanev1.MultiClusterEngine{ + ObjectMeta: metav1.ObjectMeta{Name: "mce"}, + Spec: backplanev1.MultiClusterEngineSpec{TargetNamespace: "multicluster-engine"}, + } + + tests := []struct { + name string + component string + oldResources []backplanev1.ManagedResource + newResources []backplanev1.ManagedResource + setupClient func(t *testing.T) client.Client + verify func(t *testing.T, c client.Client) + expectRequeue bool + }{ + { + name: "resource removed from chart and owned by MCE is deleted", + component: "example", + oldResources: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "old-deploy", "multicluster-engine"), + }, + newResources: nil, + setupClient: func(t *testing.T) client.Client { + deploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-deploy", + Namespace: "multicluster-engine", + Labels: map[string]string{"backplaneconfig.name": "mce"}, + }, + } + return fake.NewClientBuilder().WithScheme(s).WithObjects(deploy).Build() + }, + verify: func(t *testing.T, c client.Client) { + err := c.Get(context.TODO(), types.NamespacedName{Name: "old-deploy", + Namespace: "multicluster-engine"}, &appsv1.Deployment{}) + if err == nil { + t.Errorf("expected old-deploy to be deleted, but it still exists") + } + }, + }, + { + name: "resource removed from chart but manually recreated without MCE label is left alone", + component: "example", + oldResources: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "adopted-deploy", "multicluster-engine"), + }, + newResources: nil, + setupClient: func(t *testing.T) client.Client { + deploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "adopted-deploy", + Namespace: "multicluster-engine", + // No backplaneconfig.name label - simulates a resource manually recreated + // by a user after MCE deleted it, or never owned by MCE. + }, + } + return fake.NewClientBuilder().WithScheme(s).WithObjects(deploy).Build() + }, + verify: func(t *testing.T, c client.Client) { + err := c.Get(context.TODO(), types.NamespacedName{Name: "adopted-deploy", + Namespace: "multicluster-engine"}, &appsv1.Deployment{}) + if err != nil { + t.Errorf("expected adopted-deploy to be left alone, but got error: %v", err) + } + }, + }, + { + name: "resource still present in current templates is not touched", + component: "example", + oldResources: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "kept-deploy", "multicluster-engine"), + }, + newResources: []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "kept-deploy", "multicluster-engine"), + }, + setupClient: func(t *testing.T) client.Client { + deploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kept-deploy", + Namespace: "multicluster-engine", + Labels: map[string]string{"backplaneconfig.name": "mce"}, + }, + } + return fake.NewClientBuilder().WithScheme(s).WithObjects(deploy).Build() + }, + verify: func(t *testing.T, c client.Client) { + err := c.Get(context.TODO(), types.NamespacedName{Name: "kept-deploy", + Namespace: "multicluster-engine"}, &appsv1.Deployment{}) + if err != nil { + t.Errorf("expected kept-deploy to still exist, got error: %v", err) + } + }, + }, + { + name: "legacy console-mce ServiceMonitor is cleaned up even with no tracked history (ACM-40355)", + component: backplanev1.ConsoleMCE, + oldResources: nil, // Simulates an InternalEngineComponent CR from before resource tracking existed. + newResources: nil, + setupClient: func(t *testing.T) client.Client { + sm := &promv1.ServiceMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "console-mce-monitor", + Namespace: "multicluster-engine", + Labels: map[string]string{"backplaneconfig.name": "mce"}, + }, + } + return fake.NewClientBuilder().WithScheme(s).WithObjects(sm).Build() + }, + verify: func(t *testing.T, c client.Client) { + err := c.Get(context.TODO(), types.NamespacedName{Name: "console-mce-monitor", + Namespace: "multicluster-engine"}, &promv1.ServiceMonitor{}) + if err == nil { + t.Errorf("expected legacy console-mce-monitor ServiceMonitor to be deleted, but it still exists") + } + }, + }, + { + name: "legacy console-mce ServiceMonitor absent from cluster is a no-op", + component: backplanev1.ConsoleMCE, + oldResources: nil, + newResources: nil, + setupClient: func(t *testing.T) client.Client { + return fake.NewClientBuilder().WithScheme(s).Build() + }, + verify: func(t *testing.T, c client.Client) { + // Nothing to verify beyond "no error/requeue", asserted below. + }, + }, + { + name: "legacy cleanup does not run for unrelated components", + component: "example", + setupClient: func(t *testing.T) client.Client { + sm := &promv1.ServiceMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "console-mce-monitor", + Namespace: "multicluster-engine", + Labels: map[string]string{"backplaneconfig.name": "mce"}, + }, + } + return fake.NewClientBuilder().WithScheme(s).WithObjects(sm).Build() + }, + verify: func(t *testing.T, c client.Client) { + err := c.Get(context.TODO(), types.NamespacedName{Name: "console-mce-monitor", + Namespace: "multicluster-engine"}, &promv1.ServiceMonitor{}) + if err != nil { + t.Errorf("console-mce-monitor should only be cleaned up for console-mce, got error: %v", err) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := tt.setupClient(t) + r := &MultiClusterEngineReconciler{Client: c} + + result, err := r.cleanupOrphanedManagedResources(context.TODO(), mce, tt.component, + tt.oldResources, tt.newResources) + + if err != nil { + t.Errorf("cleanupOrphanedManagedResources() unexpected error: %v", err) + } + if tt.expectRequeue && result == (ctrl.Result{}) { + t.Errorf("cleanupOrphanedManagedResources() expected requeue, got empty result") + } + if !tt.expectRequeue && result != (ctrl.Result{}) { + t.Errorf("cleanupOrphanedManagedResources() expected no requeue, got %v", result) + } + + if tt.verify != nil { + tt.verify(t, c) + } + }) + } +} diff --git a/controllers/toggle_components.go b/controllers/toggle_components.go index 6ca4608a7..4ff8249b4 100644 --- a/controllers/toggle_components.go +++ b/controllers/toggle_components.go @@ -65,6 +65,16 @@ func (r *MultiClusterEngineReconciler) ensureConsoleMCE(ctx context.Context, mce return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ConsoleMCE) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ConsoleMCE, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ConsoleMCE); err != nil { return result, err @@ -84,6 +94,12 @@ func (r *MultiClusterEngineReconciler) ensureConsoleMCE(ctx context.Context, mce } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ConsoleMCE, newManagedResources); err != nil { + return ctrl.Result{}, err + } + // Check console-mce deployment health before adding plugin consoleDeployment := &appsv1.Deployment{} err := r.Client.Get(ctx, namespacedName, consoleDeployment) @@ -108,6 +124,11 @@ func (r *MultiClusterEngineReconciler) ensureNoConsoleMCE(ctx context.Context, m namespacedName := types.NamespacedName{Name: "console-mce-console", Namespace: mce.Spec.TargetNamespace} r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ConsoleMCE) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ConsoleMCE); (result != ctrl.Result{}) || err != nil { @@ -139,6 +160,15 @@ func (r *MultiClusterEngineReconciler) ensureNoConsoleMCE(ctx context.Context, m return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ConsoleMCE, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Deletes all templates for _, template := range templates { // Skip NetworkPolicy resources - they are managed by ensureNetworkPolicies with create-once pattern @@ -184,6 +214,16 @@ func (r *MultiClusterEngineReconciler) ensureManagedServiceAccount(ctx context.C return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ManagedServiceAccount) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ManagedServiceAccount, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ManagedServiceAccount); err != nil { return result, err @@ -211,12 +251,23 @@ func (r *MultiClusterEngineReconciler) ensureManagedServiceAccount(ctx context.C if missingCRDErrorOccured { return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ManagedServiceAccount, newManagedResources); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } func (r *MultiClusterEngineReconciler) ensureNoManagedServiceAccount(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ManagedServiceAccount) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ManagedServiceAccount); (result != ctrl.Result{}) || err != nil { @@ -233,6 +284,15 @@ func (r *MultiClusterEngineReconciler) ensureNoManagedServiceAccount(ctx context return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ManagedServiceAccount, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.AddComponent(toggle.DisabledStatus(types.NamespacedName{Name: "managedservice", Namespace: mce.Spec.TargetNamespace}, []*unstructured.Unstructured{})) @@ -279,6 +339,16 @@ func (r *MultiClusterEngineReconciler) ensureFleetNavigation(ctx context.Context return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.FleetNavigation) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.FleetNavigation, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + missingCRDErrorOccured := false for _, template := range templates { applyReleaseVersionAnnotation(template) @@ -298,12 +368,23 @@ func (r *MultiClusterEngineReconciler) ensureFleetNavigation(ctx context.Context if missingCRDErrorOccured { return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.FleetNavigation, newManagedResources); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } func (r *MultiClusterEngineReconciler) ensureNoFleetNavigation(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.FleetNavigation) + if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.FleetNavigation); (result != ctrl.Result{}) || err != nil { return result, err @@ -318,6 +399,15 @@ func (r *MultiClusterEngineReconciler) ensureNoFleetNavigation(ctx context.Conte return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.FleetNavigation, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.AddComponent(toggle.DisabledStatus(types.NamespacedName{Name: backplanev1.FleetNavigation, Namespace: mce.Spec.TargetNamespace}, []*unstructured.Unstructured{})) @@ -424,6 +514,16 @@ func (r *MultiClusterEngineReconciler) ensureDiscovery(ctx context.Context, mce return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.Discovery) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.Discovery, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.Discovery); err != nil { return result, err @@ -443,6 +543,12 @@ func (r *MultiClusterEngineReconciler) ensureDiscovery(ctx context.Context, mce } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.Discovery, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -450,6 +556,11 @@ func (r *MultiClusterEngineReconciler) ensureNoDiscovery(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { namespacedName := types.NamespacedName{Name: "discovery-operator", Namespace: mce.Spec.TargetNamespace} + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.Discovery) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.Discovery); (result != ctrl.Result{}) || err != nil { @@ -467,6 +578,15 @@ func (r *MultiClusterEngineReconciler) ensureNoDiscovery(ctx context.Context, return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.Discovery, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -509,6 +629,16 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPI(ctx context.Context, mce return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPI) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPI, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPI); err != nil { return result, err @@ -528,6 +658,12 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPI(ctx context.Context, mce } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ClusterAPI, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -535,6 +671,11 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPI(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { namespacedName := types.NamespacedName{Name: "capi-controller-manager", Namespace: mce.Spec.TargetNamespace} + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPI) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ClusterAPI); (result != ctrl.Result{}) || err != nil { @@ -552,6 +693,15 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPI(ctx context.Context, return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPI, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -595,6 +745,16 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderAWS(ctx context.C return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPIProviderAWS); err != nil { return result, err @@ -614,6 +774,12 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderAWS(ctx context.C } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -621,6 +787,11 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderAWS(ctx context mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { namespacedName := types.NamespacedName{Name: "capa-controller-manager", Namespace: mce.Spec.TargetNamespace} + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ClusterAPIProviderAWS); (result != ctrl.Result{}) || err != nil { @@ -638,6 +809,15 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderAWS(ctx context return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -685,6 +865,16 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderAzure(ctx context return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview, + oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPIProviderAzurePreview); err != nil { return result, err @@ -704,12 +894,23 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderAzure(ctx context } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderAzure(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview) + namespacedName := types.NamespacedName{Name: "azureserviceoperator-controller-manager", Namespace: mce.Spec.TargetNamespace} r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) @@ -736,6 +937,15 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderAzure(ctx conte return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview, + oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -779,6 +989,16 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderMetal(ctx context return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPIProviderMetal); err != nil { return result, err @@ -798,6 +1018,12 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderMetal(ctx context } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -805,6 +1031,11 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderMetal(ctx conte mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { namespacedName := types.NamespacedName{Name: "mce-capm3-controller-manager", Namespace: mce.Spec.TargetNamespace} + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ClusterAPIProviderMetal); (result != ctrl.Result{}) || err != nil { @@ -822,6 +1053,15 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderMetal(ctx conte return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -867,6 +1107,16 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderOA(ctx context.Co return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPIProviderOA); err != nil { return result, err @@ -886,6 +1136,12 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderOA(ctx context.Co } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -898,6 +1154,11 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderOA(ctx context. r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ClusterAPIProviderOA); (result != ctrl.Result{}) || err != nil { @@ -915,6 +1176,15 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderOA(ctx context. return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Deletes all templates for _, template := range templates { // Skip NetworkPolicy resources - they are managed by ensureNetworkPolicies with create-once pattern @@ -954,6 +1224,17 @@ func (r *MultiClusterEngineReconciler) ensureHive(ctx context.Context, mce *back return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. Note: this only covers the chart-rendered templates below, not the + // HiveConfig custom resource applied separately at the end of this function. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.Hive) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.Hive, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.Hive); err != nil { return result, err @@ -973,6 +1254,12 @@ func (r *MultiClusterEngineReconciler) ensureHive(ctx context.Context, mce *back } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.Hive, newManagedResources); err != nil { + return ctrl.Result{}, err + } + hiveTemplate := hive.HiveConfig(mce) return r.ensureUnstructuredResource(ctx, mce, hiveTemplate) } @@ -982,6 +1269,11 @@ func (r *MultiClusterEngineReconciler) ensureNoHive(ctx context.Context, mce *ba namespacedName := types.NamespacedName{Name: "hive-operator", Namespace: mce.Spec.TargetNamespace} + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.Hive) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.Hive); (result != ctrl.Result{}) || err != nil { @@ -999,6 +1291,15 @@ func (r *MultiClusterEngineReconciler) ensureNoHive(ctx context.Context, mce *ba return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.Hive, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -1060,6 +1361,16 @@ func (r *MultiClusterEngineReconciler) ensureAssistedService(ctx context.Context return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.AssistedService) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.AssistedService, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.AssistedService); err != nil { return result, err @@ -1079,6 +1390,12 @@ func (r *MultiClusterEngineReconciler) ensureAssistedService(ctx context.Context } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.AssistedService, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -1091,6 +1408,11 @@ func (r *MultiClusterEngineReconciler) ensureNoAssistedService(ctx context.Conte } namespacedName := types.NamespacedName{Name: "infrastructure-operator", Namespace: targetNamespace} + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.AssistedService) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.AssistedService); (result != ctrl.Result{}) || err != nil { @@ -1109,6 +1431,15 @@ func (r *MultiClusterEngineReconciler) ensureNoAssistedService(ctx context.Conte return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.AssistedService, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -1164,6 +1495,16 @@ func (r *MultiClusterEngineReconciler) ensureServerFoundation(ctx context.Contex return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ServerFoundation) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ServerFoundation, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ServerFoundation); err != nil { return result, err @@ -1183,12 +1524,23 @@ func (r *MultiClusterEngineReconciler) ensureServerFoundation(ctx context.Contex } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ServerFoundation, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } func (r *MultiClusterEngineReconciler) ensureNoServerFoundation(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ServerFoundation) + // Ensure that the InternalHubComponent CR instance is created for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ServerFoundation); (result != ctrl.Result{}) || err != nil { @@ -1206,6 +1558,15 @@ func (r *MultiClusterEngineReconciler) ensureNoServerFoundation(ctx context.Cont return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ServerFoundation, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + namespacedName := types.NamespacedName{Name: "ocm-controller", Namespace: mce.Spec.TargetNamespace} r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -1263,6 +1624,16 @@ func (r *MultiClusterEngineReconciler) ensureImageBasedInstallOperator(ctx conte return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator, + oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ImageBasedInstallOperator); err != nil { @@ -1283,6 +1654,12 @@ func (r *MultiClusterEngineReconciler) ensureImageBasedInstallOperator(ctx conte } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -1292,6 +1669,11 @@ func (r *MultiClusterEngineReconciler) ensureNoImageBasedInstallOperator(ctx con targetNamespace := mce.Spec.TargetNamespace namespacedName := types.NamespacedName{Name: "image-based-install-operator", Namespace: targetNamespace} + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ImageBasedInstallOperator); (result != ctrl.Result{}) || err != nil { @@ -1309,6 +1691,15 @@ func (r *MultiClusterEngineReconciler) ensureNoImageBasedInstallOperator(ctx con return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator, + oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -1366,6 +1757,16 @@ func (r *MultiClusterEngineReconciler) ensureClusterLifecycle(ctx context.Contex return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterLifecycle) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterLifecycle, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterLifecycle); err != nil { return result, err @@ -1385,12 +1786,23 @@ func (r *MultiClusterEngineReconciler) ensureClusterLifecycle(ctx context.Contex } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ClusterLifecycle, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } func (r *MultiClusterEngineReconciler) ensureNoClusterLifecycle(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterLifecycle) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ClusterLifecycle); (result != ctrl.Result{}) || err != nil { @@ -1408,6 +1820,15 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterLifecycle(ctx context.Cont return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterLifecycle, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + if utils.DeployOnOCP() { namespacedName := types.NamespacedName{Name: "cluster-curator-controller", Namespace: mce.Spec.TargetNamespace} r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) @@ -1465,6 +1886,17 @@ func (r *MultiClusterEngineReconciler) ensureClusterManager(ctx context.Context, return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. Note: this only covers the chart-rendered templates below, not the + // ClusterManager custom resource or TLS profile ConfigMaps applied separately below. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterManager) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterManager, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterManager); err != nil { return result, err @@ -1484,6 +1916,12 @@ func (r *MultiClusterEngineReconciler) ensureClusterManager(ctx context.Context, } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ClusterManager, newManagedResources); err != nil { + return ctrl.Result{}, err + } + // Apply clustermanager cmTemplate := foundation.ClusterManager(mce, r.CacheSpec.ImageOverrides) if err := ctrl.SetControllerReference(mce, cmTemplate, r.Scheme); err != nil { @@ -1510,6 +1948,11 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterManager(ctx context.Contex mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { namespacedName := types.NamespacedName{Name: "cluster-manager", Namespace: mce.Spec.TargetNamespace} + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterManager) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ClusterManager); (result != ctrl.Result{}) || err != nil { @@ -1527,6 +1970,15 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterManager(ctx context.Contex return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterManager, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) r.StatusManager.RemoveComponent(status.ClusterManagerStatus{ @@ -1601,6 +2053,16 @@ func (r *MultiClusterEngineReconciler) ensureClusterPermission(ctx context.Conte return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterPermission) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterPermission, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterPermission); err != nil { return result, err @@ -1620,6 +2082,12 @@ func (r *MultiClusterEngineReconciler) ensureClusterPermission(ctx context.Conte } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ClusterPermission, newManagedResources); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -1628,6 +2096,11 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterPermission(ctx context.Con namespacedName := types.NamespacedName{Name: "cluster-permission", Namespace: mce.Spec.TargetNamespace} + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterPermission) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ClusterPermission); (result != ctrl.Result{}) || err != nil { @@ -1645,6 +2118,15 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterPermission(ctx context.Con return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterPermission, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) @@ -1692,6 +2174,16 @@ func (r *MultiClusterEngineReconciler) ensureHyperShift(ctx context.Context, mce return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.HyperShift) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.HyperShift, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.HyperShift); err != nil { return result, err @@ -1723,12 +2215,23 @@ func (r *MultiClusterEngineReconciler) ensureHyperShift(ctx context.Context, mce if missingCRDErrorOccured { return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.HyperShift, newManagedResources); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } func (r *MultiClusterEngineReconciler) ensureNoHyperShift(ctx context.Context, mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) { + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.HyperShift) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.HyperShift); (result != ctrl.Result{}) || err != nil { @@ -1786,6 +2289,15 @@ func (r *MultiClusterEngineReconciler) ensureNoHyperShift(ctx context.Context, return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.HyperShift, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Deletes all templates for _, template := range templates { // Skip NetworkPolicy resources - they are managed by ensureNetworkPolicies with create-once pattern @@ -1964,6 +2476,16 @@ func (r *MultiClusterEngineReconciler) ensureClusterProxyAddon(ctx context.Conte return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterProxyAddon) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterProxyAddon, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterProxyAddon); err != nil { return result, err @@ -1987,6 +2509,12 @@ func (r *MultiClusterEngineReconciler) ensureClusterProxyAddon(ctx context.Conte if missingCRDErrorOccured { return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.ClusterProxyAddon, newManagedResources); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -2003,6 +2531,11 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterProxyAddon(ctx context.Con r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) r.StatusManager.AddComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{})) + // Snapshot the resources previously recorded for this component before removing the + // InternalEngineComponent tracking CR below, so orphaned resources can still be identified + // and cleaned up later in this function (see managed_resources.go). + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterProxyAddon) + // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.ClusterProxyAddon); (result != ctrl.Result{}) || err != nil { @@ -2020,6 +2553,15 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterProxyAddon(ctx context.Con return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart), in addition + // to the resources rendered below. See managed_resources.go. + newManagedResources := extractManagedResources(templates) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterProxyAddon, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Deletes all templates for _, template := range templates { // Skip NetworkPolicy resources - they are managed by ensureNetworkPolicies with create-once pattern @@ -2080,6 +2622,19 @@ func (r *MultiClusterEngineReconciler) ensureMaestro(ctx context.Context, return ctrl.Result{RequeueAfter: requeuePeriod}, nil } + // Clean up any resources that were deployed by a previous version of this component's + // templates but are no longer rendered (e.g. a resource removed from the chart). See + // managed_resources.go. Note: this only covers the chart-rendered templates below, not the + // gRPC server ConfigMap/Route managed separately below, and is a no-op if maestro is later + // disabled, since ensureNoMaestro removes the whole "maestro" namespace instead of individual + // templates. + newManagedResources := extractManagedResources(templates) + oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.MaestroPreview) + if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.MaestroPreview, oldManagedResources, + newManagedResources); result != (ctrl.Result{}) || err != nil { + return result, err + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.MaestroPreview); err != nil { return result, errors.Wrapf(err, "failed to apply deployment config overrides for maestro") @@ -2094,6 +2649,12 @@ func (r *MultiClusterEngineReconciler) ensureMaestro(ctx context.Context, } } + // Record the resources currently managed by this component so future reconciles can detect + // and clean up resources that are later removed from the chart. + if err := r.updateManagedResources(ctx, mce, backplanev1.MaestroPreview, newManagedResources); err != nil { + return ctrl.Result{}, err + } + // Ensure the cluster-manager gRPC server (conductor) ConfigMap exists with the database password if err := r.ensureClusterManagerGRPCServerConfigMap(ctx, mce, ocmHubNS, "grpc-server-config", dbPassword); err != nil { return ctrl.Result{}, err diff --git a/pkg/templates/crds/internal/internal-engine-component.yaml b/pkg/templates/crds/internal/internal-engine-component.yaml index 76d897967..435d0b74b 100644 --- a/pkg/templates/crds/internal/internal-engine-component.yaml +++ b/pkg/templates/crds/internal/internal-engine-component.yaml @@ -19,6 +19,34 @@ spec: properties: spec: type: object + properties: + managedResources: + description: >- + ManagedResources tracks the resources currently rendered and applied for this + component. It is refreshed on every reconcile and used to detect resources + that were deployed by a previous version of the component's chart but are no + longer part of its rendered templates, so they can be safely cleaned up during + upgrades. + type: array + items: + type: object + required: + - apiVersion + - kind + - name + properties: + apiVersion: + description: APIVersion of the resource (e.g. "monitoring.coreos.com/v1"). + type: string + kind: + description: Kind of the resource (e.g. "ServiceMonitor"). + type: string + name: + description: Name of the resource. + type: string + namespace: + description: Namespace of the resource. Empty for cluster-scoped resources. + type: string # either Namespaced or Cluster scope: Namespaced names: From e5a0ab79d55dcb123b1c996f5104222322e2a584 Mon Sep 17 00:00:00 2001 From: dislbenn Date: Tue, 8 Sep 2026 10:03:00 -0400 Subject: [PATCH 2/2] undo Signed-off-by: dislbenn --- controllers/managed_resources.go | 84 +++++++++-- controllers/managed_resources_test.go | 112 ++++++++++++++- controllers/toggle_components.go | 193 ++++++++++++++++++++------ 3 files changed, 331 insertions(+), 58 deletions(-) diff --git a/controllers/managed_resources.go b/controllers/managed_resources.go index 956d21819..024e4aad1 100644 --- a/controllers/managed_resources.go +++ b/controllers/managed_resources.go @@ -5,6 +5,8 @@ package controllers import ( "context" "fmt" + "strings" + "time" backplanev1 "github.com/stolostron/backplane-operator/api/v1" @@ -46,11 +48,32 @@ var legacyManagedResources = map[string][]backplanev1.ManagedResource{ }, } -// managedResourceKey returns a string uniquely identifying a ManagedResource for set comparisons. +// managedResourceKey returns a string uniquely identifying a ManagedResource, including its +// APIVersion, for exact-match comparisons (see managedResourcesEqual). func managedResourceKey(resource backplanev1.ManagedResource) string { return fmt.Sprintf("%s/%s/%s/%s", resource.APIVersion, resource.Kind, resource.Namespace, resource.Name) } +// apiGroupFromAPIVersion extracts the API group from an apiVersion string (e.g. "apps/v1" -> +// "apps", "v1" -> "" for the core group), ignoring the version component. +func apiGroupFromAPIVersion(apiVersion string) string { + if idx := strings.Index(apiVersion, "/"); idx != -1 { + return apiVersion[:idx] + } + return "" +} + +// managedResourceIdentityKey returns a version-independent identity for a ManagedResource (API +// group + kind + namespace + name). Kubernetes objects are identified by group/kind/namespace/name; +// the API version is just an alternate representation of the same underlying object. Using the +// full APIVersion-sensitive key here would cause a pure version bump in a chart (e.g. a CRD moving +// from v1beta1 to v1) to be misdetected as the resource being removed, triggering an unnecessary +// delete-then-recreate cycle in cleanupOrphanedManagedResources instead of an in-place update. +func managedResourceIdentityKey(resource backplanev1.ManagedResource) string { + return fmt.Sprintf("%s/%s/%s/%s", apiGroupFromAPIVersion(resource.APIVersion), resource.Kind, + resource.Namespace, resource.Name) +} + // extractManagedResources builds the list of resources represented by the given rendered // templates. NetworkPolicy resources are intentionally excluded because they are managed // separately by ensureNetworkPolicies using a create-once pattern. @@ -92,22 +115,24 @@ func managedResourcesEqual(a, b []backplanev1.ManagedResource) bool { } // getManagedResources returns the resources currently recorded on the component's -// InternalEngineComponent CR. It returns nil (without error) if the CR does not exist, since -// callers treat "no tracked resources" as an empty diff baseline rather than a failure. +// InternalEngineComponent CR. It returns (nil, nil) if the CR does not exist, since callers treat +// "no tracked resources" as an empty diff baseline rather than a failure. Any other error is +// returned to the caller rather than swallowed, since silently treating a transient read failure +// as "no history" could cause cleanupOrphanedManagedResources to miss real orphans, or worse, lose +// the tracked history permanently if the caller goes on to delete the InternalEngineComponent CR. func (r *MultiClusterEngineReconciler) getManagedResources(ctx context.Context, mce *backplanev1.MultiClusterEngine, - component string) []backplanev1.ManagedResource { + component string) ([]backplanev1.ManagedResource, error) { iec := &backplanev1.InternalEngineComponent{} if err := r.Client.Get(ctx, types.NamespacedName{Name: component, Namespace: mce.Spec.TargetNamespace}, iec); err != nil { - if !apierrors.IsNotFound(err) { - log.Error(err, "failed to get InternalEngineComponent while reading managed resources", - "Component", component, "Namespace", mce.Spec.TargetNamespace) + if apierrors.IsNotFound(err) { + return nil, nil } - return nil + return nil, fmt.Errorf("failed to get InternalEngineComponent %s/%s: %v", mce.Spec.TargetNamespace, component, err) } - return iec.Spec.ManagedResources + return iec.Spec.ManagedResources, nil } // updateManagedResources patches the component's InternalEngineComponent CR with the current list @@ -145,13 +170,22 @@ func (r *MultiClusterEngineReconciler) updateManagedResources(ctx context.Contex // templates but are no longer rendered. Deletion is delegated to deleteTemplate, which only // removes resources that still carry this operator's backplaneconfig.name ownership label, so // resources that were manually recreated (and therefore lack that label) are left untouched. +// +// This function is best-effort and does not stop at the first candidate that fails or needs a +// requeue: on the disable path, the calling ensureNoXxx function must delete the +// InternalEngineComponent tracking CR promptly (other operators watch for its removal as a +// signal), so this reconcile is the only chance to use the resource history captured before that +// CR is gone. Stopping early would leave every remaining candidate un-attempted, and a future +// reconcile would have no history left to retry them with. Attempting every candidate in one pass +// instead means only a genuinely finalizer-blocked resource is left for the caller to +// report/requeue on; unrelated candidates are still cleaned up. func (r *MultiClusterEngineReconciler) cleanupOrphanedManagedResources(ctx context.Context, mce *backplanev1.MultiClusterEngine, component string, oldResources, newResources []backplanev1.ManagedResource) (ctrl.Result, error) { current := make(map[string]struct{}, len(newResources)) for _, resource := range newResources { - current[managedResourceKey(resource)] = struct{}{} + current[managedResourceIdentityKey(resource)] = struct{}{} } // Merge in any known legacy resources for this component that predate resource tracking (see @@ -165,9 +199,15 @@ func (r *MultiClusterEngineReconciler) cleanupOrphanedManagedResources(ctx conte orphanCandidates = append(orphanCandidates, legacy) } + var ( + firstErr error + needsRequeue bool + requeueAfter time.Duration + ) + seenCandidates := make(map[string]struct{}, len(orphanCandidates)) for _, resource := range orphanCandidates { - key := managedResourceKey(resource) + key := managedResourceIdentityKey(resource) if _, alreadyHandled := seenCandidates[key]; alreadyHandled { continue } @@ -187,10 +227,28 @@ func (r *MultiClusterEngineReconciler) cleanupOrphanedManagedResources(ctx conte "Component", component, "APIVersion", resource.APIVersion, "Kind", resource.Kind, "Name", resource.Name, "Namespace", resource.Namespace) - if result, err := r.deleteTemplate(ctx, mce, stub); result != (ctrl.Result{}) || err != nil { - return result, err + result, err := r.deleteTemplate(ctx, mce, stub) + if err != nil { + log.Error(err, "failed to clean up orphaned managed resource; continuing with remaining resources", + "Component", component, "Kind", resource.Kind, "Name", resource.Name, "Namespace", resource.Namespace) + if firstErr == nil { + firstErr = err + } + continue + } + if result != (ctrl.Result{}) { + needsRequeue = true + if result.RequeueAfter > requeueAfter { + requeueAfter = result.RequeueAfter + } } } + if firstErr != nil { + return ctrl.Result{}, firstErr + } + if needsRequeue { + return ctrl.Result{RequeueAfter: requeueAfter}, nil + } return ctrl.Result{}, nil } diff --git a/controllers/managed_resources_test.go b/controllers/managed_resources_test.go index f37ddc1d4..0bd2660ab 100644 --- a/controllers/managed_resources_test.go +++ b/controllers/managed_resources_test.go @@ -4,6 +4,7 @@ package controllers import ( "context" + "fmt" "testing" promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" @@ -158,8 +159,8 @@ func TestGetAndUpdateManagedResources(t *testing.T) { r := &MultiClusterEngineReconciler{Client: fakeClient} // No InternalEngineComponent exists yet - should return nil without error. - if got := r.getManagedResources(context.TODO(), mce, "console-mce"); got != nil { - t.Errorf("getManagedResources() with no CR = %v, want nil", got) + if got, err := r.getManagedResources(context.TODO(), mce, "console-mce"); got != nil || err != nil { + t.Errorf("getManagedResources() with no CR = (%v, %v), want (nil, nil)", got, err) } // updateManagedResources should be a no-op (not an error) when the CR doesn't exist yet. @@ -184,7 +185,10 @@ func TestGetAndUpdateManagedResources(t *testing.T) { t.Fatalf("updateManagedResources() returned error: %v", err) } - got := r.getManagedResources(context.TODO(), mce, "console-mce") + got, err := r.getManagedResources(context.TODO(), mce, "console-mce") + if err != nil { + t.Fatalf("getManagedResources() returned error: %v", err) + } if !managedResourcesEqual(got, initial) { t.Errorf("getManagedResources() = %v, want %v", got, initial) } @@ -201,7 +205,10 @@ func TestGetAndUpdateManagedResources(t *testing.T) { t.Fatalf("updateManagedResources() returned error: %v", err) } - got = r.getManagedResources(context.TODO(), mce, "console-mce") + got, err = r.getManagedResources(context.TODO(), mce, "console-mce") + if err != nil { + t.Fatalf("getManagedResources() returned error: %v", err) + } if !managedResourcesEqual(got, updated) { t.Errorf("getManagedResources() after update = %v, want %v", got, updated) } @@ -302,6 +309,40 @@ func TestCleanupOrphanedManagedResources(t *testing.T) { } }, }, + { + // A chart bumping a resource's apiVersion (e.g. a CRD moving from v1beta1 to v1) must + // not be treated as that resource being removed from the chart: the underlying + // Kubernetes object is identified by group/kind/namespace/name, not apiVersion, so + // deleting it here would cause an unnecessary delete-then-recreate cycle instead of an + // in-place update via applyTemplate. + name: "resource with only an apiVersion change is not treated as orphaned", + component: "example", + oldResources: []backplanev1.ManagedResource{ + newManagedResource("example.com/v1beta1", "Widget", "my-widget", "multicluster-engine"), + }, + newResources: []backplanev1.ManagedResource{ + newManagedResource("example.com/v1", "Widget", "my-widget", "multicluster-engine"), + }, + setupClient: func(t *testing.T) client.Client { + widget := &unstructured.Unstructured{} + widget.SetAPIVersion("example.com/v1beta1") + widget.SetKind("Widget") + widget.SetName("my-widget") + widget.SetNamespace("multicluster-engine") + widget.SetLabels(map[string]string{"backplaneconfig.name": "mce"}) + return fake.NewClientBuilder().WithScheme(s).WithObjects(widget).Build() + }, + verify: func(t *testing.T, c client.Client) { + widget := &unstructured.Unstructured{} + widget.SetAPIVersion("example.com/v1beta1") + widget.SetKind("Widget") + err := c.Get(context.TODO(), types.NamespacedName{Name: "my-widget", + Namespace: "multicluster-engine"}, widget) + if err != nil { + t.Errorf("expected my-widget to survive a version-only chart change, got error: %v", err) + } + }, + }, { name: "legacy console-mce ServiceMonitor is cleaned up even with no tracked history (ACM-40355)", component: backplanev1.ConsoleMCE, @@ -384,3 +425,66 @@ func TestCleanupOrphanedManagedResources(t *testing.T) { }) } } + +// errorOnDeleteClient simulates a transient error deleting a specific named resource, while +// deletions of any other resource proceed normally. Used to verify that +// cleanupOrphanedManagedResources is best-effort: it must still attempt (and succeed at) deleting +// every other orphan candidate instead of stopping at the first one that fails. Unlike MCH, +// backplane-operator's deleteTemplate doesn't poll for finalizer-blocked termination - it just +// calls Delete and returns - so a transient Delete error is the realistic failure mode here. +type errorOnDeleteClient struct { + client.Client + failName string +} + +func (c *errorOnDeleteClient) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + if obj.GetName() == c.failName { + return fmt.Errorf("simulated transient delete error") + } + return c.Client.Delete(ctx, obj, opts...) +} + +func TestCleanupOrphanedManagedResources_BestEffort(t *testing.T) { + s := newManagedResourcesTestScheme(t) + + mce := &backplanev1.MultiClusterEngine{ + ObjectMeta: metav1.ObjectMeta{Name: "mce"}, + Spec: backplanev1.MultiClusterEngineSpec{TargetNamespace: "multicluster-engine"}, + } + + labels := map[string]string{"backplaneconfig.name": "mce"} + failingDeploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "failing-deploy", Namespace: "multicluster-engine", Labels: labels}, + } + cleanDeploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "clean-deploy", Namespace: "multicluster-engine", Labels: labels}, + } + + fakeClient := fake.NewClientBuilder().WithScheme(s).WithObjects(failingDeploy, cleanDeploy).Build() + c := &errorOnDeleteClient{Client: fakeClient, failName: "failing-deploy"} + r := &MultiClusterEngineReconciler{Client: c} + + oldResources := []backplanev1.ManagedResource{ + newManagedResource("apps/v1", "Deployment", "failing-deploy", "multicluster-engine"), + newManagedResource("apps/v1", "Deployment", "clean-deploy", "multicluster-engine"), + } + + _, err := r.cleanupOrphanedManagedResources(context.TODO(), mce, "example", oldResources, nil) + if err == nil { + t.Fatalf("cleanupOrphanedManagedResources() expected error due to failing-deploy, got nil") + } + + // The failing resource should still exist... + if err := c.Get(context.TODO(), types.NamespacedName{Name: "failing-deploy", Namespace: "multicluster-engine"}, + &appsv1.Deployment{}); err != nil { + t.Errorf("expected failing-deploy to still exist after a failed delete, got error: %v", err) + } + + // ...but clean-deploy must still have been deleted in the same pass, instead of being skipped + // because an earlier candidate failed. + err = c.Get(context.TODO(), types.NamespacedName{Name: "clean-deploy", Namespace: "multicluster-engine"}, + &appsv1.Deployment{}) + if err == nil { + t.Errorf("expected clean-deploy to be deleted even though failing-deploy errored") + } +} diff --git a/controllers/toggle_components.go b/controllers/toggle_components.go index 4ff8249b4..d31232b52 100644 --- a/controllers/toggle_components.go +++ b/controllers/toggle_components.go @@ -69,7 +69,10 @@ func (r *MultiClusterEngineReconciler) ensureConsoleMCE(ctx context.Context, mce // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ConsoleMCE) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ConsoleMCE) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ConsoleMCE, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -102,7 +105,7 @@ func (r *MultiClusterEngineReconciler) ensureConsoleMCE(ctx context.Context, mce // Check console-mce deployment health before adding plugin consoleDeployment := &appsv1.Deployment{} - err := r.Client.Get(ctx, namespacedName, consoleDeployment) + err = r.Client.Get(ctx, namespacedName, consoleDeployment) if err != nil { log.Error(err, "Failed to get console-mce deployment for addon. Requeuing.") return ctrl.Result{RequeueAfter: requeuePeriod}, nil @@ -127,7 +130,10 @@ func (r *MultiClusterEngineReconciler) ensureNoConsoleMCE(ctx context.Context, m // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ConsoleMCE) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ConsoleMCE) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -218,7 +224,10 @@ func (r *MultiClusterEngineReconciler) ensureManagedServiceAccount(ctx context.C // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ManagedServiceAccount) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ManagedServiceAccount) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ManagedServiceAccount, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -266,7 +275,10 @@ func (r *MultiClusterEngineReconciler) ensureNoManagedServiceAccount(ctx context // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ManagedServiceAccount) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ManagedServiceAccount) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -343,7 +355,10 @@ func (r *MultiClusterEngineReconciler) ensureFleetNavigation(ctx context.Context // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.FleetNavigation) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.FleetNavigation) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.FleetNavigation, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -383,7 +398,10 @@ func (r *MultiClusterEngineReconciler) ensureNoFleetNavigation(ctx context.Conte // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.FleetNavigation) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.FleetNavigation) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.ensureNoInternalEngineComponent(ctx, mce, backplanev1.FleetNavigation); (result != ctrl.Result{}) || err != nil { @@ -518,7 +536,10 @@ func (r *MultiClusterEngineReconciler) ensureDiscovery(ctx context.Context, mce // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.Discovery) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.Discovery) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.Discovery, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -559,7 +580,10 @@ func (r *MultiClusterEngineReconciler) ensureNoDiscovery(ctx context.Context, // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.Discovery) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.Discovery) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -633,7 +657,10 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPI(ctx context.Context, mce // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPI) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPI) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPI, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -674,7 +701,10 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPI(ctx context.Context, // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPI) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPI) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -749,7 +779,10 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderAWS(ctx context.C // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -790,7 +823,10 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderAWS(ctx context // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAWS) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -869,7 +905,10 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderAzure(ctx context // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -909,7 +948,10 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderAzure(ctx conte // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderAzurePreview) + if err != nil { + return ctrl.Result{}, err + } namespacedName := types.NamespacedName{Name: "azureserviceoperator-controller-manager", Namespace: mce.Spec.TargetNamespace} @@ -993,7 +1035,10 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderMetal(ctx context // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -1034,7 +1079,10 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderMetal(ctx conte // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderMetal) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -1111,7 +1159,10 @@ func (r *MultiClusterEngineReconciler) ensureClusterAPIProviderOA(ctx context.Co // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -1157,7 +1208,10 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterAPIProviderOA(ctx context. // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterAPIProviderOA) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -1229,7 +1283,10 @@ func (r *MultiClusterEngineReconciler) ensureHive(ctx context.Context, mce *back // managed_resources.go. Note: this only covers the chart-rendered templates below, not the // HiveConfig custom resource applied separately at the end of this function. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.Hive) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.Hive) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.Hive, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -1272,7 +1329,10 @@ func (r *MultiClusterEngineReconciler) ensureNoHive(ctx context.Context, mce *ba // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.Hive) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.Hive) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -1305,7 +1365,7 @@ func (r *MultiClusterEngineReconciler) ensureNoHive(ctx context.Context, mce *ba // Delete hivconfig hiveConfig := hive.HiveConfig(mce) - err := r.Client.Get(ctx, types.NamespacedName{Name: "hive"}, hiveConfig) + err = r.Client.Get(ctx, types.NamespacedName{Name: "hive"}, hiveConfig) if err == nil { // If resource exists, delete err := r.Client.Delete(ctx, hiveConfig) if err != nil && !apierrors.IsNotFound(err) { @@ -1365,7 +1425,10 @@ func (r *MultiClusterEngineReconciler) ensureAssistedService(ctx context.Context // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.AssistedService) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.AssistedService) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.AssistedService, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -1411,7 +1474,10 @@ func (r *MultiClusterEngineReconciler) ensureNoAssistedService(ctx context.Conte // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.AssistedService) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.AssistedService) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -1499,7 +1565,10 @@ func (r *MultiClusterEngineReconciler) ensureServerFoundation(ctx context.Contex // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ServerFoundation) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ServerFoundation) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ServerFoundation, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -1539,7 +1608,10 @@ func (r *MultiClusterEngineReconciler) ensureNoServerFoundation(ctx context.Cont // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ServerFoundation) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ServerFoundation) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is created for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -1628,7 +1700,10 @@ func (r *MultiClusterEngineReconciler) ensureImageBasedInstallOperator(ctx conte // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -1672,7 +1747,10 @@ func (r *MultiClusterEngineReconciler) ensureNoImageBasedInstallOperator(ctx con // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ImageBasedInstallOperator) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -1761,7 +1839,10 @@ func (r *MultiClusterEngineReconciler) ensureClusterLifecycle(ctx context.Contex // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterLifecycle) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterLifecycle) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterLifecycle, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -1801,7 +1882,10 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterLifecycle(ctx context.Cont // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterLifecycle) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterLifecycle) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -1891,7 +1975,10 @@ func (r *MultiClusterEngineReconciler) ensureClusterManager(ctx context.Context, // managed_resources.go. Note: this only covers the chart-rendered templates below, not the // ClusterManager custom resource or TLS profile ConfigMaps applied separately below. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterManager) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterManager) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterManager, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -1928,7 +2015,7 @@ func (r *MultiClusterEngineReconciler) ensureClusterManager(ctx context.Context, return ctrl.Result{}, errors.Wrapf(err, "Error setting controller reference on resource %s", cmTemplate.GetName()) } force := true - err := r.Client.Patch(ctx, cmTemplate, client.Apply, &client.PatchOptions{Force: &force, FieldManager: "backplane-operator"}) + err = r.Client.Patch(ctx, cmTemplate, client.Apply, &client.PatchOptions{Force: &force, FieldManager: "backplane-operator"}) if err != nil { return ctrl.Result{}, errors.Wrapf(err, "error applying object Name: %s Kind: %s", cmTemplate.GetName(), cmTemplate.GetKind()) } @@ -1951,7 +2038,10 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterManager(ctx context.Contex // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterManager) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterManager) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -1994,7 +2084,7 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterManager(ctx context.Contex Kind: "ClusterManager", }, ) - err := r.Client.Get(ctx, types.NamespacedName{Name: "cluster-manager"}, clusterManager) + err = r.Client.Get(ctx, types.NamespacedName{Name: "cluster-manager"}, clusterManager) if err == nil { // If resource exists, delete err := r.Client.Delete(ctx, clusterManager) if err != nil { @@ -2057,7 +2147,10 @@ func (r *MultiClusterEngineReconciler) ensureClusterPermission(ctx context.Conte // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterPermission) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterPermission) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterPermission, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -2099,7 +2192,10 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterPermission(ctx context.Con // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterPermission) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterPermission) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -2178,7 +2274,10 @@ func (r *MultiClusterEngineReconciler) ensureHyperShift(ctx context.Context, mce // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.HyperShift) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.HyperShift) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.HyperShift, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -2230,7 +2329,10 @@ func (r *MultiClusterEngineReconciler) ensureNoHyperShift(ctx context.Context, // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.HyperShift) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.HyperShift) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -2480,7 +2582,10 @@ func (r *MultiClusterEngineReconciler) ensureClusterProxyAddon(ctx context.Conte // templates but are no longer rendered (e.g. a resource removed from the chart). See // managed_resources.go. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterProxyAddon) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterProxyAddon) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.ClusterProxyAddon, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err @@ -2534,7 +2639,10 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterProxyAddon(ctx context.Con // Snapshot the resources previously recorded for this component before removing the // InternalEngineComponent tracking CR below, so orphaned resources can still be identified // and cleaned up later in this function (see managed_resources.go). - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.ClusterProxyAddon) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.ClusterProxyAddon) + if err != nil { + return ctrl.Result{}, err + } // Ensure that the InternalHubComponent CR instance is deleted for component in MCE. if result, err := r.ensureNoInternalEngineComponent(ctx, mce, @@ -2629,7 +2737,10 @@ func (r *MultiClusterEngineReconciler) ensureMaestro(ctx context.Context, // disabled, since ensureNoMaestro removes the whole "maestro" namespace instead of individual // templates. newManagedResources := extractManagedResources(templates) - oldManagedResources := r.getManagedResources(ctx, mce, backplanev1.MaestroPreview) + oldManagedResources, err := r.getManagedResources(ctx, mce, backplanev1.MaestroPreview) + if err != nil { + return ctrl.Result{}, err + } if result, err := r.cleanupOrphanedManagedResources(ctx, mce, backplanev1.MaestroPreview, oldManagedResources, newManagedResources); result != (ctrl.Result{}) || err != nil { return result, err