From 6fffbca173de767334243b2bed04ce763d78b8cf Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Fri, 27 Sep 2024 15:57:55 +0400 Subject: [PATCH 1/4] Make TemplateChains namespace-scoped --- api/v1alpha1/clustertemplatechain_types.go | 1 - api/v1alpha1/servicetemplatechain_types.go | 1 - .../templatemanagement_controller.go | 2 +- .../templatemanagement_controller_test.go | 10 ++++--- internal/templateutil/state.go | 8 +++-- .../webhook/templatemanagement_webhook.go | 2 +- .../templatemanagement_webhook_test.go | 30 +++++++++++-------- ...mc.mirantis.com_clustertemplatechains.yaml | 2 +- ...mc.mirantis.com_servicetemplatechains.yaml | 2 +- test/objects/templatechain/templatechain.go | 6 ++++ 10 files changed, 39 insertions(+), 25 deletions(-) diff --git a/api/v1alpha1/clustertemplatechain_types.go b/api/v1alpha1/clustertemplatechain_types.go index f20b4bf57..ab723db9f 100644 --- a/api/v1alpha1/clustertemplatechain_types.go +++ b/api/v1alpha1/clustertemplatechain_types.go @@ -19,7 +19,6 @@ import ( ) // +kubebuilder:object:root=true -// +kubebuilder:resource:scope=Cluster // ClusterTemplateChain is the Schema for the clustertemplatechains API type ClusterTemplateChain struct { diff --git a/api/v1alpha1/servicetemplatechain_types.go b/api/v1alpha1/servicetemplatechain_types.go index 96eb4eb06..b725dd2d0 100644 --- a/api/v1alpha1/servicetemplatechain_types.go +++ b/api/v1alpha1/servicetemplatechain_types.go @@ -19,7 +19,6 @@ import ( ) // +kubebuilder:object:root=true -// +kubebuilder:resource:scope=Cluster // ServiceTemplateChain is the Schema for the servicetemplatechains API type ServiceTemplateChain struct { diff --git a/internal/controller/templatemanagement_controller.go b/internal/controller/templatemanagement_controller.go index a6be1ebf9..45bc6a0d1 100644 --- a/internal/controller/templatemanagement_controller.go +++ b/internal/controller/templatemanagement_controller.go @@ -67,7 +67,7 @@ func (r *TemplateManagementReconciler) Reconcile(ctx context.Context, req ctrl.R if err != nil { return ctrl.Result{}, err } - expectedState, err := templateutil.ParseAccessRules(ctx, r.Client, templateMgmt.Spec.AccessRules, currentState) + expectedState, err := templateutil.ParseAccessRules(ctx, r.Client, r.SystemNamespace, templateMgmt.Spec.AccessRules, currentState) if err != nil { return ctrl.Result{}, err } diff --git a/internal/controller/templatemanagement_controller_test.go b/internal/controller/templatemanagement_controller_test.go index 214e0f4bb..a848dfca7 100644 --- a/internal/controller/templatemanagement_controller_test.go +++ b/internal/controller/templatemanagement_controller_test.go @@ -124,6 +124,7 @@ var _ = Describe("Template Management Controller", func() { } ctChain := chain.NewClusterTemplateChain( chain.WithName(ctChainName), + chain.WithNamespace(systemNamespace.Name), chain.WithSupportedTemplates(supportedClusterTemplates), ) @@ -133,6 +134,7 @@ var _ = Describe("Template Management Controller", func() { } stChain := chain.NewServiceTemplateChain( chain.WithName(stChainName), + chain.WithNamespace(systemNamespace.Name), chain.WithSupportedTemplates(supportedServiceTemplates), ) @@ -172,12 +174,12 @@ var _ = Describe("Template Management Controller", func() { Expect(k8sClient.Create(ctx, tm)).To(Succeed()) } By("creating the custom resource for the Kind ClusterTemplateChain") - err = k8sClient.Get(ctx, types.NamespacedName{Name: ctChainName}, ctChain) + err = k8sClient.Get(ctx, types.NamespacedName{Name: ctChainName, Namespace: systemNamespace.Name}, ctChain) if err != nil && errors.IsNotFound(err) { Expect(k8sClient.Create(ctx, ctChain)).To(Succeed()) } By("creating the custom resource for the Kind ServiceTemplateChain") - err = k8sClient.Get(ctx, types.NamespacedName{Name: stChainName}, stChain) + err = k8sClient.Get(ctx, types.NamespacedName{Name: stChainName, Namespace: systemNamespace.Name}, stChain) if err != nil && errors.IsNotFound(err) { Expect(k8sClient.Create(ctx, stChain)).To(Succeed()) } @@ -199,13 +201,13 @@ var _ = Describe("Template Management Controller", func() { } ctChain := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} - err := k8sClient.Get(ctx, types.NamespacedName{Name: ctChainName}, ctChain) + err := k8sClient.Get(ctx, types.NamespacedName{Name: ctChainName, Namespace: systemNamespace.Name}, ctChain) Expect(err).NotTo(HaveOccurred()) By("Cleanup the specific resource instance ClusterTemplateChain") Expect(k8sClient.Delete(ctx, ctChain)).To(Succeed()) stChain := &hmcmirantiscomv1alpha1.ServiceTemplateChain{} - err = k8sClient.Get(ctx, types.NamespacedName{Name: stChainName}, stChain) + err = k8sClient.Get(ctx, types.NamespacedName{Name: stChainName, Namespace: systemNamespace.Name}, stChain) Expect(err).NotTo(HaveOccurred()) By("Cleanup the specific resource instance ServiceTemplateChain") Expect(k8sClient.Delete(ctx, stChain)).To(Succeed()) diff --git a/internal/templateutil/state.go b/internal/templateutil/state.go index b7db545e9..48ec7cf59 100644 --- a/internal/templateutil/state.go +++ b/internal/templateutil/state.go @@ -93,7 +93,7 @@ func GetCurrentTemplatesState(ctx context.Context, cl client.Client, systemNames }, nil } -func ParseAccessRules(ctx context.Context, cl client.Client, rules []hmc.AccessRule, currentState State) (State, error) { +func ParseAccessRules(ctx context.Context, cl client.Client, systemNamespace string, rules []hmc.AccessRule, currentState State) (State, error) { var errs error expectedState := currentState @@ -109,7 +109,8 @@ func ParseAccessRules(ctx context.Context, cl client.Client, rules []hmc.AccessR for _, ctChainName := range rule.ClusterTemplateChains { ctChain := &hmc.ClusterTemplateChain{} err := cl.Get(ctx, client.ObjectKey{ - Name: ctChainName, + Name: ctChainName, + Namespace: systemNamespace, }, ctChain) if err != nil { errs = errors.Join(errs, err) @@ -122,7 +123,8 @@ func ParseAccessRules(ctx context.Context, cl client.Client, rules []hmc.AccessR for _, stChainName := range rule.ServiceTemplateChains { stChain := &hmc.ServiceTemplateChain{} err := cl.Get(ctx, client.ObjectKey{ - Name: stChainName, + Name: stChainName, + Namespace: systemNamespace, }, stChain) if err != nil { errs = errors.Join(errs, err) diff --git a/internal/webhook/templatemanagement_webhook.go b/internal/webhook/templatemanagement_webhook.go index 5b9a53e7c..01536aa6c 100644 --- a/internal/webhook/templatemanagement_webhook.go +++ b/internal/webhook/templatemanagement_webhook.go @@ -79,7 +79,7 @@ func (v *TemplateManagementValidator) ValidateUpdate(ctx context.Context, _ runt return nil, fmt.Errorf("could not get current templates state: %v", err) } - expectedState, err := templateutil.ParseAccessRules(ctx, v.Client, newTm.Spec.AccessRules, currentState) + expectedState, err := templateutil.ParseAccessRules(ctx, v.Client, v.SystemNamespace, newTm.Spec.AccessRules, currentState) if err != nil { return nil, fmt.Errorf("failed to parse access rules for TemplateManagement: %v", err) } diff --git a/internal/webhook/templatemanagement_webhook_test.go b/internal/webhook/templatemanagement_webhook_test.go index 50e429587..4868f62b6 100644 --- a/internal/webhook/templatemanagement_webhook_test.go +++ b/internal/webhook/templatemanagement_webhook_test.go @@ -139,18 +139,24 @@ func TestTemplateManagementValidateUpdate(t *testing.T) { ClusterTemplateChains: []string{azureCtChainName}, } - awsCtChain := chain.NewClusterTemplateChain(chain.WithName(awsCtChainName), chain.WithSupportedTemplates( - []v1alpha1.SupportedTemplate{ - {Name: awsStandaloneCpTemplateName}, - {Name: awsHostedCpTemplateName}, - }, - )) - azureCtChain := chain.NewClusterTemplateChain(chain.WithName(azureCtChainName), chain.WithSupportedTemplates( - []v1alpha1.SupportedTemplate{ - {Name: azureStandaloneCpTemplateName}, - {Name: azureHostedCpTemplateName}, - }, - )) + awsCtChain := chain.NewClusterTemplateChain( + chain.WithName(awsCtChainName), + chain.WithNamespace(utils.DefaultSystemNamespace), + chain.WithSupportedTemplates( + []v1alpha1.SupportedTemplate{ + {Name: awsStandaloneCpTemplateName}, + {Name: awsHostedCpTemplateName}, + }, + )) + azureCtChain := chain.NewClusterTemplateChain( + chain.WithName(azureCtChainName), + chain.WithNamespace(utils.DefaultSystemNamespace), + chain.WithSupportedTemplates( + []v1alpha1.SupportedTemplate{ + {Name: azureStandaloneCpTemplateName}, + {Name: azureHostedCpTemplateName}, + }, + )) hmcClusterTemplate := template.WithLabels(map[string]string{v1alpha1.HMCManagedLabelKey: v1alpha1.HMCManagedLabelValue}) diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplatechains.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplatechains.yaml index f2f8a3ccc..451d901af 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplatechains.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_clustertemplatechains.yaml @@ -12,7 +12,7 @@ spec: listKind: ClusterTemplateChainList plural: clustertemplatechains singular: clustertemplatechain - scope: Cluster + scope: Namespaced versions: - name: v1alpha1 schema: diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplatechains.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplatechains.yaml index ce4ca88b1..53913c138 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplatechains.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_servicetemplatechains.yaml @@ -12,7 +12,7 @@ spec: listKind: ServiceTemplateChainList plural: servicetemplatechains singular: servicetemplatechain - scope: Cluster + scope: Namespaced versions: - name: v1alpha1 schema: diff --git a/test/objects/templatechain/templatechain.go b/test/objects/templatechain/templatechain.go index ec0c60dee..105993591 100644 --- a/test/objects/templatechain/templatechain.go +++ b/test/objects/templatechain/templatechain.go @@ -65,6 +65,12 @@ func WithName(name string) Opt { } } +func WithNamespace(namespace string) Opt { + return func(tc *TemplateChain) { + tc.Namespace = namespace + } +} + func WithSupportedTemplates(supportedTemplates []v1alpha1.SupportedTemplate) Opt { return func(tc *TemplateChain) { tc.Spec.SupportedTemplates = supportedTemplates From 41374f5e5ba8e19b67684cdf90a72fb01d5a5e74 Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Fri, 27 Sep 2024 17:29:58 +0400 Subject: [PATCH 2/4] Add template chain controller --- api/v1alpha1/clustertemplate_types.go | 2 + api/v1alpha1/clustertemplatechain_types.go | 14 + api/v1alpha1/servicetemplate_types.go | 2 + api/v1alpha1/servicetemplatechain_types.go | 14 + cmd/main.go | 87 +++-- .../controller/templatechain_controller.go | 216 +++++++++++++ .../templatechain_controller_test.go | 299 ++++++++++++++++++ .../templatemanagement_controller_test.go | 37 +-- .../hmc/templates/rbac/controller/roles.yaml | 4 +- test/objects/template/template.go | 9 + 10 files changed, 629 insertions(+), 55 deletions(-) create mode 100644 internal/controller/templatechain_controller.go create mode 100644 internal/controller/templatechain_controller_test.go diff --git a/api/v1alpha1/clustertemplate_types.go b/api/v1alpha1/clustertemplate_types.go index 2af641b4f..36db285e9 100644 --- a/api/v1alpha1/clustertemplate_types.go +++ b/api/v1alpha1/clustertemplate_types.go @@ -18,6 +18,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ClusterTemplateKind = "ClusterTemplate" + // ClusterTemplateSpec defines the desired state of ClusterTemplate type ClusterTemplateSpec struct { TemplateSpecCommon `json:",inline"` diff --git a/api/v1alpha1/clustertemplatechain_types.go b/api/v1alpha1/clustertemplatechain_types.go index ab723db9f..d9684f796 100644 --- a/api/v1alpha1/clustertemplatechain_types.go +++ b/api/v1alpha1/clustertemplatechain_types.go @@ -18,6 +18,20 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ClusterTemplateChainKind = "ClusterTemplateChain" + +func (*ClusterTemplateChain) Kind() string { + return ClusterTemplateChainKind +} + +func (*ClusterTemplateChain) TemplateKind() string { + return ClusterTemplateKind +} + +func (t *ClusterTemplateChain) GetSpec() *TemplateChainSpec { + return &t.Spec +} + // +kubebuilder:object:root=true // ClusterTemplateChain is the Schema for the clustertemplatechains API diff --git a/api/v1alpha1/servicetemplate_types.go b/api/v1alpha1/servicetemplate_types.go index e9d083d18..731486192 100644 --- a/api/v1alpha1/servicetemplate_types.go +++ b/api/v1alpha1/servicetemplate_types.go @@ -18,6 +18,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ServiceTemplateKind = "ServiceTemplate" + // ServiceTemplateSpec defines the desired state of ServiceTemplate type ServiceTemplateSpec struct { TemplateSpecCommon `json:",inline"` diff --git a/api/v1alpha1/servicetemplatechain_types.go b/api/v1alpha1/servicetemplatechain_types.go index b725dd2d0..ff812f33e 100644 --- a/api/v1alpha1/servicetemplatechain_types.go +++ b/api/v1alpha1/servicetemplatechain_types.go @@ -18,6 +18,20 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ServiceTemplateChainKind = "ServiceTemplateChain" + +func (*ServiceTemplateChain) Kind() string { + return ServiceTemplateChainKind +} + +func (*ServiceTemplateChain) TemplateKind() string { + return ServiceTemplateKind +} + +func (t *ServiceTemplateChain) GetSpec() *TemplateChainSpec { + return &t.Spec +} + // +kubebuilder:object:root=true // ServiceTemplateChain is the Schema for the servicetemplatechains API diff --git a/cmd/main.go b/cmd/main.go index 75b9d6f00..6e949490e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -232,6 +232,24 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "TemplateManagement") os.Exit(1) } + + templateChainReconciler := controller.TemplateChainReconciler{ + Client: mgr.GetClient(), + SystemNamespace: currentNamespace, + } + if err = (&controller.ClusterTemplateChainReconciler{ + TemplateChainReconciler: templateChainReconciler, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ClusterTemplateChain") + os.Exit(1) + } + if err = (&controller.ServiceTemplateChainReconciler{ + TemplateChainReconciler: templateChainReconciler, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ServiceTemplateChain") + os.Exit(1) + } + if err = mgr.Add(&controller.Poller{ Client: mgr.GetClient(), Config: mgr.GetConfig(), @@ -274,37 +292,8 @@ func main() { } if enableWebhook { - if err := (&hmcwebhook.ManagedClusterValidator{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "ManagedCluster") - os.Exit(1) - } - if err := (&hmcwebhook.ManagementValidator{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "Management") - os.Exit(1) - } - if err := (&hmcwebhook.TemplateManagementValidator{ - SystemNamespace: currentNamespace, - }).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "TemplateManagement") - } - if err := (&hmcwebhook.ClusterTemplateChainValidator{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "ClusterTemplateChain") - os.Exit(1) - } - if err := (&hmcwebhook.ServiceTemplateChainValidator{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "ServiceTemplateChain") - os.Exit(1) - } - if err := (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "ClusterTemplate") - os.Exit(1) - } - if err := (&hmcwebhook.ServiceTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "ServiceTemplate") - os.Exit(1) - } - if err := (&hmcwebhook.ProviderTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "ProviderTemplate") + if err := setupWebhooks(mgr, currentNamespace); err != nil { + setupLog.Error(err, "failed to setup webhooks") os.Exit(1) } } @@ -315,3 +304,39 @@ func main() { os.Exit(1) } } + +func setupWebhooks(mgr ctrl.Manager, currentNamespace string) error { + if err := (&hmcwebhook.ManagedClusterValidator{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ManagedCluster") + return err + } + if err := (&hmcwebhook.ManagementValidator{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "Management") + return err + } + if err := (&hmcwebhook.TemplateManagementValidator{SystemNamespace: currentNamespace}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "TemplateManagement") + return err + } + if err := (&hmcwebhook.ClusterTemplateChainValidator{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ClusterTemplateChain") + return err + } + if err := (&hmcwebhook.ServiceTemplateChainValidator{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ServiceTemplateChain") + return err + } + if err := (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ClusterTemplate") + return err + } + if err := (&hmcwebhook.ServiceTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ServiceTemplate") + return err + } + if err := (&hmcwebhook.ProviderTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ProviderTemplate") + return err + } + return nil +} diff --git a/internal/controller/templatechain_controller.go b/internal/controller/templatechain_controller.go new file mode 100644 index 000000000..6e7d15041 --- /dev/null +++ b/internal/controller/templatechain_controller.go @@ -0,0 +1,216 @@ +// Copyright 2024 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controller + +import ( + "context" + "errors" + "fmt" + + helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2" + sourcev1 "github.com/fluxcd/source-controller/api/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + hmc "github.com/Mirantis/hmc/api/v1alpha1" +) + +// TemplateChainReconciler reconciles a TemplateChain object +type TemplateChainReconciler struct { + client.Client + SystemNamespace string +} + +type ClusterTemplateChainReconciler struct { + TemplateChainReconciler +} + +type ServiceTemplateChainReconciler struct { + TemplateChainReconciler +} + +// TemplateChain is the interface defining a list of methods to interact with templatechains +type TemplateChain interface { + client.Object + Kind() string + TemplateKind() string + GetSpec() *hmc.TemplateChainSpec +} + +func (r *ClusterTemplateChainReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + l := log.FromContext(ctx).WithValues("ClusterTemplateChainController", req.NamespacedName) + l.Info("Reconciling ClusterTemplateChain") + + clusterTemplateChain := &hmc.ClusterTemplateChain{} + err := r.Get(ctx, req.NamespacedName, clusterTemplateChain) + if err != nil { + if apierrors.IsNotFound(err) { + l.Info("ClusterTemplateChain not found, ignoring since object must be deleted") + return ctrl.Result{}, nil + } + l.Error(err, "Failed to get ClusterTemplateChain") + return ctrl.Result{}, err + } + return r.ReconcileTemplateChain(ctx, clusterTemplateChain) +} + +func (r *ServiceTemplateChainReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + l := log.FromContext(ctx).WithValues("ServiceTemplateChainReconciler", req.NamespacedName) + l.Info("Reconciling ServiceTemplateChain") + + serviceTemplateChain := &hmc.ServiceTemplateChain{} + err := r.Get(ctx, req.NamespacedName, serviceTemplateChain) + if err != nil { + if apierrors.IsNotFound(err) { + l.Info("ServiceTemplateChain not found, ignoring since object must be deleted") + return ctrl.Result{}, nil + } + l.Error(err, "Failed to get ServiceTemplateChain") + return ctrl.Result{}, err + } + return r.ReconcileTemplateChain(ctx, serviceTemplateChain) +} + +func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, templateChain TemplateChain) (ctrl.Result, error) { + l := log.FromContext(ctx) + + systemTemplates, existingTemplates, err := getCurrentTemplates(ctx, r.Client, templateChain.TemplateKind(), r.SystemNamespace, templateChain.GetNamespace()) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to get current templates: %v", err) + } + + var errs error + + keepTemplate := make(map[string]bool) + for _, supportedTemplate := range templateChain.GetSpec().SupportedTemplates { + meta := metav1.ObjectMeta{ + Name: supportedTemplate.Name, + Namespace: templateChain.GetNamespace(), + Labels: map[string]string{ + hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue, + }, + } + keepTemplate[supportedTemplate.Name] = true + + source, found := systemTemplates[supportedTemplate.Name] + if !found { + errs = errors.Join(errs, fmt.Errorf("source %s %s/%s is not found", templateChain.TemplateKind(), r.SystemNamespace, supportedTemplate.Name)) + continue + } + + templateSpec := hmc.TemplateSpecCommon{ + Helm: hmc.HelmSpec{ + ChartRef: &helmcontrollerv2.CrossNamespaceSourceReference{ + Kind: sourcev1.HelmChartKind, + Name: source.GetSpec().Helm.ChartName, + Namespace: r.SystemNamespace, + }, + }, + } + + var target client.Object + switch templateChain.Kind() { + case hmc.ClusterTemplateChainKind: + target = &hmc.ClusterTemplate{ObjectMeta: meta, Spec: hmc.ClusterTemplateSpec{ + TemplateSpecCommon: templateSpec, + }} + case hmc.ServiceTemplateChainKind: + target = &hmc.ServiceTemplate{ObjectMeta: meta, Spec: hmc.ServiceTemplateSpec{ + TemplateSpecCommon: templateSpec, + }} + default: + return ctrl.Result{}, fmt.Errorf("invalid TemplateChain kind. Supported kinds are %s and %s", hmc.ClusterTemplateChainKind, hmc.ServiceTemplateChainKind) + } + err := r.Create(ctx, target) + if err == nil { + l.Info(fmt.Sprintf("%s was successfully created", templateChain.TemplateKind()), "namespace", templateChain.GetNamespace(), "name", supportedTemplate) + continue + } + if !apierrors.IsAlreadyExists(err) { + errs = errors.Join(errs, err) + } + } + for _, template := range existingTemplates { + if !keepTemplate[template.GetName()] { + l.Info(fmt.Sprintf("Deleting %s", templateChain.TemplateKind()), "namespace", templateChain.GetNamespace(), "name", template.GetName()) + err := r.Delete(ctx, template) + if err == nil { + l.Info(fmt.Sprintf("%s was deleted", templateChain.TemplateKind()), "namespace", templateChain.GetNamespace(), "name", template.GetName()) + continue + } + if !apierrors.IsNotFound(err) { + errs = errors.Join(errs, err) + } + } + } + return ctrl.Result{}, nil +} + +func getCurrentTemplates(ctx context.Context, cl client.Client, templateKind, systemNamespace, targetNamespace string) (map[string]Template, []Template, error) { + var templates []Template + + switch templateKind { + case hmc.ClusterTemplateKind: + ctList := &hmc.ClusterTemplateList{} + err := cl.List(ctx, ctList) + if err != nil { + return nil, nil, err + } + for _, template := range ctList.Items { + templates = append(templates, &template) + } + case hmc.ServiceTemplateKind: + stList := &hmc.ServiceTemplateList{} + err := cl.List(ctx, stList) + if err != nil { + return nil, nil, err + } + for _, template := range stList.Items { + templates = append(templates, &template) + } + default: + return nil, nil, fmt.Errorf("invalid Template kind. Supported kinds are %s and %s", hmc.ClusterTemplateKind, hmc.ServiceTemplateKind) + } + systemTemplates := make(map[string]Template) + var managedTemplates []Template + + for _, template := range templates { + if template.GetNamespace() == systemNamespace { + systemTemplates[template.GetName()] = template + continue + } + if template.GetNamespace() == targetNamespace && template.GetLabels()[hmc.HMCManagedLabelKey] == "true" { + managedTemplates = append(managedTemplates, template) + } + } + return systemTemplates, managedTemplates, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *ClusterTemplateChainReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&hmc.ClusterTemplateChain{}). + Complete(r) +} + +// SetupWithManager sets up the controller with the Manager. +func (r *ServiceTemplateChainReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&hmc.ServiceTemplateChain{}). + Complete(r) +} diff --git a/internal/controller/templatechain_controller_test.go b/internal/controller/templatechain_controller_test.go new file mode 100644 index 000000000..1648a86ef --- /dev/null +++ b/internal/controller/templatechain_controller_test.go @@ -0,0 +1,299 @@ +// Copyright 2024 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controller + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + crclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + hmcmirantiscomv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/internal/utils" + "github.com/Mirantis/hmc/test/objects/template" +) + +var _ = Describe("Template Chain Controller", func() { + Context("When reconciling a resource", func() { + const ( + ctChainName = "ct-chain" + stChainName = "st-chain" + ) + + ctx := context.Background() + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-chains", + }, + } + + templateHelmSpec := hmcmirantiscomv1alpha1.HelmSpec{ChartName: "test"} + + ctTemplates := map[string]*hmcmirantiscomv1alpha1.ClusterTemplate{ + // Should be created in target namespace + "test": template.NewClusterTemplate( + template.WithName("test"), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithHelmSpec(templateHelmSpec), + ), + // Should be created in target namespace + "ct0": template.NewClusterTemplate( + template.WithName("ct0"), + 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"), + template.WithNamespace(namespace.Name), + template.WithHelmSpec(templateHelmSpec), + ), + } + stTemplates := map[string]*hmcmirantiscomv1alpha1.ServiceTemplate{ + // Should be created in target namespace + "test": template.NewServiceTemplate( + template.WithName("test"), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithHelmSpec(templateHelmSpec), + ), + // Should be created in target namespace + "st0": template.NewServiceTemplate( + template.WithName("st0"), + 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"), + template.WithNamespace(namespace.Name), + template.WithHelmSpec(templateHelmSpec), + ), + } + + ctChainNamespacedName := types.NamespacedName{ + Name: ctChainName, + Namespace: namespace.Name, + } + + stChainNamespacedName := types.NamespacedName{ + Name: stChainName, + Namespace: namespace.Name, + } + clusterTemplateChain := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} + serviceTemplateChain := &hmcmirantiscomv1alpha1.ServiceTemplateChain{} + supportedClusterTemplates := []hmcmirantiscomv1alpha1.SupportedTemplate{ + { + Name: "test", + }, + { + Name: "ct0", + }, + // Does not exist in the system namespace + { + Name: "ct3", + }, + } + supportedServiceTemplates := []hmcmirantiscomv1alpha1.SupportedTemplate{ + { + Name: "test", + }, + { + Name: "st0", + }, + // Does not exist in the system namespace + { + Name: "st3", + }, + } + + BeforeEach(func() { + By("creating the system and test namespaces") + for _, ns := range []string{namespace.Name, utils.DefaultSystemNamespace} { + namespace := &corev1.Namespace{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: ns}, namespace) + if err != nil && errors.IsNotFound(err) { + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: ns, + }, + } + Expect(k8sClient.Create(ctx, namespace)).To(Succeed()) + } + } + + By("creating the custom resource for the Kind ClusterTemplateChain") + err := k8sClient.Get(ctx, ctChainNamespacedName, clusterTemplateChain) + if err != nil && errors.IsNotFound(err) { + resource := &hmcmirantiscomv1alpha1.ClusterTemplateChain{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctChainName, + Namespace: namespace.Name, + }, + Spec: hmcmirantiscomv1alpha1.TemplateChainSpec{SupportedTemplates: supportedClusterTemplates}, + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } + By("creating the custom resource for the Kind ServiceTemplateChain") + err = k8sClient.Get(ctx, stChainNamespacedName, serviceTemplateChain) + if err != nil && errors.IsNotFound(err) { + resource := &hmcmirantiscomv1alpha1.ServiceTemplateChain{ + ObjectMeta: metav1.ObjectMeta{ + Name: stChainName, + Namespace: namespace.Name, + }, + Spec: hmcmirantiscomv1alpha1.TemplateChainSpec{SupportedTemplates: supportedServiceTemplates}, + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } + By("creating the custom resource for the Kind ClusterTemplate") + for name, template := range ctTemplates { + ct := &hmcmirantiscomv1alpha1.ClusterTemplate{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: utils.DefaultSystemNamespace}, ct) + if err != nil && errors.IsNotFound(err) { + Expect(k8sClient.Create(ctx, template)).To(Succeed()) + } + } + for name, template := range stTemplates { + st := &hmcmirantiscomv1alpha1.ServiceTemplate{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: utils.DefaultSystemNamespace}, st) + if err != nil && errors.IsNotFound(err) { + Expect(k8sClient.Create(ctx, template)).To(Succeed()) + } + } + }) + + AfterEach(func() { + for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ + ctTemplates["test"], ctTemplates["ct0"], ctTemplates["ct1"], ctTemplates["ct2"], + } { + err := k8sClient.Delete(ctx, template) + Expect(crclient.IgnoreNotFound(err)).To(Succeed()) + } + for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{ + stTemplates["test"], stTemplates["st0"], stTemplates["st1"], stTemplates["st2"], + } { + err := k8sClient.Delete(ctx, template) + Expect(crclient.IgnoreNotFound(err)).To(Succeed()) + } + + clusterTemplateChainResource := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} + err := k8sClient.Get(ctx, ctChainNamespacedName, clusterTemplateChainResource) + Expect(err).NotTo(HaveOccurred()) + + By("Cleanup the specific resource instance ClusterTemplateChain") + Expect(k8sClient.Delete(ctx, clusterTemplateChainResource)).To(Succeed()) + + serviceTemplateChainResource := &hmcmirantiscomv1alpha1.ServiceTemplateChain{} + err = k8sClient.Get(ctx, stChainNamespacedName, serviceTemplateChainResource) + Expect(err).NotTo(HaveOccurred()) + + By("Cleanup the specific resource instance ServiceTemplateChain") + Expect(k8sClient.Delete(ctx, serviceTemplateChainResource)).To(Succeed()) + + By("Cleanup the namespace") + err = k8sClient.Get(ctx, types.NamespacedName{Name: namespace.Name}, namespace) + Expect(err).NotTo(HaveOccurred()) + Expect(crclient.IgnoreNotFound(k8sClient.Delete(ctx, namespace))).To(Succeed()) + }) + It("should successfully reconcile the resource", func() { + By("Get unmanaged templates before the reconciliation to verify it wasn't changed") + ctUnmanagedBefore := &hmcmirantiscomv1alpha1.ClusterTemplate{} + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace.Name, Name: "ct2"}, ctUnmanagedBefore) + Expect(err).NotTo(HaveOccurred()) + stUnmanagedBefore := &hmcmirantiscomv1alpha1.ServiceTemplate{} + err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace.Name, Name: "st2"}, stUnmanagedBefore) + Expect(err).NotTo(HaveOccurred()) + + templateChainReconciler := TemplateChainReconciler{ + Client: k8sClient, + SystemNamespace: utils.DefaultSystemNamespace, + } + By("Reconciling the ClusterTemplateChain resource") + clusterTemplateChainReconciler := &ClusterTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} + _, err = clusterTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: ctChainNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + + By("Reconciling the ServiceTemplateChain resource") + serviceTemplateChainReconciler := &ServiceTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} + _, err = serviceTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: stChainNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + + /* + Expected state: + * test/test - should be created + * test/ct0 - should be created + * test/ct1 - should be deleted + * test/ct2 - should be unchanged (unmanaged by HMC) + + * test/test - should be created + * test/st0 - should be created + * test/st1 - should be deleted + * test/st2 - should be unchanged (unmanaged by HMC) + */ + verifyTemplateCreated(ctx, namespace.Name, ctTemplates["test"]) + verifyTemplateCreated(ctx, namespace.Name, ctTemplates["ct0"]) + verifyTemplateDeleted(ctx, namespace.Name, ctTemplates["ct1"]) + verifyTemplateUnchanged(ctx, namespace.Name, ctUnmanagedBefore, ctTemplates["ct2"]) + + verifyTemplateCreated(ctx, namespace.Name, stTemplates["test"]) + verifyTemplateCreated(ctx, namespace.Name, stTemplates["st0"]) + verifyTemplateDeleted(ctx, namespace.Name, stTemplates["st1"]) + verifyTemplateUnchanged(ctx, namespace.Name, stUnmanagedBefore, stTemplates["st2"]) + }) + }) +}) + +func verifyTemplateCreated(ctx context.Context, namespace string, tpl crclient.Object) { + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: tpl.GetName()}, tpl) + Expect(err).NotTo(HaveOccurred()) + checkHMCManagedLabelExistence(tpl.GetLabels()) +} + +func verifyTemplateDeleted(ctx context.Context, namespace string, tpl crclient.Object) { + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: tpl.GetName()}, tpl) + Expect(err).To(HaveOccurred()) +} + +func verifyTemplateUnchanged(ctx context.Context, namespace string, oldTpl, newTpl crclient.Object) { + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: newTpl.GetName()}, newTpl) + Expect(err).NotTo(HaveOccurred()) + Expect(oldTpl).To(Equal(newTpl)) + Expect(newTpl.GetLabels()).NotTo(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) +} + +func checkHMCManagedLabelExistence(labels map[string]string) { + Expect(labels).To(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) +} diff --git a/internal/controller/templatemanagement_controller_test.go b/internal/controller/templatemanagement_controller_test.go index a848dfca7..eb8312d91 100644 --- a/internal/controller/templatemanagement_controller_test.go +++ b/internal/controller/templatemanagement_controller_test.go @@ -211,6 +211,21 @@ var _ = Describe("Template Management Controller", func() { Expect(err).NotTo(HaveOccurred()) By("Cleanup the specific resource instance ServiceTemplateChain") Expect(k8sClient.Delete(ctx, stChain)).To(Succeed()) + + for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ct1, ct2, ct3, ctUnmanaged} { + for _, ns := range []*corev1.Namespace{systemNamespace, namespace1, namespace2, namespace3} { + template.Namespace = ns.Name + err := k8sClient.Delete(ctx, template) + Expect(crclient.IgnoreNotFound(err)).To(Succeed()) + } + } + for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{st1, st2, st3, stUnmanaged} { + for _, ns := range []*corev1.Namespace{systemNamespace, namespace1, namespace2, namespace3} { + template.Namespace = ns.Name + err := k8sClient.Delete(ctx, template) + Expect(crclient.IgnoreNotFound(err)).To(Succeed()) + } + } }) It("should successfully reconcile the resource", func() { By("Get unmanaged templates before the reconciliation to verify it wasn't changed") @@ -264,25 +279,3 @@ var _ = Describe("Template Management Controller", func() { }) }) }) - -func verifyTemplateCreated(ctx context.Context, namespace string, tpl crclient.Object) { - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: tpl.GetName()}, tpl) - Expect(err).NotTo(HaveOccurred()) - checkHMCManagedLabelExistence(tpl.GetLabels()) -} - -func verifyTemplateDeleted(ctx context.Context, namespace string, tpl crclient.Object) { - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: tpl.GetName()}, tpl) - Expect(err).To(HaveOccurred()) -} - -func verifyTemplateUnchanged(ctx context.Context, namespace string, oldTpl, newTpl crclient.Object) { - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: newTpl.GetName()}, newTpl) - Expect(err).NotTo(HaveOccurred()) - Expect(oldTpl).To(Equal(newTpl)) - Expect(newTpl.GetLabels()).NotTo(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) -} - -func checkHMCManagedLabelExistence(labels map[string]string) { - Expect(labels).To(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) -} diff --git a/templates/provider/hmc/templates/rbac/controller/roles.yaml b/templates/provider/hmc/templates/rbac/controller/roles.yaml index 8dea78223..05177d3d3 100644 --- a/templates/provider/hmc/templates/rbac/controller/roles.yaml +++ b/templates/provider/hmc/templates/rbac/controller/roles.yaml @@ -43,13 +43,13 @@ rules: - hmc.mirantis.com resources: - templatemanagements + - clustertemplatechains + - servicetemplatechains verbs: {{ include "rbac.editorVerbs" . | nindent 4 }} - apiGroups: - hmc.mirantis.com resources: - releases - - clustertemplatechains - - servicetemplatechains verbs: - get - list diff --git a/test/objects/template/template.go b/test/objects/template/template.go index 668f40191..f3184be39 100644 --- a/test/objects/template/template.go +++ b/test/objects/template/template.go @@ -92,6 +92,15 @@ func WithLabels(labels map[string]string) Opt { } } +func ManagedByHMC() Opt { + return func(t *Template) { + if t.Labels == nil { + t.Labels = make(map[string]string) + } + t.Labels[v1alpha1.HMCManagedLabelKey] = v1alpha1.HMCManagedLabelValue + } +} + func WithHelmSpec(helmSpec v1alpha1.HelmSpec) Opt { return func(t *Template) { t.Spec.Helm = helmSpec From 6e256eaa49cb28b4673a8e5068beec8e749c1b32 Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Fri, 27 Sep 2024 20:01:32 +0400 Subject: [PATCH 3/4] Rework TemplateManagement controller --- .../templatechain_controller_test.go | 39 +-- .../templatemanagement_controller.go | 261 +++++++++++------- .../templatemanagement_controller_test.go | 191 ++++--------- internal/templateutil/state.go | 196 ------------- .../webhook/templatemanagement_webhook.go | 58 +--- .../templatemanagement_webhook_test.go | 182 ------------ test/objects/templatechain/templatechain.go | 9 + 7 files changed, 254 insertions(+), 682 deletions(-) delete mode 100644 internal/templateutil/state.go diff --git a/internal/controller/templatechain_controller_test.go b/internal/controller/templatechain_controller_test.go index 1648a86ef..6dccf012d 100644 --- a/internal/controller/templatechain_controller_test.go +++ b/internal/controller/templatechain_controller_test.go @@ -16,6 +16,7 @@ package controller import ( "context" + "fmt" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -263,35 +264,39 @@ var _ = Describe("Template Chain Controller", func() { * test/st1 - should be deleted * test/st2 - should be unchanged (unmanaged by HMC) */ - verifyTemplateCreated(ctx, namespace.Name, ctTemplates["test"]) - verifyTemplateCreated(ctx, namespace.Name, ctTemplates["ct0"]) - verifyTemplateDeleted(ctx, namespace.Name, ctTemplates["ct1"]) - verifyTemplateUnchanged(ctx, namespace.Name, ctUnmanagedBefore, ctTemplates["ct2"]) - verifyTemplateCreated(ctx, namespace.Name, stTemplates["test"]) - verifyTemplateCreated(ctx, namespace.Name, stTemplates["st0"]) - verifyTemplateDeleted(ctx, namespace.Name, stTemplates["st1"]) - verifyTemplateUnchanged(ctx, namespace.Name, stUnmanagedBefore, stTemplates["st2"]) + verifyObjectCreated(ctx, namespace.Name, ctTemplates["test"]) + verifyObjectCreated(ctx, namespace.Name, ctTemplates["ct0"]) + verifyObjectDeleted(ctx, namespace.Name, ctTemplates["ct1"]) + verifyObjectUnchanged(ctx, namespace.Name, ctUnmanagedBefore, ctTemplates["ct2"]) + + verifyObjectCreated(ctx, namespace.Name, stTemplates["test"]) + verifyObjectCreated(ctx, namespace.Name, stTemplates["st0"]) + verifyObjectDeleted(ctx, namespace.Name, stTemplates["st1"]) + verifyObjectUnchanged(ctx, namespace.Name, stUnmanagedBefore, stTemplates["st2"]) }) }) }) -func verifyTemplateCreated(ctx context.Context, namespace string, tpl crclient.Object) { - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: tpl.GetName()}, tpl) +func verifyObjectCreated(ctx context.Context, namespace string, obj crclient.Object) { + By(fmt.Sprintf("Verifying existence of %s/%s", namespace, obj.GetName())) + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: obj.GetName()}, obj) Expect(err).NotTo(HaveOccurred()) - checkHMCManagedLabelExistence(tpl.GetLabels()) + checkHMCManagedLabelExistence(obj.GetLabels()) } -func verifyTemplateDeleted(ctx context.Context, namespace string, tpl crclient.Object) { - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: tpl.GetName()}, tpl) +func verifyObjectDeleted(ctx context.Context, namespace string, obj crclient.Object) { + By(fmt.Sprintf("Verifying %s/%s is deleted", namespace, obj.GetName())) + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: obj.GetName()}, obj) Expect(err).To(HaveOccurred()) } -func verifyTemplateUnchanged(ctx context.Context, namespace string, oldTpl, newTpl crclient.Object) { - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: newTpl.GetName()}, newTpl) +func verifyObjectUnchanged(ctx context.Context, namespace string, oldObj, newObj crclient.Object) { + By(fmt.Sprintf("Verifying %s/%s is unchanged", namespace, newObj.GetName())) + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: newObj.GetName()}, newObj) Expect(err).NotTo(HaveOccurred()) - Expect(oldTpl).To(Equal(newTpl)) - Expect(newTpl.GetLabels()).NotTo(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) + Expect(oldObj).To(Equal(newObj)) + Expect(newObj.GetLabels()).NotTo(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) } func checkHMCManagedLabelExistence(labels map[string]string) { diff --git a/internal/controller/templatemanagement_controller.go b/internal/controller/templatemanagement_controller.go index 45bc6a0d1..0384f6982 100644 --- a/internal/controller/templatemanagement_controller.go +++ b/internal/controller/templatemanagement_controller.go @@ -19,16 +19,15 @@ import ( "errors" "fmt" - helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2" - sourcev1 "github.com/fluxcd/source-controller/api/v1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" hmc "github.com/Mirantis/hmc/api/v1alpha1" - "github.com/Mirantis/hmc/internal/templateutil" ) // TemplateManagementReconciler reconciles a TemplateManagement object @@ -63,24 +62,74 @@ func (r *TemplateManagementReconciler) Reconcile(ctx context.Context, req ctrl.R err = errors.Join(err, r.updateStatus(ctx, templateMgmt)) }() - currentState, err := templateutil.GetCurrentTemplatesState(ctx, r.Client, r.SystemNamespace) + systemCtChains, managedCtChains, err := r.getCurrentTemplateChains(ctx, hmc.ClusterTemplateChainKind) if err != nil { return ctrl.Result{}, err } - expectedState, err := templateutil.ParseAccessRules(ctx, r.Client, r.SystemNamespace, templateMgmt.Spec.AccessRules, currentState) + systemStChains, managedStChains, err := r.getCurrentTemplateChains(ctx, hmc.ServiceTemplateChainKind) if err != nil { return ctrl.Result{}, err } + keepCtChains := make(map[string]bool) + keepStChains := make(map[string]bool) + var errs error - err = r.distributeTemplates(ctx, expectedState.ClusterTemplatesState, templateutil.ClusterTemplateKind) - if err != nil { - errs = errors.Join(errs, err) + for _, rule := range templateMgmt.Spec.AccessRules { + namespaces, err := getTargetNamespaces(ctx, r.Client, rule.TargetNamespaces) + if err != nil { + return ctrl.Result{}, err + } + for _, namespace := range namespaces { + for _, ctChain := range rule.ClusterTemplateChains { + keepCtChains[getNamespacedName(namespace, ctChain)] = true + if systemCtChains[ctChain] == nil { + errs = errors.Join(errs, fmt.Errorf("ClusterTemplateChain %s/%s is not found", r.SystemNamespace, ctChain)) + continue + } + err = r.createTemplateChain(ctx, systemCtChains[ctChain], namespace) + if err != nil { + errs = errors.Join(errs, err) + continue + } + } + for _, stChain := range rule.ServiceTemplateChains { + keepStChains[getNamespacedName(namespace, stChain)] = true + if systemStChains[stChain] == nil { + errs = errors.Join(errs, fmt.Errorf("ServiceTemplateChain %s/%s is not found", r.SystemNamespace, stChain)) + continue + } + err = r.createTemplateChain(ctx, systemStChains[stChain], namespace) + if err != nil { + errs = errors.Join(errs, err) + continue + } + } + } } - err = r.distributeTemplates(ctx, expectedState.ServiceTemplatesState, templateutil.ServiceTemplateKind) - if err != nil { - errs = errors.Join(errs, err) + + managedChains := append(managedCtChains, managedStChains...) + for _, managedChain := range managedChains { + keep := false + templateNamespacedName := getNamespacedName(managedChain.GetNamespace(), managedChain.GetName()) + switch managedChain.Kind() { + case hmc.ClusterTemplateChainKind: + keep = keepCtChains[templateNamespacedName] + case hmc.ServiceTemplateChainKind: + keep = keepStChains[templateNamespacedName] + default: + errs = errors.Join(errs, fmt.Errorf("invalid TemplateChain kind. Supported kinds are %s and %s", hmc.ClusterTemplateChainKind, hmc.ServiceTemplateChainKind)) + } + + if !keep { + err := r.deleteTemplateChain(ctx, managedChain) + if err != nil { + errs = errors.Join(errs, err) + continue + } + } } + if errs != nil { return ctrl.Result{}, errs } @@ -89,115 +138,129 @@ func (r *TemplateManagementReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, nil } -func (r *TemplateManagementReconciler) updateStatus(ctx context.Context, templateMgmt *hmc.TemplateManagement) error { - if err := r.Status().Update(ctx, templateMgmt); err != nil { - return fmt.Errorf("failed to update status for TemplateManagement %s: %w", templateMgmt.Name, err) +func getNamespacedName(namespace, name string) string { + return fmt.Sprintf("%s/%s", namespace, name) +} + +func (r *TemplateManagementReconciler) getCurrentTemplateChains(ctx context.Context, templateChainKind string) (map[string]TemplateChain, []TemplateChain, error) { + var templateChains []TemplateChain + switch templateChainKind { + case hmc.ClusterTemplateChainKind: + ctChainList := &hmc.ClusterTemplateChainList{} + err := r.List(ctx, ctChainList) + if err != nil { + return nil, nil, err + } + for _, chain := range ctChainList.Items { + templateChains = append(templateChains, &chain) + } + case hmc.ServiceTemplateChainKind: + stChainList := &hmc.ServiceTemplateChainList{} + err := r.List(ctx, stChainList) + if err != nil { + return nil, nil, err + } + for _, chain := range stChainList.Items { + templateChains = append(templateChains, &chain) + } + default: + return nil, nil, fmt.Errorf("invalid TemplateChain kind. Supported kinds are %s and %s", hmc.ClusterTemplateChainKind, hmc.ServiceTemplateChainKind) } - return nil + + systemTemplateChains := make(map[string]TemplateChain) + var managedTemplateChains []TemplateChain + for _, chain := range templateChains { + if chain.GetNamespace() == r.SystemNamespace { + systemTemplateChains[chain.GetName()] = chain + continue + } + if chain.GetLabels()[hmc.HMCManagedLabelKey] == hmc.HMCManagedLabelValue { + managedTemplateChains = append(managedTemplateChains, chain) + } + } + return systemTemplateChains, managedTemplateChains, nil } -func (r *TemplateManagementReconciler) distributeTemplates(ctx context.Context, state map[string]map[string]bool, kind string) error { - var errs error - for name, namespaces := range state { - err := r.applyTemplates(ctx, kind, name, namespaces) +func getTargetNamespaces(ctx context.Context, cl client.Client, targetNamespaces hmc.TargetNamespaces) ([]string, error) { + if len(targetNamespaces.List) > 0 { + return targetNamespaces.List, nil + } + var selector labels.Selector + var err error + if targetNamespaces.StringSelector != "" { + selector, err = labels.Parse(targetNamespaces.StringSelector) + if err != nil { + return nil, err + } + } else { + selector, err = metav1.LabelSelectorAsSelector(targetNamespaces.Selector) if err != nil { - errs = errors.Join(errs, err) + return nil, fmt.Errorf("failed to construct selector from the namespaces selector %s: %w", targetNamespaces.Selector, err) } } - if errs != nil { - return errs + + namespaces := &corev1.NamespaceList{} + listOpts := &client.ListOptions{} + if selector.String() != "" { + listOpts = &client.ListOptions{LabelSelector: selector} } - return nil + err = cl.List(ctx, namespaces, listOpts) + if err != nil { + return []string{}, err + } + result := make([]string, len(namespaces.Items)) + for i, ns := range namespaces.Items { + result[i] = ns.Name + } + return result, nil } -func (r *TemplateManagementReconciler) applyTemplates(ctx context.Context, kind string, name string, namespaces map[string]bool) error { +func (r *TemplateManagementReconciler) createTemplateChain(ctx context.Context, source TemplateChain, targetNamespace string) error { l := ctrl.LoggerFrom(ctx) + meta := metav1.ObjectMeta{ - Name: name, + Name: source.GetName(), + Namespace: targetNamespace, Labels: map[string]string{ hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue, }, } + var target TemplateChain + switch source.Kind() { + case hmc.ClusterTemplateChainKind: + target = &hmc.ClusterTemplateChain{ObjectMeta: meta, Spec: *source.GetSpec()} + case hmc.ServiceTemplateChainKind: + target = &hmc.ServiceTemplateChain{ObjectMeta: meta, Spec: *source.GetSpec()} + } - sourceFound := false - chartName := "" - - switch kind { - case templateutil.ClusterTemplateKind: - source := &hmc.ClusterTemplate{} - err := r.Get(ctx, client.ObjectKey{ - Namespace: r.SystemNamespace, - Name: name, - }, source) - if err == nil { - sourceFound = true - chartName = source.Spec.Helm.ChartName - } else if !apierrors.IsNotFound(err) { - return err - } - case templateutil.ServiceTemplateKind: - source := &hmc.ServiceTemplate{} - err := r.Get(ctx, client.ObjectKey{ - Namespace: r.SystemNamespace, - Name: name, - }, source) - if err == nil { - sourceFound = true - chartName = source.Spec.Helm.ChartName - } else if !apierrors.IsNotFound(err) { - return err + err := r.Create(ctx, target) + if err != nil { + if apierrors.IsAlreadyExists(err) { + return nil } - default: - return fmt.Errorf("invalid kind %s. Only %s or %s kinds are supported", kind, templateutil.ClusterTemplateKind, templateutil.ServiceTemplateKind) + return err } + l.Info(fmt.Sprintf("%s was successfully created", source.Kind()), "namespace", targetNamespace, "name", source.GetName()) + return nil +} - spec := hmc.TemplateSpecCommon{ - Helm: hmc.HelmSpec{ - ChartRef: &helmcontrollerv2.CrossNamespaceSourceReference{ - Kind: sourcev1.HelmChartKind, - Name: chartName, - Namespace: r.SystemNamespace, - }, - }, - } - var errs error - for ns, keep := range namespaces { - var target client.Object - meta.Namespace = ns - if kind == templateutil.ClusterTemplateKind { - target = &hmc.ClusterTemplate{ObjectMeta: meta, Spec: hmc.ClusterTemplateSpec{TemplateSpecCommon: spec}} - } - if kind == templateutil.ServiceTemplateKind { - target = &hmc.ServiceTemplate{ObjectMeta: meta, Spec: hmc.ServiceTemplateSpec{TemplateSpecCommon: spec}} - } - if keep { - if !sourceFound { - errs = errors.Join(errs, fmt.Errorf("source %s %s/%s is not found", kind, r.SystemNamespace, name)) - continue - } - l.Info(fmt.Sprintf("Creating %s", kind), "namespace", ns, "name", name) - err := r.Create(ctx, target) - if err == nil { - l.Info(fmt.Sprintf("%s was successfully created", kind), "namespace", ns, "name", name) - continue - } - if !apierrors.IsAlreadyExists(err) { - errs = errors.Join(errs, err) - } - } else { - l.Info(fmt.Sprintf("Deleting %s", kind), "namespace", ns, "name", name) - err := r.Delete(ctx, target) - if err == nil { - l.Info(fmt.Sprintf("%s was deleted", kind), "namespace", ns, "name", name) - continue - } - if !apierrors.IsNotFound(err) { - errs = errors.Join(errs, err) - } +func (r *TemplateManagementReconciler) deleteTemplateChain(ctx context.Context, chain TemplateChain) error { + l := ctrl.LoggerFrom(ctx) + + err := r.Delete(ctx, chain) + if err != nil { + if apierrors.IsNotFound(err) { + return nil } + return err } - if errs != nil { - return errs + l.Info(fmt.Sprintf("%s was successfully deleted", chain.Kind()), "namespace", chain.GetNamespace(), "name", chain.GetName()) + return nil +} + +func (r *TemplateManagementReconciler) updateStatus(ctx context.Context, templateMgmt *hmc.TemplateManagement) error { + if err := r.Status().Update(ctx, templateMgmt); err != nil { + return fmt.Errorf("failed to update status for TemplateManagement %s: %w", templateMgmt.Name, err) } return nil } diff --git a/internal/controller/templatemanagement_controller_test.go b/internal/controller/templatemanagement_controller_test.go index eb8312d91..49e41f674 100644 --- a/internal/controller/templatemanagement_controller_test.go +++ b/internal/controller/templatemanagement_controller_test.go @@ -26,32 +26,26 @@ import ( crclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" - hmcmirantiscomv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1" - "github.com/Mirantis/hmc/test/objects/template" - chain "github.com/Mirantis/hmc/test/objects/templatechain" + hmc "github.com/Mirantis/hmc/api/v1alpha1" + tc "github.com/Mirantis/hmc/test/objects/templatechain" tm "github.com/Mirantis/hmc/test/objects/templatemanagement" ) var _ = Describe("Template Management Controller", func() { Context("When reconciling a resource", func() { const ( - tmName = "hmc-tm" - ctChainName = "hmc-ct-chain" - stChainName = "hmc-st-chain" + tmName = "hmc-tm" + ctChainName = "hmc-ct-chain" + stChainName = "hmc-st-chain" + ctChainToDeleteName = "hmc-ct-chain-to-delete" + stChainToDeleteName = "hmc-st-chain-to-delete" namespace1Name = "namespace1" namespace2Name = "namespace2" namespace3Name = "namespace3" - ct1Name = "tmpl" - ct2Name = "ct2" - ct3Name = "ct3" - ctUnmanagedName = "ct-unmanaged" - - st1Name = "tmpl" - st2Name = "st2" - st3Name = "st3" - stUnmanagedName = "st-unmanaged" + ctChainUnmanagedName = "ct-chain-unmanaged" + stChainUnmanagedName = "st-chain-unmanaged" ) ctx := context.Background() @@ -76,11 +70,10 @@ var _ = Describe("Template Management Controller", func() { } namespace3 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace3Name}} - accessRules := []hmcmirantiscomv1alpha1.AccessRule{ + accessRules := []hmc.AccessRule{ { // Target namespaces: namespace1, namespace2 - // ClusterTemplates: ct1, ct2 - TargetNamespaces: hmcmirantiscomv1alpha1.TargetNamespaces{ + TargetNamespaces: hmc.TargetNamespaces{ Selector: &metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ { @@ -95,9 +88,7 @@ var _ = Describe("Template Management Controller", func() { }, { // Target namespace: namespace1 - // ClusterTemplates: ct1, ct2 - // ServiceTemplates: st1, st2 - TargetNamespaces: hmcmirantiscomv1alpha1.TargetNamespaces{ + TargetNamespaces: hmc.TargetNamespaces{ StringSelector: "environment=dev", }, ClusterTemplateChains: []string{ctChainName}, @@ -105,8 +96,7 @@ var _ = Describe("Template Management Controller", func() { }, { // Target namespace: namespace3 - // ServiceTemplates: st1, st2 - TargetNamespaces: hmcmirantiscomv1alpha1.TargetNamespaces{ + TargetNamespaces: hmc.TargetNamespaces{ List: []string{namespace3Name}, }, ServiceTemplateChains: []string{stChainName}, @@ -118,46 +108,14 @@ var _ = Describe("Template Management Controller", func() { tm.WithAccessRules(accessRules), ) - supportedClusterTemplates := []hmcmirantiscomv1alpha1.SupportedTemplate{ - {Name: ct1Name}, - {Name: ct2Name}, - } - ctChain := chain.NewClusterTemplateChain( - chain.WithName(ctChainName), - chain.WithNamespace(systemNamespace.Name), - chain.WithSupportedTemplates(supportedClusterTemplates), - ) - - supportedServiceTemplates := []hmcmirantiscomv1alpha1.SupportedTemplate{ - {Name: st1Name}, - {Name: st2Name}, - } - stChain := chain.NewServiceTemplateChain( - chain.WithName(stChainName), - chain.WithNamespace(systemNamespace.Name), - chain.WithSupportedTemplates(supportedServiceTemplates), - ) + ctChain := tc.NewClusterTemplateChain(tc.WithName(ctChainName), tc.WithNamespace(systemNamespace.Name), tc.ManagedByHMC()) + stChain := tc.NewServiceTemplateChain(tc.WithName(stChainName), tc.WithNamespace(systemNamespace.Name), tc.ManagedByHMC()) - templateHelmSpec := hmcmirantiscomv1alpha1.HelmSpec{ChartName: "test"} - ct1 := template.NewClusterTemplate(template.WithName(ct1Name), template.WithNamespace(systemNamespace.Name), template.WithHelmSpec(templateHelmSpec)) - ct2 := template.NewClusterTemplate(template.WithName(ct2Name), template.WithNamespace(systemNamespace.Name), template.WithHelmSpec(templateHelmSpec)) - ct3 := template.NewClusterTemplate( - template.WithName(ct3Name), - template.WithNamespace(namespace2Name), - template.WithHelmSpec(templateHelmSpec), - template.WithLabels(map[string]string{hmcmirantiscomv1alpha1.HMCManagedLabelKey: hmcmirantiscomv1alpha1.HMCManagedLabelValue}), - ) - ctUnmanaged := template.NewClusterTemplate(template.WithName(ctUnmanagedName), template.WithNamespace(namespace1Name), template.WithHelmSpec(templateHelmSpec)) + ctChainToDelete := tc.NewClusterTemplateChain(tc.WithName(ctChainToDeleteName), tc.WithNamespace(namespace2Name), tc.ManagedByHMC()) + stChainToDelete := tc.NewServiceTemplateChain(tc.WithName(stChainToDeleteName), tc.WithNamespace(namespace3Name), tc.ManagedByHMC()) - st1 := template.NewServiceTemplate(template.WithName(st1Name), template.WithNamespace(systemNamespace.Name), template.WithHelmSpec(templateHelmSpec)) - st2 := template.NewServiceTemplate(template.WithName(st2Name), template.WithNamespace(systemNamespace.Name), template.WithHelmSpec(templateHelmSpec)) - st3 := template.NewServiceTemplate( - template.WithName(st3Name), - template.WithNamespace(namespace2Name), - template.WithHelmSpec(templateHelmSpec), - template.WithLabels(map[string]string{hmcmirantiscomv1alpha1.HMCManagedLabelKey: hmcmirantiscomv1alpha1.HMCManagedLabelValue}), - ) - stUnmanaged := template.NewServiceTemplate(template.WithName(stUnmanagedName), template.WithNamespace(namespace2Name), template.WithHelmSpec(templateHelmSpec)) + ctChainUnmanaged := tc.NewClusterTemplateChain(tc.WithName(ctChainUnmanagedName), tc.WithNamespace(namespace1Name)) + stChainUnmanaged := tc.NewServiceTemplateChain(tc.WithName(stChainUnmanagedName), tc.WithNamespace(namespace2Name)) BeforeEach(func() { By("creating test namespaces") @@ -173,67 +131,48 @@ var _ = Describe("Template Management Controller", func() { if err != nil && errors.IsNotFound(err) { Expect(k8sClient.Create(ctx, tm)).To(Succeed()) } - By("creating the custom resource for the Kind ClusterTemplateChain") - err = k8sClient.Get(ctx, types.NamespacedName{Name: ctChainName, Namespace: systemNamespace.Name}, ctChain) - if err != nil && errors.IsNotFound(err) { - Expect(k8sClient.Create(ctx, ctChain)).To(Succeed()) - } - By("creating the custom resource for the Kind ServiceTemplateChain") - err = k8sClient.Get(ctx, types.NamespacedName{Name: stChainName, Namespace: systemNamespace.Name}, stChain) - if err != nil && errors.IsNotFound(err) { - Expect(k8sClient.Create(ctx, stChain)).To(Succeed()) - } - By("creating ClusterTemplates and ServiceTemplates") - for _, template := range []crclient.Object{ct1, ct2, ct3, ctUnmanaged, st1, st2, st3, stUnmanaged} { - err = k8sClient.Get(ctx, types.NamespacedName{Name: template.GetName(), Namespace: template.GetNamespace()}, template) + + By("creating custom resources for the Kind ClusterTemplateChain and ServiceTemplateChain") + for _, chain := range []crclient.Object{ + ctChain, ctChainToDelete, ctChainUnmanaged, + stChain, stChainToDelete, stChainUnmanaged, + } { + err = k8sClient.Get(ctx, types.NamespacedName{Name: chain.GetName(), Namespace: chain.GetNamespace()}, chain) if err != nil && errors.IsNotFound(err) { - Expect(k8sClient.Create(ctx, template)).To(Succeed()) + Expect(k8sClient.Create(ctx, chain)).To(Succeed()) } } }) AfterEach(func() { - for _, ns := range []*corev1.Namespace{namespace1, namespace2, namespace3} { - err := k8sClient.Get(ctx, types.NamespacedName{Name: ns.Name}, ns) - Expect(err).NotTo(HaveOccurred()) - By("Cleanup the namespace") - Expect(k8sClient.Delete(ctx, ns)).To(Succeed()) - } - - ctChain := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} - err := k8sClient.Get(ctx, types.NamespacedName{Name: ctChainName, Namespace: systemNamespace.Name}, ctChain) - Expect(err).NotTo(HaveOccurred()) - By("Cleanup the specific resource instance ClusterTemplateChain") - Expect(k8sClient.Delete(ctx, ctChain)).To(Succeed()) - - stChain := &hmcmirantiscomv1alpha1.ServiceTemplateChain{} - err = k8sClient.Get(ctx, types.NamespacedName{Name: stChainName, Namespace: systemNamespace.Name}, stChain) - Expect(err).NotTo(HaveOccurred()) - By("Cleanup the specific resource instance ServiceTemplateChain") - Expect(k8sClient.Delete(ctx, stChain)).To(Succeed()) - - for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ct1, ct2, ct3, ctUnmanaged} { + for _, chain := range []*hmc.ClusterTemplateChain{ctChain, ctChainToDelete, ctChainUnmanaged} { for _, ns := range []*corev1.Namespace{systemNamespace, namespace1, namespace2, namespace3} { - template.Namespace = ns.Name - err := k8sClient.Delete(ctx, template) + chain.Namespace = ns.Name + err := k8sClient.Delete(ctx, chain) Expect(crclient.IgnoreNotFound(err)).To(Succeed()) } } - for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{st1, st2, st3, stUnmanaged} { + for _, chain := range []*hmc.ServiceTemplateChain{stChain, stChainToDelete, stChainUnmanaged} { for _, ns := range []*corev1.Namespace{systemNamespace, namespace1, namespace2, namespace3} { - template.Namespace = ns.Name - err := k8sClient.Delete(ctx, template) + chain.Namespace = ns.Name + err := k8sClient.Delete(ctx, chain) Expect(crclient.IgnoreNotFound(err)).To(Succeed()) } } + for _, ns := range []*corev1.Namespace{namespace1, namespace2, namespace3} { + err := k8sClient.Get(ctx, types.NamespacedName{Name: ns.Name}, ns) + Expect(err).NotTo(HaveOccurred()) + By("Cleanup the namespace") + Expect(k8sClient.Delete(ctx, ns)).To(Succeed()) + } }) It("should successfully reconcile the resource", func() { - By("Get unmanaged templates before the reconciliation to verify it wasn't changed") - ctUnmanagedBefore := &hmcmirantiscomv1alpha1.ClusterTemplate{} - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace1Name, Name: ctUnmanaged.Name}, ctUnmanagedBefore) + By("Get unmanaged template chains before the reconciliation to verify it wasn't changed") + ctChainUnmanagedBefore := &hmc.ClusterTemplateChain{} + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: ctChainUnmanaged.Namespace, Name: ctChainUnmanaged.Name}, ctChainUnmanagedBefore) Expect(err).NotTo(HaveOccurred()) - stUnmanagedBefore := &hmcmirantiscomv1alpha1.ServiceTemplate{} - err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace2Name, Name: stUnmanaged.Name}, stUnmanagedBefore) + stChainUnmanagedBefore := &hmc.ServiceTemplateChain{} + err = k8sClient.Get(ctx, types.NamespacedName{Namespace: stChainUnmanaged.Namespace, Name: stChainUnmanaged.Name}, stChainUnmanagedBefore) Expect(err).NotTo(HaveOccurred()) By("Reconciling the created resource") @@ -247,35 +186,25 @@ var _ = Describe("Template Management Controller", func() { Expect(err).NotTo(HaveOccurred()) /* Expected state: - * namespace1/ct1 - should be created - * namespace1/ct2 - should be created - * namespace1/ctUnmanaged - should be unchanged (unmanaged by HMC) - * namespace1/st1 - should be created - * namespace1/st2 - should be created - - * namespace2/ct1 - should be created - * namespace2/ct2 - should be created - * namespace2/ct3 - should be deleted - * namespace2/st3 - should be deleted - * namespace2/stUnmanaged - should be unchanged (unmanaged by HMC) - - * namespace3/st1 - should be created - * namespace3/st2 - should be created + * namespace1/hmc-ct-chain - should be created + * namespace1/hmc-st-chain - should be created + * namespace2/hmc-ct-chain - should be created + * namespace3/hmc-st-chain - should be created + * namespace1/ct-chain-unmanaged - should be unchanged (unmanaged by HMC) + * namespace2/st-chain-unmanaged - should be unchanged (unmanaged by HMC) + * namespace2/hmc-ct-chain-to-delete - should be deleted + * namespace3/hmc-st-chain-to-delete - should be deleted */ - verifyTemplateCreated(ctx, namespace1Name, ct1) - verifyTemplateCreated(ctx, namespace1Name, ct2) - verifyTemplateUnchanged(ctx, namespace1Name, ctUnmanagedBefore, ctUnmanaged) - verifyTemplateCreated(ctx, namespace1Name, st1) - verifyTemplateCreated(ctx, namespace1Name, st2) + verifyObjectCreated(ctx, namespace1Name, ctChain) + verifyObjectCreated(ctx, namespace1Name, stChain) + verifyObjectCreated(ctx, namespace2Name, ctChain) + verifyObjectCreated(ctx, namespace3Name, stChain) - verifyTemplateCreated(ctx, namespace2Name, ct1) - verifyTemplateCreated(ctx, namespace2Name, ct2) - verifyTemplateDeleted(ctx, namespace2Name, ct3) - verifyTemplateDeleted(ctx, namespace2Name, st3) - verifyTemplateUnchanged(ctx, namespace2Name, stUnmanagedBefore, stUnmanaged) + verifyObjectUnchanged(ctx, namespace1Name, ctChainUnmanaged, ctChainUnmanagedBefore) + verifyObjectUnchanged(ctx, namespace2Name, stChainUnmanaged, stChainUnmanagedBefore) - verifyTemplateCreated(ctx, namespace3Name, st1) - verifyTemplateCreated(ctx, namespace3Name, st2) + verifyObjectDeleted(ctx, namespace2Name, ctChainToDelete) + verifyObjectDeleted(ctx, namespace3Name, stChainToDelete) }) }) }) diff --git a/internal/templateutil/state.go b/internal/templateutil/state.go deleted file mode 100644 index 48ec7cf59..000000000 --- a/internal/templateutil/state.go +++ /dev/null @@ -1,196 +0,0 @@ -// Copyright 2024 -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package templateutil - -import ( - "context" - "errors" - "fmt" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/client" - - hmc "github.com/Mirantis/hmc/api/v1alpha1" -) - -const ( - ClusterTemplateKind = "ClusterTemplate" - ServiceTemplateKind = "ServiceTemplate" -) - -type State struct { - // ClusterTemplatesState is a map where keys are ClusterTemplate names and values is the map of namespaces - // where this ClusterTemplate should be distributed - ClusterTemplatesState map[string]map[string]bool - // ServiceTemplatesState is a map where keys are ServiceTemplate names and values is the map of namespaces - // where this ServiceTemplate should be distributed - ServiceTemplatesState map[string]map[string]bool -} - -func GetCurrentTemplatesState(ctx context.Context, cl client.Client, systemNamespace string) (State, error) { - listOpts := &client.ListOptions{ - LabelSelector: labels.SelectorFromSet(map[string]string{hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue}), - Namespace: "", - } - clusterTemplatesList, serviceTemplatesList := &metav1.PartialObjectMetadataList{}, &metav1.PartialObjectMetadataList{} - - for _, kind := range []string{ClusterTemplateKind, ServiceTemplateKind} { - partialList := &metav1.PartialObjectMetadataList{} - partialList.SetGroupVersionKind(schema.GroupVersionKind{ - Group: hmc.GroupVersion.Group, - Version: hmc.GroupVersion.Version, - Kind: kind, - }) - err := cl.List(ctx, partialList, listOpts) - if err != nil { - return State{}, err - } - if kind == ClusterTemplateKind { - clusterTemplatesList = partialList - } - if kind == ServiceTemplateKind { - serviceTemplatesList = partialList - } - } - - clusterTemplates, serviceTemplates := make(map[string]map[string]bool), make(map[string]map[string]bool) - for _, ct := range clusterTemplatesList.Items { - if ct.Namespace == systemNamespace { - continue - } - if clusterTemplates[ct.Name] == nil { - clusterTemplates[ct.Name] = make(map[string]bool) - } - clusterTemplates[ct.Name][ct.Namespace] = false - } - for _, st := range serviceTemplatesList.Items { - if st.Namespace == systemNamespace { - continue - } - if serviceTemplates[st.Name] == nil { - serviceTemplates[st.Name] = make(map[string]bool) - } - serviceTemplates[st.Name][st.Namespace] = false - } - return State{ - ClusterTemplatesState: clusterTemplates, - ServiceTemplatesState: serviceTemplates, - }, nil -} - -func ParseAccessRules(ctx context.Context, cl client.Client, systemNamespace string, rules []hmc.AccessRule, currentState State) (State, error) { - var errs error - - expectedState := currentState - if expectedState.ClusterTemplatesState == nil { - expectedState.ClusterTemplatesState = make(map[string]map[string]bool) - } - if expectedState.ServiceTemplatesState == nil { - expectedState.ServiceTemplatesState = make(map[string]map[string]bool) - } - for _, rule := range rules { - var clusterTemplates []string - var serviceTemplates []string - for _, ctChainName := range rule.ClusterTemplateChains { - ctChain := &hmc.ClusterTemplateChain{} - err := cl.Get(ctx, client.ObjectKey{ - Name: ctChainName, - Namespace: systemNamespace, - }, ctChain) - if err != nil { - errs = errors.Join(errs, err) - continue - } - for _, supportedTemplate := range ctChain.Spec.SupportedTemplates { - clusterTemplates = append(clusterTemplates, supportedTemplate.Name) - } - } - for _, stChainName := range rule.ServiceTemplateChains { - stChain := &hmc.ServiceTemplateChain{} - err := cl.Get(ctx, client.ObjectKey{ - Name: stChainName, - Namespace: systemNamespace, - }, stChain) - if err != nil { - errs = errors.Join(errs, err) - continue - } - for _, supportedTemplate := range stChain.Spec.SupportedTemplates { - serviceTemplates = append(serviceTemplates, supportedTemplate.Name) - } - } - namespaces, err := getTargetNamespaces(ctx, cl, rule.TargetNamespaces) - if err != nil { - return State{}, err - } - for _, ct := range clusterTemplates { - if expectedState.ClusterTemplatesState[ct] == nil { - expectedState.ClusterTemplatesState[ct] = make(map[string]bool) - } - for _, ns := range namespaces { - expectedState.ClusterTemplatesState[ct][ns] = true - } - } - for _, st := range serviceTemplates { - if expectedState.ServiceTemplatesState[st] == nil { - expectedState.ServiceTemplatesState[st] = make(map[string]bool) - } - for _, ns := range namespaces { - expectedState.ServiceTemplatesState[st][ns] = true - } - } - } - if errs != nil { - return State{}, errs - } - return expectedState, nil -} - -func getTargetNamespaces(ctx context.Context, cl client.Client, targetNamespaces hmc.TargetNamespaces) ([]string, error) { - if len(targetNamespaces.List) > 0 { - return targetNamespaces.List, nil - } - var selector labels.Selector - var err error - if targetNamespaces.StringSelector != "" { - selector, err = labels.Parse(targetNamespaces.StringSelector) - if err != nil { - return nil, err - } - } else { - selector, err = metav1.LabelSelectorAsSelector(targetNamespaces.Selector) - if err != nil { - return nil, fmt.Errorf("failed to construct selector from the namespaces selector %s: %w", targetNamespaces.Selector, err) - } - } - - namespaces := &corev1.NamespaceList{} - listOpts := &client.ListOptions{} - if selector.String() != "" { - listOpts = &client.ListOptions{LabelSelector: selector} - } - err = cl.List(ctx, namespaces, listOpts) - if err != nil { - return []string{}, err - } - result := make([]string, len(namespaces.Items)) - for i, ns := range namespaces.Items { - result[i] = ns.Name - } - return result, nil -} diff --git a/internal/webhook/templatemanagement_webhook.go b/internal/webhook/templatemanagement_webhook.go index 01536aa6c..9d7470adb 100644 --- a/internal/webhook/templatemanagement_webhook.go +++ b/internal/webhook/templatemanagement_webhook.go @@ -18,10 +18,7 @@ import ( "context" "errors" "fmt" - "sort" - "strings" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -30,7 +27,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/Mirantis/hmc/api/v1alpha1" - "github.com/Mirantis/hmc/internal/templateutil" ) var errTemplateManagementDeletionForbidden = errors.New("TemplateManagement deletion is forbidden") @@ -69,62 +65,10 @@ func (v *TemplateManagementValidator) ValidateCreate(ctx context.Context, _ runt } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (v *TemplateManagementValidator) ValidateUpdate(ctx context.Context, _ runtime.Object, newObj runtime.Object) (admission.Warnings, error) { - newTm, ok := newObj.(*v1alpha1.TemplateManagement) - if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expected TemplateManagement but got a %T", newObj)) - } - currentState, err := templateutil.GetCurrentTemplatesState(ctx, v.Client, v.SystemNamespace) - if err != nil { - return nil, fmt.Errorf("could not get current templates state: %v", err) - } - - expectedState, err := templateutil.ParseAccessRules(ctx, v.Client, v.SystemNamespace, newTm.Spec.AccessRules, currentState) - if err != nil { - return nil, fmt.Errorf("failed to parse access rules for TemplateManagement: %v", err) - } - - warnings := admission.Warnings{} - for templateName, namespaces := range expectedState.ClusterTemplatesState { - for namespace, keep := range namespaces { - if !keep { - managedClusters, err := getManagedClustersForTemplate(ctx, v.Client, namespace, templateName) - if err != nil { - return nil, err - } - - if len(managedClusters) > 0 { - errMsg := fmt.Sprintf("ClusterTemplate \"%s/%s\" can't be removed: found ManagedClusters that reference it: ", namespace, templateName) - sort.Slice(managedClusters, func(i, j int) bool { - return managedClusters[i].Name < managedClusters[j].Name - }) - for _, cluster := range managedClusters { - errMsg += fmt.Sprintf("\"%s/%s\", ", cluster.Namespace, cluster.Name) - } - warnings = append(warnings, strings.TrimRight(errMsg, ", ")) - } - } - } - } - if len(warnings) > 0 { - sort.Strings(warnings) - return warnings, fmt.Errorf("can not apply new access rules") - } +func (*TemplateManagementValidator) ValidateUpdate(_ context.Context, _ runtime.Object, _ runtime.Object) (admission.Warnings, error) { return nil, nil } -func getManagedClustersForTemplate(ctx context.Context, cl client.Client, namespace, templateName string) ([]v1alpha1.ManagedCluster, error) { - managedClusters := &v1alpha1.ManagedClusterList{} - err := cl.List(ctx, managedClusters, - client.InNamespace(namespace), - client.MatchingFields{v1alpha1.TemplateKey: templateName}, - ) - if err != nil { - return nil, err - } - return managedClusters.Items, nil -} - // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. func (v *TemplateManagementValidator) ValidateDelete(ctx context.Context, _ runtime.Object) (admission.Warnings, error) { partialList := &metav1.PartialObjectMetadataList{} diff --git a/internal/webhook/templatemanagement_webhook_test.go b/internal/webhook/templatemanagement_webhook_test.go index 4868f62b6..45d4a4718 100644 --- a/internal/webhook/templatemanagement_webhook_test.go +++ b/internal/webhook/templatemanagement_webhook_test.go @@ -19,7 +19,6 @@ import ( "testing" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -27,10 +26,7 @@ import ( "github.com/Mirantis/hmc/api/v1alpha1" "github.com/Mirantis/hmc/internal/utils" - "github.com/Mirantis/hmc/test/objects/managedcluster" "github.com/Mirantis/hmc/test/objects/management" - "github.com/Mirantis/hmc/test/objects/template" - chain "github.com/Mirantis/hmc/test/objects/templatechain" tm "github.com/Mirantis/hmc/test/objects/templatemanagement" "github.com/Mirantis/hmc/test/scheme" ) @@ -85,184 +81,6 @@ func TestTemplateManagementValidateCreate(t *testing.T) { } } -func TestTemplateManagementValidateUpdate(t *testing.T) { - g := NewWithT(t) - - ctx := context.Background() - - namespaceDevName := "dev" - namespaceProdName := "prod" - - namespaceDev := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespaceDevName, - Labels: map[string]string{ - "environment": "dev", - }, - }, - } - namespaceProd := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespaceProdName, - Labels: map[string]string{ - "environment": "prod", - }, - }, - } - - awsCtChainName := "aws-chain" - azureCtChainName := "azure-chain" - awsStandaloneCpTemplateName := "aws-standalone-cp" - awsHostedCpTemplateName := "aws-hosted-cp" - azureStandaloneCpTemplateName := "azure-standalone-cp" - azureHostedCpTemplateName := "azure-hosted-cp" - - awsAccessRule := v1alpha1.AccessRule{ - TargetNamespaces: v1alpha1.TargetNamespaces{ - StringSelector: "environment=dev", - }, - ClusterTemplateChains: []string{awsCtChainName}, - } - - azureProdAccessRule := v1alpha1.AccessRule{ - TargetNamespaces: v1alpha1.TargetNamespaces{ - Selector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "environment", - Operator: metav1.LabelSelectorOpIn, - Values: []string{"prod"}, - }, - }, - }, - }, - ClusterTemplateChains: []string{azureCtChainName}, - } - - awsCtChain := chain.NewClusterTemplateChain( - chain.WithName(awsCtChainName), - chain.WithNamespace(utils.DefaultSystemNamespace), - chain.WithSupportedTemplates( - []v1alpha1.SupportedTemplate{ - {Name: awsStandaloneCpTemplateName}, - {Name: awsHostedCpTemplateName}, - }, - )) - azureCtChain := chain.NewClusterTemplateChain( - chain.WithName(azureCtChainName), - chain.WithNamespace(utils.DefaultSystemNamespace), - chain.WithSupportedTemplates( - []v1alpha1.SupportedTemplate{ - {Name: azureStandaloneCpTemplateName}, - {Name: azureHostedCpTemplateName}, - }, - )) - - hmcClusterTemplate := template.WithLabels(map[string]string{v1alpha1.HMCManagedLabelKey: v1alpha1.HMCManagedLabelValue}) - - tests := []struct { - name string - newTm *v1alpha1.TemplateManagement - existingObjects []runtime.Object - err string - warnings admission.Warnings - }{ - { - name: `should fail if new access rules require to remove the in-used ClusterTemplate (only aws in dev namespace is allowed)`, - newTm: tm.NewTemplateManagement(tm.WithAccessRules([]v1alpha1.AccessRule{awsAccessRule})), - existingObjects: []runtime.Object{ - awsCtChain, - azureCtChain, - template.NewClusterTemplate(hmcClusterTemplate, template.WithNamespace(namespaceDevName), template.WithName(awsStandaloneCpTemplateName)), - template.NewClusterTemplate(hmcClusterTemplate, template.WithNamespace(namespaceProdName), template.WithName(awsHostedCpTemplateName)), - template.NewClusterTemplate(hmcClusterTemplate, template.WithNamespace(namespaceDevName), template.WithName(azureStandaloneCpTemplateName)), - template.NewClusterTemplate(hmcClusterTemplate, template.WithNamespace(namespaceProdName), template.WithName(azureHostedCpTemplateName)), - template.NewClusterTemplate(template.WithName("unmanaged")), - managedcluster.NewManagedCluster( - managedcluster.WithName("aws-standalone"), - managedcluster.WithNamespace(namespaceDevName), - managedcluster.WithTemplate(awsStandaloneCpTemplateName), - ), - managedcluster.NewManagedCluster( - managedcluster.WithName("aws-hosted"), - managedcluster.WithNamespace(namespaceProdName), - managedcluster.WithTemplate(awsHostedCpTemplateName), - ), - managedcluster.NewManagedCluster( - managedcluster.WithName("azure-standalone"), - managedcluster.WithNamespace(namespaceDevName), - managedcluster.WithTemplate(azureStandaloneCpTemplateName), - ), - managedcluster.NewManagedCluster( - managedcluster.WithName("azure-hosted-1"), - managedcluster.WithNamespace(namespaceProdName), - managedcluster.WithTemplate(azureHostedCpTemplateName), - ), - managedcluster.NewManagedCluster( - managedcluster.WithName("azure-hosted-2"), - managedcluster.WithNamespace(namespaceProdName), - managedcluster.WithTemplate(azureHostedCpTemplateName), - ), - }, - warnings: admission.Warnings{ - "ClusterTemplate \"dev/azure-standalone-cp\" can't be removed: found ManagedClusters that reference it: \"dev/azure-standalone\"", - "ClusterTemplate \"prod/aws-hosted-cp\" can't be removed: found ManagedClusters that reference it: \"prod/aws-hosted\"", - "ClusterTemplate \"prod/azure-hosted-cp\" can't be removed: found ManagedClusters that reference it: \"prod/azure-hosted-1\", \"prod/azure-hosted-2\"", - }, - err: "can not apply new access rules", - }, - { - name: "should succeed if new access rules don't affect in-used ClusterTemplates (only azure hosted in prod namespace is allowed)", - newTm: tm.NewTemplateManagement(tm.WithAccessRules([]v1alpha1.AccessRule{azureProdAccessRule})), - existingObjects: []runtime.Object{ - awsCtChain, - azureCtChain, - template.NewClusterTemplate(hmcClusterTemplate, template.WithNamespace(namespaceDevName), template.WithName(awsStandaloneCpTemplateName)), - template.NewClusterTemplate(hmcClusterTemplate, template.WithNamespace(namespaceProdName), template.WithName(awsHostedCpTemplateName)), - template.NewClusterTemplate(hmcClusterTemplate, template.WithNamespace(namespaceDevName), template.WithName(azureStandaloneCpTemplateName)), - template.NewClusterTemplate(hmcClusterTemplate, template.WithNamespace(namespaceProdName), template.WithName(azureHostedCpTemplateName)), - template.NewClusterTemplate(template.WithName("unmanaged")), - managedcluster.NewManagedCluster( - managedcluster.WithName("azure-hosted-1"), - managedcluster.WithNamespace(namespaceProdName), - managedcluster.WithTemplate(azureHostedCpTemplateName), - ), - managedcluster.NewManagedCluster( - managedcluster.WithName("azure-hosted-2"), - managedcluster.WithNamespace(namespaceProdName), - managedcluster.WithTemplate(azureHostedCpTemplateName), - ), - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - obj := []runtime.Object{namespaceDev, namespaceProd} - c := fake.NewClientBuilder(). - WithScheme(scheme.Scheme). - WithRuntimeObjects(append(obj, tt.existingObjects...)...). - WithIndex(&v1alpha1.ManagedCluster{}, v1alpha1.TemplateKey, v1alpha1.ExtractTemplateName). - Build() - validator := &TemplateManagementValidator{Client: c, SystemNamespace: utils.DefaultSystemNamespace} - warn, err := validator.ValidateUpdate(ctx, tm.NewTemplateManagement(), tt.newTm) - if tt.err != "" { - g.Expect(err).To(HaveOccurred()) - if err.Error() != tt.err { - t.Fatalf("expected error '%s', got error: %s", tt.err, err.Error()) - } - } else { - g.Expect(err).To(Succeed()) - } - if len(tt.warnings) > 0 { - g.Expect(warn).To(Equal(tt.warnings)) - } else { - g.Expect(warn).To(BeEmpty()) - } - }) - } -} - func TestTemplateManagementValidateDelete(t *testing.T) { g := NewWithT(t) diff --git a/test/objects/templatechain/templatechain.go b/test/objects/templatechain/templatechain.go index 105993591..ac3296008 100644 --- a/test/objects/templatechain/templatechain.go +++ b/test/objects/templatechain/templatechain.go @@ -71,6 +71,15 @@ func WithNamespace(namespace string) Opt { } } +func ManagedByHMC() Opt { + return func(t *TemplateChain) { + if t.Labels == nil { + t.Labels = make(map[string]string) + } + t.Labels[v1alpha1.HMCManagedLabelKey] = v1alpha1.HMCManagedLabelValue + } +} + func WithSupportedTemplates(supportedTemplates []v1alpha1.SupportedTemplate) Opt { return func(tc *TemplateChain) { tc.Spec.SupportedTemplates = supportedTemplates From b3bb2babd58ca2f890dbb7d095e1eacbacb3742e Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Tue, 1 Oct 2024 00:42:56 +0400 Subject: [PATCH 4/4] Set managed-by-chain label on managed templates --- .../controller/templatechain_controller.go | 16 +- .../templatechain_controller_test.go | 197 +++++++++++------- 2 files changed, 127 insertions(+), 86 deletions(-) diff --git a/internal/controller/templatechain_controller.go b/internal/controller/templatechain_controller.go index 6e7d15041..ca2406e7b 100644 --- a/internal/controller/templatechain_controller.go +++ b/internal/controller/templatechain_controller.go @@ -30,6 +30,8 @@ import ( hmc "github.com/Mirantis/hmc/api/v1alpha1" ) +const HMCManagedByChainLabelKey = "hmc.mirantis.com/managed-by-chain" + // TemplateChainReconciler reconciles a TemplateChain object type TemplateChainReconciler struct { client.Client @@ -89,7 +91,7 @@ func (r *ServiceTemplateChainReconciler) Reconcile(ctx context.Context, req ctrl func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, templateChain TemplateChain) (ctrl.Result, error) { l := log.FromContext(ctx) - systemTemplates, existingTemplates, err := getCurrentTemplates(ctx, r.Client, templateChain.TemplateKind(), r.SystemNamespace, templateChain.GetNamespace()) + systemTemplates, managedTemplates, err := getCurrentTemplates(ctx, r.Client, templateChain.TemplateKind(), r.SystemNamespace, templateChain.GetNamespace(), templateChain.GetName()) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to get current templates: %v", err) } @@ -102,7 +104,8 @@ func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, te Name: supportedTemplate.Name, Namespace: templateChain.GetNamespace(), Labels: map[string]string{ - hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue, + hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue, + HMCManagedByChainLabelKey: templateChain.GetName(), }, } keepTemplate[supportedTemplate.Name] = true @@ -145,7 +148,7 @@ func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, te errs = errors.Join(errs, err) } } - for _, template := range existingTemplates { + for _, template := range managedTemplates { if !keepTemplate[template.GetName()] { l.Info(fmt.Sprintf("Deleting %s", templateChain.TemplateKind()), "namespace", templateChain.GetNamespace(), "name", template.GetName()) err := r.Delete(ctx, template) @@ -161,7 +164,7 @@ func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, te return ctrl.Result{}, nil } -func getCurrentTemplates(ctx context.Context, cl client.Client, templateKind, systemNamespace, targetNamespace string) (map[string]Template, []Template, error) { +func getCurrentTemplates(ctx context.Context, cl client.Client, templateKind, systemNamespace, targetNamespace, templateChainName string) (map[string]Template, []Template, error) { var templates []Template switch templateKind { @@ -194,7 +197,10 @@ func getCurrentTemplates(ctx context.Context, cl client.Client, templateKind, sy systemTemplates[template.GetName()] = template continue } - if template.GetNamespace() == targetNamespace && template.GetLabels()[hmc.HMCManagedLabelKey] == "true" { + labels := template.GetLabels() + if template.GetNamespace() == targetNamespace && + labels[hmc.HMCManagedLabelKey] == "true" && + labels[HMCManagedByChainLabelKey] == templateChainName { managedTemplates = append(managedTemplates, template) } } diff --git a/internal/controller/templatechain_controller_test.go b/internal/controller/templatechain_controller_test.go index 6dccf012d..7b3048e9f 100644 --- a/internal/controller/templatechain_controller_test.go +++ b/internal/controller/templatechain_controller_test.go @@ -35,8 +35,10 @@ import ( var _ = Describe("Template Chain Controller", func() { Context("When reconciling a resource", func() { const ( - ctChainName = "ct-chain" - stChainName = "st-chain" + ctChain1Name = "ct-chain-1" + ctChain2Name = "ct-chain-2" + stChain1Name = "st-chain-1" + stChain2Name = "st-chain-2" ) ctx := context.Background() @@ -67,6 +69,7 @@ var _ = Describe("Template Chain Controller", func() { template.WithName("ct1"), template.WithNamespace(namespace.Name), template.WithHelmSpec(templateHelmSpec), + template.WithLabels(map[string]string{HMCManagedByChainLabelKey: ctChain1Name}), template.ManagedByHMC(), ), // Should be unchanged (unmanaged) @@ -94,6 +97,7 @@ var _ = Describe("Template Chain Controller", func() { template.WithName("st1"), template.WithNamespace(namespace.Name), template.WithHelmSpec(templateHelmSpec), + template.WithLabels(map[string]string{HMCManagedByChainLabelKey: stChain1Name}), template.ManagedByHMC(), ), // Should be unchanged (unmanaged) @@ -104,40 +108,44 @@ var _ = Describe("Template Chain Controller", func() { ), } - ctChainNamespacedName := types.NamespacedName{ - Name: ctChainName, - Namespace: namespace.Name, + ctChainNames := []types.NamespacedName{ + getNamespacedChainName(namespace.Name, ctChain1Name), + getNamespacedChainName(namespace.Name, ctChain2Name), } - - stChainNamespacedName := types.NamespacedName{ - Name: stChainName, - Namespace: namespace.Name, + stChainNames := []types.NamespacedName{ + getNamespacedChainName(namespace.Name, stChain1Name), + getNamespacedChainName(namespace.Name, stChain2Name), } - clusterTemplateChain := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} - serviceTemplateChain := &hmcmirantiscomv1alpha1.ServiceTemplateChain{} - supportedClusterTemplates := []hmcmirantiscomv1alpha1.SupportedTemplate{ - { - Name: "test", - }, - { - Name: "ct0", - }, - // Does not exist in the system namespace - { - Name: "ct3", + + supportedClusterTemplates := map[string][]hmcmirantiscomv1alpha1.SupportedTemplate{ + ctChain1Name: { + { + Name: "test", + }, + { + Name: "ct0", + }, + // Does not exist in the system namespace + { + Name: "ct3", + }, }, + ctChain2Name: {}, } - supportedServiceTemplates := []hmcmirantiscomv1alpha1.SupportedTemplate{ - { - Name: "test", - }, - { - Name: "st0", - }, - // Does not exist in the system namespace - { - Name: "st3", + supportedServiceTemplates := map[string][]hmcmirantiscomv1alpha1.SupportedTemplate{ + stChain1Name: { + { + Name: "test", + }, + { + Name: "st0", + }, + // Does not exist in the system namespace + { + Name: "st3", + }, }, + stChain2Name: {}, } BeforeEach(func() { @@ -155,41 +163,46 @@ var _ = Describe("Template Chain Controller", func() { } } - By("creating the custom resource for the Kind ClusterTemplateChain") - err := k8sClient.Get(ctx, ctChainNamespacedName, clusterTemplateChain) - if err != nil && errors.IsNotFound(err) { - resource := &hmcmirantiscomv1alpha1.ClusterTemplateChain{ - ObjectMeta: metav1.ObjectMeta{ - Name: ctChainName, - Namespace: namespace.Name, - }, - Spec: hmcmirantiscomv1alpha1.TemplateChainSpec{SupportedTemplates: supportedClusterTemplates}, + By("creating the custom resources for the Kind ClusterTemplateChain") + for _, chain := range ctChainNames { + clusterTemplateChain := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} + err := k8sClient.Get(ctx, chain, clusterTemplateChain) + if err != nil && errors.IsNotFound(err) { + resource := &hmcmirantiscomv1alpha1.ClusterTemplateChain{ + ObjectMeta: metav1.ObjectMeta{ + Name: chain.Name, + Namespace: chain.Namespace, + }, + Spec: hmcmirantiscomv1alpha1.TemplateChainSpec{SupportedTemplates: supportedClusterTemplates[chain.Name]}, + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } - Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } - By("creating the custom resource for the Kind ServiceTemplateChain") - err = k8sClient.Get(ctx, stChainNamespacedName, serviceTemplateChain) - if err != nil && errors.IsNotFound(err) { - resource := &hmcmirantiscomv1alpha1.ServiceTemplateChain{ - ObjectMeta: metav1.ObjectMeta{ - Name: stChainName, - Namespace: namespace.Name, - }, - Spec: hmcmirantiscomv1alpha1.TemplateChainSpec{SupportedTemplates: supportedServiceTemplates}, + for _, chain := range stChainNames { + serviceTemplateChain := &hmcmirantiscomv1alpha1.ServiceTemplateChain{} + err := k8sClient.Get(ctx, chain, serviceTemplateChain) + if err != nil && errors.IsNotFound(err) { + resource := &hmcmirantiscomv1alpha1.ServiceTemplateChain{ + ObjectMeta: metav1.ObjectMeta{ + Name: chain.Name, + Namespace: chain.Namespace, + }, + Spec: hmcmirantiscomv1alpha1.TemplateChainSpec{SupportedTemplates: supportedServiceTemplates[chain.Name]}, + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } - Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } By("creating the custom resource for the Kind ClusterTemplate") for name, template := range ctTemplates { ct := &hmcmirantiscomv1alpha1.ClusterTemplate{} - err = k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: utils.DefaultSystemNamespace}, ct) + err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: utils.DefaultSystemNamespace}, ct) if err != nil && errors.IsNotFound(err) { Expect(k8sClient.Create(ctx, template)).To(Succeed()) } } for name, template := range stTemplates { st := &hmcmirantiscomv1alpha1.ServiceTemplate{} - err = k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: utils.DefaultSystemNamespace}, st) + err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: utils.DefaultSystemNamespace}, st) if err != nil && errors.IsNotFound(err) { Expect(k8sClient.Create(ctx, template)).To(Succeed()) } @@ -210,22 +223,25 @@ var _ = Describe("Template Chain Controller", func() { Expect(crclient.IgnoreNotFound(err)).To(Succeed()) } - clusterTemplateChainResource := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} - err := k8sClient.Get(ctx, ctChainNamespacedName, clusterTemplateChainResource) - Expect(err).NotTo(HaveOccurred()) - - By("Cleanup the specific resource instance ClusterTemplateChain") - Expect(k8sClient.Delete(ctx, clusterTemplateChainResource)).To(Succeed()) + for _, chain := range ctChainNames { + clusterTemplateChainResource := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} + err := k8sClient.Get(ctx, chain, clusterTemplateChainResource) + Expect(err).NotTo(HaveOccurred()) + By("Cleanup the specific resource instance ClusterTemplateChain") + Expect(k8sClient.Delete(ctx, clusterTemplateChainResource)).To(Succeed()) + } - serviceTemplateChainResource := &hmcmirantiscomv1alpha1.ServiceTemplateChain{} - err = k8sClient.Get(ctx, stChainNamespacedName, serviceTemplateChainResource) - Expect(err).NotTo(HaveOccurred()) + for _, chain := range stChainNames { + serviceTemplateChainResource := &hmcmirantiscomv1alpha1.ServiceTemplateChain{} + err := k8sClient.Get(ctx, chain, serviceTemplateChainResource) + Expect(err).NotTo(HaveOccurred()) - By("Cleanup the specific resource instance ServiceTemplateChain") - Expect(k8sClient.Delete(ctx, serviceTemplateChainResource)).To(Succeed()) + By("Cleanup the specific resource instance ServiceTemplateChain") + Expect(k8sClient.Delete(ctx, serviceTemplateChainResource)).To(Succeed()) + } By("Cleanup the namespace") - err = k8sClient.Get(ctx, types.NamespacedName{Name: namespace.Name}, namespace) + err := k8sClient.Get(ctx, types.NamespacedName{Name: namespace.Name}, namespace) Expect(err).NotTo(HaveOccurred()) Expect(crclient.IgnoreNotFound(k8sClient.Delete(ctx, namespace))).To(Succeed()) }) @@ -242,42 +258,57 @@ var _ = Describe("Template Chain Controller", func() { Client: k8sClient, SystemNamespace: utils.DefaultSystemNamespace, } - By("Reconciling the ClusterTemplateChain resource") - clusterTemplateChainReconciler := &ClusterTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} - _, err = clusterTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: ctChainNamespacedName}) - Expect(err).NotTo(HaveOccurred()) + By("Reconciling the ClusterTemplateChain resources") + for _, chain := range ctChainNames { + clusterTemplateChainReconciler := &ClusterTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} + _, err = clusterTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: chain}) + Expect(err).NotTo(HaveOccurred()) + } - By("Reconciling the ServiceTemplateChain resource") - serviceTemplateChainReconciler := &ServiceTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} - _, err = serviceTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: stChainNamespacedName}) - Expect(err).NotTo(HaveOccurred()) + By("Reconciling the ServiceTemplateChain resources") + for _, chain := range stChainNames { + serviceTemplateChainReconciler := &ServiceTemplateChainReconciler{TemplateChainReconciler: templateChainReconciler} + _, err = serviceTemplateChainReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: chain}) + Expect(err).NotTo(HaveOccurred()) + } /* Expected state: - * test/test - should be created - * test/ct0 - should be created - * test/ct1 - should be deleted - * test/ct2 - should be unchanged (unmanaged by HMC) + * 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/test - should be created - * test/st0 - should be created - * test/st1 - should be deleted - * test/st2 - 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) */ verifyObjectCreated(ctx, namespace.Name, ctTemplates["test"]) + checkHMCManagedByChainLabelExistence(ctTemplates["test"].GetLabels(), ctChain1Name) verifyObjectCreated(ctx, namespace.Name, ctTemplates["ct0"]) + checkHMCManagedByChainLabelExistence(ctTemplates["test"].GetLabels(), ctChain1Name) verifyObjectDeleted(ctx, namespace.Name, ctTemplates["ct1"]) verifyObjectUnchanged(ctx, namespace.Name, ctUnmanagedBefore, ctTemplates["ct2"]) verifyObjectCreated(ctx, namespace.Name, stTemplates["test"]) + checkHMCManagedByChainLabelExistence(stTemplates["test"].GetLabels(), stChain1Name) verifyObjectCreated(ctx, namespace.Name, stTemplates["st0"]) + checkHMCManagedByChainLabelExistence(stTemplates["st0"].GetLabels(), stChain1Name) verifyObjectDeleted(ctx, namespace.Name, stTemplates["st1"]) verifyObjectUnchanged(ctx, namespace.Name, stUnmanagedBefore, stTemplates["st2"]) }) }) }) +func getNamespacedChainName(namespace, name string) types.NamespacedName { + return types.NamespacedName{ + Name: name, + Namespace: namespace, + } +} + func verifyObjectCreated(ctx context.Context, namespace string, obj crclient.Object) { By(fmt.Sprintf("Verifying existence of %s/%s", namespace, obj.GetName())) err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: obj.GetName()}, obj) @@ -302,3 +333,7 @@ func verifyObjectUnchanged(ctx context.Context, namespace string, oldObj, newObj func checkHMCManagedLabelExistence(labels map[string]string) { Expect(labels).To(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) } + +func checkHMCManagedByChainLabelExistence(labels map[string]string, chainName string) { + Expect(labels).To(HaveKeyWithValue(HMCManagedByChainLabelKey, chainName)) +}