From 33f4aa742788329dea0377f9234b7c4644da7b8f Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Fri, 18 Oct 2024 22:40:32 +0400 Subject: [PATCH 1/3] Set availableUpgrades in ManagedCluster status --- api/v1alpha1/managedcluster_types.go | 7 +- api/v1alpha1/zz_generated.deepcopy.go | 5 ++ .../controller/managedcluster_controller.go | 65 ++++++++++++++++++- .../controller/templatechain_controller.go | 8 +++ .../hmc.mirantis.com_managedclusters.yaml | 19 +++++- 5 files changed, 99 insertions(+), 5 deletions(-) diff --git a/api/v1alpha1/managedcluster_types.go b/api/v1alpha1/managedcluster_types.go index 60920c732..9698fe566 100644 --- a/api/v1alpha1/managedcluster_types.go +++ b/api/v1alpha1/managedcluster_types.go @@ -95,8 +95,13 @@ type ManagedClusterStatus struct { // Currently compatible exact Kubernetes version of the cluster. Being set only if // provided by the corresponding ClusterTemplate. KubernetesVersion string `json:"k8sVersion,omitempty"` - // Conditions contains details for the current state of the ManagedCluster + // Conditions contains details for the current state of the ManagedCluster. Conditions []metav1.Condition `json:"conditions,omitempty"` + + // AvailableUpgrades is the list of ClusterTemplate names to which + // this cluster can be upgraded. It can be an empty array, which means no upgrades are + // available. + AvailableUpgrades []AvailableUpgrade `json:"availableUpgrades,omitempty"` // ObservedGeneration is the last observed generation. ObservedGeneration int64 `json:"observedGeneration,omitempty"` } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 0e7794334..742313395 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -538,6 +538,11 @@ func (in *ManagedClusterStatus) DeepCopyInto(out *ManagedClusterStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.AvailableUpgrades != nil { + in, out := &in.AvailableUpgrades, &out.AvailableUpgrades + *out = make([]AvailableUpgrade, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ManagedClusterStatus. diff --git a/internal/controller/managedcluster_controller.go b/internal/controller/managedcluster_controller.go index b0a6a1b93..a3992f4c9 100644 --- a/internal/controller/managedcluster_controller.go +++ b/internal/controller/managedcluster_controller.go @@ -39,6 +39,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -180,11 +181,12 @@ func (r *ManagedClusterReconciler) Update(ctx context.Context, managedCluster *h managedCluster.InitConditions() } + template := &hmc.ClusterTemplate{} + defer func() { - err = errors.Join(err, r.updateStatus(ctx, managedCluster)) + err = errors.Join(err, r.updateStatus(ctx, managedCluster, template)) }() - template := &hmc.ClusterTemplate{} templateRef := client.ObjectKey{Name: managedCluster.Spec.Template, Namespace: managedCluster.Namespace} if err := r.Get(ctx, templateRef, template); err != nil { l.Error(err, "Failed to get Template") @@ -419,7 +421,7 @@ func validateReleaseWithValues(ctx context.Context, actionConfig *action.Configu return nil } -func (r *ManagedClusterReconciler) updateStatus(ctx context.Context, managedCluster *hmc.ManagedCluster) error { +func (r *ManagedClusterReconciler) updateStatus(ctx context.Context, managedCluster *hmc.ManagedCluster, template *hmc.ClusterTemplate) error { managedCluster.Status.ObservedGeneration = managedCluster.Generation warnings := "" errs := "" @@ -451,6 +453,11 @@ func (r *ManagedClusterReconciler) updateStatus(ctx context.Context, managedClus condition.Message = errs } apimeta.SetStatusCondition(managedCluster.GetConditions(), condition) + + err := r.setAvailableUpgrades(ctx, managedCluster, template) + if err != nil { + return errors.New("failed to set available upgrades") + } if err := r.Status().Update(ctx, managedCluster); err != nil { return fmt.Errorf("failed to update status for managedCluster %s/%s: %w", managedCluster.Namespace, managedCluster.Name, err) } @@ -997,6 +1004,29 @@ func setIdentityHelmValues(values *apiextensionsv1.JSON, idRef *corev1.ObjectRef }, nil } +func (r *ManagedClusterReconciler) setAvailableUpgrades(ctx context.Context, managedCluster *hmc.ManagedCluster, template *hmc.ClusterTemplate) error { + if template == nil { + return nil + } + chainName := template.Labels[HMCManagedByChainLabelKey] + if chainName == "" { + return nil + } + chain := &hmc.ClusterTemplateChain{} + err := r.Get(ctx, types.NamespacedName{Namespace: template.Namespace, Name: chainName}, chain) + if err != nil { + return err + } + + for _, supportedTemplate := range chain.Spec.SupportedTemplates { + if supportedTemplate.Name == template.Name { + managedCluster.Status.AvailableUpgrades = supportedTemplate.AvailableUpgrades + return nil + } + } + return nil +} + // SetupWithManager sets up the controller with the Manager. func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). @@ -1019,5 +1049,34 @@ func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { } }), ). + Watches(&hmc.ClusterTemplateChain{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []ctrl.Request { + chain := &hmc.ClusterTemplateChain{} + err := r.Client.Get(ctx, types.NamespacedName{Namespace: o.GetNamespace(), Name: o.GetName()}, chain) + if err != nil { + return []ctrl.Request{} + } + + var req []ctrl.Request + for _, template := range getTemplateNamesManagedByChain(chain) { + managedClusters := &hmc.ManagedClusterList{} + err = r.Client.List(ctx, managedClusters, + client.InNamespace(o.GetNamespace()), + client.MatchingFields{hmc.TemplateKey: template}) + if err != nil { + return []ctrl.Request{} + } + for _, cluster := range managedClusters.Items { + req = append(req, reconcile.Request{ + NamespacedName: client.ObjectKey{ + Namespace: cluster.Namespace, + Name: cluster.Name, + }, + }) + } + } + return req + }), + ). Complete(r) } diff --git a/internal/controller/templatechain_controller.go b/internal/controller/templatechain_controller.go index 758a7c36d..093428585 100644 --- a/internal/controller/templatechain_controller.go +++ b/internal/controller/templatechain_controller.go @@ -211,6 +211,14 @@ func getCurrentTemplates(ctx context.Context, cl client.Client, templateKind, sy return systemTemplates, managedTemplates, nil } +func getTemplateNamesManagedByChain(chain templateChain) []string { + result := make([]string, 0, len(chain.GetSpec().SupportedTemplates)) + for _, template := range chain.GetSpec().SupportedTemplates { + result = append(result, template.Name) + } + return result +} + // SetupWithManager sets up the controller with the Manager. func (r *ClusterTemplateChainReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml index d3ad6d5f5..c449c0d16 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml @@ -132,9 +132,26 @@ spec: status: description: ManagedClusterStatus defines the observed state of ManagedCluster properties: + availableUpgrades: + description: |- + AvailableUpgrades is the list of ClusterTemplate names to which + this cluster can be upgraded. It can be an empty array, which means no upgrades are + available. + items: + description: AvailableUpgrade is the definition of the available + upgrade for the Template + properties: + name: + description: Name is the name of the Template to which the upgrade + is available. + type: string + required: + - name + type: object + type: array conditions: description: Conditions contains details for the current state of - the ManagedCluster + the ManagedCluster. items: description: Condition contains details for one aspect of the current state of this API Resource. From c8508d47608992af9aef7af1f25340ad9d866a31 Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Fri, 18 Oct 2024 22:59:21 +0400 Subject: [PATCH 2/3] Validate availability of the ManagedCluster update --- api/v1alpha1/common.go | 39 ++++++++- .../controller/managedcluster_controller.go | 29 ++++--- internal/webhook/managedcluster_webhook.go | 22 ++++- .../webhook/managedcluster_webhook_test.go | 82 ++++++++++++++++++- test/objects/managedcluster/managedcluster.go | 6 ++ 5 files changed, 162 insertions(+), 16 deletions(-) diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index 31400432f..3bbc56611 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -71,7 +71,15 @@ func SetupIndexers(ctx context.Context, mgr ctrl.Manager) error { return err } - return SetupManagedClusterServicesIndexer(ctx, mgr) + if err := SetupManagedClusterServicesIndexer(ctx, mgr); err != nil { + return err + } + + if err := SetupClusterTemplateChainIndexer(ctx, mgr); err != nil { + return err + } + + return SetupServiceTemplateChainIndexer(ctx, mgr) } const TemplateKey = ".spec.template" @@ -121,3 +129,32 @@ func ExtractServiceTemplateName(rawObj client.Object) []string { return templates } + +const SupportedTemplateKey = ".spec.supportedTemplates[].Name" + +func SetupClusterTemplateChainIndexer(ctx context.Context, mgr ctrl.Manager) error { + return mgr.GetFieldIndexer().IndexField(ctx, &ClusterTemplateChain{}, SupportedTemplateKey, ExtractSupportedTemplatesNames) +} + +func SetupServiceTemplateChainIndexer(ctx context.Context, mgr ctrl.Manager) error { + return mgr.GetFieldIndexer().IndexField(ctx, &ServiceTemplateChain{}, SupportedTemplateKey, ExtractSupportedTemplatesNames) +} + +func ExtractSupportedTemplatesNames(rawObj client.Object) []string { + chainSpec := TemplateChainSpec{} + switch chain := rawObj.(type) { + case *ClusterTemplateChain: + chainSpec = chain.Spec + case *ServiceTemplateChain: + chainSpec = chain.Spec + default: + return nil + } + + supportedTemplates := make([]string, 0, len(chainSpec.SupportedTemplates)) + for _, t := range chainSpec.SupportedTemplates { + supportedTemplates = append(supportedTemplates, t.Name) + } + + return supportedTemplates +} diff --git a/internal/controller/managedcluster_controller.go b/internal/controller/managedcluster_controller.go index a3992f4c9..587eb422f 100644 --- a/internal/controller/managedcluster_controller.go +++ b/internal/controller/managedcluster_controller.go @@ -1008,22 +1008,31 @@ func (r *ManagedClusterReconciler) setAvailableUpgrades(ctx context.Context, man if template == nil { return nil } - chainName := template.Labels[HMCManagedByChainLabelKey] - if chainName == "" { - return nil - } - chain := &hmc.ClusterTemplateChain{} - err := r.Get(ctx, types.NamespacedName{Namespace: template.Namespace, Name: chainName}, chain) + chains := &hmc.ClusterTemplateChainList{} + err := r.List(ctx, chains, + client.InNamespace(template.Namespace), + client.MatchingFields{hmc.SupportedTemplateKey: template.GetName()}, + ) if err != nil { return err } - for _, supportedTemplate := range chain.Spec.SupportedTemplates { - if supportedTemplate.Name == template.Name { - managedCluster.Status.AvailableUpgrades = supportedTemplate.AvailableUpgrades - return nil + availableUpgradesMap := make(map[string]hmc.AvailableUpgrade) + for _, chain := range chains.Items { + for _, supportedTemplate := range chain.Spec.SupportedTemplates { + if supportedTemplate.Name == template.Name { + for _, availableUpgrade := range supportedTemplate.AvailableUpgrades { + availableUpgradesMap[availableUpgrade.Name] = availableUpgrade + } + } } } + availableUpgrades := make([]hmc.AvailableUpgrade, 0, len(availableUpgradesMap)) + for _, availableUpgrade := range availableUpgradesMap { + availableUpgrades = append(availableUpgrades, availableUpgrade) + } + + managedCluster.Status.AvailableUpgrades = availableUpgrades return nil } diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go index 90f286ea4..9f4424bd8 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "github.com/Masterminds/semver/v3" @@ -38,6 +39,8 @@ type ManagedClusterValidator struct { const invalidManagedClusterMsg = "the ManagedCluster is invalid" +var errClusterUpgradeForbidden = errors.New("cluster upgrade is forbidden") + func (v *ManagedClusterValidator) SetupWebhookWithManager(mgr ctrl.Manager) error { v.Client = mgr.GetClient() return ctrl.NewWebhookManagedBy(mgr). @@ -89,12 +92,21 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("expected ManagedCluster but got a %T", newObj)) } - template, err := v.getManagedClusterTemplate(ctx, newManagedCluster.Namespace, newManagedCluster.Spec.Template) + oldTemplate := oldManagedCluster.Spec.Template + newTemplate := newManagedCluster.Spec.Template + + template, err := v.getManagedClusterTemplate(ctx, newManagedCluster.Namespace, newTemplate) if err != nil { return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) } - if oldManagedCluster.Spec.Template != newManagedCluster.Spec.Template { + if oldTemplate != newTemplate { + isUpgradeAvailable := validateAvailableUpgrade(oldManagedCluster, newTemplate) + if !isUpgradeAvailable { + msg := fmt.Sprintf("Cluster can't be upgraded from %s to %s. This upgrade sequence is not allowed", oldTemplate, newTemplate) + return admission.Warnings{msg}, errClusterUpgradeForbidden + } + if err := isTemplateValid(template); err != nil { return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) } @@ -111,6 +123,12 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne return nil, nil } +func validateAvailableUpgrade(oldManagedCluster *hmcv1alpha1.ManagedCluster, newTemplate string) bool { + return slices.ContainsFunc(oldManagedCluster.Status.AvailableUpgrades, func(au hmcv1alpha1.AvailableUpgrade) bool { + return newTemplate == au.Name + }) +} + func validateK8sCompatibility(ctx context.Context, cl client.Client, template *hmcv1alpha1.ClusterTemplate, mc *hmcv1alpha1.ManagedCluster) error { if len(mc.Spec.Services) == 0 || template.Status.KubernetesVersion == "" { return nil // nothing to do diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index fd1d76132..53731af3d 100644 --- a/internal/webhook/managedcluster_webhook_test.go +++ b/internal/webhook/managedcluster_webhook_test.go @@ -257,6 +257,11 @@ func TestManagedClusterValidateCreate(t *testing.T) { } func TestManagedClusterValidateUpdate(t *testing.T) { + const ( + upgradeTargetTemplateName = "upgrade-target-template" + unmanagedByHMCTemplateName = "unmanaged-template" + ) + g := NewWithT(t) ctx := admission.NewContextWithRequest(context.Background(), admission.Request{ @@ -274,8 +279,11 @@ func TestManagedClusterValidateUpdate(t *testing.T) { warnings admission.Warnings }{ { - name: "should fail if the new cluster template was found but is invalid (some validation error)", - oldManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(testTemplateName)), + name: "update spec.template: should fail if the new cluster template was found but is invalid (some validation error)", + oldManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithAvailableUpgrades([]v1alpha1.AvailableUpgrade{{Name: newTemplateName}}), + ), newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(newTemplateName)), existingObjects: []runtime.Object{ mgmt, @@ -290,7 +298,75 @@ func TestManagedClusterValidateUpdate(t *testing.T) { err: "the ManagedCluster is invalid: the template is not valid: validation error example", }, { - name: "should succeed if template is not changed", + name: "update spec.template: should fail if the template is not in the list of available", + oldManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithCredential(testCredentialName), + managedcluster.WithAvailableUpgrades([]v1alpha1.AvailableUpgrade{}), + ), + newManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(upgradeTargetTemplateName), + managedcluster.WithCredential(testCredentialName), + ), + existingObjects: []runtime.Object{ + mgmt, cred, + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + ), + template.NewClusterTemplate( + template.WithName(upgradeTargetTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + ), + }, + warnings: admission.Warnings{fmt.Sprintf("Cluster can't be upgraded from %s to %s. This upgrade sequence is not allowed", testTemplateName, upgradeTargetTemplateName)}, + err: "cluster upgrade is forbidden", + }, + { + name: "update spec.template: should succeed if the template is in the list of available", + oldManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithCredential(testCredentialName), + managedcluster.WithAvailableUpgrades([]v1alpha1.AvailableUpgrade{{Name: newTemplateName}}), + ), + newManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(newTemplateName), + managedcluster.WithCredential(testCredentialName), + ), + existingObjects: []runtime.Object{ + mgmt, cred, + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + ), + template.NewClusterTemplate( + template.WithName(newTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + ), + }, + }, + { + name: "should succeed if spec.template is not changed", oldManagedCluster: managedcluster.NewManagedCluster( managedcluster.WithClusterTemplate(testTemplateName), managedcluster.WithConfig(`{"foo":"bar"}`), diff --git a/test/objects/managedcluster/managedcluster.go b/test/objects/managedcluster/managedcluster.go index fc6c00770..48d9ecf44 100644 --- a/test/objects/managedcluster/managedcluster.go +++ b/test/objects/managedcluster/managedcluster.go @@ -87,3 +87,9 @@ func WithCredential(credName string) Opt { p.Spec.Credential = credName } } + +func WithAvailableUpgrades(availableUpgrades []v1alpha1.AvailableUpgrade) Opt { + return func(p *v1alpha1.ManagedCluster) { + p.Status.AvailableUpgrades = availableUpgrades + } +} From ab3f0337f9f6e8dbd90946329697383318ada829 Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Mon, 28 Oct 2024 19:10:16 +0400 Subject: [PATCH 3/3] Reflect available upgrades in cluster status: addressed comments * Several watcher improvements: events, code enhancements * Changed availableUpgrades type into []string --- api/v1alpha1/managedcluster_types.go | 2 +- api/v1alpha1/zz_generated.deepcopy.go | 2 +- .../controller/managedcluster_controller.go | 25 +++++++++++-------- internal/webhook/managedcluster_webhook.go | 9 +------ .../webhook/managedcluster_webhook_test.go | 6 ++--- .../hmc.mirantis.com_managedclusters.yaml | 11 +------- test/objects/managedcluster/managedcluster.go | 2 +- 7 files changed, 23 insertions(+), 34 deletions(-) diff --git a/api/v1alpha1/managedcluster_types.go b/api/v1alpha1/managedcluster_types.go index 9698fe566..3f5e40bc1 100644 --- a/api/v1alpha1/managedcluster_types.go +++ b/api/v1alpha1/managedcluster_types.go @@ -101,7 +101,7 @@ type ManagedClusterStatus struct { // AvailableUpgrades is the list of ClusterTemplate names to which // this cluster can be upgraded. It can be an empty array, which means no upgrades are // available. - AvailableUpgrades []AvailableUpgrade `json:"availableUpgrades,omitempty"` + AvailableUpgrades []string `json:"availableUpgrades,omitempty"` // ObservedGeneration is the last observed generation. ObservedGeneration int64 `json:"observedGeneration,omitempty"` } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 742313395..c0d4b348d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -540,7 +540,7 @@ func (in *ManagedClusterStatus) DeepCopyInto(out *ManagedClusterStatus) { } if in.AvailableUpgrades != nil { in, out := &in.AvailableUpgrades, &out.AvailableUpgrades - *out = make([]AvailableUpgrade, len(*in)) + *out = make([]string, len(*in)) copy(*out, *in) } } diff --git a/internal/controller/managedcluster_controller.go b/internal/controller/managedcluster_controller.go index 587eb422f..3b8805317 100644 --- a/internal/controller/managedcluster_controller.go +++ b/internal/controller/managedcluster_controller.go @@ -39,7 +39,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -47,9 +46,12 @@ import ( capz "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" capv "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/yaml" @@ -1027,9 +1029,9 @@ func (r *ManagedClusterReconciler) setAvailableUpgrades(ctx context.Context, man } } } - availableUpgrades := make([]hmc.AvailableUpgrade, 0, len(availableUpgradesMap)) + availableUpgrades := make([]string, 0, len(availableUpgradesMap)) for _, availableUpgrade := range availableUpgradesMap { - availableUpgrades = append(availableUpgrades, availableUpgrade) + availableUpgrades = append(availableUpgrades, availableUpgrade.Name) } managedCluster.Status.AvailableUpgrades = availableUpgrades @@ -1060,23 +1062,22 @@ func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&hmc.ClusterTemplateChain{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []ctrl.Request { - chain := &hmc.ClusterTemplateChain{} - err := r.Client.Get(ctx, types.NamespacedName{Namespace: o.GetNamespace(), Name: o.GetName()}, chain) - if err != nil { - return []ctrl.Request{} + chain, ok := o.(*hmc.ClusterTemplateChain) + if !ok { + return nil } var req []ctrl.Request for _, template := range getTemplateNamesManagedByChain(chain) { managedClusters := &hmc.ManagedClusterList{} - err = r.Client.List(ctx, managedClusters, - client.InNamespace(o.GetNamespace()), + err := r.Client.List(ctx, managedClusters, + client.InNamespace(chain.Namespace), client.MatchingFields{hmc.TemplateKey: template}) if err != nil { return []ctrl.Request{} } for _, cluster := range managedClusters.Items { - req = append(req, reconcile.Request{ + req = append(req, ctrl.Request{ NamespacedName: client.ObjectKey{ Namespace: cluster.Namespace, Name: cluster.Name, @@ -1086,6 +1087,10 @@ func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { } return req }), + builder.WithPredicates(predicate.Funcs{ + UpdateFunc: func(event.UpdateEvent) bool { return false }, + GenericFunc: func(event.GenericEvent) bool { return false }, + }), ). Complete(r) } diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go index 9f4424bd8..25970c588 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -101,8 +101,7 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne } if oldTemplate != newTemplate { - isUpgradeAvailable := validateAvailableUpgrade(oldManagedCluster, newTemplate) - if !isUpgradeAvailable { + if !slices.Contains(oldManagedCluster.Status.AvailableUpgrades, newTemplate) { msg := fmt.Sprintf("Cluster can't be upgraded from %s to %s. This upgrade sequence is not allowed", oldTemplate, newTemplate) return admission.Warnings{msg}, errClusterUpgradeForbidden } @@ -123,12 +122,6 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne return nil, nil } -func validateAvailableUpgrade(oldManagedCluster *hmcv1alpha1.ManagedCluster, newTemplate string) bool { - return slices.ContainsFunc(oldManagedCluster.Status.AvailableUpgrades, func(au hmcv1alpha1.AvailableUpgrade) bool { - return newTemplate == au.Name - }) -} - func validateK8sCompatibility(ctx context.Context, cl client.Client, template *hmcv1alpha1.ClusterTemplate, mc *hmcv1alpha1.ManagedCluster) error { if len(mc.Spec.Services) == 0 || template.Status.KubernetesVersion == "" { return nil // nothing to do diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index 53731af3d..4d8cc84a5 100644 --- a/internal/webhook/managedcluster_webhook_test.go +++ b/internal/webhook/managedcluster_webhook_test.go @@ -282,7 +282,7 @@ func TestManagedClusterValidateUpdate(t *testing.T) { name: "update spec.template: should fail if the new cluster template was found but is invalid (some validation error)", oldManagedCluster: managedcluster.NewManagedCluster( managedcluster.WithClusterTemplate(testTemplateName), - managedcluster.WithAvailableUpgrades([]v1alpha1.AvailableUpgrade{{Name: newTemplateName}}), + managedcluster.WithAvailableUpgrades([]string{newTemplateName}), ), newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(newTemplateName)), existingObjects: []runtime.Object{ @@ -302,7 +302,7 @@ func TestManagedClusterValidateUpdate(t *testing.T) { oldManagedCluster: managedcluster.NewManagedCluster( managedcluster.WithClusterTemplate(testTemplateName), managedcluster.WithCredential(testCredentialName), - managedcluster.WithAvailableUpgrades([]v1alpha1.AvailableUpgrade{}), + managedcluster.WithAvailableUpgrades([]string{}), ), newManagedCluster: managedcluster.NewManagedCluster( managedcluster.WithClusterTemplate(upgradeTargetTemplateName), @@ -337,7 +337,7 @@ func TestManagedClusterValidateUpdate(t *testing.T) { oldManagedCluster: managedcluster.NewManagedCluster( managedcluster.WithClusterTemplate(testTemplateName), managedcluster.WithCredential(testCredentialName), - managedcluster.WithAvailableUpgrades([]v1alpha1.AvailableUpgrade{{Name: newTemplateName}}), + managedcluster.WithAvailableUpgrades([]string{newTemplateName}), ), newManagedCluster: managedcluster.NewManagedCluster( managedcluster.WithClusterTemplate(newTemplateName), diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml index c449c0d16..59dacde72 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_managedclusters.yaml @@ -138,16 +138,7 @@ spec: this cluster can be upgraded. It can be an empty array, which means no upgrades are available. items: - description: AvailableUpgrade is the definition of the available - upgrade for the Template - properties: - name: - description: Name is the name of the Template to which the upgrade - is available. - type: string - required: - - name - type: object + type: string type: array conditions: description: Conditions contains details for the current state of diff --git a/test/objects/managedcluster/managedcluster.go b/test/objects/managedcluster/managedcluster.go index 48d9ecf44..4bc2d5db1 100644 --- a/test/objects/managedcluster/managedcluster.go +++ b/test/objects/managedcluster/managedcluster.go @@ -88,7 +88,7 @@ func WithCredential(credName string) Opt { } } -func WithAvailableUpgrades(availableUpgrades []v1alpha1.AvailableUpgrade) Opt { +func WithAvailableUpgrades(availableUpgrades []string) Opt { return func(p *v1alpha1.ManagedCluster) { p.Status.AvailableUpgrades = availableUpgrades }