Merge pull request openshift#9256 from r4f4/aws-edge-default-instance…

…-fix

OCPBUGS-45218: aws: fix perm requirement for edge nodes

pkg/asset/installconfig/aws/permissions.go

@@ -75,6 +75,10 @@ const (
 	// PermissionCarrierGateway is a permission set required when an edge compute pool with WL zones is set in the install-config.
 	PermissionCarrierGateway PermissionGroup = "permission-create-carrier-gateway"
 
+	// PermissionEdgeDefaultInstance is a permission set required when an edge compute pool is set without an instance
+	// type in the install-config.
+	PermissionEdgeDefaultInstance PermissionGroup = "permission-edge-default-instance"
+
 	// PermissionMintCreds is a permission set required when minting credentials.
 	PermissionMintCreds PermissionGroup = "permission-mint-creds"
 
@@ -347,6 +351,10 @@ var permissions = map[PermissionGroup][]string{
 		// Needed to delete Carrier Gateways.
 		"ec2:DeleteCarrierGateway",
 	},
+	PermissionEdgeDefaultInstance: {
+		// Needed to filter zones by instance type
+		"ec2:DescribeInstanceTypeOfferings",
+	},
 	// From: https://github.com/openshift/cloud-credential-operator/blob/master/pkg/aws/utils.go
 	// TODO: export these in CCO so we don't have to duplicate them here.
 	PermissionMintCreds: {
@@ -542,6 +550,10 @@ func RequiredPermissionGroups(ic *types.InstallConfig) []PermissionGroup {
 		permissionGroups = append(permissionGroups, PermissionCarrierGateway)
 	}
 
+	if includesEdgeDefaultInstanceType(ic) {
+		permissionGroups = append(permissionGroups, PermissionEdgeDefaultInstance)
+	}
+
 	return permissionGroups
 }
 
@@ -736,3 +748,16 @@ func includesWavelengthZones(installConfig *types.InstallConfig) bool {
 
 	return false
 }
+
+// includesEdgeDefaultInstanceType checks if any edge machine pool is specified without an instance type.
+func includesEdgeDefaultInstanceType(installConfig *types.InstallConfig) bool {
+	for _, mpool := range installConfig.Compute {
+		if mpool.Name != types.MachinePoolEdgeRoleName {
+			continue
+		}
+		if mpool.Platform.AWS == nil || len(mpool.Platform.AWS.InstanceType) == 0 {
+			return true
+		}
+	}
+	return false
+}

pkg/asset/installconfig/aws/permissions_test.go

@@ -857,3 +857,68 @@ func TestIncludesWavelengthZones(t *testing.T) {
 		})
 	})
 }
+
+func TestIncludesEdgeDefaultInstance(t *testing.T) {
+	t.Run("Should be true when at least one edge compute pool specified", func(t *testing.T) {
+		t.Run("without platform", func(t *testing.T) {
+			ic := validInstallConfig()
+			ic.Compute = append(ic.Compute, types.MachinePool{
+				Name: "edge",
+			})
+			ic.Compute = append(ic.Compute, types.MachinePool{
+				Name: "edge",
+				Platform: types.MachinePoolPlatform{
+					AWS: &aws.MachinePool{
+						Zones:        []string{"us-west-2-pdx-1a", "us-west-2-wl1-sea-wlz-1"},
+						InstanceType: "m6a.xlarge",
+					},
+				},
+			})
+			requiredPerms := RequiredPermissionGroups(ic)
+			assert.Contains(t, requiredPerms, PermissionEdgeDefaultInstance)
+		})
+		t.Run("without instance type", func(t *testing.T) {
+			ic := validInstallConfig()
+			ic.Compute = append(ic.Compute, types.MachinePool{
+				Name: "edge",
+				Platform: types.MachinePoolPlatform{
+					AWS: &aws.MachinePool{
+						Zones: []string{"us-west-2-pdx-1a", "us-west-2-wl1-sea-wlz-1"},
+					},
+				},
+			})
+			ic.Compute = append(ic.Compute, types.MachinePool{
+				Name: "edge",
+				Platform: types.MachinePoolPlatform{
+					AWS: &aws.MachinePool{
+						Zones:        []string{"us-west-2-pdx-1a", "us-west-2-wl1-sea-wlz-1"},
+						InstanceType: "m6a.xlarge",
+					},
+				},
+			})
+			requiredPerms := RequiredPermissionGroups(ic)
+			assert.Contains(t, requiredPerms, PermissionEdgeDefaultInstance)
+		})
+	})
+	t.Run("Should be false when", func(t *testing.T) {
+		t.Run("edge compute specified with instance type", func(t *testing.T) {
+			ic := validInstallConfig()
+			ic.Compute = append(ic.Compute, types.MachinePool{
+				Name: "edge",
+				Platform: types.MachinePoolPlatform{
+					AWS: &aws.MachinePool{
+						Zones:        []string{"us-west-1a", "us-west-2-pdx-1a"},
+						InstanceType: "m6a.xlarge",
+					},
+				},
+			})
+			requiredPerms := RequiredPermissionGroups(ic)
+			assert.NotContains(t, requiredPerms, PermissionEdgeDefaultInstance)
+		})
+		t.Run("edge compute not specified", func(t *testing.T) {
+			ic := validInstallConfig()
+			requiredPerms := RequiredPermissionGroups(ic)
+			assert.NotContains(t, requiredPerms, PermissionEdgeDefaultInstance)
+		})
+	})
+}

pkg/asset/machines/worker.go

@@ -405,10 +405,9 @@ func (w *Worker) Generate(ctx context.Context, dependencies asset.Parents) error
 				instanceTypes := awsdefaults.InstanceTypes(installConfig.Config.Platform.AWS.Region, arch, configv1.HighlyAvailableTopologyMode)
 				switch pool.Name {
 				case types.MachinePoolEdgeRoleName:
-					ok := awsSetPreferredInstanceByEdgeZone(ctx, instanceTypes, installConfig.AWS, zones)
-					if !ok {
-						logrus.Warnf("failed to find preferred instance type for one or more zones in the %s pool, using default: %s", pool.Name, instanceTypes[0])
-						mpool.InstanceType = instanceTypes[0]
+					if !awsSetPreferredInstanceByEdgeZone(ctx, instanceTypes, installConfig.AWS, zones) {
+						// Using the default instance type from the non-edge pool often fails.
+						return fmt.Errorf("failed to find instance type for one or more zones in the %s pool. Please specify an instance type in the install-config.yaml", pool.Name)
 					}
 				default:
 					mpool.InstanceType, err = aws.PreferredInstanceType(ctx, installConfig.AWS, instanceTypes, mpool.Zones)