diff --git a/pkg/providers/tinkerbell/upgrade.go b/pkg/providers/tinkerbell/upgrade.go index 29633a179ba7..5c05901ba08b 100644 --- a/pkg/providers/tinkerbell/upgrade.go +++ b/pkg/providers/tinkerbell/upgrade.go @@ -9,6 +9,7 @@ import ( rufiov1 "github.com/tinkerbell/rufio/api/v1alpha1" tinkv1alpha1 "github.com/tinkerbell/tink/pkg/apis/core/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kerrors "k8s.io/apimachinery/pkg/util/errors" "github.com/aws/eks-anywhere/pkg/api/v1alpha1" "github.com/aws/eks-anywhere/pkg/cluster" @@ -244,7 +245,22 @@ func (p *Provider) ValidateNewSpec(ctx context.Context, cluster *types.Cluster, oSpec := prevDatacenterConfig.Spec nSpec := p.datacenterConfig.Spec - prevMachineConfigRefs := machineRefSliceToMap(prevSpec.MachineConfigRefs()) + currCPMachineRef := prevSpec.Spec.ControlPlaneConfiguration.MachineGroupRef + desiCPMachineRef := clusterSpec.Cluster.Spec.ControlPlaneConfiguration.MachineGroupRef + if !currCPMachineRef.Equal(desiCPMachineRef) { + return errors.New("control plane machine config reference is immutable") + } + + err = validateWorkerNodeGroupMachineConfigRefsUnchanged(prevSpec.Spec.WorkerNodeGroupConfigurations, + clusterSpec.Cluster.Spec.WorkerNodeGroupConfigurations) + if err != nil { + return err + } + + // Validate worker node group machine configs are the same. + prevMachineConfigRefs := collection.ToMap(prevSpec.MachineConfigRefs(), func(v v1alpha1.Ref) string { + return v.Name + }) desiredWorkerNodeGroups := clusterSpec.Cluster.Spec.WorkerNodeGroupConfigurations currentWorkerNodeGroups := collection.ToMap(prevSpec.Spec.WorkerNodeGroupConfigurations, @@ -279,7 +295,7 @@ func (p *Provider) ValidateNewSpec(ctx context.Context, cluster *types.Cluster, } if nSpec.TinkerbellIP != oSpec.TinkerbellIP { - return fmt.Errorf("spec.TinkerbellIP is immutable. Previous value %s, New value %s", oSpec.TinkerbellIP, nSpec.TinkerbellIP) + return fmt.Errorf("spec.tinkerbellIP is immutable; previous = %s, new = %s", oSpec.TinkerbellIP, nSpec.TinkerbellIP) } // for any operation other than k8s version change, hookImageURL is immutable @@ -292,6 +308,30 @@ func (p *Provider) ValidateNewSpec(ctx context.Context, cluster *types.Cluster, return nil } +func validateWorkerNodeGroupMachineConfigRefsUnchanged(current, desired []v1alpha1.WorkerNodeGroupConfiguration) error { + lookup := collection.ToMap(desired, func(v v1alpha1.WorkerNodeGroupConfiguration) string { + return v.Name + }) + + var errs []error + + // For every current worker node group that still exists in the desired config, ensure the + // machine config is still the same. + for _, curr := range current { + desi, exists := lookup[curr.Name] + if !exists { + continue + } + + if !curr.MachineGroupRef.Equal(desi.MachineGroupRef) { + errs = append(errs, + fmt.Errorf("%v: worker node group machine config reference is immutable", curr.Name)) + } + } + + return kerrors.NewAggregate(errs) +} + func (p *Provider) UpgradeNeeded(_ context.Context, _, _ *cluster.Spec, _ *types.Cluster) (bool, error) { // TODO: Figure out if something is needed here return false, nil @@ -368,14 +408,6 @@ func (p *Provider) validateMachineConfigImmutability(ctx context.Context, cluste return nil } -func machineRefSliceToMap(machineRefs []v1alpha1.Ref) map[string]v1alpha1.Ref { - refMap := make(map[string]v1alpha1.Ref, len(machineRefs)) - for _, ref := range machineRefs { - refMap[ref.Name] = ref - } - return refMap -} - // PreCoreComponentsUpgrade staisfies the Provider interface. func (p *Provider) PreCoreComponentsUpgrade( ctx context.Context, diff --git a/pkg/providers/tinkerbell/upgrade_test.go b/pkg/providers/tinkerbell/upgrade_test.go index 8b24ac175f7a..053706a1bc07 100644 --- a/pkg/providers/tinkerbell/upgrade_test.go +++ b/pkg/providers/tinkerbell/upgrade_test.go @@ -799,6 +799,86 @@ func TestProvider_ValidateNewSpec_ChangeWorkerNodeGroupMachineRef(t *testing.T) docker, helm, kubectl) provider.stackInstaller = stackInstaller + kubectl.EXPECT(). + GetEksaCluster(ctx, desiredClusterSpec.ManagementCluster, + desiredClusterSpec.Cluster.Spec.ManagementCluster.Name). + Return(currentClusterSpec.Cluster, nil) + kubectl.EXPECT(). + GetEksaTinkerbellDatacenterConfig(ctx, currentClusterSpec.Cluster.Spec.DatacenterRef.Name, + cluster.KubeconfigFile, currentClusterSpec.Cluster.Namespace). + Return(datacenterConfig, nil) + + err := provider.ValidateNewSpec(ctx, desiredClusterSpec.ManagementCluster, desiredClusterSpec) + if err == nil || !strings.Contains(err.Error(), "worker node group machine config reference is immutable") { + t.Fatalf("Expected error containing 'worker node group machine config reference is immutable' but received: %v", err) + } +} + +func TestProvider_ValidateNewSpec_ChangeControlPlaneMachineRefToExisting(t *testing.T) { + clusterSpecManifest := "cluster_tinkerbell_stacked_etcd.yaml" + mockCtrl := gomock.NewController(t) + currentClusterSpec := givenClusterSpec(t, clusterSpecManifest) + datacenterConfig := givenDatacenterConfig(t, clusterSpecManifest) + machineConfigs := givenMachineConfigs(t, clusterSpecManifest) + docker := stackmocks.NewMockDocker(mockCtrl) + helm := stackmocks.NewMockHelm(mockCtrl) + kubectl := mocks.NewMockProviderKubectlClient(mockCtrl) + stackInstaller := stackmocks.NewMockStackInstaller(mockCtrl) + writer := filewritermocks.NewMockFileWriter(mockCtrl) + ctx := context.Background() + + cluster := &types.Cluster{Name: "test", KubeconfigFile: "kubeconfig-file"} + currentClusterSpec.ManagementCluster = cluster + + desiredClusterSpec := currentClusterSpec.DeepCopy() + + // Change an existing worker node groups machine config reference to an existing machine config. + desiredClusterSpec.Cluster.Spec.ControlPlaneConfiguration.MachineGroupRef.Name = "test-md" + + provider := newTinkerbellProvider(datacenterConfig, machineConfigs, currentClusterSpec.Cluster, writer, + docker, helm, kubectl) + provider.stackInstaller = stackInstaller + + kubectl.EXPECT(). + GetEksaCluster(ctx, desiredClusterSpec.ManagementCluster, + desiredClusterSpec.Cluster.Spec.ManagementCluster.Name). + Return(currentClusterSpec.Cluster, nil) + kubectl.EXPECT(). + GetEksaTinkerbellDatacenterConfig(ctx, currentClusterSpec.Cluster.Spec.DatacenterRef.Name, + cluster.KubeconfigFile, currentClusterSpec.Cluster.Namespace). + Return(datacenterConfig, nil) + + err := provider.ValidateNewSpec(ctx, desiredClusterSpec.ManagementCluster, desiredClusterSpec) + if err == nil || !strings.Contains(err.Error(), "control plane machine config reference is immutable") { + t.Fatalf("Expected error containing 'control plane machine config reference is immutable' but received: %v", err) + } +} + +func TestProvider_ValidateNewSpec_ChangeWorkerNodeGroupMachineRefToExisting(t *testing.T) { + clusterSpecManifest := "cluster_tinkerbell_stacked_etcd.yaml" + mockCtrl := gomock.NewController(t) + currentClusterSpec := givenClusterSpec(t, clusterSpecManifest) + datacenterConfig := givenDatacenterConfig(t, clusterSpecManifest) + machineConfigs := givenMachineConfigs(t, clusterSpecManifest) + docker := stackmocks.NewMockDocker(mockCtrl) + helm := stackmocks.NewMockHelm(mockCtrl) + kubectl := mocks.NewMockProviderKubectlClient(mockCtrl) + stackInstaller := stackmocks.NewMockStackInstaller(mockCtrl) + writer := filewritermocks.NewMockFileWriter(mockCtrl) + ctx := context.Background() + + cluster := &types.Cluster{Name: "test", KubeconfigFile: "kubeconfig-file"} + currentClusterSpec.ManagementCluster = cluster + + desiredClusterSpec := currentClusterSpec.DeepCopy() + + // Change an existing worker node groups machine config reference to an existing machine config. + desiredClusterSpec.Cluster.Spec.WorkerNodeGroupConfigurations[0].MachineGroupRef.Name = "test-cp" + + provider := newTinkerbellProvider(datacenterConfig, machineConfigs, currentClusterSpec.Cluster, writer, + docker, helm, kubectl) + provider.stackInstaller = stackInstaller + kubectl.EXPECT(). GetEksaCluster(ctx, desiredClusterSpec.ManagementCluster, desiredClusterSpec.Cluster.Spec.ManagementCluster.Name). @@ -820,8 +900,8 @@ func TestProvider_ValidateNewSpec_ChangeWorkerNodeGroupMachineRef(t *testing.T) AnyTimes() err := provider.ValidateNewSpec(ctx, desiredClusterSpec.ManagementCluster, desiredClusterSpec) - if err == nil || !strings.Contains(err.Error(), "cannot add or remove MachineConfigs") { - t.Fatalf("Expected error containing 'cannot add or remove MachineConfigs' but received: %v", err) + if err == nil || !strings.Contains(err.Error(), "worker node group machine config reference is immutable") { + t.Fatalf("Expected error containing 'worker node group machine config reference is immutable' but received: %v", err) } } diff --git a/pkg/validations/upgradevalidations/preflightvalidations_test.go b/pkg/validations/upgradevalidations/preflightvalidations_test.go index 1bc99bb7d334..ba045e2db2a8 100644 --- a/pkg/validations/upgradevalidations/preflightvalidations_test.go +++ b/pkg/validations/upgradevalidations/preflightvalidations_test.go @@ -201,7 +201,7 @@ func TestPreflightValidationsTinkerbell(t *testing.T) { workerResponse: nil, nodeResponse: nil, crdResponse: nil, - wantErr: composeError("spec.TinkerbellIP is immutable. Previous value 4.5.6.7, New value 1.2.3.4"), + wantErr: composeError("spec.tinkerbellIP is immutable; previous = 4.5.6.7, new = 1.2.3.4"), modifyDatacenterFunc: func(s *anywherev1.TinkerbellDatacenterConfig) { s.Spec.TinkerbellIP = "4.5.6.7" },