Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions pkg/types/aws/validation/featuregates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
59 changes: 59 additions & 0 deletions pkg/types/aws/validation/featuregates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
19 changes: 6 additions & 13 deletions pkg/types/defaults/machinepools.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
}
Expand Down
73 changes: 34 additions & 39 deletions pkg/types/defaults/machinepools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -148,78 +149,72 @@ 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,
},
}

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")
Expand Down
27 changes: 14 additions & 13 deletions pkg/types/validation/featuregate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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$`,
},
}

Expand Down
17 changes: 0 additions & 17 deletions pkg/types/validation/featuregates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment thread
tthvo marked this conversation as resolved.
Resolved
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,
Expand Down
19 changes: 19 additions & 0 deletions pkg/types/validation/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
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{}
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/validation/machinepools.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading