SPLAT-2167: Added dedicated host support for AWS

manifests/0000_30_cluster-api_01_credentials-request.yaml

@@ -20,18 +20,21 @@ spec:
     statementEntries:
     - effect: Allow
       action:
+      - ec2:AllocateHosts
       - ec2:CreateTags
       - ec2:DescribeAvailabilityZones
       - ec2:DescribeDhcpOptions
       - ec2:DescribeImages
       - ec2:DescribeInstances
+      - ec2:DescribeInstanceTypes
       - ec2:DescribeInternetGateways
       - ec2:DescribeSecurityGroups
       - ec2:DescribeSubnets
       - ec2:DescribeVpcs
       - ec2:DescribeNetworkInterfaces
       - ec2:DescribeNetworkInterfaceAttribute
       - ec2:ModifyNetworkInterfaceAttribute
+      - ec2:ReleaseHosts
       - ec2:RunInstances
       - ec2:TerminateInstances
       - elasticloadbalancing:DescribeLoadBalancers

pkg/controllers/machinesetsync/machineset_sync_controller.go

@@ -1332,10 +1332,6 @@ func compareCAPIInfraMachineTemplates(platform configv1.PlatformType, infraMachi
 
 	switch platform {
 	case configv1.AWSPlatformType:
-		diffOpts = append(diffOpts,
-			util.WithIgnoreField("spec", "template", "spec", "hostID"),
-			util.WithIgnoreField("spec", "template", "spec", "hostAffinity"),
-		)
 	case configv1.OpenStackPlatformType:
 	case configv1.PowerVSPlatformType:
 	default:

pkg/controllers/machinesync/machine_sync_controller.go

@@ -1506,7 +1506,7 @@ func setChangedMAPIMachineStatusFields(existingMAPIMachine, convertedMAPIMachine
 	convertedMAPIMachine.Status.AuthoritativeAPI = existingMAPIMachine.Status.AuthoritativeAPI
 	convertedMAPIMachine.Status.SynchronizedGeneration = existingMAPIMachine.Status.SynchronizedGeneration
 	convertedMAPIMachine.Status.LastOperation = existingMAPIMachine.Status.LastOperation
-	convertedMAPIMachine.Status.ProviderStatus = existingMAPIMachine.Status.ProviderStatus
+	// ProviderStatus is handled separately by setChangedMAPIMachineProviderStatusFields
 
 	// Finally overwrite the entire existingMAPIMachine status with the convertedMAPIMachine status.
 	existingMAPIMachine.Status = convertedMAPIMachine.Status

pkg/controllers/machinesync/machine_sync_mapi2capi_infrastructure.go

@@ -252,15 +252,6 @@ func compareCAPIInfraMachines(platform configv1.PlatformType, infraMachine1, inf
 	// Make per provider adjustments to the differ.
 	switch platform {
 	case configv1.AWSPlatformType:
-		// Ignore HostAffinity and HostID fields to prevent conversion loops.
-		// CAPA defaults HostAffinity to "host" when not set, which would cause
-		// continuous drift detection.
-		// TODO: These fields will be properly converted
-		// when MAPI HostPlacement feature is stable.
-		diffOpts = append(diffOpts,
-			util.WithIgnoreField("spec", "hostID"),
-			util.WithIgnoreField("spec", "hostAffinity"),
-		)
 	case configv1.OpenStackPlatformType:
 	case configv1.PowerVSPlatformType:
 	default:

pkg/conversion/capi2mapi/aws.go

@@ -19,6 +19,8 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"regexp"
+	"sort"
 	"strings"
 
 	mapiv1beta1 "github.com/openshift/api/machine/v1beta1"
@@ -40,14 +42,27 @@ var (
 	errCAPIMachineSetAWSMachineTemplateAWSClusterCannotBeNil = errors.New("provided MachineSet, AWSMachineTemplate and AWSCluster can not be nil")
 	errNilLoadBalancer                                       = errors.New("nil load balancer")
 	errUnsupportedLoadBalancerType                           = errors.New("unsupported load balancer type")
+
+	// awsDedicatedHostNamePattern is used to validate the id of a dedicated host.
+	awsDedicatedHostNamePattern = regexp.MustCompile(`^h-(?:[0-9a-f]{8}|[0-9a-f]{17})$`)
 )
 
 const (
-	errUnsupportedCAPATenancy     = "unable to convert tenancy, unknown value"
-	errUnsupportedCAPAMarketType  = "unable to convert market type, unknown value"
-	errUnsupportedHTTPTokensState = "unable to convert httpTokens state, unknown value" //nolint:gosec // This is an error message, not a credential
-	defaultIdentityName           = "default"
-	defaultCredentialsSecretName  = "aws-cloud-credentials" //#nosec G101 -- False positive, not actually a credential.
+	errUnsupportedCAPATenancy      = "unable to convert tenancy, unknown value"
+	errUnsupportedCAPAMarketType   = "unable to convert market type, unknown value"
+	errUnsupportedHTTPTokensState  = "unable to convert httpTokens state, unknown value" //nolint:gosec // This is an error message, not a credential
+	defaultIdentityName            = "default"
+	defaultCredentialsSecretName   = "aws-cloud-credentials" //#nosec G101 -- False positive, not actually a credential.
+	errUnsupportedHostAffinityType = "unable to convert hostAffinity, unknown value"
+	errHostIDRequired              = "id is required and must start with 'h-' followed by 8 or 17 lowercase hexadecimal characters (0-9 and a-f)"
+	errHostIDInvalidFormat         = "id must start with 'h-' followed by 8 or 17 lowercase hexadecimal characters (0-9 and a-f)"
+
+	// TenancyDefault default setting for tenancy.
+	TenancyDefault = "default"
+	// TenancyDedicated dedicated setting for tenancy.
+	TenancyDedicated = "dedicated"
+	// TenancyHost host setting for tenancy.
+	TenancyHost = "host"
 )
 
 // machineAndAWSMachineAndAWSCluster stores the details of a Cluster API Machine and AWSMachine and AWSCluster.
@@ -174,6 +189,12 @@ func (m machineAndAWSMachineAndAWSCluster) toProviderSpec() (*mapiv1beta1.AWSMac
 		MarketType:              mapiAWSMarketType,
 	}
 
+	// Dedicated host support
+	mapaProviderConfig.Placement.Host, errs = convertAWSDedicatedHostToMAPI(m.awsMachine.Spec, mapaTenancy, fldPath)
+	if len(errs) > 0 {
+		errors = append(errors, errs...)
+	}
+
 	secretRef, errs := handleAWSIdentityRef(fldPath.Child("identityRef"), m.awsCluster.Spec.IdentityRef)
 
 	if len(errs) > 0 {
@@ -220,9 +241,164 @@ func (m machineAndAWSMachineAndAWSCluster) toProviderStatus() *mapiv1beta1.AWSMa
 		Conditions:    convertCAPAMachineConditionsToMAPIMachineAWSProviderConditions(m.awsMachine),
 	}
 
+	// Convert DedicatedHost status if present
+	if m.awsMachine.Status.DedicatedHost != nil {
+		dedicatedHostID := ptr.Deref(m.awsMachine.Status.DedicatedHost.ID, "")
+		if dedicatedHostID != "" {
+			s.DedicatedHost = &mapiv1beta1.DedicatedHostStatus{
+				ID: dedicatedHostID,
+			}
+		}
+	}
+
 	return s
 }
 
+func convertDynamicHostAllocationTags(dynamicHostAllocation *awsv1.DynamicHostAllocationSpec) *mapiv1beta1.DynamicHostAllocationSpec {
+	if dynamicHostAllocation == nil || dynamicHostAllocation.Tags == nil {
+		return nil
+	}
+
+	// Collect keys first to ensure deterministic ordering
+	keys := make([]string, 0, len(dynamicHostAllocation.Tags))
+	for key := range dynamicHostAllocation.Tags {
+		keys = append(keys, key)
+	}
+
+	sort.Strings(keys)
+
+	// Build tags slice in sorted order
+	tags := make([]mapiv1beta1.TagSpecification, 0, len(keys))
+	for _, key := range keys {
+		tags = append(tags, mapiv1beta1.TagSpecification{
+			Name:  key,
+			Value: dynamicHostAllocation.Tags[key],
+		})
+	}
+
+	// Only create DynamicHostAllocationSpec if there are tags.
+	// The MAPI API has MinProperties=1 validation, so an empty struct is invalid.
+	if len(tags) == 0 {
+		return nil
+	}
+
+	return &mapiv1beta1.DynamicHostAllocationSpec{
+		Tags: &tags,
+	}
+}
+
+func convertHostAffinityHost(spec awsv1.AWSMachineSpec, fldPath *field.Path) (*mapiv1beta1.HostPlacement, field.ErrorList) {
+	var errorList field.ErrorList
+
+	hasDynamicHostAllocation := spec.DynamicHostAllocation != nil
+	hasHostID := spec.HostID != nil
+
+	// For "host" affinity, either HostID (user-provided) or DynamicHostAllocation must be set
+	if !hasHostID && !hasDynamicHostAllocation {
+		errorList = append(errorList, field.Required(fldPath.Child("dedicatedHost"), "either id or dynamicHostAllocation is required when hostAffinity is host"))
+		return nil, errorList
+	}
+
+	if hasHostID && hasDynamicHostAllocation {
+		errorList = append(errorList, field.Invalid(fldPath.Child("dedicatedHost"), spec.HostID, "id and dynamicHostAllocation are mutually exclusive"))
+		return nil, errorList
+	}
+
+	if hasHostID {
+		// User-provided host ID
+		if !awsDedicatedHostNamePattern.MatchString(*spec.HostID) {
+			errorList = append(errorList, field.Invalid(fldPath.Child("dedicatedHost").Child("id"), *spec.HostID, errHostIDInvalidFormat))
+			return nil, errorList
+		}
+
+		return &mapiv1beta1.HostPlacement{
+			Affinity: ptr.To(mapiv1beta1.HostAffinityDedicatedHost),
+			DedicatedHost: &mapiv1beta1.DedicatedHost{
+				AllocationStrategy: ptr.To(mapiv1beta1.AllocationStrategyUserProvided),
+				ID:                 *spec.HostID,
+			},
+		}, nil
+	}
+
+	// Dynamic host allocation
+	dynamicAlloc := convertDynamicHostAllocationTags(spec.DynamicHostAllocation)
+
+	return &mapiv1beta1.HostPlacement{
+		Affinity: ptr.To(mapiv1beta1.HostAffinityDedicatedHost),
+		DedicatedHost: &mapiv1beta1.DedicatedHost{
+			AllocationStrategy:    ptr.To(mapiv1beta1.AllocationStrategyDynamic),
+			DynamicHostAllocation: dynamicAlloc,
+		},
+	}, nil
+}
+
+func convertHostAffinityDefault(spec awsv1.AWSMachineSpec, tenancy mapiv1beta1.InstanceTenancy, fldPath *field.Path) (*mapiv1beta1.HostPlacement, field.ErrorList) {
+	var errorList field.ErrorList
+
+	hasDynamicHostAllocation := spec.DynamicHostAllocation != nil
+	hasHostID := spec.HostID != nil
+
+	// DynamicHostAllocation is not allowed with "default" affinity
+	if hasDynamicHostAllocation {
+		errorList = append(errorList, field.Invalid(fldPath.Child("dynamicHostAllocation"), spec.DynamicHostAllocation, "dynamicHostAllocation is only allowed when hostAffinity is host"))
+		return nil, errorList
+	}
+
+	// Only create a HostPlacement when:
+	// 1. Tenancy is "host" (even without a specific HostID), OR
+	// 2. There's an explicit HostID (user-provided dedicated host)
+	// Otherwise, return nil to indicate no dedicated host configuration
+	if tenancy != mapiv1beta1.HostTenancy && !hasHostID {
+		return nil, errorList
+	}
+
+	host := &mapiv1beta1.HostPlacement{
+		Affinity: ptr.To(mapiv1beta1.HostAffinityAnyAvailable),
+	}
+
+	// For "default", host ID is optional
+	if hasHostID {
+		if !awsDedicatedHostNamePattern.MatchString(*spec.HostID) {
+			errorList = append(errorList, field.Invalid(fldPath.Child("dedicatedHost").Child("id"), *spec.HostID, errHostIDInvalidFormat))
+			return nil, errorList
+		}
+
+		host.DedicatedHost = &mapiv1beta1.DedicatedHost{
+			AllocationStrategy: ptr.To(mapiv1beta1.AllocationStrategyUserProvided),
+			ID:                 *spec.HostID,
+		}
+	}
+
+	return host, errorList
+}
+
+func convertAWSDedicatedHostToMAPI(spec awsv1.AWSMachineSpec, tenancy mapiv1beta1.InstanceTenancy, fldPath *field.Path) (*mapiv1beta1.HostPlacement, field.ErrorList) {
+	var (
+		errorList field.ErrorList
+		host      *mapiv1beta1.HostPlacement
+	)
+
+	if spec.HostAffinity == nil {
+		// When HostAffinity is not set, validate that dedicated host fields are also not set.  Default host affinity is default so host ID does not need to be checked.
+		if spec.DynamicHostAllocation != nil {
+			errorList = append(errorList, field.Invalid(fldPath.Child("dynamicHostAllocation"), spec.DynamicHostAllocation, "dynamicHostAllocation is only allowed when hostAffinity is host"))
+		}
+
+		return host, errorList
+	}
+
+	switch *spec.HostAffinity {
+	case "host":
+		host, errorList = convertHostAffinityHost(spec, fldPath)
+	case "default":
+		host, errorList = convertHostAffinityDefault(spec, tenancy, fldPath)
+	default:
+		errorList = append(errorList, field.Invalid(fldPath.Child("hostAffinity"), spec.HostAffinity, errUnsupportedHostAffinityType))
+	}
+
+	return host, errorList
+}
+
 func convertCAPAMachineConditionsToMAPIMachineAWSProviderConditions(awsMachine *awsv1.AWSMachine) []metav1.Condition {
 	if ptr.Deref(awsMachine.Status.InstanceState, "") == awsv1.InstanceStateRunning {
 		// Set conditionSuccess
@@ -400,11 +576,24 @@ func convertAWSFiltersToMAPI(capiFilters []awsv1.Filter) []mapiv1beta1.Filter {
 }
 
 func convertAWSTagsToMAPI(capiTags awsv1.Tags) []mapiv1beta1.TagSpecification {
+	if len(capiTags) == 0 {
+		return []mapiv1beta1.TagSpecification{}
+	}
+
+	// Collect keys first to ensure deterministic ordering
+	keys := make([]string, 0, len(capiTags))
+	for key := range capiTags {
+		keys = append(keys, key)
+	}
+
+	sort.Strings(keys)
+
+	// Build tags slice in sorted order
 	mapiTags := make([]mapiv1beta1.TagSpecification, 0, len(capiTags))
-	for key, value := range capiTags {
+	for _, key := range keys {
 		mapiTags = append(mapiTags, mapiv1beta1.TagSpecification{
 			Name:  key,
-			Value: value,
+			Value: capiTags[key],
 		})
 	}
 

pkg/conversion/capi2mapi/aws_fuzz_test.go

@@ -29,6 +29,7 @@ import (
 	conversiontest "github.com/openshift/cluster-capi-operator/pkg/conversion/test/fuzz"
 
 	runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
+	"k8s.io/utils/ptr"
 	awsv1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -155,7 +156,7 @@ func awsMachineFuzzerFuncs(codecs runtimeserializer.CodecFactory) []interface{}
 		func(spec *awsv1.AWSMachineSpec, c randfill.Continue) {
 			c.FillNoCustom(spec)
 
-			fuzzAWSMachineSpecTenancy(&spec.Tenancy, c)
+			fuzzAWSMachineSpecTenancy(spec, c)
 			fuzzAWSMachineSpecMarketType(&spec.MarketType, c)
 			fuzzAWSMachineSpecCPUOptions(&spec.CPUOptions, c)
 
@@ -197,17 +198,34 @@ func awsMachineFuzzerFuncs(codecs runtimeserializer.CodecFactory) []interface{}
 	}
 }
 
-func fuzzAWSMachineSpecTenancy(tenancy *string, c randfill.Continue) {
-	switch c.Int31n(4) {
-	case 0:
-		*tenancy = "default"
-	case 1:
-		*tenancy = "dedicated"
-	case 2:
-		*tenancy = "host"
-	case 3:
-		*tenancy = ""
+type awsTenancyConfig struct {
+	tenancy               string
+	hostAffinity          *string
+	hostID                *string
+	dynamicHostAllocation *awsv1.DynamicHostAllocationSpec
+}
+
+func fuzzAWSMachineSpecTenancy(spec *awsv1.AWSMachineSpec, c randfill.Continue) {
+	configs := []awsTenancyConfig{
+		{tenancy: capi2mapi.TenancyDefault},
+		{tenancy: capi2mapi.TenancyDefault, hostAffinity: ptr.To("default")},
+		{tenancy: capi2mapi.TenancyDedicated},
+		{tenancy: capi2mapi.TenancyDedicated, hostAffinity: ptr.To("default")},
+		{tenancy: capi2mapi.TenancyHost, hostAffinity: ptr.To("default")},
+		{tenancy: capi2mapi.TenancyHost, hostAffinity: ptr.To("default"), hostID: ptr.To("h-0123456789abcdef0")},
+		{tenancy: capi2mapi.TenancyHost, hostAffinity: ptr.To("host"), hostID: ptr.To("h-0123456789abcdef0")},
+		{tenancy: ""},
+		{tenancy: capi2mapi.TenancyHost, hostAffinity: ptr.To("host"), dynamicHostAllocation: &awsv1.DynamicHostAllocationSpec{}},
+		{tenancy: capi2mapi.TenancyHost, hostAffinity: ptr.To("host"), dynamicHostAllocation: &awsv1.DynamicHostAllocationSpec{
+			Tags: map[string]string{"test-key-1": "test-value-1", "test-key-2": "test-value-2"},
+		}},
 	}
+
+	cfg := configs[c.Int31n(int32(len(configs)))]
+	spec.Tenancy = cfg.tenancy
+	spec.HostAffinity = cfg.hostAffinity
+	spec.HostID = cfg.hostID
+	spec.DynamicHostAllocation = cfg.dynamicHostAllocation
 }
 
 func fuzzAWSMachineSpecMarketType(marketType *awsv1.MarketType, c randfill.Continue) {