From 827d92618decf31e1f8071d9c2b4950ddbf7d8dd Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Wed, 11 Dec 2024 11:54:10 +0400 Subject: [PATCH] Fix missing spec fields in distributed templates Closes #778 --- .../controller/templatechain_controller.go | 55 +++++++--------- .../templatechain_controller_test.go | 65 +++++++++++++++---- 2 files changed, 76 insertions(+), 44 deletions(-) diff --git a/internal/controller/templatechain_controller.go b/internal/controller/templatechain_controller.go index 1eae1d58c..5d50207da 100644 --- a/internal/controller/templatechain_controller.go +++ b/internal/controller/templatechain_controller.go @@ -94,7 +94,7 @@ func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, te return ctrl.Result{}, nil } - systemTemplates, err := r.getSystemTemplates(ctx) + systemTemplates, err := r.getTemplates(ctx, &client.ListOptions{Namespace: r.SystemNamespace}) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to get system templates: %w", err) } @@ -119,22 +119,26 @@ func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, te continue } - helmSpec := hmc.HelmSpec{ - ChartRef: source.GetCommonStatus().ChartRef, - } - var target client.Object switch r.templateKind { case hmc.ClusterTemplateKind: - target = &hmc.ClusterTemplate{ObjectMeta: meta, Spec: hmc.ClusterTemplateSpec{ - Helm: helmSpec, - }} + clusterTemplate, ok := source.(*hmc.ClusterTemplate) + if !ok { + return ctrl.Result{}, fmt.Errorf("type assertion failed: expected ClusterTemplate but got %T", source) + } + spec := clusterTemplate.Spec + spec.Helm = hmc.HelmSpec{ChartRef: clusterTemplate.Status.ChartRef} + target = &hmc.ClusterTemplate{ObjectMeta: meta, Spec: spec} case hmc.ServiceTemplateKind: - target = &hmc.ServiceTemplate{ObjectMeta: meta, Spec: hmc.ServiceTemplateSpec{ - Helm: helmSpec, - }} + serviceTemplate, ok := source.(*hmc.ServiceTemplate) + if !ok { + return ctrl.Result{}, fmt.Errorf("type assertion failed: expected ServiceTemplate but got %T", source) + } + spec := serviceTemplate.Spec + spec.Helm = hmc.HelmSpec{ChartRef: serviceTemplate.Status.ChartRef} + target = &hmc.ServiceTemplate{ObjectMeta: meta, Spec: spec} default: - return ctrl.Result{}, fmt.Errorf("invalid TemplateChain kind. Supported kinds are %s and %s", hmc.ClusterTemplateChainKind, hmc.ServiceTemplateChainKind) + return ctrl.Result{}, fmt.Errorf("invalid Template kind. Supported kinds are %s and %s", hmc.ClusterTemplateKind, hmc.ServiceTemplateKind) } operation, err := ctrl.CreateOrUpdate(ctx, r.Client, target, func() error { @@ -157,40 +161,27 @@ func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, te return ctrl.Result{}, errs } -func (r *TemplateChainReconciler) getSystemTemplates(ctx context.Context) (systemTemplates map[string]templateCommon, _ error) { - templates, err := r.getTemplates(ctx, r.templateKind, &client.ListOptions{Namespace: r.SystemNamespace}) - if err != nil { - return nil, err - } - - systemTemplates = make(map[string]templateCommon, len(templates)) - for _, tmpl := range templates { - systemTemplates[tmpl.GetName()] = tmpl - } - return systemTemplates, nil -} - -func (r *TemplateChainReconciler) getTemplates(ctx context.Context, templateKind string, opts *client.ListOptions) ([]templateCommon, error) { - var templates []templateCommon +func (r *TemplateChainReconciler) getTemplates(ctx context.Context, opts *client.ListOptions) (map[string]templateCommon, error) { + templates := make(map[string]templateCommon) - switch templateKind { + switch r.templateKind { case hmc.ClusterTemplateKind: ctList := &hmc.ClusterTemplateList{} - err := r.Client.List(ctx, ctList, opts) + err := r.List(ctx, ctList, opts) if err != nil { return nil, err } for _, template := range ctList.Items { - templates = append(templates, &template) + templates[template.Name] = &template } case hmc.ServiceTemplateKind: stList := &hmc.ServiceTemplateList{} - err := r.Client.List(ctx, stList, opts) + err := r.List(ctx, stList, opts) if err != nil { return nil, err } for _, template := range stList.Items { - templates = append(templates, &template) + templates[template.Name] = &template } default: return nil, fmt.Errorf("invalid Template kind. Supported kinds are %s and %s", hmc.ClusterTemplateKind, hmc.ServiceTemplateKind) diff --git a/internal/controller/templatechain_controller_test.go b/internal/controller/templatechain_controller_test.go index 7ca319c3a..33ab0ae2d 100644 --- a/internal/controller/templatechain_controller_test.go +++ b/internal/controller/templatechain_controller_test.go @@ -60,6 +60,9 @@ var _ = Describe("Template Chain Controller", func() { Name: chartName, } + ctProviders := hmcmirantiscomv1alpha1.Providers{"ct-provider-test"} + stProviders := hmcmirantiscomv1alpha1.Providers{"st-provider-test"} + ctTemplates := map[string]*hmcmirantiscomv1alpha1.ClusterTemplate{ // Should be created in target namespace "test": template.NewClusterTemplate( @@ -193,6 +196,7 @@ var _ = Describe("Template Chain Controller", func() { ct := &hmcmirantiscomv1alpha1.ClusterTemplate{} err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: utils.DefaultSystemNamespace}, ct) if err != nil && errors.IsNotFound(err) { + template.Spec.Providers = ctProviders Expect(k8sClient.Create(ctx, template)).To(Succeed()) } template.Status.ChartRef = chartRef @@ -203,6 +207,7 @@ var _ = Describe("Template Chain Controller", func() { st := &hmcmirantiscomv1alpha1.ServiceTemplate{} err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: utils.DefaultSystemNamespace}, st) if err != nil && errors.IsNotFound(err) { + template.Spec.Providers = stProviders Expect(k8sClient.Create(ctx, template)).To(Succeed()) } template.Status.ChartRef = chartRef @@ -335,23 +340,17 @@ var _ = Describe("Template Chain Controller", func() { Name: stChain2Name, UID: stChainUIDs[stChain2Name], } - verifyObjectCreated(ctx, namespace.Name, ctTemplates["test"]) - verifyOwnerReferenceExistence(ctTemplates["test"], ct1OwnerRef) - verifyObjectCreated(ctx, namespace.Name, ctTemplates["ct0"]) + verifyClusterTemplateCreated(ctx, namespace.Name, "test", ct1OwnerRef) + verifyClusterTemplateCreated(ctx, namespace.Name, "ct0") - verifyObjectCreated(ctx, namespace.Name, ctTemplates["ct1"]) - verifyOwnerReferenceExistence(ctTemplates["ct1"], ct1OwnerRef) - verifyOwnerReferenceExistence(ctTemplates["ct1"], ct2OwnerRef) + verifyClusterTemplateCreated(ctx, namespace.Name, "ct1", ct1OwnerRef, ct2OwnerRef) verifyObjectUnchanged(ctx, namespace.Name, ctUnmanagedBefore, ctTemplates["ct2"]) - verifyObjectCreated(ctx, namespace.Name, stTemplates["test"]) - verifyOwnerReferenceExistence(stTemplates["test"], st1OwnerRef) - verifyObjectCreated(ctx, namespace.Name, stTemplates["st0"]) + verifyServiceTemplateCreated(ctx, namespace.Name, "test", st1OwnerRef) + verifyServiceTemplateCreated(ctx, namespace.Name, "st0") - verifyObjectCreated(ctx, namespace.Name, stTemplates["st1"]) - verifyOwnerReferenceExistence(stTemplates["st1"], st1OwnerRef) - verifyOwnerReferenceExistence(stTemplates["st1"], st2OwnerRef) + verifyServiceTemplateCreated(ctx, namespace.Name, "st1", st1OwnerRef, st2OwnerRef) verifyObjectUnchanged(ctx, namespace.Name, stUnmanagedBefore, stTemplates["st2"]) }) @@ -394,3 +393,45 @@ func verifyOwnerReferenceExistence(obj crclient.Object, ownerRef metav1.OwnerRef func checkHMCManagedLabelExistence(labels map[string]string) { Expect(labels).To(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedLabelKey, hmcmirantiscomv1alpha1.HMCManagedLabelValue)) } + +func verifyClusterTemplateCreated(ctx context.Context, namespace, name string, ownerRef ...metav1.OwnerReference) { + By(fmt.Sprintf("Verifying the ClusterTemplate %s/%s", namespace, name)) + source := &hmcmirantiscomv1alpha1.ClusterTemplate{} + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: utils.DefaultSystemNamespace, Name: name}, source) + Expect(err).NotTo(HaveOccurred()) + + target := &hmcmirantiscomv1alpha1.ClusterTemplate{} + err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: source.Name}, target) + Expect(err).NotTo(HaveOccurred()) + + expectedSpec := source.Spec + expectedSpec.Helm.ChartSpec = nil + expectedSpec.Helm.ChartRef = source.Status.ChartRef + Expect(expectedSpec).To(Equal(target.Spec)) + + checkHMCManagedLabelExistence(target.GetLabels()) + for _, or := range ownerRef { + verifyOwnerReferenceExistence(target, or) + } +} + +func verifyServiceTemplateCreated(ctx context.Context, namespace, name string, ownerRef ...metav1.OwnerReference) { + By(fmt.Sprintf("Verifying the ServiceTemplate %s/%s", namespace, name)) + source := &hmcmirantiscomv1alpha1.ServiceTemplate{} + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: utils.DefaultSystemNamespace, Name: name}, source) + Expect(err).NotTo(HaveOccurred()) + + target := &hmcmirantiscomv1alpha1.ServiceTemplate{} + err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, target) + Expect(err).NotTo(HaveOccurred()) + + expectedSpec := source.Spec + expectedSpec.Helm.ChartSpec = nil + expectedSpec.Helm.ChartRef = source.Status.ChartRef + Expect(expectedSpec).To(Equal(target.Spec)) + + checkHMCManagedLabelExistence(target.GetLabels()) + for _, or := range ownerRef { + verifyOwnerReferenceExistence(target, or) + } +}