Skip to content

Commit

Permalink
Validate availability of the ManagedCluster update
Browse files Browse the repository at this point in the history
  • Loading branch information
eromanova committed Oct 24, 2024
1 parent 2f30141 commit 8fbf9f0
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 26 deletions.
39 changes: 38 additions & 1 deletion api/v1alpha1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
2 changes: 2 additions & 0 deletions api/v1alpha1/managedcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ const (
HMCManagedLabelKey = "hmc.mirantis.com/managed"
HMCManagedLabelValue = "true"

HMCManagedByChainLabelKey = "hmc.mirantis.com/managed-by-chain"

ClusterNameLabelKey = "cluster.x-k8s.io/cluster-name"
)

Expand Down
34 changes: 22 additions & 12 deletions internal/controller/managedcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1011,22 +1010,33 @@ 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]bool)
for _, chain := range chains.Items {
for _, supportedTemplate := range chain.Spec.SupportedTemplates {
if supportedTemplate.Name == template.Name {
for _, upgrade := range supportedTemplate.AvailableUpgrades {
availableUpgradesMap[upgrade.Name] = true
}
}
}
}
availableUpgrades := make([]hmc.AvailableUpgrade, 0, len(availableUpgradesMap))
for availableUpgrade := range availableUpgradesMap {
availableUpgrades = append(availableUpgrades, hmc.AvailableUpgrade{
Name: availableUpgrade,
})
}

managedCluster.Status.AvailableUpgrades = availableUpgrades
return nil
}

Expand Down Expand Up @@ -1057,7 +1067,7 @@ func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
templates := &hmc.ClusterTemplateList{}
err := r.Client.List(ctx, templates,
client.InNamespace(o.GetNamespace()),
client.MatchingLabels{HMCManagedByChainLabelKey: o.GetName()})
client.MatchingLabels{hmc.HMCManagedByChainLabelKey: o.GetName()})
if err != nil {
return []ctrl.Request{}
}
Expand Down
8 changes: 3 additions & 5 deletions internal/controller/templatechain_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ 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
Expand Down Expand Up @@ -102,8 +100,8 @@ func (r *TemplateChainReconciler) ReconcileTemplateChain(ctx context.Context, te
Name: supportedTemplate.Name,
Namespace: templateChain.GetNamespace(),
Labels: map[string]string{
hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue,
HMCManagedByChainLabelKey: templateChain.GetName(),
hmc.HMCManagedLabelKey: hmc.HMCManagedLabelValue,
hmc.HMCManagedByChainLabelKey: templateChain.GetName(),
},
}
keepTemplate[supportedTemplate.Name] = struct{}{}
Expand Down Expand Up @@ -203,7 +201,7 @@ func getCurrentTemplates(ctx context.Context, cl client.Client, templateKind, sy
labels := template.GetLabels()
if template.GetNamespace() == targetNamespace &&
labels[hmc.HMCManagedLabelKey] == hmc.HMCManagedLabelValue &&
labels[HMCManagedByChainLabelKey] == templateChainName {
labels[hmc.HMCManagedByChainLabelKey] == templateChainName {
managedTemplates = append(managedTemplates, template)
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/controller/templatechain_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,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.WithLabels(map[string]string{hmcmirantiscomv1alpha1.HMCManagedByChainLabelKey: ctChain1Name}),
template.ManagedByHMC(),
),
// Should be unchanged (unmanaged)
Expand Down Expand Up @@ -104,7 +104,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.WithLabels(map[string]string{hmcmirantiscomv1alpha1.HMCManagedByChainLabelKey: stChain1Name}),
template.ManagedByHMC(),
),
// Should be unchanged (unmanaged)
Expand Down Expand Up @@ -338,5 +338,5 @@ func checkHMCManagedLabelExistence(labels map[string]string) {
}

func checkHMCManagedByChainLabelExistence(labels map[string]string, chainName string) {
Expect(labels).To(HaveKeyWithValue(HMCManagedByChainLabelKey, chainName))
Expect(labels).To(HaveKeyWithValue(hmcmirantiscomv1alpha1.HMCManagedByChainLabelKey, chainName))
}
23 changes: 21 additions & 2 deletions internal/webhook/managedcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ package webhook

import (
"context"
"errors"
"fmt"
"slices"
"strings"

"github.com/Masterminds/semver/v3"
Expand All @@ -37,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).
Expand Down Expand Up @@ -88,12 +92,21 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj run
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)
}
Expand All @@ -110,6 +123,12 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj run
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
Expand Down
82 changes: 79 additions & 3 deletions internal/webhook/managedcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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,
Expand All @@ -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"}`),
Expand Down
6 changes: 6 additions & 0 deletions test/objects/managedcluster/managedcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

0 comments on commit 8fbf9f0

Please sign in to comment.