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..024e4aad1 --- /dev/null +++ b/controllers/managed_resources.go @@ -0,0 +1,254 @@ +// Copyright Contributors to the Open Cluster Management project + +package controllers + +import ( + "context" + "fmt" + "strings" + "time" + + 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, 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. +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, 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, 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) { + return nil, nil + } + return nil, fmt.Errorf("failed to get InternalEngineComponent %s/%s: %v", mce.Spec.TargetNamespace, component, err) + } + + return iec.Spec.ManagedResources, nil +} + +// 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. +// +// 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[managedResourceIdentityKey(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) + } + + var ( + firstErr error + needsRequeue bool + requeueAfter time.Duration + ) + + seenCandidates := make(map[string]struct{}, len(orphanCandidates)) + for _, resource := range orphanCandidates { + key := managedResourceIdentityKey(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) + + 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 new file mode 100644 index 000000000..0bd2660ab --- /dev/null +++ b/controllers/managed_resources_test.go @@ -0,0 +1,490 @@ +// Copyright Contributors to the Open Cluster Management project + +package controllers + +import ( + "context" + "fmt" + "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, 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. + 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, 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) + } + + // 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, 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) + } +} + +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) + } + }, + }, + { + // 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, + 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) + } + }) + } +} + +// 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 6ca4608a7..d31232b52 100644 --- a/controllers/toggle_components.go +++ b/controllers/toggle_components.go @@ -65,6 +65,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ConsoleMCE); err != nil { return result, err @@ -84,9 +97,15 @@ 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) + 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 @@ -108,6 +127,14 @@ 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, 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, backplanev1.ConsoleMCE); (result != ctrl.Result{}) || err != nil { @@ -139,6 +166,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 +220,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ManagedServiceAccount); err != nil { return result, err @@ -211,12 +260,26 @@ 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, 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, backplanev1.ManagedServiceAccount); (result != ctrl.Result{}) || err != nil { @@ -233,6 +296,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 +351,19 @@ 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, 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 + } + missingCRDErrorOccured := false for _, template := range templates { applyReleaseVersionAnnotation(template) @@ -298,12 +383,26 @@ 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, 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 { return result, err @@ -318,6 +417,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 +532,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.Discovery); err != nil { return result, err @@ -443,6 +564,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 +577,14 @@ 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, 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, backplanev1.Discovery); (result != ctrl.Result{}) || err != nil { @@ -467,6 +602,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 +653,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPI); err != nil { return result, err @@ -528,6 +685,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 +698,14 @@ 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, 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, backplanev1.ClusterAPI); (result != ctrl.Result{}) || err != nil { @@ -552,6 +723,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 +775,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPIProviderAWS); err != nil { return result, err @@ -614,6 +807,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 +820,14 @@ 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, 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, backplanev1.ClusterAPIProviderAWS); (result != ctrl.Result{}) || err != nil { @@ -638,6 +845,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 +901,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPIProviderAzurePreview); err != nil { return result, err @@ -704,12 +933,26 @@ 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, 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} r.StatusManager.RemoveComponent(toggle.EnabledStatus(namespacedName)) @@ -736,6 +979,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 +1031,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPIProviderMetal); err != nil { return result, err @@ -798,6 +1063,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 +1076,14 @@ 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, 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, backplanev1.ClusterAPIProviderMetal); (result != ctrl.Result{}) || err != nil { @@ -822,6 +1101,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 +1155,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterAPIProviderOA); err != nil { return result, err @@ -886,6 +1187,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 +1205,14 @@ 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, 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, backplanev1.ClusterAPIProviderOA); (result != ctrl.Result{}) || err != nil { @@ -915,6 +1230,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 +1278,20 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.Hive); err != nil { return result, err @@ -973,6 +1311,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 +1326,14 @@ 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, 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, backplanev1.Hive); (result != ctrl.Result{}) || err != nil { @@ -999,12 +1351,21 @@ 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{})) // 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) { @@ -1060,6 +1421,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.AssistedService); err != nil { return result, err @@ -1079,6 +1453,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 +1471,14 @@ 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, 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, backplanev1.AssistedService); (result != ctrl.Result{}) || err != nil { @@ -1109,6 +1497,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 +1561,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ServerFoundation); err != nil { return result, err @@ -1183,12 +1593,26 @@ 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, 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, backplanev1.ServerFoundation); (result != ctrl.Result{}) || err != nil { @@ -1206,6 +1630,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 +1696,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ImageBasedInstallOperator); err != nil { @@ -1283,6 +1729,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 +1744,14 @@ 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, 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, backplanev1.ImageBasedInstallOperator); (result != ctrl.Result{}) || err != nil { @@ -1309,6 +1769,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 +1835,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterLifecycle); err != nil { return result, err @@ -1385,12 +1867,26 @@ 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, 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, backplanev1.ClusterLifecycle); (result != ctrl.Result{}) || err != nil { @@ -1408,6 +1904,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 +1970,20 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterManager); err != nil { return result, err @@ -1484,13 +2003,19 @@ 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 { 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()) } @@ -1510,6 +2035,14 @@ 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, 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, backplanev1.ClusterManager); (result != ctrl.Result{}) || err != nil { @@ -1527,6 +2060,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{ @@ -1542,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 { @@ -1601,6 +2143,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterPermission); err != nil { return result, err @@ -1620,6 +2175,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 +2189,14 @@ 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, 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, backplanev1.ClusterPermission); (result != ctrl.Result{}) || err != nil { @@ -1645,6 +2214,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 +2270,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.HyperShift); err != nil { return result, err @@ -1723,12 +2314,26 @@ 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, 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, backplanev1.HyperShift); (result != ctrl.Result{}) || err != nil { @@ -1786,6 +2391,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 +2578,19 @@ 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, 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 + } + // Apply deployment config overrides if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterProxyAddon); err != nil { return result, err @@ -1987,6 +2614,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 +2636,14 @@ 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, 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, backplanev1.ClusterProxyAddon); (result != ctrl.Result{}) || err != nil { @@ -2020,6 +2661,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 +2730,22 @@ 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, 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 + } + // 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 +2760,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: