From e2acc451aae08625fbad3b652e6be39908f23c75 Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Thu, 7 Nov 2024 19:53:15 +0400 Subject: [PATCH] Add ownerReference on managed Templates --- api/v1alpha1/clustertemplate_types.go | 16 -- api/v1alpha1/providertemplate_types.go | 7 - api/v1alpha1/servicetemplate_types.go | 16 -- api/v1alpha1/templatechain_common.go | 2 - .../multiclusterservice_controller_test.go | 3 +- internal/controller/suite_test.go | 2 +- internal/controller/template_controller.go | 108 ++++++++++++- .../controller/template_controller_test.go | 6 +- .../controller/templatechain_controller.go | 147 +----------------- .../templatechain_controller_test.go | 106 +++++-------- .../templatemanagement_controller.go | 3 - test/objects/template/template.go | 12 ++ 12 files changed, 172 insertions(+), 256 deletions(-) diff --git a/api/v1alpha1/clustertemplate_types.go b/api/v1alpha1/clustertemplate_types.go index 036021bc8..9018a01e9 100644 --- a/api/v1alpha1/clustertemplate_types.go +++ b/api/v1alpha1/clustertemplate_types.go @@ -15,12 +15,10 @@ package v1alpha1 import ( - "context" "fmt" "github.com/Masterminds/semver/v3" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -108,20 +106,6 @@ func (t *ClusterTemplate) GetCommonStatus() *TemplateStatusCommon { return &t.Status.TemplateStatusCommon } -// IsOrphaned checks whether the Template is orphaned or not. -func (t *ClusterTemplate) IsOrphaned(ctx context.Context, cl client.Client) (bool, error) { - list := new(ClusterTemplateChainList) - if err := cl.List(ctx, list, client.InNamespace(t.Namespace), client.MatchingFields{SupportedTemplateKey: t.Name}); err != nil { - return false, fmt.Errorf("failed to list %s: %w", list.GroupVersionKind(), err) - } - for _, chain := range list.Items { - if chain.DeletionTimestamp == nil { - return false, nil - } - } - return true, nil -} - // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:resource:shortName=clustertmpl diff --git a/api/v1alpha1/providertemplate_types.go b/api/v1alpha1/providertemplate_types.go index a6c843fb1..66e82f1bb 100644 --- a/api/v1alpha1/providertemplate_types.go +++ b/api/v1alpha1/providertemplate_types.go @@ -15,11 +15,9 @@ package v1alpha1 import ( - "context" "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) // ProviderTemplateKind denotes the providertemplate resource Kind. @@ -68,11 +66,6 @@ func (t *ProviderTemplate) GetCommonStatus() *TemplateStatusCommon { return &t.Status.TemplateStatusCommon } -// IsOrphaned checks whether the Template is orphaned or not. -func (*ProviderTemplate) IsOrphaned(_ context.Context, _ client.Client) (bool, error) { - return false, nil -} - // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:resource:shortName=providertmpl,scope=Cluster diff --git a/api/v1alpha1/servicetemplate_types.go b/api/v1alpha1/servicetemplate_types.go index f7b084fae..97d52fb3e 100644 --- a/api/v1alpha1/servicetemplate_types.go +++ b/api/v1alpha1/servicetemplate_types.go @@ -15,12 +15,10 @@ package v1alpha1 import ( - "context" "fmt" "github.com/Masterminds/semver/v3" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -87,20 +85,6 @@ func (t *ServiceTemplate) GetCommonStatus() *TemplateStatusCommon { return &t.Status.TemplateStatusCommon } -// IsOrphaned checks whether the Template is orphaned or not. -func (t *ServiceTemplate) IsOrphaned(ctx context.Context, cl client.Client) (bool, error) { - list := new(ServiceTemplateChainList) - if err := cl.List(ctx, list, client.InNamespace(t.Namespace), client.MatchingFields{SupportedTemplateKey: t.Name}); err != nil { - return false, fmt.Errorf("failed to list %s: %w", list.GroupVersionKind(), err) - } - for _, chain := range list.Items { - if chain.DeletionTimestamp == nil { - return false, nil - } - } - return true, nil -} - // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:resource:shortName=svctmpl diff --git a/api/v1alpha1/templatechain_common.go b/api/v1alpha1/templatechain_common.go index efffd1642..36a2d29c3 100644 --- a/api/v1alpha1/templatechain_common.go +++ b/api/v1alpha1/templatechain_common.go @@ -14,8 +14,6 @@ package v1alpha1 -const TemplateChainFinalizer = "hmc.mirantis.com/template-chain" - // TemplateChainSpec defines the observed state of TemplateChain type TemplateChainSpec struct { // SupportedTemplates is the list of supported Templates definitions and all available upgrade sequences for it. diff --git a/internal/controller/multiclusterservice_controller_test.go b/internal/controller/multiclusterservice_controller_test.go index 817eba6f5..049c69a48 100644 --- a/internal/controller/multiclusterservice_controller_test.go +++ b/internal/controller/multiclusterservice_controller_test.go @@ -150,8 +150,9 @@ var _ = Describe("MultiClusterService Controller", func() { By("reconciling ServiceTemplate used by MultiClusterService") templateReconciler := TemplateReconciler{ - Client: k8sClient, + Client: mgrClient, downloadHelmChartFunc: fakeDownloadHelmChartFunc, + templateKind: hmc.ServiceTemplateKind, } serviceTemplateReconciler := &ServiceTemplateReconciler{TemplateReconciler: templateReconciler} _, err = serviceTemplateReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: serviceTemplateRef}) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 22602dbc5..e805f260c 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -144,7 +144,7 @@ var _ = BeforeSuite(func() { }) Expect(err).NotTo(HaveOccurred()) mgrClient = mgr.GetClient() - Expect(mgr).NotTo(BeNil()) + Expect(mgrClient).NotTo(BeNil()) err = hmcmirantiscomv1alpha1.SetupIndexers(ctx, mgr) Expect(err).NotTo(HaveOccurred()) diff --git a/internal/controller/template_controller.go b/internal/controller/template_controller.go index 90a997f29..300321c90 100644 --- a/internal/controller/template_controller.go +++ b/internal/controller/template_controller.go @@ -29,6 +29,8 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -50,10 +52,11 @@ const ( // TemplateReconciler reconciles a *Template object type TemplateReconciler struct { client.Client - downloadHelmChartFunc func(context.Context, *sourcev1.Artifact) (*chart.Chart, error) - SystemNamespace string + SystemNamespace string + templateKind string + DefaultRegistryConfig helm.DefaultRegistryConfig } @@ -84,6 +87,16 @@ func (r *ClusterTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, err } + changed, err := r.setTemplateChainOwnership(ctx, clusterTemplate) + if err != nil { + l.Error(err, "Failed to set OwnerReferences") + return ctrl.Result{}, err + } + if changed { + l.Info("Updating OwnerReferences with associated ClusterTemplateChain") + return ctrl.Result{RequeueAfter: DefaultRequeueInterval}, r.Update(ctx, clusterTemplate) + } + result, err := r.ReconcileTemplate(ctx, clusterTemplate) if err != nil { l.Error(err, "failed to reconcile template") @@ -117,6 +130,17 @@ func (r *ServiceTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Requ l.Error(err, "Failed to get ServiceTemplate") return ctrl.Result{}, err } + + changed, err := r.setTemplateChainOwnership(ctx, serviceTemplate) + if err != nil { + l.Error(err, "Failed to set OwnerReferences") + return ctrl.Result{}, err + } + if changed { + l.Info("Updating OwnerReferences with associated ServiceTemplateChain") + return ctrl.Result{RequeueAfter: DefaultRequeueInterval}, r.Update(ctx, serviceTemplate) + } + return r.ReconcileTemplate(ctx, serviceTemplate) } @@ -168,7 +192,6 @@ type templateCommon interface { GetHelmSpec() *hmc.HelmSpec GetCommonStatus() *hmc.TemplateStatusCommon FillStatusWithProviders(map[string]string) error - IsOrphaned(context.Context, client.Client) (bool, error) } func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template templateCommon) (ctrl.Result, error) { @@ -272,6 +295,42 @@ func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template tem return ctrl.Result{}, r.updateStatus(ctx, template, "") } +func (r *TemplateReconciler) setTemplateChainOwnership(ctx context.Context, template templateCommon) (changed bool, err error) { + if template.GetNamespace() == r.SystemNamespace { + return false, nil + } + + opts := &client.ListOptions{ + Namespace: template.GetNamespace(), + FieldSelector: fields.SelectorFromSet(fields.Set{hmc.TemplateChainSupportedTemplatesIndexKey: template.GetName()}), + } + switch r.templateKind { + case hmc.ClusterTemplateKind: + chainList := &hmc.ClusterTemplateChainList{} + if err = r.List(ctx, chainList, opts); err != nil { + return false, err + } + for _, chain := range chainList.Items { + if utils.AddOwnerReference(template, &chain) { + changed = true + } + } + case hmc.ServiceTemplateKind: + chainList := &hmc.ServiceTemplateChainList{} + if err = r.List(ctx, chainList, opts); err != nil { + return false, err + } + for _, chain := range chainList.Items { + if utils.AddOwnerReference(template, &chain) { + changed = true + } + } + default: + return false, fmt.Errorf("invalid Template kind %s. Supported kinds are %s and %s", r.templateKind, hmc.ClusterTemplateKind, hmc.ServiceTemplateKind) + } + return changed, nil +} + func templateManagedByHMC(template templateCommon) bool { return template.GetLabels()[hmc.HMCManagedLabelKey] == hmc.HMCManagedLabelValue } @@ -425,20 +484,63 @@ func (r *ClusterTemplateReconciler) validateCompatibilityAttrs(ctx context.Conte // SetupWithManager sets up the controller with the Manager. func (r *ClusterTemplateReconciler) SetupWithManager(mgr ctrl.Manager) error { + r.templateKind = hmc.ClusterTemplateKind return ctrl.NewControllerManagedBy(mgr). For(&hmc.ClusterTemplate{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). + Watches(&hmc.ClusterTemplateChain{}, + handler.EnqueueRequestsFromMapFunc(func(_ context.Context, o client.Object) []ctrl.Request { + chain, ok := o.(*hmc.ClusterTemplateChain) + if !ok { + return nil + } + return r.getRequestsForSupportedTemplates(chain) + }), + builder.WithPredicates(predicate.Funcs{ + UpdateFunc: func(event.UpdateEvent) bool { return false }, + GenericFunc: func(event.GenericEvent) bool { return false }, + }), + ). Complete(r) } // SetupWithManager sets up the controller with the Manager. func (r *ServiceTemplateReconciler) SetupWithManager(mgr ctrl.Manager) error { + r.templateKind = hmc.ServiceTemplateKind return ctrl.NewControllerManagedBy(mgr). For(&hmc.ServiceTemplate{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). + Watches(&hmc.ServiceTemplateChain{}, + handler.EnqueueRequestsFromMapFunc(func(_ context.Context, o client.Object) []ctrl.Request { + chain, ok := o.(*hmc.ServiceTemplateChain) + if !ok { + return nil + } + return r.getRequestsForSupportedTemplates(chain) + }), + builder.WithPredicates(predicate.Funcs{ + UpdateFunc: func(event.UpdateEvent) bool { return false }, + GenericFunc: func(event.GenericEvent) bool { return false }, + }), + ). Complete(r) } +func (r *TemplateReconciler) getRequestsForSupportedTemplates(chain templateChain) []ctrl.Request { + if chain.GetNamespace() == r.SystemNamespace { + return []ctrl.Request{} + } + supportedTemplates := chain.GetSpec().SupportedTemplates + requests := make([]ctrl.Request, 0, len(supportedTemplates)) + for _, template := range supportedTemplates { + requests = append(requests, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: template.Name}, + }) + } + return requests +} + // SetupWithManager sets up the controller with the Manager. func (r *ProviderTemplateReconciler) SetupWithManager(mgr ctrl.Manager) error { + r.templateKind = hmc.ProviderTemplateKind return ctrl.NewControllerManagedBy(mgr). For(&hmc.ProviderTemplate{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Watches(&hmc.Release{}, diff --git a/internal/controller/template_controller_test.go b/internal/controller/template_controller_test.go index ceddb6201..b8f84d8ea 100644 --- a/internal/controller/template_controller_test.go +++ b/internal/controller/template_controller_test.go @@ -181,16 +181,19 @@ var _ = Describe("Template Controller", func() { downloadHelmChartFunc: fakeDownloadHelmChartFunc, } By("Reconciling the ClusterTemplate resource") + templateReconciler.templateKind = hmcmirantiscomv1alpha1.ClusterTemplateKind clusterTemplateReconciler := &ClusterTemplateReconciler{TemplateReconciler: templateReconciler} _, err := clusterTemplateReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) Expect(err).NotTo(HaveOccurred()) By("Reconciling the ServiceTemplate resource") + templateReconciler.templateKind = hmcmirantiscomv1alpha1.ServiceTemplateKind serviceTemplateReconciler := &ServiceTemplateReconciler{TemplateReconciler: templateReconciler} _, err = serviceTemplateReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) Expect(err).NotTo(HaveOccurred()) By("Reconciling the ProviderTemplate resource") + templateReconciler.templateKind = hmcmirantiscomv1alpha1.ProviderTemplateKind providerTemplateReconciler := &ProviderTemplateReconciler{TemplateReconciler: templateReconciler} _, err = providerTemplateReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) Expect(err).NotTo(HaveOccurred()) @@ -301,8 +304,9 @@ var _ = Describe("Template Controller", func() { By("Reconciling the cluster template") clusterTemplateReconciler := &ClusterTemplateReconciler{TemplateReconciler: TemplateReconciler{ - Client: k8sClient, + Client: mgrClient, downloadHelmChartFunc: fakeDownloadHelmChartFunc, + templateKind: hmcmirantiscomv1alpha1.ClusterTemplateKind, }} _, err := clusterTemplateReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{ Name: clusterTemplateName, diff --git a/internal/controller/templatechain_controller.go b/internal/controller/templatechain_controller.go index cb51af7ae..e8e86951a 100644 --- a/internal/controller/templatechain_controller.go +++ b/internal/controller/templatechain_controller.go @@ -19,20 +19,13 @@ import ( "errors" "fmt" - "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" hmc "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/internal/utils" ) // TemplateChainReconciler reconciles a TemplateChain object @@ -72,11 +65,7 @@ func (r *ClusterTemplateChainReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, err } - if !clusterTemplateChain.DeletionTimestamp.IsZero() { - l.Info("Deleting ClusterTemplateChain") - return r.Delete(ctx, clusterTemplateChain) - } - return r.Update(ctx, clusterTemplateChain) + return r.ReconcileTemplateChain(ctx, clusterTemplateChain) } func (r *ServiceTemplateChainReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -94,33 +83,18 @@ func (r *ServiceTemplateChainReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, err } - if !serviceTemplateChain.DeletionTimestamp.IsZero() { - l.Info("Deleting ServiceTemplateChain") - return r.Delete(ctx, serviceTemplateChain) - } - return r.Update(ctx, serviceTemplateChain) + return r.ReconcileTemplateChain(ctx, serviceTemplateChain) } -func (r *TemplateChainReconciler) Update(ctx context.Context, templateChain templateChain) (ctrl.Result, error) { +func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, templateChain templateChain) (ctrl.Result, error) { l := ctrl.LoggerFrom(ctx) - if controllerutil.AddFinalizer(templateChain, hmc.TemplateChainFinalizer) { - if err := r.Client.Update(ctx, templateChain); err != nil { - l.Error(err, "Failed to update TemplateChain finalizers") - return ctrl.Result{}, err - } - return ctrl.Result{}, nil - } - systemTemplates, err := r.getSystemTemplates(ctx) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to get system templates: %w", err) } - var ( - errs error - keepTemplate = make(map[string]struct{}, len(templateChain.GetSpec().SupportedTemplates)) - ) + var errs error for _, supportedTemplate := range templateChain.GetSpec().SupportedTemplates { meta := metav1.ObjectMeta{ Name: supportedTemplate.Name, @@ -129,7 +103,6 @@ func (r *TemplateChainReconciler) Update(ctx context.Context, templateChain temp hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue, }, } - keepTemplate[supportedTemplate.Name] = struct{}{} source, found := systemTemplates[supportedTemplate.Name] if !found { @@ -159,6 +132,8 @@ func (r *TemplateChainReconciler) Update(ctx context.Context, templateChain temp return ctrl.Result{}, fmt.Errorf("invalid TemplateChain kind. Supported kinds are %s and %s", hmc.ClusterTemplateChainKind, hmc.ServiceTemplateChainKind) } + utils.AddOwnerReference(target, templateChain) + if err := r.Create(ctx, target); err == nil { l.Info(r.templateKind+" was successfully created", "template namespace", templateChain.GetNamespace(), "template name", supportedTemplate.Name) continue @@ -168,24 +143,8 @@ func (r *TemplateChainReconciler) Update(ctx context.Context, templateChain temp errs = errors.Join(errs, err) } } - if errs != nil { - return ctrl.Result{}, errs - } - - return ctrl.Result{}, r.removeOrphanedTemplates(ctx, templateChain) -} - -func (r *TemplateChainReconciler) Delete(ctx context.Context, chain templateChain) (ctrl.Result, error) { - if err := r.removeOrphanedTemplates(ctx, chain); err != nil { - return ctrl.Result{}, err - } - // Removing finalizer in the end of cleanup - if controllerutil.RemoveFinalizer(chain, hmc.TemplateChainFinalizer) { - ctrl.LoggerFrom(ctx).Info("Removing TemplateChain finalizer") - return ctrl.Result{}, r.Client.Update(ctx, chain) - } - return ctrl.Result{}, nil + return ctrl.Result{}, errs } func (r *TemplateChainReconciler) getSystemTemplates(ctx context.Context) (systemTemplates map[string]templateCommon, _ error) { @@ -229,39 +188,6 @@ func (r *TemplateChainReconciler) getTemplates(ctx context.Context, templateKind return templates, nil } -func (r TemplateChainReconciler) removeOrphanedTemplates(ctx context.Context, chain templateChain) error { - l := log.FromContext(ctx) - - managedTemplates, err := r.getTemplates(ctx, r.templateKind, &client.ListOptions{ - Namespace: chain.GetNamespace(), - LabelSelector: labels.SelectorFromSet(map[string]string{hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue}), - }) - if err != nil { - return err - } - - // Removing templates not managed by any chain - var errs error - for _, tmpl := range managedTemplates { - orphaned, err := tmpl.IsOrphaned(ctx, r.Client) - if err != nil { - errs = errors.Join(errs, err) - continue - } - if orphaned { - ll := l.WithValues("template kind", r.templateKind, "template namespace", tmpl.GetNamespace(), "template name", tmpl.GetName()) - ll.Info("Deleting Template") - - if err := r.Client.Delete(ctx, tmpl); client.IgnoreNotFound(err) != nil { - errs = errors.Join(errs, err) - continue - } - ll.Info("Template has been deleted") - } - } - return errs -} - func getTemplateNamesManagedByChain(chain templateChain) []string { result := make([]string, 0, len(chain.GetSpec().SupportedTemplates)) for _, tmpl := range chain.GetSpec().SupportedTemplates { @@ -270,35 +196,12 @@ func getTemplateNamesManagedByChain(chain templateChain) []string { return result } -var tmEvents = predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - // Only trigger if access rules were changed - oldObject, ok := e.ObjectOld.(*hmc.TemplateManagement) - if !ok { - return false - } - newObject, ok := e.ObjectNew.(*hmc.TemplateManagement) - if !ok { - return false - } - return !equality.Semantic.DeepEqual(oldObject.Spec.AccessRules, newObject.Spec.AccessRules) - }, - DeleteFunc: func(event.DeleteEvent) bool { return false }, - GenericFunc: func(event.GenericEvent) bool { return false }, -} - // SetupWithManager sets up the controller with the Manager. func (r *ClusterTemplateChainReconciler) SetupWithManager(mgr ctrl.Manager) error { r.templateKind = hmc.ClusterTemplateKind return ctrl.NewControllerManagedBy(mgr). For(&hmc.ClusterTemplateChain{}). - Watches(&hmc.TemplateManagement{}, - handler.EnqueueRequestsFromMapFunc(func(_ context.Context, o client.Object) []ctrl.Request { - return getTemplateChainRequests(o, r.SystemNamespace, hmc.ClusterTemplateKind) - }), - builder.WithPredicates(tmEvents), - ). Complete(r) } @@ -308,39 +211,5 @@ func (r *ServiceTemplateChainReconciler) SetupWithManager(mgr ctrl.Manager) erro return ctrl.NewControllerManagedBy(mgr). For(&hmc.ServiceTemplateChain{}). - Watches(&hmc.TemplateManagement{}, - handler.EnqueueRequestsFromMapFunc(func(_ context.Context, o client.Object) []ctrl.Request { - return getTemplateChainRequests(o, r.SystemNamespace, hmc.ServiceTemplateKind) - }), - builder.WithPredicates(tmEvents), - ). Complete(r) } - -func getTemplateChainRequests(o client.Object, systemNamespace, kind string) (reqs []ctrl.Request) { - tm, ok := o.(*hmc.TemplateManagement) - if !ok { - return nil - } - f := func(chains []string) { - for _, c := range chains { - reqs = append(reqs, ctrl.Request{ - NamespacedName: client.ObjectKey{ - Namespace: systemNamespace, - Name: c, - }, - }) - } - } - if kind == hmc.ClusterTemplateChainKind { - for _, ar := range tm.Spec.AccessRules { - f(ar.ClusterTemplateChains) - } - } - if kind == hmc.ServiceTemplateChainKind { - for _, ar := range tm.Spec.AccessRules { - f(ar.ServiceTemplateChains) - } - } - return reqs -} diff --git a/internal/controller/templatechain_controller_test.go b/internal/controller/templatechain_controller_test.go index b8de60147..04c56f118 100644 --- a/internal/controller/templatechain_controller_test.go +++ b/internal/controller/templatechain_controller_test.go @@ -72,13 +72,6 @@ var _ = Describe("Template Chain Controller", func() { template.WithNamespace(utils.DefaultSystemNamespace), template.WithHelmSpec(templateHelmSpec), ), - // Should be deleted (not in the list of supported templates) - "ct1": template.NewClusterTemplate( - template.WithName("ct1"), - template.WithNamespace(namespace.Name), - template.WithHelmSpec(templateHelmSpec), - template.ManagedByHMC(), - ), // Should be unchanged (unmanaged) "ct2": template.NewClusterTemplate( template.WithName("ct2"), @@ -99,13 +92,6 @@ var _ = Describe("Template Chain Controller", func() { template.WithNamespace(utils.DefaultSystemNamespace), template.WithHelmSpec(templateHelmSpec), ), - // Should be deleted (not in the list of supported templates) - "st1": template.NewServiceTemplate( - template.WithName("st1"), - template.WithNamespace(namespace.Name), - template.WithHelmSpec(templateHelmSpec), - template.ManagedByHMC(), - ), // Should be unchanged (unmanaged) "st2": template.NewServiceTemplate( template.WithName("st2"), @@ -146,6 +132,9 @@ var _ = Describe("Template Chain Controller", func() { stChain2Name: {}, } + ctChainUIDs := make(map[string]types.UID) + stChainUIDs := make(map[string]types.UID) + BeforeEach(func() { By("creating the system and test namespaces") for _, ns := range []string{namespace.Name, utils.DefaultSystemNamespace} { @@ -220,18 +209,9 @@ var _ = Describe("Template Chain Controller", func() { Expect(err).NotTo(HaveOccurred()) By("Cleanup the specific resource instance ClusterTemplateChain") - Expect(mgrClient.Delete(ctx, clusterTemplateChainResource)).To(Succeed()) - - // Wait until the cached client reflects the deletion by checking the deletion timestamp - Eventually(func() bool { - err := mgrClient.Get(ctx, chain, clusterTemplateChainResource) - if err != nil { - return errors.IsNotFound(err) - } - return !clusterTemplateChainResource.DeletionTimestamp.IsZero() - }, 1*time.Minute, 5*time.Second).Should(BeTrue(), "ClusterTemplateChain should have a deletion timestamp") + Expect(k8sClient.Delete(ctx, clusterTemplateChainResource)).To(Succeed()) - // Running reconcile to remove the finalizer and delete the ClusterTemplateChain + // Running reconcile to delete the ClusterTemplateChain templateChainReconciler.templateKind = hmcmirantiscomv1alpha1.ClusterTemplateKind clusterTemplateChainReconciler := &ClusterTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} _, err = clusterTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: chain}) @@ -239,13 +219,8 @@ var _ = Describe("Template Chain Controller", func() { Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, chain, clusterTemplateChainResource).Should(HaveOccurred()) } - // These ClusterTemplates should be deleted once the ClusterTemplateChain is deleted - for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ctTemplates["test"], ctTemplates["ct0"]} { - verifyObjectDeleted(ctx, template.Namespace, template) - } - for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ctTemplates["ct1"], ctTemplates["ct2"]} { - err := k8sClient.Delete(ctx, template) - Expect(crclient.IgnoreNotFound(err)).To(Succeed()) + for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ctTemplates["test"], ctTemplates["ct0"], ctTemplates["ct2"]} { + Expect(crclient.IgnoreNotFound(k8sClient.Delete(ctx, template))).To(Succeed()) } for _, chain := range stChainNames { @@ -256,30 +231,16 @@ var _ = Describe("Template Chain Controller", func() { By("Cleanup the specific resource instance ServiceTemplateChain") Expect(k8sClient.Delete(ctx, serviceTemplateChainResource)).To(Succeed()) - // Wait until the cached client reflects the deletion by checking the deletion timestamp - Eventually(func() bool { - err := mgrClient.Get(ctx, chain, serviceTemplateChainResource) - if err != nil { - return errors.IsNotFound(err) - } - return !serviceTemplateChainResource.DeletionTimestamp.IsZero() - }, 1*time.Minute, 5*time.Second).Should(BeTrue(), "ServiceTemplateChain should have a deletion timestamp") - - // Running reconcile to remove the finalizer and delete the ServiceTemplateChain + // Running reconcile to delete the ServiceTemplateChain templateChainReconciler.templateKind = hmcmirantiscomv1alpha1.ServiceTemplateKind serviceTemplateChainReconciler := &ServiceTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} _, err = serviceTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: chain}) Expect(err).NotTo(HaveOccurred()) - Eventually(mgrClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, chain, serviceTemplateChainResource).Should(HaveOccurred()) + Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, chain, serviceTemplateChainResource).Should(HaveOccurred()) } - // These ServiceTemplates should be deleted once the ServiceTemplateChain is deleted - for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{stTemplates["test"], stTemplates["st0"]} { - verifyObjectDeleted(ctx, template.Namespace, template) - } - for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{stTemplates["st1"], stTemplates["st2"]} { - err := k8sClient.Delete(ctx, template) - Expect(crclient.IgnoreNotFound(err)).To(Succeed()) + for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{stTemplates["test"], stTemplates["st0"], stTemplates["st2"]} { + Expect(crclient.IgnoreNotFound(k8sClient.Delete(ctx, template))).To(Succeed()) } By("Cleanup the namespace") @@ -302,59 +263,65 @@ var _ = Describe("Template Chain Controller", func() { } By("Reconciling the ClusterTemplateChain resources") for _, chain := range ctChainNames { - // First Reconcile should only add finalizer templateChainReconciler.templateKind = hmcmirantiscomv1alpha1.ClusterTemplateKind clusterTemplateChainReconciler := &ClusterTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} _, err = clusterTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: chain}) Expect(err).NotTo(HaveOccurred()) ctChain := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} - Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: chain.Namespace, Name: chain.Name}, ctChain)).NotTo(HaveOccurred()) - Expect(ctChain.Finalizers).Should(ContainElement(hmcmirantiscomv1alpha1.TemplateChainFinalizer)) - - // Second Reconcile should reconcile the TemplateChain - _, err = clusterTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: chain}) + err = k8sClient.Get(ctx, types.NamespacedName{Namespace: chain.Namespace, Name: chain.Name}, ctChain) + ctChainUIDs[ctChain.Name] = ctChain.UID Expect(err).NotTo(HaveOccurred()) } By("Reconciling the ServiceTemplateChain resources") for _, chain := range stChainNames { - // First Reconcile should only add finalizer templateChainReconciler.templateKind = hmcmirantiscomv1alpha1.ServiceTemplateKind serviceTemplateChainReconciler := &ServiceTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} _, err = serviceTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: chain}) Expect(err).NotTo(HaveOccurred()) stChain := &hmcmirantiscomv1alpha1.ServiceTemplateChain{} - Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: chain.Namespace, Name: chain.Name}, stChain)).NotTo(HaveOccurred()) - Expect(stChain.Finalizers).Should(ContainElement(hmcmirantiscomv1alpha1.TemplateChainFinalizer)) - - // Second Reconcile should reconcile the TemplateChain - _, err = serviceTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: chain}) + err = k8sClient.Get(ctx, types.NamespacedName{Namespace: chain.Namespace, Name: chain.Name}, stChain) + stChainUIDs[stChain.Name] = stChain.UID Expect(err).NotTo(HaveOccurred()) } /* - Expected state: + Expected state after Reconcile: * test-chains/test - should be created * test-chains/ct0 - should be created - * test-chains/ct1 - should be deleted * test-chains/ct2 - should be unchanged (unmanaged by HMC) * test-chains/test - should be created * test-chains/st0 - should be created - * test-chains/st1 - should be deleted * test-chains/st2 - should be unchanged (unmanaged by HMC) */ + ct1OwnerRef := metav1.OwnerReference{ + APIVersion: hmcmirantiscomv1alpha1.GroupVersion.String(), + Kind: hmcmirantiscomv1alpha1.ClusterTemplateChainKind, + Name: ctChain1Name, + UID: ctChainUIDs[ctChain1Name], + } + st1OwnerRef := metav1.OwnerReference{ + APIVersion: hmcmirantiscomv1alpha1.GroupVersion.String(), + Kind: hmcmirantiscomv1alpha1.ServiceTemplateChainKind, + Name: stChain1Name, + UID: stChainUIDs[stChain1Name], + } verifyObjectCreated(ctx, namespace.Name, ctTemplates["test"]) + verifyOwnerReferenceExistence(ctTemplates["test"], ct1OwnerRef) verifyObjectCreated(ctx, namespace.Name, ctTemplates["ct0"]) - verifyObjectDeleted(ctx, namespace.Name, ctTemplates["ct1"]) + verifyOwnerReferenceExistence(ctTemplates["ct0"], ct1OwnerRef) + verifyObjectUnchanged(ctx, namespace.Name, ctUnmanagedBefore, ctTemplates["ct2"]) verifyObjectCreated(ctx, namespace.Name, stTemplates["test"]) + verifyOwnerReferenceExistence(stTemplates["test"], st1OwnerRef) verifyObjectCreated(ctx, namespace.Name, stTemplates["st0"]) - verifyObjectDeleted(ctx, namespace.Name, stTemplates["st1"]) + verifyOwnerReferenceExistence(stTemplates["st0"], st1OwnerRef) + verifyObjectUnchanged(ctx, namespace.Name, stUnmanagedBefore, stTemplates["st2"]) }) }) @@ -388,6 +355,11 @@ func verifyObjectUnchanged(ctx context.Context, namespace string, oldObj, newObj Expect(newObj.GetLabels()).NotTo(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) } +func verifyOwnerReferenceExistence(obj crclient.Object, ownerRef metav1.OwnerReference) { + By(fmt.Sprintf("Verifying owner reference existence on %s/%s", obj.GetNamespace(), obj.GetName())) + Expect(obj.GetOwnerReferences()).To(ContainElement(ownerRef)) +} + func checkHMCManagedLabelExistence(labels map[string]string) { Expect(labels).To(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) } diff --git a/internal/controller/templatemanagement_controller.go b/internal/controller/templatemanagement_controller.go index 93d08fe5d..bb0d40620 100644 --- a/internal/controller/templatemanagement_controller.go +++ b/internal/controller/templatemanagement_controller.go @@ -231,9 +231,6 @@ func (r *TemplateManagementReconciler) createTemplateChain(ctx context.Context, Labels: map[string]string{ hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue, }, - Finalizers: []string{ - hmc.TemplateChainFinalizer, - }, } var target templateChain kind := source.GetObjectKind().GroupVersionKind().Kind diff --git a/test/objects/template/template.go b/test/objects/template/template.go index 8518c861d..c26a50668 100644 --- a/test/objects/template/template.go +++ b/test/objects/template/template.go @@ -41,6 +41,10 @@ type ( func NewClusterTemplate(opts ...Opt) *v1alpha1.ClusterTemplate { t := &v1alpha1.ClusterTemplate{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: v1alpha1.ClusterTemplateKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: DefaultName, Namespace: DefaultNamespace, @@ -56,6 +60,10 @@ func NewClusterTemplate(opts ...Opt) *v1alpha1.ClusterTemplate { func NewServiceTemplate(opts ...Opt) *v1alpha1.ServiceTemplate { t := &v1alpha1.ServiceTemplate{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: v1alpha1.ServiceTemplateKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: DefaultName, Namespace: DefaultNamespace, @@ -71,6 +79,10 @@ func NewServiceTemplate(opts ...Opt) *v1alpha1.ServiceTemplate { func NewProviderTemplate(opts ...Opt) *v1alpha1.ProviderTemplate { t := &v1alpha1.ProviderTemplate{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: v1alpha1.ProviderTemplateKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: DefaultName, },