diff --git a/pkg/clusterapi/name.go b/pkg/clusterapi/name.go index e1b20a5d5dd1..b72fa50e81e4 100644 --- a/pkg/clusterapi/name.go +++ b/pkg/clusterapi/name.go @@ -7,10 +7,16 @@ import ( "github.com/aws/eks-anywhere/pkg/api/v1alpha1" "github.com/aws/eks-anywhere/pkg/cluster" + "github.com/aws/eks-anywhere/pkg/logger" ) var nameRegex = regexp.MustCompile(`(.*?)(-)(\d+)$`) +// IncrementName takes an object name and increments the suffix number by one. +// This method is used for updating objects (e.g. machinetemplate, kubeadmconfigtemplate) that are either immutable +// or require recreation to trigger machine rollout. The original object name should follow the name convention of +// alphanumeric followed by dash digits, e.g. abc-1, md-0, kct-2. An error will be raised if the original name does not follow +// this pattern. func IncrementName(name string) (string, error) { match := nameRegex.FindStringSubmatch(name) if match == nil { @@ -25,6 +31,19 @@ func IncrementName(name string) (string, error) { return ObjectName(match[1], n+1), nil } +// IncrementNameWithFallbackDefault calls the IncrementName and fallbacks to use the default name if IncrementName +// returns an error. This method is used to accommodate for any objects with name breaking changes from a previous version. +// For example, in beta capi snowmachinetemplate is named after the eks-a snowmachineconfig name, without the '-1' suffix. +// We set the object name to the default new machinetemplate name after detecting the invalid old name. +func IncrementNameWithFallbackDefault(name, defaultName string) string { + n, err := IncrementName(name) + if err != nil { + logger.V(4).Info("Unable to increment object name (might due to changes of name format), fallback to the default name", "error", err.Error()) + return defaultName + } + return n +} + func ObjectName(baseName string, version int) string { return fmt.Sprintf("%s-%d", baseName, version) } diff --git a/pkg/clusterapi/name_test.go b/pkg/clusterapi/name_test.go index f0a22694cd87..56338527868b 100644 --- a/pkg/clusterapi/name_test.go +++ b/pkg/clusterapi/name_test.go @@ -42,6 +42,35 @@ func TestIncrementName(t *testing.T) { } } +func TestIncrementNameWithFallbackDefault(t *testing.T) { + tests := []struct { + name string + oldName string + defaultName string + want string + }{ + { + name: "valid", + oldName: "cluster-1", + defaultName: "default", + want: "cluster-2", + }, + { + name: "invalid format", + oldName: "cluster-1a", + defaultName: "default", + want: "default", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + got := clusterapi.IncrementNameWithFallbackDefault(tt.oldName, tt.defaultName) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + func TestObjectName(t *testing.T) { tests := []struct { name string diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index d9fecec29e0c..cae2b608a6cd 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -13,6 +13,7 @@ const ( CapvSystemNamespace = "capv-system" CaptSystemNamespace = "capt-system" CapaSystemNamespace = "capa-system" + CapasSystemNamespace = "capas-system" CertManagerNamespace = "cert-manager" DefaultNamespace = "default" EtcdAdmBootstrapProviderSystemNamespace = "etcdadm-bootstrap-provider-system" diff --git a/pkg/executables/clusterctl.go b/pkg/executables/clusterctl.go index e63b942eb22f..8c93f61d2024 100644 --- a/pkg/executables/clusterctl.go +++ b/pkg/executables/clusterctl.go @@ -300,6 +300,7 @@ var providerNamespaces = map[string]string{ constants.DockerProviderName: constants.CapdSystemNamespace, constants.CloudStackProviderName: constants.CapcSystemNamespace, constants.AWSProviderName: constants.CapaSystemNamespace, + constants.SnowProviderName: constants.CapasSystemNamespace, etcdadmBootstrapProviderName: constants.EtcdAdmBootstrapProviderSystemNamespace, etcdadmControllerProviderName: constants.EtcdAdmControllerSystemNamespace, kubeadmBootstrapProviderName: constants.CapiKubeadmBootstrapSystemNamespace, diff --git a/pkg/providers/snow/objects.go b/pkg/providers/snow/objects.go index 07a7ff190504..2884f1e9d21a 100644 --- a/pkg/providers/snow/objects.go +++ b/pkg/providers/snow/objects.go @@ -23,11 +23,8 @@ func ControlPlaneObjects(ctx context.Context, clusterSpec *cluster.Spec, kubeCli if err != nil { return nil, err } - name, err := NewMachineTemplateName(new, old) - if err != nil { - return nil, err - } - new.SetName(name) + + new.SetName(NewMachineTemplateName(new, old)) kubeadmControlPlane, err := KubeadmControlPlane(clusterSpec, new) if err != nil { @@ -98,17 +95,11 @@ func WorkersMachineAndConfigTemplate(ctx context.Context, kubeClient kubernetes. } // compare the old and new kubeadmConfigTemplate to determine whether to recreate new kubeadmConfigTemplate object - configName, err := NewKubeadmConfigTemplateName(newConfigTemplate, oldConfigTemplate) - if err != nil { - return nil, nil, err - } + configName := NewKubeadmConfigTemplateName(newConfigTemplate, oldConfigTemplate) // compare the old and new machineTemplate as well as kubeadmConfigTemplate to determine whether to recreate // new machineTemplate object - machineName, err := NewWorkerMachineTemplateName(newMachineTemplate, oldMachineTemplate, newConfigTemplate, oldConfigTemplate) - if err != nil { - return nil, nil, err - } + machineName := NewWorkerMachineTemplateName(newMachineTemplate, oldMachineTemplate, newConfigTemplate, oldConfigTemplate) newConfigTemplate.SetName(configName) newMachineTemplate.SetName(machineName) @@ -120,48 +111,42 @@ func WorkersMachineAndConfigTemplate(ctx context.Context, kubeClient kubernetes. return machines, configs, nil } -func NewMachineTemplateName(new, old *snowv1.AWSSnowMachineTemplate) (string, error) { +func NewMachineTemplateName(new, old *snowv1.AWSSnowMachineTemplate) string { if old == nil { - return new.GetName(), nil + return new.GetName() } if equality.Semantic.DeepDerivative(new.Spec, old.Spec) { - return old.GetName(), nil + return old.GetName() } - return clusterapi.IncrementName(old.GetName()) + return clusterapi.IncrementNameWithFallbackDefault(old.GetName(), new.GetName()) } -func NewWorkerMachineTemplateName(newMt, oldMt *snowv1.AWSSnowMachineTemplate, newKct, oldKct *bootstrapv1.KubeadmConfigTemplate) (string, error) { - name, err := NewMachineTemplateName(newMt, oldMt) - if err != nil { - return "", err - } +func NewWorkerMachineTemplateName(newMt, oldMt *snowv1.AWSSnowMachineTemplate, newKct, oldKct *bootstrapv1.KubeadmConfigTemplate) string { + name := NewMachineTemplateName(newMt, oldMt) if oldKct == nil { - return name, nil + return name } if recreateKubeadmConfigTemplateNeeded(newKct, oldKct) { - name, err = clusterapi.IncrementName(oldMt.GetName()) - if err != nil { - return "", err - } + name = clusterapi.IncrementNameWithFallbackDefault(oldMt.GetName(), newMt.GetName()) } - return name, nil + return name } -func NewKubeadmConfigTemplateName(new, old *bootstrapv1.KubeadmConfigTemplate) (string, error) { +func NewKubeadmConfigTemplateName(new, old *bootstrapv1.KubeadmConfigTemplate) string { if old == nil { - return new.GetName(), nil + return new.GetName() } if recreateKubeadmConfigTemplateNeeded(new, old) { - return clusterapi.IncrementName(old.GetName()) + return clusterapi.IncrementNameWithFallbackDefault(old.GetName(), new.GetName()) } - return old.GetName(), nil + return old.GetName() } func recreateKubeadmConfigTemplateNeeded(new, old *bootstrapv1.KubeadmConfigTemplate) bool { diff --git a/pkg/providers/snow/objects_test.go b/pkg/providers/snow/objects_test.go index e3e3f9a2bd87..6644e9c44a27 100644 --- a/pkg/providers/snow/objects_test.go +++ b/pkg/providers/snow/objects_test.go @@ -58,6 +58,45 @@ func TestControlPlaneObjects(t *testing.T) { g.Expect(got).To(Equal([]runtime.Object{wantCAPICluster(), wantSnowCluster(), kcp, mt})) } +func TestControlPlaneObjectsUpgradeFromBetaMachineTemplateName(t *testing.T) { + g := newSnowTest(t) + mt := wantSnowMachineTemplate() + g.kubeconfigClient.EXPECT(). + Get( + g.ctx, + "snow-test", + constants.EksaSystemNamespace, + &controlplanev1.KubeadmControlPlane{}, + ). + DoAndReturn(func(_ context.Context, _, _ string, obj *controlplanev1.KubeadmControlPlane) error { + obj.Spec.MachineTemplate.InfrastructureRef.Name = "test-cp" + return nil + }) + g.kubeconfigClient.EXPECT(). + Get( + g.ctx, + "test-cp", + constants.EksaSystemNamespace, + &snowv1.AWSSnowMachineTemplate{}, + ). + DoAndReturn(func(_ context.Context, _, _ string, obj *snowv1.AWSSnowMachineTemplate) error { + mt.DeepCopyInto(obj) + obj.SetName("test-cp") + obj.Spec.Template.Spec.InstanceType = "updated-instance-type" + return nil + }) + + wantMachineTemplateName := "snow-test-control-plane-1" + mt.SetName(wantMachineTemplateName) + mt.Spec.Template.Spec.InstanceType = "sbe-c.large" + kcp := wantKubeadmControlPlane() + kcp.Spec.MachineTemplate.InfrastructureRef.Name = wantMachineTemplateName + + got, err := snow.ControlPlaneObjects(g.ctx, g.clusterSpec, g.kubeconfigClient) + g.Expect(err).To(Succeed()) + g.Expect(got).To(Equal([]runtime.Object{wantCAPICluster(), wantSnowCluster(), kcp, mt})) +} + func TestControlPlaneObjectsOldControlPlaneNotExists(t *testing.T) { g := newSnowTest(t) mt := wantSnowMachineTemplate() @@ -198,6 +237,65 @@ func TestWorkersObjects(t *testing.T) { g.Expect(got).To(Equal([]runtime.Object{md, wantKubeadmConfigTemplate(), mt})) } +func TestWorkersObjectsFromBetaMachineTemplateName(t *testing.T) { + g := newSnowTest(t) + mt := wantSnowMachineTemplate() + g.kubeconfigClient.EXPECT(). + Get( + g.ctx, + "snow-test-md-0", + constants.EksaSystemNamespace, + &clusterv1.MachineDeployment{}, + ). + DoAndReturn(func(_ context.Context, _, _ string, obj *clusterv1.MachineDeployment) error { + wantMachineDeployment().DeepCopyInto(obj) + obj.Spec.Template.Spec.InfrastructureRef.Name = "test-wn" + obj.Spec.Template.Spec.Bootstrap.ConfigRef.Name = "test-wn" + return nil + }) + g.kubeconfigClient.EXPECT(). + Get( + g.ctx, + "test-wn", + constants.EksaSystemNamespace, + &bootstrapv1.KubeadmConfigTemplate{}, + ). + DoAndReturn(func(_ context.Context, _, _ string, obj *bootstrapv1.KubeadmConfigTemplate) error { + wantKubeadmConfigTemplate().DeepCopyInto(obj) + obj.SetName("test-wn") + obj.Spec.Template.Spec.PreKubeadmCommands = []string{ + "new command", + } + return nil + }) + g.kubeconfigClient.EXPECT(). + Get( + g.ctx, + "test-wn", + constants.EksaSystemNamespace, + &snowv1.AWSSnowMachineTemplate{}, + ). + DoAndReturn(func(_ context.Context, _, _ string, obj *snowv1.AWSSnowMachineTemplate) error { + mt.DeepCopyInto(obj) + obj.SetName("test-wn") + obj.Spec.Template.Spec.InstanceType = "updated-instance-type" + return nil + }) + + wantMachineTemplateName := "snow-test-md-0-1" + mt.SetName(wantMachineTemplateName) + md := wantMachineDeployment() + md.Spec.Template.Spec.InfrastructureRef.Name = wantMachineTemplateName + kct := wantKubeadmConfigTemplate() + wantKctName := "snow-test-md-0-1" + kct.SetName(wantKctName) + md.Spec.Template.Spec.Bootstrap.ConfigRef.Name = wantKctName + + got, err := snow.WorkersObjects(g.ctx, g.clusterSpec, g.kubeconfigClient) + g.Expect(err).To(Succeed()) + g.Expect(got).To(Equal([]runtime.Object{md, kct, mt})) +} + func TestWorkersObjectsOldMachineDeploymentNotExists(t *testing.T) { g := newSnowTest(t) mt := wantSnowMachineTemplate() diff --git a/pkg/providers/snow/snow.go b/pkg/providers/snow/snow.go index f981b4867f58..8e1fe671f5bc 100644 --- a/pkg/providers/snow/snow.go +++ b/pkg/providers/snow/snow.go @@ -184,7 +184,7 @@ func (p *SnowProvider) EnvMap(clusterSpec *cluster.Spec) (map[string]string, err func (p *SnowProvider) GetDeployments() map[string][]string { return map[string][]string{ - "capas-system": {"capas-controller-manager"}, + constants.CapasSystemNamespace: {"capas-controller-manager"}, } } @@ -231,7 +231,15 @@ func (p *SnowProvider) GenerateMHC(clusterSpec *cluster.Spec) ([]byte, error) { } func (p *SnowProvider) ChangeDiff(currentSpec, newSpec *cluster.Spec) *types.ComponentChangeDiff { - return nil + if currentSpec.VersionsBundle.Snow.Version == newSpec.VersionsBundle.Snow.Version { + return nil + } + + return &types.ComponentChangeDiff{ + ComponentName: constants.SnowProviderName, + NewVersion: newSpec.VersionsBundle.Snow.Version, + OldVersion: currentSpec.VersionsBundle.Snow.Version, + } } func (p *SnowProvider) RunPostControlPlaneUpgrade(ctx context.Context, oldClusterSpec *cluster.Spec, clusterSpec *cluster.Spec, workloadCluster *types.Cluster, managementCluster *types.Cluster) error { diff --git a/pkg/providers/snow/snow_test.go b/pkg/providers/snow/snow_test.go index dafdc4c9db97..9e1e14812396 100644 --- a/pkg/providers/snow/snow_test.go +++ b/pkg/providers/snow/snow_test.go @@ -739,3 +739,25 @@ func TestUpgradeNeededMachineConfigs(t *testing.T) { }) } } + +func TestChangeDiffNoChange(t *testing.T) { + g := NewWithT(t) + provider := givenProvider(t) + clusterSpec := givenEmptyClusterSpec() + g.Expect(provider.ChangeDiff(clusterSpec, clusterSpec)).To(BeNil()) +} + +func TestChangeDiffWithChange(t *testing.T) { + g := NewWithT(t) + provider := givenProvider(t) + clusterSpec := givenEmptyClusterSpec() + newClusterSpec := clusterSpec.DeepCopy() + clusterSpec.VersionsBundle.Snow.Version = "v1.0.2" + newClusterSpec.VersionsBundle.Snow.Version = "v1.0.3" + want := &types.ComponentChangeDiff{ + ComponentName: "snow", + NewVersion: "v1.0.3", + OldVersion: "v1.0.2", + } + g.Expect(provider.ChangeDiff(clusterSpec, newClusterSpec)).To(Equal(want)) +}