diff --git a/pkg/types/aws/validation/featuregates.go b/pkg/types/aws/validation/featuregates.go index c3a8b85e53c..629a71f24f6 100644 --- a/pkg/types/aws/validation/featuregates.go +++ b/pkg/types/aws/validation/featuregates.go @@ -44,5 +44,17 @@ func GatedFeatures(c *types.InstallConfig) []featuregates.GatedInstallConfigFeat } } + // Check if any compute pool has ClusterAPI management configured + for idx, compute := range c.Compute { + if compute.Management == types.ClusterAPI { + gatedFeatures = append(gatedFeatures, featuregates.GatedInstallConfigFeature{ + FeatureGateName: features.FeatureGateClusterAPIMachineManagementAWS, + Condition: true, + Field: field.NewPath("compute").Index(idx).Child("management"), + }) + break + } + } + return gatedFeatures } diff --git a/pkg/types/aws/validation/featuregates_test.go b/pkg/types/aws/validation/featuregates_test.go index 301a155a27e..35984e4b479 100644 --- a/pkg/types/aws/validation/featuregates_test.go +++ b/pkg/types/aws/validation/featuregates_test.go @@ -44,6 +44,65 @@ func TestGatedFeatures(t *testing.T) { }, expectedFeatureGates: []configv1.FeatureGateName{}, }, + { + name: "CAPI management configured for worker pool", + installConfig: &types.InstallConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: types.InstallConfigVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + Platform: types.Platform{ + AWS: &aws.Platform{ + Region: "us-east-1", + }, + }, + Compute: []types.MachinePool{ + { + Name: types.MachinePoolComputeRoleName, + Management: types.ClusterAPI, + Platform: types.MachinePoolPlatform{ + AWS: &aws.MachinePool{}, + }, + }, + }, + }, + expectedFeatureGates: []configv1.FeatureGateName{features.FeatureGateClusterAPIMachineManagementAWS}, + }, + { + name: "CAPI management configured for edge pool", + installConfig: &types.InstallConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: types.InstallConfigVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + Platform: types.Platform{ + AWS: &aws.Platform{ + Region: "us-east-1", + }, + }, + Compute: []types.MachinePool{ + { + Name: types.MachinePoolComputeRoleName, + Management: types.MachineAPI, + Platform: types.MachinePoolPlatform{ + AWS: &aws.MachinePool{}, + }, + }, + { + Name: types.MachinePoolEdgeRoleName, + Management: types.ClusterAPI, + Platform: types.MachinePoolPlatform{ + AWS: &aws.MachinePool{}, + }, + }, + }, + }, + expectedFeatureGates: []configv1.FeatureGateName{features.FeatureGateClusterAPIMachineManagementAWS}, + }, { name: "dedicated hosts configured", installConfig: &types.InstallConfig{ diff --git a/pkg/types/defaults/machinepools.go b/pkg/types/defaults/machinepools.go index 9af35b5c82a..9bb18118adc 100644 --- a/pkg/types/defaults/machinepools.go +++ b/pkg/types/defaults/machinepools.go @@ -40,21 +40,14 @@ func SetMachinePoolDefaults(p *types.MachinePool, platform *types.Platform, fgat } } - // Set management to ClusterAPI if the appropriate feature gate is enabled and management is unspecified - if p.Management == "" { - if p.Name == types.MachinePoolControlPlaneRoleName && fgates.Enabled(features.FeatureGateClusterAPIControlPlaneInstall) { - p.Management = types.ClusterAPI - } - if p.Name == types.MachinePoolComputeRoleName && fgates.Enabled(features.FeatureGateClusterAPIComputeInstall) { - p.Management = types.ClusterAPI - } - if p.Name == types.MachinePoolEdgeRoleName && fgates.Enabled(features.FeatureGateClusterAPIComputeInstall) { - p.Management = types.ClusterAPI - } - } - switch platform.Name() { case aws.Name: + if p.Management == "" { + if (p.Name == types.MachinePoolComputeRoleName || p.Name == types.MachinePoolEdgeRoleName) && + fgates.Enabled(features.FeatureGateClusterAPIMachineManagementAWS) { + p.Management = types.ClusterAPI + } + } if p.Platform.AWS == nil && platform.AWS.DefaultMachinePlatform != nil { p.Platform.AWS = &aws.MachinePool{} } diff --git a/pkg/types/defaults/machinepools_test.go b/pkg/types/defaults/machinepools_test.go index d520515e9b4..eb90161e349 100644 --- a/pkg/types/defaults/machinepools_test.go +++ b/pkg/types/defaults/machinepools_test.go @@ -7,6 +7,7 @@ import ( configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/aws" "github.com/openshift/installer/pkg/types/gcp" ) @@ -148,70 +149,63 @@ func TestSetMachinePoolDefaultsWithFeatureGates(t *testing.T) { name string pool *types.MachinePool platform *types.Platform - featureSet configv1.FeatureSet + featureGates []string expectedManagement types.MachineManagementAPI }{ { - name: "control plane with DevPreviewNoUpgrade feature set", - pool: &types.MachinePool{Name: types.MachinePoolControlPlaneRoleName}, - platform: &types.Platform{}, - featureSet: configv1.DevPreviewNoUpgrade, - expectedManagement: types.ClusterAPI, - }, - { - name: "control plane with default feature set", - pool: &types.MachinePool{Name: types.MachinePoolControlPlaneRoleName}, - platform: &types.Platform{}, - featureSet: configv1.Default, - expectedManagement: "", - }, - { - name: "compute with DevPreviewNoUpgrade feature set", + name: "AWS compute sets ClusterAPI management when ClusterAPIMachineManagementAWS enabled", pool: &types.MachinePool{Name: types.MachinePoolComputeRoleName}, - platform: &types.Platform{}, - featureSet: configv1.DevPreviewNoUpgrade, + platform: &types.Platform{AWS: &aws.Platform{}}, + featureGates: []string{"ClusterAPIMachineManagementAWS=True"}, expectedManagement: types.ClusterAPI, }, { - name: "compute with default feature set", + name: "AWS compute management is unchanged when ClusterAPIMachineManagementAWS disabled", pool: &types.MachinePool{Name: types.MachinePoolComputeRoleName}, - platform: &types.Platform{}, - featureSet: configv1.Default, + platform: &types.Platform{AWS: &aws.Platform{}}, + featureGates: []string{"ClusterAPIMachineManagementAWS=False"}, expectedManagement: "", }, { - name: "edge compute with DevPreviewNoUpgrade feature set", + name: "AWS edge compute sets ClusterAPI management when ClusterAPIMachineManagementAWS enabled", pool: &types.MachinePool{Name: types.MachinePoolEdgeRoleName}, - platform: &types.Platform{}, - featureSet: configv1.DevPreviewNoUpgrade, + platform: &types.Platform{AWS: &aws.Platform{}}, + featureGates: []string{"ClusterAPIMachineManagementAWS=True"}, expectedManagement: types.ClusterAPI, }, { - name: "edge compute with default feature set", + name: "AWS edge compute management is unchanged when ClusterAPIMachineManagementAWS disabled", pool: &types.MachinePool{Name: types.MachinePoolEdgeRoleName}, - platform: &types.Platform{}, - featureSet: configv1.Default, + platform: &types.Platform{AWS: &aws.Platform{}}, + featureGates: []string{"ClusterAPIMachineManagementAWS=False"}, expectedManagement: "", }, { - name: "control plane with management already set", - pool: &types.MachinePool{Name: types.MachinePoolControlPlaneRoleName, Management: types.MachineAPI}, - platform: &types.Platform{}, - featureSet: configv1.DevPreviewNoUpgrade, - expectedManagement: types.MachineAPI, + name: "AWS control plane is unaffected by ClusterAPIMachineManagementAWS", + pool: &types.MachinePool{Name: types.MachinePoolControlPlaneRoleName}, + platform: &types.Platform{AWS: &aws.Platform{}}, + featureGates: []string{"ClusterAPIMachineManagementAWS=True"}, + expectedManagement: "", + }, + { + name: "non-AWS compute is unaffected by ClusterAPIMachineManagementAWS", + pool: &types.MachinePool{Name: types.MachinePoolComputeRoleName}, + platform: &types.Platform{GCP: &gcp.Platform{}}, + featureGates: []string{"ClusterAPIMachineManagementAWS=True"}, + expectedManagement: "", }, { - name: "compute with management already set", + name: "AWS compute management is not overwritten when already set", pool: &types.MachinePool{Name: types.MachinePoolComputeRoleName, Management: types.MachineAPI}, - platform: &types.Platform{}, - featureSet: configv1.DevPreviewNoUpgrade, + platform: &types.Platform{AWS: &aws.Platform{}}, + featureGates: []string{"ClusterAPIMachineManagementAWS=True"}, expectedManagement: types.MachineAPI, }, { - name: "edge compute with management already set", + name: "AWS edge compute management is not overwritten when already set", pool: &types.MachinePool{Name: types.MachinePoolEdgeRoleName, Management: types.MachineAPI}, - platform: &types.Platform{}, - featureSet: configv1.DevPreviewNoUpgrade, + platform: &types.Platform{AWS: &aws.Platform{}}, + featureGates: []string{"ClusterAPIMachineManagementAWS=True"}, expectedManagement: types.MachineAPI, }, } @@ -219,7 +213,8 @@ func TestSetMachinePoolDefaultsWithFeatureGates(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { config := &types.InstallConfig{ - FeatureSet: tc.featureSet, + FeatureSet: configv1.CustomNoUpgrade, + FeatureGates: tc.featureGates, } SetMachinePoolDefaults(tc.pool, tc.platform, config.EnabledFeatureGates()) assert.Equal(t, tc.expectedManagement, tc.pool.Management, "unexpected management API") diff --git a/pkg/types/validation/featuregate_test.go b/pkg/types/validation/featuregate_test.go index 49aa710d930..b1ff2be0b02 100644 --- a/pkg/types/validation/featuregate_test.go +++ b/pkg/types/validation/featuregate_test.go @@ -267,41 +267,42 @@ func TestFeatureGates(t *testing.T) { }(), }, { - name: "Control Plane CAPI machine management is allowed with DevPreviewNoUpgrade Feature Set", + name: "Compute CAPI machine management is allowed with DevPreviewNoUpgrade Feature Set", installConfig: func() *types.InstallConfig { c := validInstallConfig() c.FeatureSet = v1.DevPreviewNoUpgrade - c.ControlPlane.Management = types.ClusterAPI + c.Compute[0].Management = types.ClusterAPI return c }(), }, { - name: "Compute CAPI machine management is allowed with DevPreviewNoUpgrade Feature Set", + name: "Edge Compute CAPI machine management is allowed with DevPreviewNoUpgrade Feature Set", installConfig: func() *types.InstallConfig { c := validInstallConfig() c.FeatureSet = v1.DevPreviewNoUpgrade - c.Compute[0].Management = types.ClusterAPI + c.Compute = append(c.Compute, *validMachinePool("edge")) + c.Compute[1].Management = types.ClusterAPI return c }(), }, { - name: "Edge Compute CAPI machine management is allowed with DevPreviewNoUpgrade Feature Set", + name: "Compute CAPI machine management is allowed with TechPreviewNoUpgrade Feature Set", installConfig: func() *types.InstallConfig { c := validInstallConfig() - c.FeatureSet = v1.DevPreviewNoUpgrade - c.Compute = append(c.Compute, *validMachinePool("edge")) - c.Compute[1].Management = types.ClusterAPI + c.FeatureSet = v1.TechPreviewNoUpgrade + c.Compute[0].Management = types.ClusterAPI return c }(), }, { - name: "Control Plane CAPI machine management is not allowed with Default Feature Set", + name: "Edge Compute CAPI machine management is allowed with TechPreviewNoUpgrade Feature Set", installConfig: func() *types.InstallConfig { c := validInstallConfig() - c.ControlPlane.Management = types.ClusterAPI + c.FeatureSet = v1.TechPreviewNoUpgrade + c.Compute = append(c.Compute, *validMachinePool("edge")) + c.Compute[1].Management = types.ClusterAPI return c }(), - expected: `^controlPlane.management: Forbidden: this field is protected by the ClusterAPIControlPlaneInstall feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set$`, }, { name: "Compute CAPI machine management is not allowed with the Default Feature Set", @@ -310,7 +311,7 @@ func TestFeatureGates(t *testing.T) { c.Compute[0].Management = types.ClusterAPI return c }(), - expected: `^compute.management: Forbidden: this field is protected by the ClusterAPIComputeInstall feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set$`, + expected: `^\Qcompute[0].management: Forbidden: this field is protected by the ClusterAPIMachineManagementAWS feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set\E$`, }, { name: "Edge compute CAPI machine management is not allowed with the Default Feature Set", @@ -320,7 +321,7 @@ func TestFeatureGates(t *testing.T) { c.Compute[1].Management = types.ClusterAPI return c }(), - expected: `^compute.management: Forbidden: this field is protected by the ClusterAPIComputeInstall feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set$`, + expected: `^\Qcompute[1].management: Forbidden: this field is protected by the ClusterAPIMachineManagementAWS feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set\E$`, }, } diff --git a/pkg/types/validation/featuregates.go b/pkg/types/validation/featuregates.go index 3e276510375..8d9efa6a10c 100644 --- a/pkg/types/validation/featuregates.go +++ b/pkg/types/validation/featuregates.go @@ -41,23 +41,6 @@ func validateMachinePoolFeatureGates(c *types.InstallConfig) []featuregates.Gate Condition: c.OSImageStream != rhcos.GetDefaultOSImageStream(c), Field: field.NewPath("osImageStream"), }, - { - FeatureGateName: features.FeatureGateClusterAPIControlPlaneInstall, - Condition: c.ControlPlane != nil && c.ControlPlane.Management == types.ClusterAPI, - Field: field.NewPath("controlPlane", "management"), - }, - { - FeatureGateName: features.FeatureGateClusterAPIComputeInstall, - Condition: func() bool { - for _, compute := range c.Compute { - if compute.Management == types.ClusterAPI { - return true - } - } - return false - }(), - Field: field.NewPath("compute", "management"), - }, { FeatureGateName: features.FeatureGateConfigurablePKI, Condition: c.PKI != nil, diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index ac4ab5f640d..14f97978b36 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -1689,6 +1689,25 @@ func validateGatedFeatures(c *types.InstallConfig) field.ErrorList { return allErrs } +func validateMachineManagement(platform *types.Platform, p *types.MachinePool, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if p.Management != types.ClusterAPI { + return allErrs + } + + switch platform.Name() { + case aws.Name: + // ATM, ClusterAPI management is only supported for worker and edge compute pool + if p.Name == types.MachinePoolControlPlaneRoleName { + allErrs = append(allErrs, field.Invalid(fldPath, p.Management, fmt.Sprintf("%s machines cannot be managed by Cluster API", p.Name))) + } + default: + allErrs = append(allErrs, field.Invalid(fldPath, p.Management, fmt.Sprintf("machines cannot be managed by Cluster API for platform %s", platform.Name()))) + } + return allErrs +} + // validateReleaseArchitecture ensures a compatible payload is used according to the desired architecture of the cluster. func validateReleaseArchitecture(controlPlanePool *types.MachinePool, computePool []types.MachinePool, releaseArch types.Architecture) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/types/validation/machinepools.go b/pkg/types/validation/machinepools.go index 160e58a2d65..5b5afa46db8 100644 --- a/pkg/types/validation/machinepools.go +++ b/pkg/types/validation/machinepools.go @@ -79,7 +79,7 @@ func ValidateMachinePool(platform *types.Platform, p *types.MachinePool, fldPath } allErrs = append(allErrs, validateDiskSetup(p, fldPath.Child("diskSetup"))...) - + allErrs = append(allErrs, validateMachineManagement(platform, p, fldPath.Child("management"))...) allErrs = append(allErrs, validateMachinePoolPlatform(platform, &p.Platform, p, fldPath.Child("platform"))...) return allErrs } diff --git a/pkg/types/validation/machinepools_test.go b/pkg/types/validation/machinepools_test.go index d8119ed73ab..913e8a2f61b 100644 --- a/pkg/types/validation/machinepools_test.go +++ b/pkg/types/validation/machinepools_test.go @@ -251,6 +251,46 @@ func TestValidateMachinePool(t *testing.T) { }(), valid: true, }, + { + name: "valid CAPI management on AWS worker", + platform: &types.Platform{AWS: &aws.Platform{Region: "us-east-1"}}, + pool: func() *types.MachinePool { + p := validMachinePool(types.MachinePoolComputeRoleName) + p.Management = types.ClusterAPI + return p + }(), + valid: true, + }, + { + name: "valid CAPI management on AWS edge", + platform: &types.Platform{AWS: &aws.Platform{Region: "us-east-1"}}, + pool: func() *types.MachinePool { + p := validMachinePool(types.MachinePoolEdgeRoleName) + p.Management = types.ClusterAPI + return p + }(), + valid: true, + }, + { + name: "invalid CAPI management on AWS master", + platform: &types.Platform{AWS: &aws.Platform{Region: "us-east-1"}}, + pool: func() *types.MachinePool { + p := validMachinePool(types.MachinePoolControlPlaneRoleName) + p.Management = types.ClusterAPI + return p + }(), + expectedError: `master machines cannot be managed by Cluster API`, + }, + { + name: "invalid CAPI management on GCP", + platform: &types.Platform{GCP: &gcp.Platform{Region: "us-east-1"}}, + pool: func() *types.MachinePool { + p := validMachinePool(types.MachinePoolComputeRoleName) + p.Management = types.ClusterAPI + return p + }(), + expectedError: `machines cannot be managed by Cluster API for platform gcp`, + }, { name: "valid multiple disks", platform: &types.Platform{Azure: &azure.Platform{Region: "eastus"}},