From 5e41e6e4cc683b2f9f0652d20a4ea17afb314f9b Mon Sep 17 00:00:00 2001 From: vr4manta Date: Thu, 13 Nov 2025 11:59:06 -0500 Subject: [PATCH 1/5] Add IPI installation on AWS dedicated hosts --- .../install.openshift.io_installconfigs.yaml | 200 ++++++++++++++++++ pkg/asset/installconfig/aws/dedicatedhosts.go | 55 +++++ pkg/asset/installconfig/aws/metadata.go | 27 +++ pkg/asset/installconfig/aws/validation.go | 106 ++++++++++ .../installconfig/aws/validation_test.go | 151 +++++++++++++ pkg/asset/machines/aws/machines.go | 27 +++ pkg/asset/machines/aws/machinesets.go | 6 + pkg/asset/machines/worker.go | 9 + pkg/asset/machines/worker_test.go | 106 +++++----- pkg/types/aws/machinepool.go | 55 +++++ pkg/types/aws/validation/machinepool.go | 53 ++++- pkg/types/aws/validation/machinepool_test.go | 162 +++++++++++++- pkg/types/aws/validation/platform.go | 2 +- pkg/types/aws/zz_generated.deepcopy.go | 47 ++++ pkg/types/machinepools.go | 2 + pkg/types/validation/machinepools.go | 4 +- 16 files changed, 957 insertions(+), 55 deletions(-) create mode 100644 pkg/asset/installconfig/aws/dedicatedhosts.go diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index b3eb531e5f0..3f0a929e810 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -199,6 +199,56 @@ spec: - AMDEncryptedVirtualizationNestedPaging type: string type: object + hostPlacement: + description: |- + hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host + for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL), + and to gain visibility and control over instance placement on a physical server. + When omitted, the instance is not constrained to a dedicated host. + properties: + affinity: + description: |- + affinity specifies the affinity setting for the instance. + Allowed values are AnyAvailable and DedicatedHost. + When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set. + When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host. + enum: + - DedicatedHost + - AnyAvailable + type: string + dedicatedHost: + description: |- + dedicatedHost specifies the exact host that an instance should be restarted on if stopped. + dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise. + items: + description: DedicatedHost represents the configuration + for the usage of dedicated host. + properties: + id: + description: |- + id identifies the AWS Dedicated Host on which the instance must run. + The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f). + Must be exactly 19 characters in length. + maxLength: 19 + minLength: 19 + type: string + x-kubernetes-validations: + - message: hostID must start with 'h-' followed + by 17 lowercase hexadecimal characters (0-9 + and a-f) + rule: self.matches('^h-[0-9a-f]{17}$') + required: + - id + type: object + type: array + required: + - affinity + type: object + x-kubernetes-validations: + - message: dedicatedHost is required when affinity is DedicatedHost, + and forbidden otherwise + rule: 'has(self.affinity) && self.affinity == ''DedicatedHost'' + ? has(self.dedicatedHost) : !has(self.dedicatedHost)' iamProfile: description: |- IAMProfile is the name of the IAM instance profile to use for the machine. @@ -1759,6 +1809,56 @@ spec: - AMDEncryptedVirtualizationNestedPaging type: string type: object + hostPlacement: + description: |- + hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host + for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL), + and to gain visibility and control over instance placement on a physical server. + When omitted, the instance is not constrained to a dedicated host. + properties: + affinity: + description: |- + affinity specifies the affinity setting for the instance. + Allowed values are AnyAvailable and DedicatedHost. + When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set. + When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host. + enum: + - DedicatedHost + - AnyAvailable + type: string + dedicatedHost: + description: |- + dedicatedHost specifies the exact host that an instance should be restarted on if stopped. + dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise. + items: + description: DedicatedHost represents the configuration + for the usage of dedicated host. + properties: + id: + description: |- + id identifies the AWS Dedicated Host on which the instance must run. + The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f). + Must be exactly 19 characters in length. + maxLength: 19 + minLength: 19 + type: string + x-kubernetes-validations: + - message: hostID must start with 'h-' followed + by 17 lowercase hexadecimal characters (0-9 + and a-f) + rule: self.matches('^h-[0-9a-f]{17}$') + required: + - id + type: object + type: array + required: + - affinity + type: object + x-kubernetes-validations: + - message: dedicatedHost is required when affinity is DedicatedHost, + and forbidden otherwise + rule: 'has(self.affinity) && self.affinity == ''DedicatedHost'' + ? has(self.dedicatedHost) : !has(self.dedicatedHost)' iamProfile: description: |- IAMProfile is the name of the IAM instance profile to use for the machine. @@ -3259,6 +3359,56 @@ spec: - AMDEncryptedVirtualizationNestedPaging type: string type: object + hostPlacement: + description: |- + hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host + for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL), + and to gain visibility and control over instance placement on a physical server. + When omitted, the instance is not constrained to a dedicated host. + properties: + affinity: + description: |- + affinity specifies the affinity setting for the instance. + Allowed values are AnyAvailable and DedicatedHost. + When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set. + When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host. + enum: + - DedicatedHost + - AnyAvailable + type: string + dedicatedHost: + description: |- + dedicatedHost specifies the exact host that an instance should be restarted on if stopped. + dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise. + items: + description: DedicatedHost represents the configuration + for the usage of dedicated host. + properties: + id: + description: |- + id identifies the AWS Dedicated Host on which the instance must run. + The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f). + Must be exactly 19 characters in length. + maxLength: 19 + minLength: 19 + type: string + x-kubernetes-validations: + - message: hostID must start with 'h-' followed + by 17 lowercase hexadecimal characters (0-9 + and a-f) + rule: self.matches('^h-[0-9a-f]{17}$') + required: + - id + type: object + type: array + required: + - affinity + type: object + x-kubernetes-validations: + - message: dedicatedHost is required when affinity is DedicatedHost, + and forbidden otherwise + rule: 'has(self.affinity) && self.affinity == ''DedicatedHost'' + ? has(self.dedicatedHost) : !has(self.dedicatedHost)' iamProfile: description: |- IAMProfile is the name of the IAM instance profile to use for the machine. @@ -4959,6 +5109,56 @@ spec: - AMDEncryptedVirtualizationNestedPaging type: string type: object + hostPlacement: + description: |- + hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host + for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL), + and to gain visibility and control over instance placement on a physical server. + When omitted, the instance is not constrained to a dedicated host. + properties: + affinity: + description: |- + affinity specifies the affinity setting for the instance. + Allowed values are AnyAvailable and DedicatedHost. + When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set. + When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host. + enum: + - DedicatedHost + - AnyAvailable + type: string + dedicatedHost: + description: |- + dedicatedHost specifies the exact host that an instance should be restarted on if stopped. + dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise. + items: + description: DedicatedHost represents the configuration + for the usage of dedicated host. + properties: + id: + description: |- + id identifies the AWS Dedicated Host on which the instance must run. + The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f). + Must be exactly 19 characters in length. + maxLength: 19 + minLength: 19 + type: string + x-kubernetes-validations: + - message: hostID must start with 'h-' followed + by 17 lowercase hexadecimal characters (0-9 + and a-f) + rule: self.matches('^h-[0-9a-f]{17}$') + required: + - id + type: object + type: array + required: + - affinity + type: object + x-kubernetes-validations: + - message: dedicatedHost is required when affinity is DedicatedHost, + and forbidden otherwise + rule: 'has(self.affinity) && self.affinity == ''DedicatedHost'' + ? has(self.dedicatedHost) : !has(self.dedicatedHost)' iamProfile: description: |- IAMProfile is the name of the IAM instance profile to use for the machine. diff --git a/pkg/asset/installconfig/aws/dedicatedhosts.go b/pkg/asset/installconfig/aws/dedicatedhosts.go new file mode 100644 index 00000000000..6abbd89184e --- /dev/null +++ b/pkg/asset/installconfig/aws/dedicatedhosts.go @@ -0,0 +1,55 @@ +package aws + +import ( + "context" + "fmt" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/ec2" + "github.com/sirupsen/logrus" +) + +// Host holds metadata for a dedicated host. +type Host struct { + // ID the ID of the host. + ID string + // Zone the zone the host belongs to. + Zone string + // Tags is the map of the Host's tags. + Tags Tags +} + +// dedicatedHosts retrieves a list of dedicated hosts and returns them in a map keyed by the host ID. +func dedicatedHosts(ctx context.Context, client *ec2.Client, hosts []string) (map[string]Host, error) { + hostsByID := map[string]Host{} + + input := &ec2.DescribeHostsInput{} + if len(hosts) > 0 { + input.HostIds = hosts + } + + paginator := ec2.NewDescribeHostsPaginator(client, input) + for paginator.HasMorePages() { + page, err := paginator.NextPage(ctx) + if err != nil { + return nil, fmt.Errorf("fetching dedicated hosts: %w", err) + } + + for _, host := range page.Hosts { + id := aws.ToString(host.HostId) + if id == "" { + // Skip entries lacking an ID (should not happen) + continue + } + + logrus.Debugf("Found dedicated host: %s", id) + hostsByID[id] = Host{ + ID: id, + Zone: aws.ToString(host.AvailabilityZone), + Tags: FromAWSTags(host.Tags), + } + } + } + + return hostsByID, nil +} diff --git a/pkg/asset/installconfig/aws/metadata.go b/pkg/asset/installconfig/aws/metadata.go index 5051824a0be..7d3827bea95 100644 --- a/pkg/asset/installconfig/aws/metadata.go +++ b/pkg/asset/installconfig/aws/metadata.go @@ -25,6 +25,7 @@ type Metadata struct { instanceTypes map[string]InstanceType images map[string]ImageInfo + Hosts map[string]Host Region string `json:"region,omitempty"` ProvidedSubnets []typesaws.Subnet `json:"subnets,omitempty"` Services []typesaws.ServiceEndpoint `json:"services,omitempty"` @@ -406,3 +407,29 @@ func (m *Metadata) Images(ctx context.Context, amiID string) (ImageInfo, error) return imageInfo, nil } + +// DedicatedHosts retrieves all hosts available for use to verify against this installation for configured region. +func (m *Metadata) DedicatedHosts(ctx context.Context, hosts []typesaws.DedicatedHost) (map[string]Host, error) { + m.mutex.Lock() + defer m.mutex.Unlock() + + // Create array of host IDs for client + hostIDs := make([]string, len(hosts)) + for idx, host := range hosts { + hostIDs[idx] = host.ID + } + + if len(m.Hosts) == 0 { + client, err := m.EC2Client(ctx) + if err != nil { + return nil, err + } + + m.Hosts, err = dedicatedHosts(ctx, client, hostIDs) + if err != nil { + return nil, fmt.Errorf("error listing dedicated hosts: %w", err) + } + } + + return m.Hosts, nil +} diff --git a/pkg/asset/installconfig/aws/validation.go b/pkg/asset/installconfig/aws/validation.go index 0cffc8d4efe..91820fc6593 100644 --- a/pkg/asset/installconfig/aws/validation.go +++ b/pkg/asset/installconfig/aws/validation.go @@ -9,6 +9,7 @@ import ( "net/url" "slices" "sort" + "strings" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/ec2" @@ -115,6 +116,12 @@ func validatePlatform(ctx context.Context, meta *Metadata, fldPath *field.Path, allErrs = append(allErrs, validateSharedVPC(ctx, meta, fldPath.Child("vpc").Child("subnets"))...) } if platform.DefaultMachinePlatform != nil { + // Dedicated hosts cannot be configured in defaultMachinePlatform + if platform.DefaultMachinePlatform.HostPlacement != nil { + defaultPath := fldPath.Child("defaultMachinePlatform").Child("hostPlacement") + errMsg := "dedicated hosts cannot be configured in defaultMachinePlatform, they must be specified per machine pool" + allErrs = append(allErrs, field.Invalid(defaultPath, platform.DefaultMachinePlatform.HostPlacement, errMsg)) + } allErrs = append(allErrs, validateMachinePool(ctx, meta, fldPath.Child("defaultMachinePlatform"), platform, platform.DefaultMachinePlatform, controlPlaneReq, "", "")...) } return allErrs @@ -479,6 +486,8 @@ func validateMachinePool(ctx context.Context, meta *Metadata, fldPath *field.Pat } } + allErrs = append(allErrs, validateHostPlacement(ctx, meta, fldPath, pool, poolName)...) + return allErrs } @@ -497,6 +506,103 @@ func translateEC2Arches(arches []string) sets.Set[string] { return res } +// validateHostPlacement validates the HostPlacement for all instances. +// HostPlacement must not have any duplicate dedicated hosts. +// Dedicated Host must belong to a region and zone that this cluster is being installed into. +// Dedicated Host must be only host in the region and zone for this install-config. +// Dedicated Host must not have tags: +// - "kubernetes.io/cluster/: owned" +// - "sigs.k8s.io/cluster-api-provider-aws/cluster/: owned". +func validateHostPlacement(ctx context.Context, meta *Metadata, fldPath *field.Path, pool *awstypes.MachinePool, poolName string) field.ErrorList { + allErrs := field.ErrorList{} + + if pool.HostPlacement == nil { + return allErrs + } + + if pool.HostPlacement.Affinity != nil && *pool.HostPlacement.Affinity == awstypes.HostAffinityDedicatedHost { + placementPath := fldPath.Child("hostPlacement") + if pool.HostPlacement.DedicatedHost != nil { + configuredHosts := pool.HostPlacement.DedicatedHost + + // Check for duplicate host IDs in the configuration + { + seenHostIDs := make(map[string]int) + for idx, host := range configuredHosts { + if firstIdx, exists := seenHostIDs[host.ID]; exists { + dhPath := placementPath.Child("dedicatedHost").Index(idx) + errMsg := fmt.Sprintf("duplicate dedicated host %s (first seen at index %d)", host.ID, firstIdx) + allErrs = append(allErrs, field.Invalid(dhPath, host, errMsg)) + } else { + seenHostIDs[host.ID] = idx + } + } + } + + // Check to see if all configured hosts exist + foundHosts, err := meta.DedicatedHosts(ctx, configuredHosts) + if err != nil { + allErrs = append(allErrs, field.InternalError(placementPath.Child("dedicatedHost"), err)) + } else { + // Track hosts by zone to ensure only one host per zone + seenZones := make(map[string]int) + + // Check the returned configured hosts to see if the dedicated hosts defined in install-config exists. + for idx, host := range configuredHosts { + dhPath := placementPath.Child("dedicatedHost").Index(idx) + + // Is host in AWS? + foundHost, ok := foundHosts[host.ID] + if !ok { + errMsg := fmt.Sprintf("dedicated host %s not found", host.ID) + allErrs = append(allErrs, field.Invalid(dhPath.Child("id"), host, errMsg)) + continue + } + + // Verify host is in the correct region + if !strings.HasPrefix(foundHost.Zone, meta.Region) { + errMsg := fmt.Sprintf("dedicated host %s is in zone %s which is not in the cluster's region %s", host.ID, foundHost.Zone, meta.Region) + allErrs = append(allErrs, field.Invalid(dhPath, host, errMsg)) + continue + } + + // Is host valid for pools region and zone config? + // Only check if zones are explicitly configured; if pool.Zones is empty, all zones are allowed + zones := pool.Zones + if len(zones) == 0 { + zones, err = meta.AvailabilityZones(ctx) + if err != nil { + allErrs = append(allErrs, field.InternalError(fldPath, fmt.Errorf("unable to retrieve availability zones: %w", err))) + } + } + if !slices.Contains(zones, foundHost.Zone) { + errMsg := fmt.Sprintf("machine pool specifies zones %v but dedicated host %s is in zone %s", pool.Zones, host.ID, foundHost.Zone) + allErrs = append(allErrs, field.Invalid(dhPath, host, errMsg)) + } + + // Check for multiple hosts in the same zone + if firstIdx, exists := seenZones[foundHost.Zone]; exists { + errMsg := fmt.Sprintf("multiple dedicated hosts configured for zone %s (host %s at index %d, host %s at index %d)", + foundHost.Zone, configuredHosts[firstIdx].ID, firstIdx, host.ID, idx) + allErrs = append(allErrs, field.Invalid(dhPath, host, errMsg)) + } else { + seenZones[foundHost.Zone] = idx + } + + // Check to make sure dedicated host is not owned by another cluster + clusterIDs := foundHost.Tags.GetClusterIDs(TagValueOwned) + if len(clusterIDs) > 0 { + allErrs = append(allErrs, field.Forbidden(dhPath, + fmt.Sprintf("Dedicated host %s is owned by other cluster %v and cannot be used for new installations, another Dedicated Host must be created separately", foundHost.ID, clusterIDs))) + } + } + } + } + } + + return allErrs +} + func validateSecurityGroupIDs(ctx context.Context, meta *Metadata, fldPath *field.Path, platform *awstypes.Platform, pool *awstypes.MachinePool) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/asset/installconfig/aws/validation_test.go b/pkg/asset/installconfig/aws/validation_test.go index f368199baa2..f870e41a1ff 100644 --- a/pkg/asset/installconfig/aws/validation_test.go +++ b/pkg/asset/installconfig/aws/validation_test.go @@ -77,6 +77,7 @@ func TestValidate(t *testing.T) { vpcTags Tags instanceTypes map[string]InstanceType images map[string]ImageInfo + hosts map[string]Host proxy string publicOnly bool expectErr string @@ -1524,6 +1525,126 @@ func TestValidate(t *testing.T) { }, expectErr: `^\Qplatform.aws.vpc.subnets: Forbidden: subnet subnet-valid-public-d is owned by other clusters [my-cluster] and cannot be used for new installations, another subnet must be created separately\E$`, }, + { + name: "valid dedicated host placement on compute", + installConfig: icBuild.build( + icBuild.withComputePlatformZones([]string{"us-east-1a"}, true, 0), + icBuild.withComputeHostPlacement([]string{"h-1234567890abcdef0"}, 0), + ), + availRegions: validAvailRegions(), + availZones: []string{"us-east-1a", "us-east-1b", "us-east-1c"}, + hosts: map[string]Host{ + "h-1234567890abcdef0": {ID: "h-1234567890abcdef0", Zone: "us-east-1a"}, + }, + }, + { + name: "valid dedicated host placement on compute with no zones specified", + installConfig: icBuild.build( + icBuild.withComputeHostPlacement([]string{"h-1234567890abcdef0"}, 0), + ), + availRegions: validAvailRegions(), + availZones: []string{"us-east-1a", "us-east-1b", "us-east-1c"}, + hosts: map[string]Host{ + "h-1234567890abcdef0": {ID: "h-1234567890abcdef0", Zone: "us-east-1a"}, + }, + }, + { + name: "invalid dedicated host not found", + installConfig: icBuild.build( + icBuild.withComputePlatformZones([]string{"us-east-1a"}, true, 0), + icBuild.withComputeHostPlacement([]string{"h-aaaaaaaaaaaaaaaaa"}, 0), + ), + availRegions: validAvailRegions(), + availZones: validAvailZones(), + hosts: map[string]Host{ + "h-1234567890abcdef0": {ID: "h-1234567890abcdef0", Zone: "us-east-1a"}, + }, + expectErr: "dedicated host h-aaaaaaaaaaaaaaaaa not found", + }, + { + name: "invalid dedicated host in wrong region", + installConfig: icBuild.build( + icBuild.withComputeHostPlacement([]string{"h-1234567890abcdef0"}, 0), + ), + availRegions: validAvailRegions(), + availZones: validAvailZones(), + hosts: map[string]Host{ + "h-1234567890abcdef0": {ID: "h-1234567890abcdef0", Zone: "us-west-2a"}, + }, + expectErr: "dedicated host h-1234567890abcdef0 is in zone us-west-2a which is not in the cluster's region us-east-1", + }, + { + name: "invalid dedicated host zone not in pool zones", + installConfig: icBuild.build( + icBuild.withComputePlatformZones([]string{"us-east-1a"}, true, 0), + icBuild.withComputeHostPlacement([]string{"h-bbbbbbbbbbbbbbbbb"}, 0), + ), + availRegions: validAvailRegions(), + availZones: validAvailZones(), + hosts: map[string]Host{ + "h-bbbbbbbbbbbbbbbbb": {ID: "h-bbbbbbbbbbbbbbbbb", Zone: "us-east-1b"}, + }, + expectErr: "machine pool specifies zones.*but dedicated host h-bbbbbbbbbbbbbbbbb is in zone us-east-1b", + }, + { + name: "dedicated host placement on compute but for a zone that pool is not using", + installConfig: icBuild.build( + icBuild.withComputePlatformZones([]string{"us-east-1b"}, true, 0), + icBuild.withComputeHostPlacementAndZone([]string{"h-1234567890abcdef0"}, "us-east-1b", 0), + ), + availRegions: validAvailRegions(), + availZones: []string{"us-east-1a", "us-east-1b", "us-east-1c"}, + hosts: map[string]Host{ + "h-1234567890abcdef0": {ID: "h-1234567890abcdef0", Zone: "us-east-1a"}, + }, + expectErr: "machine pool specifies zones.*but dedicated host h-1234567890abcdef0 is in zone us-east-1a", + }, + { + name: "duplicate dedicated host IDs in configuration", + installConfig: icBuild.build( + icBuild.withComputePlatformZones([]string{"us-east-1a"}, true, 0), + icBuild.withComputeHostPlacement([]string{"h-1234567890abcdef0", "h-1234567890abcdef0"}, 0), + ), + availRegions: validAvailRegions(), + availZones: validAvailZones(), + hosts: map[string]Host{ + "h-1234567890abcdef0": {ID: "h-1234567890abcdef0", Zone: "us-east-1a"}, + }, + expectErr: "duplicate dedicated host h-1234567890abcdef0", + }, + { + name: "multiple dedicated hosts in same zone", + installConfig: icBuild.build( + icBuild.withComputePlatformZones([]string{"us-east-1a", "us-east-1b"}, true, 0), + icBuild.withComputeHostPlacement([]string{"h-1234567890abcdef0", "h-abcdef1234567890a"}, 0), + ), + availRegions: validAvailRegions(), + availZones: validAvailZones(), + hosts: map[string]Host{ + "h-1234567890abcdef0": {ID: "h-1234567890abcdef0", Zone: "us-east-1a"}, + "h-abcdef1234567890a": {ID: "h-abcdef1234567890a", Zone: "us-east-1a"}, + }, + expectErr: "multiple dedicated hosts configured for zone us-east-1a", + }, + { + name: "dedicated hosts in defaultMachinePlatform not allowed", + installConfig: icBuild.build( + icBuild.withDefaultPlatformMachine(aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: func() *aws.HostAffinity { a := aws.HostAffinityDedicatedHost; return &a }(), + DedicatedHost: []aws.DedicatedHost{ + {ID: "h-1234567890abcdef0"}, + }, + }, + }), + ), + availRegions: validAvailRegions(), + availZones: validAvailZones(), + hosts: map[string]Host{ + "h-1234567890abcdef0": {ID: "h-1234567890abcdef0", Zone: "us-east-1a"}, + }, + expectErr: "dedicated hosts cannot be configured in defaultMachinePlatform", + }, } // Register mock http(s) responses for tests. @@ -1557,6 +1678,8 @@ func TestValidate(t *testing.T) { }, instanceTypes: test.instanceTypes, images: test.images, + Hosts: test.hosts, + Region: test.installConfig.Platform.AWS.Region, ProvidedSubnets: test.installConfig.Platform.AWS.VPC.Subnets, } @@ -2291,6 +2414,34 @@ func (icBuild icBuildForAWS) withComputePlatformZones(zones []string, overwrite } } +func (icBuild icBuildForAWS) withComputeHostPlacement(hostIDs []string, index int) icOption { + return func(ic *types.InstallConfig) { + aff := aws.HostAffinityDedicatedHost + dhs := make([]aws.DedicatedHost, 0, len(hostIDs)) + for _, id := range hostIDs { + dhs = append(dhs, aws.DedicatedHost{ID: id}) + } + ic.Compute[index].Platform.AWS.HostPlacement = &aws.HostPlacement{ + Affinity: &aff, + DedicatedHost: dhs, + } + } +} + +func (icBuild icBuildForAWS) withComputeHostPlacementAndZone(hostIDs []string, zone string, index int) icOption { + return func(ic *types.InstallConfig) { + aff := aws.HostAffinityDedicatedHost + dhs := make([]aws.DedicatedHost, 0, len(hostIDs)) + for _, id := range hostIDs { + dhs = append(dhs, aws.DedicatedHost{ID: id}) + } + ic.Compute[index].Platform.AWS.HostPlacement = &aws.HostPlacement{ + Affinity: &aff, + DedicatedHost: dhs, + } + } +} + func (icBuild icBuildForAWS) withControlPlanePlatformAMI(amiID string) icOption { return func(ic *types.InstallConfig) { ic.ControlPlane.Platform.AWS.AMIID = amiID diff --git a/pkg/asset/machines/aws/machines.go b/pkg/asset/machines/aws/machines.go index a46fb98d9cf..6072c6d4138 100644 --- a/pkg/asset/machines/aws/machines.go +++ b/pkg/asset/machines/aws/machines.go @@ -37,6 +37,7 @@ type machineProviderInput struct { publicSubnet bool securityGroupIDs []string cpuOptions *awstypes.CPUOptions + dedicatedHost string } // Machines returns a list of machines for a machinepool. @@ -305,6 +306,17 @@ func provider(in *machineProviderInput) (*machineapi.AWSMachineProviderConfig, e config.CPUOptions = &cpuOptions } + if in.dedicatedHost != "" { + config.Placement.Tenancy = machineapi.HostTenancy + config.Placement.Host = &machineapi.HostPlacement{ + Affinity: ptr.To(machineapi.HostAffinityDedicatedHost), + DedicatedHost: &machineapi.DedicatedHost{ + AllocationStrategy: ptr.To(machineapi.AllocationStrategyUserProvided), + ID: in.dedicatedHost, + }, + } + } + return config, nil } @@ -354,3 +366,18 @@ func ConfigMasters(machines []machineapi.Machine, controlPlane *machinev1.Contro providerSpec := controlPlane.Spec.Template.OpenShiftMachineV1Beta1Machine.Spec.ProviderSpec.Value.Object.(*machineapi.AWSMachineProviderConfig) providerSpec.LoadBalancers = lbrefs } + +// DedicatedHost sets dedicated hosts for the specified zone. +func DedicatedHost(hosts map[string]aws.Host, placement *awstypes.HostPlacement, zone string) string { + // If install-config has HostPlacements configured, lets check the DedicatedHosts to see if one matches our region & zone. + if placement != nil { + // We only support one host ID currently for an instance. Need to also get host that matches the zone the machines will be put into. + for _, host := range placement.DedicatedHost { + hostDetails, found := hosts[host.ID] + if found && hostDetails.Zone == zone { + return hostDetails.ID + } + } + } + return "" +} diff --git a/pkg/asset/machines/aws/machinesets.go b/pkg/asset/machines/aws/machinesets.go index 35cc1f38e69..15ec3650c9c 100644 --- a/pkg/asset/machines/aws/machinesets.go +++ b/pkg/asset/machines/aws/machinesets.go @@ -25,6 +25,7 @@ type MachineSetInput struct { Pool *types.MachinePool Role string UserDataSecret string + Hosts map[string]icaws.Host } // MachineSets returns a list of machinesets for a machinepool. @@ -87,6 +88,8 @@ func MachineSets(in *MachineSetInput) ([]*machineapi.MachineSet, error) { instanceProfile = fmt.Sprintf("%s-worker-profile", in.ClusterID) } + dedicatedHost := DedicatedHost(in.Hosts, mpool.HostPlacement, az) + provider, err := provider(&machineProviderInput{ clusterID: in.ClusterID, region: in.InstallConfigPlatformAWS.Region, @@ -103,10 +106,12 @@ func MachineSets(in *MachineSetInput) ([]*machineapi.MachineSet, error) { publicSubnet: publicSubnet, securityGroupIDs: in.Pool.Platform.AWS.AdditionalSecurityGroupIDs, cpuOptions: mpool.CPUOptions, + dedicatedHost: dedicatedHost, }) if err != nil { return nil, errors.Wrap(err, "failed to create provider") } + name := fmt.Sprintf("%s-%s-%s", in.ClusterID, in.Pool.Name, az) spec := machineapi.MachineSpec{ ProviderSpec: machineapi.ProviderSpec{ @@ -152,6 +157,7 @@ func MachineSets(in *MachineSetInput) ([]*machineapi.MachineSet, error) { }, }, } + machinesets = append(machinesets, mset) } diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index 2488f5f87a8..fb9289b1140 100644 --- a/pkg/asset/machines/worker.go +++ b/pkg/asset/machines/worker.go @@ -534,6 +534,14 @@ func (w *Worker) Generate(ctx context.Context, dependencies asset.Parents) error } } + dHosts := map[string]icaws.Host{} + if mpool.HostPlacement != nil && mpool.HostPlacement.Affinity != nil && *mpool.HostPlacement.Affinity == awstypes.HostAffinityDedicatedHost { + dHosts, err = installConfig.AWS.DedicatedHosts(ctx, mpool.HostPlacement.DedicatedHost) + if err != nil { + return fmt.Errorf("failed to retrieve dedicated hosts for compute pool: %w", err) + } + } + pool.Platform.AWS = &mpool sets, err := aws.MachineSets(&aws.MachineSetInput{ ClusterID: clusterID.InfraID, @@ -544,6 +552,7 @@ func (w *Worker) Generate(ctx context.Context, dependencies asset.Parents) error Pool: &pool, Role: pool.Name, UserDataSecret: workerUserDataSecretName, + Hosts: dHosts, }) if err != nil { return errors.Wrap(err, "failed to create worker machine objects") diff --git a/pkg/asset/machines/worker_test.go b/pkg/asset/machines/worker_test.go index ba7850919c0..bf6f2781a94 100644 --- a/pkg/asset/machines/worker_test.go +++ b/pkg/asset/machines/worker_test.go @@ -6,11 +6,12 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/ignition/machine" "github.com/openshift/installer/pkg/asset/installconfig" + icaws "github.com/openshift/installer/pkg/asset/installconfig/aws" "github.com/openshift/installer/pkg/asset/rhcos" "github.com/openshift/installer/pkg/types" awstypes "github.com/openshift/installer/pkg/types/aws" @@ -126,38 +127,40 @@ spec: for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { parents := asset.Parents{} + cfg := &types.InstallConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + SSHKey: tc.key, + BaseDomain: "test-domain", + Platform: types.Platform{ + AWS: &awstypes.Platform{ + Region: "us-east-1", + }, + }, + Compute: []types.MachinePool{ + { + Replicas: ptr.To(int64(1)), + Hyperthreading: tc.hyperthreading, + Platform: types.MachinePoolPlatform{ + AWS: &awstypes.MachinePool{ + Zones: []string{"us-east-1a"}, + InstanceType: "m5.large", + }, + }, + }, + }, + } + icAsset := installconfig.MakeAsset(cfg) + icAsset.AWS = icaws.NewMetadata(cfg.Platform.AWS.Region, cfg.Platform.AWS.VPC.Subnets, nil) parents.Add( &installconfig.ClusterID{ UUID: "test-uuid", InfraID: "test-infra-id", }, - installconfig.MakeAsset( - &types.InstallConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - }, - SSHKey: tc.key, - BaseDomain: "test-domain", - Platform: types.Platform{ - AWS: &awstypes.Platform{ - Region: "us-east-1", - }, - }, - Compute: []types.MachinePool{ - { - Replicas: pointer.Int64Ptr(1), - Hyperthreading: tc.hyperthreading, - Platform: types.MachinePoolPlatform{ - AWS: &awstypes.MachinePool{ - Zones: []string{"us-east-1a"}, - InstanceType: "m5.large", - }, - }, - }, - }, - }), + icAsset, rhcos.MakeAsset("test-image"), - (*rhcos.Release)(pointer.StringPtr("412.86.202208101040-0")), + (*rhcos.Release)(ptr.To("412.86.202208101040-0")), &machine.Worker{ File: &asset.File{ Filename: "worker-ignition", @@ -183,34 +186,35 @@ spec: func TestComputeIsNotModified(t *testing.T) { parents := asset.Parents{} - installConfig := installconfig.MakeAsset( - &types.InstallConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - }, - SSHKey: "ssh-rsa: dummy-key", - BaseDomain: "test-domain", - Platform: types.Platform{ - AWS: &awstypes.Platform{ - Region: "us-east-1", - DefaultMachinePlatform: &awstypes.MachinePool{ - InstanceType: "TEST_INSTANCE_TYPE", - }, + cfg := &types.InstallConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + SSHKey: "ssh-rsa: dummy-key", + BaseDomain: "test-domain", + Platform: types.Platform{ + AWS: &awstypes.Platform{ + Region: "us-east-1", + DefaultMachinePlatform: &awstypes.MachinePool{ + InstanceType: "TEST_INSTANCE_TYPE", }, }, - Compute: []types.MachinePool{ - { - Replicas: pointer.Int64Ptr(1), - Hyperthreading: types.HyperthreadingDisabled, - Platform: types.MachinePoolPlatform{ - AWS: &awstypes.MachinePool{ - Zones: []string{"us-east-1a"}, - InstanceType: "", - }, + }, + Compute: []types.MachinePool{ + { + Replicas: ptr.To(int64(1)), + Hyperthreading: types.HyperthreadingDisabled, + Platform: types.MachinePoolPlatform{ + AWS: &awstypes.MachinePool{ + Zones: []string{"us-east-1a"}, + InstanceType: "", }, }, }, - }) + }, + } + installConfig := installconfig.MakeAsset(cfg) + installConfig.AWS = icaws.NewMetadata(cfg.Platform.AWS.Region, cfg.Platform.AWS.VPC.Subnets, nil) parents.Add( &installconfig.ClusterID{ @@ -219,7 +223,7 @@ func TestComputeIsNotModified(t *testing.T) { }, installConfig, rhcos.MakeAsset("test-image"), - (*rhcos.Release)(pointer.StringPtr("412.86.202208101040-0")), + (*rhcos.Release)(ptr.To("412.86.202208101040-0")), &machine.Worker{ File: &asset.File{ Filename: "worker-ignition", diff --git a/pkg/types/aws/machinepool.go b/pkg/types/aws/machinepool.go index 7dc0ed540f0..71797be59f5 100644 --- a/pkg/types/aws/machinepool.go +++ b/pkg/types/aws/machinepool.go @@ -56,6 +56,14 @@ type MachinePool struct { // https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/cpu-options-supported-instances-values.html // +optional CPUOptions *CPUOptions `json:"cpuOptions,omitempty,omitzero"` + + // hostPlacement configures placement on AWS Dedicated Hosts. This allows admins to assign instances to specific host + // for a variety of needs including for regulatory compliance, to leverage existing per-socket or per-core software licenses (BYOL), + // and to gain visibility and control over instance placement on a physical server. + // When omitted, the instance is not constrained to a dedicated host. + // +openshift:enable:FeatureGate=AWSDedicatedHosts + // +optional + HostPlacement *HostPlacement `json:"hostPlacement,omitempty"` } // Set sets the values from `required` to `a`. @@ -111,6 +119,10 @@ func (a *MachinePool) Set(required *MachinePool) { if required.CPUOptions != nil { a.CPUOptions = required.CPUOptions } + + if required.HostPlacement != nil { + a.HostPlacement = required.HostPlacement + } } // EC2RootVolume defines the storage for an ec2 instance. @@ -195,3 +207,46 @@ type CPUOptions struct { // +optional ConfidentialCompute *ConfidentialComputePolicy `json:"confidentialCompute,omitempty"` } + +// HostPlacement is the type that will be used to configure the placement of AWS instances. +// This can be configured for default placement (AnyAvailable) and dedicated hosts (DedicatedHost). +// +kubebuilder:validation:XValidation:rule="has(self.affinity) && self.affinity == 'DedicatedHost' ? has(self.dedicatedHost) : !has(self.dedicatedHost)",message="dedicatedHost is required when affinity is DedicatedHost, and forbidden otherwise" +type HostPlacement struct { + // affinity specifies the affinity setting for the instance. + // Allowed values are AnyAvailable and DedicatedHost. + // When Affinity is set to DedicatedHost, an instance started onto a specific host always restarts on the same host if stopped. In this scenario, the `dedicatedHost` field must be set. + // When Affinity is set to AnyAvailable, and you stop and restart the instance, it can be restarted on any available host. + // +required + // +unionDiscriminator + Affinity *HostAffinity `json:"affinity,omitempty"` + + // dedicatedHost specifies the exact host that an instance should be restarted on if stopped. + // dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise. + // +optional + // +unionMember + DedicatedHost []DedicatedHost `json:"dedicatedHost,omitempty"` +} + +// HostAffinity selects how an instance should be placed on AWS Dedicated Hosts. +// +kubebuilder:validation:Enum:=DedicatedHost;AnyAvailable +type HostAffinity string + +const ( + // HostAffinityAnyAvailable lets the platform select any available dedicated host. + HostAffinityAnyAvailable HostAffinity = "AnyAvailable" + + // HostAffinityDedicatedHost requires specifying a particular host via dedicatedHost.host.id. + HostAffinityDedicatedHost HostAffinity = "DedicatedHost" +) + +// DedicatedHost represents the configuration for the usage of dedicated host. +type DedicatedHost struct { + // id identifies the AWS Dedicated Host on which the instance must run. + // The value must start with "h-" followed by 17 lowercase hexadecimal characters (0-9 and a-f). + // Must be exactly 19 characters in length. + // +kubebuilder:validation:XValidation:rule="self.matches('^h-[0-9a-f]{17}$')",message="hostID must start with 'h-' followed by 17 lowercase hexadecimal characters (0-9 and a-f)" + // +kubebuilder:validation:MinLength=19 + // +kubebuilder:validation:MaxLength=19 + // +required + ID string `json:"id,omitempty"` +} diff --git a/pkg/types/aws/validation/machinepool.go b/pkg/types/aws/validation/machinepool.go index 537864b45ef..c242d4e5a2e 100644 --- a/pkg/types/aws/validation/machinepool.go +++ b/pkg/types/aws/validation/machinepool.go @@ -49,7 +49,7 @@ const maxIopsThroughputRatio = 4 const gp3DefaultIOPS int32 = 3000 // ValidateMachinePool checks that the specified machine pool is valid. -func ValidateMachinePool(platform *aws.Platform, p *aws.MachinePool, fldPath *field.Path) field.ErrorList { +func ValidateMachinePool(platform *aws.Platform, p *aws.MachinePool, poolName string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} for i, zone := range p.Zones { if !strings.HasPrefix(zone, platform.Region) { @@ -69,6 +69,57 @@ func ValidateMachinePool(platform *aws.Platform, p *aws.MachinePool, fldPath *fi allErrs = append(allErrs, validateSecurityGroups(platform, p, fldPath)...) allErrs = append(allErrs, ValidateCPUOptions(p, fldPath)...) + allErrs = append(allErrs, validateHostPlacement(p, poolName, fldPath.Child("hostPlacement"))...) + + return allErrs +} + +func validateHostPlacement(p *aws.MachinePool, poolName string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + // No HostPlacement, so lets just return + if p.HostPlacement == nil { + return allErrs + } + + // Dedicated hosts are only supported on worker (compute) pools + if poolName != "" && poolName != types.MachinePoolComputeRoleName { + errMsg := fmt.Sprintf("dedicated hosts are only supported on %s pools, not on %s pools", types.MachinePoolComputeRoleName, poolName) + allErrs = append(allErrs, field.Invalid(fldPath, p.HostPlacement, errMsg)) + return allErrs + } + + // Control plane pools cannot use dedicated hosts + if poolName == "" { + errMsg := "dedicated hosts are not supported on control plane pools" + allErrs = append(allErrs, field.Invalid(fldPath, p.HostPlacement, errMsg)) + return allErrs + } + + if p.HostPlacement.Affinity == nil { + allErrs = append(allErrs, field.Required(fldPath.Child("affinity"), "affinity is required when hostPlacement is configured")) + return allErrs // Can't validate further without affinity + } + + switch *p.HostPlacement.Affinity { + case aws.HostAffinityAnyAvailable: + if len(p.HostPlacement.DedicatedHost) > 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("dedicatedHost"), "dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise")) + } + case aws.HostAffinityDedicatedHost: + if len(p.HostPlacement.DedicatedHost) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("dedicatedHost"), "dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise")) + } else { + for index, host := range p.HostPlacement.DedicatedHost { + hostPath := fldPath.Child("dedicatedHost").Index(index) + if len(host.ID) == 0 { + allErrs = append(allErrs, field.Required(hostPath.Child("id"), "a hostID must be specified when configuring 'dedicatedHost'")) + } + } + } + default: + allErrs = append(allErrs, field.NotSupported(fldPath.Child("affinity"), p.HostPlacement.Affinity, []aws.HostAffinity{aws.HostAffinityAnyAvailable, aws.HostAffinityDedicatedHost})) + } return allErrs } diff --git a/pkg/types/aws/validation/machinepool_test.go b/pkg/types/aws/validation/machinepool_test.go index ec0a9bbb278..e01b2509bdc 100644 --- a/pkg/types/aws/validation/machinepool_test.go +++ b/pkg/types/aws/validation/machinepool_test.go @@ -2,12 +2,14 @@ package validation import ( "fmt" + "strings" "testing" "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/ptr" + "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/aws" ) @@ -16,6 +18,7 @@ func TestValidateMachinePool(t *testing.T) { cases := []struct { name string pool *aws.MachinePool + poolName string expected string }{ { @@ -226,10 +229,167 @@ func TestValidateMachinePool(t *testing.T) { }, expected: `^test-path\.throughput: Invalid value: 125: throughput not supported for type gp2$`, }, + { + name: "host placement any available", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityAnyAvailable), + }, + }, + }, + { + name: "valid dedicated hosts", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityDedicatedHost), + DedicatedHost: []aws.DedicatedHost{ + { + ID: "h-09dcf61cb388b0149", + }, + }, + }, + }, + }, + { + name: "invalid dedicated hosts - missing hostID", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityDedicatedHost), + DedicatedHost: []aws.DedicatedHost{ + {}, + }, + }, + }, + expected: `^test-path.hostPlacement.dedicatedHost\[0].id: Required value: a hostID must be specified when configuring 'dedicatedHost'$`, + }, + { + name: "invalid - hostPlacement without affinity", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{}, + }, + expected: `^test-path.hostPlacement.affinity: Required value: affinity is required when hostPlacement is configured$`, + }, + { + name: "invalid unknown affinity", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinity("Unknown")), + }, + }, + expected: `^test-path.hostPlacement.affinity: Unsupported value: "Unknown": supported values: "AnyAvailable", "DedicatedHost"$`, + }, + { + name: "any available with dedicated host set", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityAnyAvailable), + DedicatedHost: []aws.DedicatedHost{{ID: "h-09dcf61cb388b0149"}}, + }, + }, + expected: `^test-path.hostPlacement.dedicatedHost: Required value: dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise$`, + }, + { + name: "invalid - DedicatedHost affinity without dedicatedHost", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityDedicatedHost), + }, + }, + expected: `^test-path.hostPlacement.dedicatedHost: Required value: dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise$`, + }, + { + name: "valid dedicated host - 8 character format", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityDedicatedHost), + DedicatedHost: []aws.DedicatedHost{ + {ID: "h-1a2b3c4d"}, + }, + }, + }, + }, + { + name: "valid dedicated host - 8 character format with zone", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityDedicatedHost), + DedicatedHost: []aws.DedicatedHost{ + { + ID: "h-9876abcd", + }, + }, + }, + }, + }, + { + name: "valid dedicated host with zone specified", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityDedicatedHost), + DedicatedHost: []aws.DedicatedHost{ + { + ID: "h-09dcf61cb388b0149", + }, + }, + }, + }, + }, + { + name: "valid dedicated host with multiple zones", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityDedicatedHost), + DedicatedHost: []aws.DedicatedHost{ + { + ID: "h-09dcf61cb388b0149", + }, + { + ID: "h-0a1b2c3d4e5f60789", + }, + }, + }, + }, + }, + { + name: "dedicated hosts on control plane not allowed", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityDedicatedHost), + DedicatedHost: []aws.DedicatedHost{ + {ID: "h-1234567890abcdef0"}, + }, + }, + }, + poolName: "", // Empty poolName indicates control plane + expected: `^test-path.hostPlacement: Invalid value:.*: dedicated hosts are not supported on control plane pools$`, + }, + { + name: "dedicated hosts on edge pool not allowed", + pool: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: ptr.To(aws.HostAffinityDedicatedHost), + DedicatedHost: []aws.DedicatedHost{ + {ID: "h-1234567890abcdef0"}, + }, + }, + }, + poolName: types.MachinePoolEdgeRoleName, + expected: `^test-path.hostPlacement: Invalid value:.*: dedicated hosts are only supported on worker pools, not on edge pools$`, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - err := ValidateMachinePool(platform, tc.pool, field.NewPath("test-path")).ToAggregate() + poolName := tc.poolName + // If poolName is not specified, default to worker pool unless the test + // is explicitly testing control plane or edge pool behavior + if poolName == "" && tc.expected != "" && + !strings.Contains(tc.expected, "control plane") && + !strings.Contains(tc.expected, "edge pool") { + poolName = types.MachinePoolComputeRoleName + } else if poolName == "" && tc.expected == "" { + poolName = types.MachinePoolComputeRoleName + } + err := ValidateMachinePool(platform, tc.pool, poolName, field.NewPath("test-path")).ToAggregate() if tc.expected == "" { assert.NoError(t, err) } else { diff --git a/pkg/types/aws/validation/platform.go b/pkg/types/aws/validation/platform.go index b9acbda4f97..7f304ea495d 100644 --- a/pkg/types/aws/validation/platform.go +++ b/pkg/types/aws/validation/platform.go @@ -61,7 +61,7 @@ func ValidatePlatform(p *aws.Platform, publish types.PublishingStrategy, cm type allErrs = append(allErrs, validateLBType(p.LBType, p.IPFamily, fldPath.Child("lbType"))...) if p.DefaultMachinePlatform != nil { - allErrs = append(allErrs, ValidateMachinePool(p, p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...) + allErrs = append(allErrs, ValidateMachinePool(p, p.DefaultMachinePlatform, types.MachinePoolDefaultConfig, fldPath.Child("defaultMachinePlatform"))...) } return allErrs diff --git a/pkg/types/aws/zz_generated.deepcopy.go b/pkg/types/aws/zz_generated.deepcopy.go index b0612b0e7a8..347aa22cdd5 100644 --- a/pkg/types/aws/zz_generated.deepcopy.go +++ b/pkg/types/aws/zz_generated.deepcopy.go @@ -26,6 +26,22 @@ func (in *CPUOptions) DeepCopy() *CPUOptions { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DedicatedHost) DeepCopyInto(out *DedicatedHost) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DedicatedHost. +func (in *DedicatedHost) DeepCopy() *DedicatedHost { + if in == nil { + return nil + } + out := new(DedicatedHost) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *EC2Metadata) DeepCopyInto(out *EC2Metadata) { *out = *in @@ -63,6 +79,32 @@ func (in *EC2RootVolume) DeepCopy() *EC2RootVolume { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HostPlacement) DeepCopyInto(out *HostPlacement) { + *out = *in + if in.Affinity != nil { + in, out := &in.Affinity, &out.Affinity + *out = new(HostAffinity) + **out = **in + } + if in.DedicatedHost != nil { + in, out := &in.DedicatedHost, &out.DedicatedHost + *out = make([]DedicatedHost, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HostPlacement. +func (in *HostPlacement) DeepCopy() *HostPlacement { + if in == nil { + return nil + } + out := new(HostPlacement) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachinePool) DeepCopyInto(out *MachinePool) { *out = *in @@ -83,6 +125,11 @@ func (in *MachinePool) DeepCopyInto(out *MachinePool) { *out = new(CPUOptions) (*in).DeepCopyInto(*out) } + if in.HostPlacement != nil { + in, out := &in.HostPlacement, &out.HostPlacement + *out = new(HostPlacement) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/types/machinepools.go b/pkg/types/machinepools.go index 799698a7bf3..78e18116233 100644 --- a/pkg/types/machinepools.go +++ b/pkg/types/machinepools.go @@ -23,6 +23,8 @@ const ( MachinePoolControlPlaneRoleName = "master" // MachinePoolArbiterRoleName name associated with the control plane machinepool for smaller sized limited nodes. MachinePoolArbiterRoleName = "arbiter" + // MachinePoolDefaultConfig name associated with the generic default configs for machine pool. + MachinePoolDefaultConfig = "default" ) // HyperthreadingMode is the mode of hyperthreading for a machine. diff --git a/pkg/types/validation/machinepools.go b/pkg/types/validation/machinepools.go index 947bc6e80e7..160e58a2d65 100644 --- a/pkg/types/validation/machinepools.go +++ b/pkg/types/validation/machinepools.go @@ -153,7 +153,9 @@ func validateMachinePoolPlatform(platform *types.Platform, p *types.MachinePoolP allErrs = append(allErrs, awsvalidation.ValidateAMIID(platform.AWS, p.AWS, fldPath.Child("aws"))...) } if p.AWS != nil { - validate(aws.Name, p.AWS, func(f *field.Path) field.ErrorList { return awsvalidation.ValidateMachinePool(platform.AWS, p.AWS, f) }) + validate(aws.Name, p.AWS, func(f *field.Path) field.ErrorList { + return awsvalidation.ValidateMachinePool(platform.AWS, p.AWS, pool.Name, f) + }) } if p.Azure != nil { validate(azure.Name, p.Azure, func(f *field.Path) field.ErrorList { From e530a6717b36c63210423e8de3fe870118358754 Mon Sep 17 00:00:00 2001 From: vr4manta Date: Thu, 5 Mar 2026 09:37:26 -0500 Subject: [PATCH 2/5] Added shared tag for BYO dedicated host --- pkg/asset/cluster/aws/aws.go | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/pkg/asset/cluster/aws/aws.go b/pkg/asset/cluster/aws/aws.go index 5ecabdcec21..836fc50fa98 100644 --- a/pkg/asset/cluster/aws/aws.go +++ b/pkg/asset/cluster/aws/aws.go @@ -44,6 +44,10 @@ func PreTerraform(ctx context.Context, clusterID string, installConfig *installc return err } + if err := tagSharedDedicatedHosts(ctx, clusterID, installConfig); err != nil { + return err + } + if err := tagSharedIAMRoles(ctx, clusterID, installConfig); err != nil { return err } @@ -244,6 +248,50 @@ func tagSharedIAMProfiles(ctx context.Context, clusterID string, installConfig * return nil } +// tagSharedDedicatedHosts tags users BYO dedicated hosts so they are not destroyed by the Installer. +func tagSharedDedicatedHosts(ctx context.Context, clusterID string, installConfig *installconfig.InstallConfig) error { + var dhNames []string + + // DH is only allowed to be created in "worker" pool at this time. No need to check default platform. + for _, compute := range installConfig.Config.Compute { + if compute.Name == "worker" { + mpool := awstypes.MachinePool{} + mpool.Set(installConfig.Config.AWS.DefaultMachinePlatform) + mpool.Set(compute.Platform.AWS) + if mpool.HostPlacement != nil && mpool.HostPlacement.DedicatedHost != nil { + for _, name := range mpool.HostPlacement.DedicatedHost { + dhNames = append(dhNames, name.ID) + } + } + } + } + + if len(dhNames) == 0 { + return nil + } + + logrus.Debugf("Tagging shared dedicated hosts: %v", dhNames) + + tagKey, tagValue := sharedTag(clusterID) + + ec2Client, err := awsconfig.NewEC2Client(ctx, awsconfig.EndpointOptions{ + Region: installConfig.Config.Platform.AWS.Region, + Endpoints: installConfig.Config.Platform.AWS.ServiceEndpoints, + }) + if err != nil { + return fmt.Errorf("failed to create EC2 client: %w", err) + } + + if _, err = ec2Client.CreateTags(ctx, &ec2.CreateTagsInput{ + Resources: dhNames, + Tags: []ec2types.Tag{{Key: &tagKey, Value: &tagValue}}, + }); err != nil { + return errors.Wrap(err, "could not add tags to dedicated host") + } + + return nil +} + func sharedTag(clusterID string) (string, string) { return fmt.Sprintf("kubernetes.io/cluster/%s", clusterID), "shared" } From 6f20dfef4b2676c52804f7f713a877e36c0a82a9 Mon Sep 17 00:00:00 2001 From: vr4manta Date: Fri, 27 Feb 2026 07:38:07 -0500 Subject: [PATCH 3/5] Added permission checking for dedicated hosts --- pkg/asset/installconfig/aws/permissions.go | 31 ++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/pkg/asset/installconfig/aws/permissions.go b/pkg/asset/installconfig/aws/permissions.go index 6970df7da31..78d1597fbe3 100644 --- a/pkg/asset/installconfig/aws/permissions.go +++ b/pkg/asset/installconfig/aws/permissions.go @@ -85,6 +85,12 @@ const ( // PermissionPassthroughCreds is a permission set required when using passthrough credentials. PermissionPassthroughCreds PermissionGroup = "permission-passthrough-creds" + + // PermissionDedicatedHosts is a permission set required when using user-provided dedicated hosts. + PermissionDedicatedHosts PermissionGroup = "permission-dedicated-hosts" + + // PermissionDynamicHostAllocation is a permission set required when cleaning up dynamic hosts that were allocated during the life of the cluster. + PermissionDynamicHostAllocation PermissionGroup = "permission-dynamic-host-allocation" ) var permissions = map[PermissionGroup][]string{ @@ -429,6 +435,14 @@ var permissions = map[PermissionGroup][]string{ "iam:GetUserPolicy", "iam:ListAccessKeys", }, + PermissionDedicatedHosts: { + // Used when user-provided dedicated hosts are configured in the install-config.yaml or when dynamic dedicated hosts are detected during cluster destroy. + "ec2:DescribeHosts", + }, + PermissionDynamicHostAllocation: { + // This is only used during cluster destroy if during cluster destroy we detect a dedicated host with appropriate tags on it. + "ec2:ReleaseHosts", + }, } // ValidateCreds will try to create an AWS session, and also verify that the current credentials @@ -564,6 +578,10 @@ func RequiredPermissionGroups(ic *types.InstallConfig) []PermissionGroup { permissionGroups = append(permissionGroups, PermissionEdgeDefaultInstance) } + if includesDedicatedHosts(ic) { + permissionGroups = append(permissionGroups, PermissionDedicatedHosts) + } + return permissionGroups } @@ -772,3 +790,16 @@ func includesEdgeDefaultInstanceType(installConfig *types.InstallConfig) bool { } return false } + +// includesDedicatedHosts checks if any dedicated hosts are specified for worker machine pools. +func includesDedicatedHosts(installConfig *types.InstallConfig) bool { + for _, mpool := range installConfig.Compute { + if mpool.Name != types.MachinePoolComputeRoleName { + continue + } + if mpool.Platform.AWS != nil && mpool.Platform.AWS.HostPlacement != nil { + return true + } + } + return false +} From dbd42b62a6bdb02aaa363a8e9056c41bbd51219e Mon Sep 17 00:00:00 2001 From: vr4manta Date: Wed, 28 Jan 2026 12:26:10 -0500 Subject: [PATCH 4/5] Added logic to handle cleaning up dynamic dedicated hosts --- pkg/destroy/aws/ec2helpers.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pkg/destroy/aws/ec2helpers.go b/pkg/destroy/aws/ec2helpers.go index f6a6ca2827e..bd1ded07355 100644 --- a/pkg/destroy/aws/ec2helpers.go +++ b/pkg/destroy/aws/ec2helpers.go @@ -162,6 +162,8 @@ func (o *ClusterUninstaller) deleteEC2(ctx context.Context, arn arn.ARN, logger return deleteEC2VPCEndpointService(ctx, o.EC2Client, id, logger) case "egress-only-internet-gateway": return deleteEgressOnlyInternetGateway(ctx, o.EC2Client, id, logger) + case "dedicated-host": + return deleteEC2DedicatedHost(ctx, o.EC2Client, id, logger) default: return errors.Errorf("unrecognized EC2 resource type %s", resourceType) } @@ -962,3 +964,27 @@ func deleteEgressOnlyInternetGateway(ctx context.Context, client *ec2v2.Client, logger.Info("Deleted") return nil } + +func deleteEC2DedicatedHost(ctx context.Context, client *ec2v2.Client, id string, logger logrus.FieldLogger) error { + // Release the host + _, err := client.ReleaseHosts(ctx, &ec2v2.ReleaseHostsInput{ + HostIds: []string{id}, + }) + if err != nil { + errCode := HandleErrorCode(err) + switch errCode { + case "InvalidHostID.NotFound": + // Host doesn't exist, nothing to do + return nil + case "UnauthorizedOperation", "AccessDenied": + // User doesn't have permission to release dedicated hosts + // This is expected when dedicated host permissions are not configured + logger.Warn("User does not have permission to release dedicated hosts, skipping") + return nil + } + return fmt.Errorf("failed to release dedicated host %s: %w", id, err) + } + + logger.Info("Released") + return nil +} From e6282c4229eb3ef7e3b02dc658dabfa0a588b24e Mon Sep 17 00:00:00 2001 From: vr4manta Date: Mon, 2 Mar 2026 07:49:27 -0500 Subject: [PATCH 5/5] Added featuregate check for BYO DH --- pkg/types/aws/validation/featuregates.go | 16 +- pkg/types/aws/validation/featuregates_test.go | 191 ++++++++++++++++++ 2 files changed, 206 insertions(+), 1 deletion(-) create mode 100644 pkg/types/aws/validation/featuregates_test.go diff --git a/pkg/types/aws/validation/featuregates.go b/pkg/types/aws/validation/featuregates.go index 4b6dd4ec06a..59cba281677 100644 --- a/pkg/types/aws/validation/featuregates.go +++ b/pkg/types/aws/validation/featuregates.go @@ -12,7 +12,7 @@ import ( // GatedFeatures determines all of the install config fields that should // be validated to ensure that the proper featuregate is enabled when the field is used. func GatedFeatures(c *types.InstallConfig) []featuregates.GatedInstallConfigFeature { - return []featuregates.GatedInstallConfigFeature{ + gatedFeatures := []featuregates.GatedInstallConfigFeature{ { FeatureGateName: features.FeatureGateAWSClusterHostedDNSInstall, Condition: c.AWS.UserProvisionedDNS == dns.UserProvisionedDNSEnabled, @@ -24,4 +24,18 @@ func GatedFeatures(c *types.InstallConfig) []featuregates.GatedInstallConfigFeat Field: field.NewPath("platform", "aws", "ipFamily"), }, } + + // Check if any compute pool has dedicated hosts configured + for idx, compute := range c.Compute { + if compute.Platform.AWS != nil && compute.Platform.AWS.HostPlacement != nil { + gatedFeatures = append(gatedFeatures, featuregates.GatedInstallConfigFeature{ + FeatureGateName: features.FeatureGateAWSDedicatedHosts, + Condition: true, + Field: field.NewPath("compute").Index(idx).Child("platform", "aws", "hostPlacement"), + }) + break // Only need to add the feature gate once + } + } + + return gatedFeatures } diff --git a/pkg/types/aws/validation/featuregates_test.go b/pkg/types/aws/validation/featuregates_test.go new file mode 100644 index 00000000000..301a155a27e --- /dev/null +++ b/pkg/types/aws/validation/featuregates_test.go @@ -0,0 +1,191 @@ +package validation + +import ( + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/aws" +) + +func TestGatedFeatures(t *testing.T) { + tests := []struct { + name string + installConfig *types.InstallConfig + expectedFeatureGates []configv1.FeatureGateName + }{ + { + name: "no gated features", + 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, + Platform: types.MachinePoolPlatform{ + AWS: &aws.MachinePool{}, + }, + }, + }, + }, + expectedFeatureGates: []configv1.FeatureGateName{}, + }, + { + name: "dedicated hosts configured", + 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, + Platform: types.MachinePoolPlatform{ + AWS: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: func() *aws.HostAffinity { + a := aws.HostAffinityDedicatedHost + return &a + }(), + DedicatedHost: []aws.DedicatedHost{ + {ID: "h-1234567890abcdef0"}, + }, + }, + }, + }, + }, + }, + }, + expectedFeatureGates: []configv1.FeatureGateName{features.FeatureGateAWSDedicatedHosts}, + }, + { + name: "dedicated hosts configured on second compute 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, + Platform: types.MachinePoolPlatform{ + AWS: &aws.MachinePool{}, + }, + }, + { + Name: "worker-special", + Platform: types.MachinePoolPlatform{ + AWS: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: func() *aws.HostAffinity { + a := aws.HostAffinityDedicatedHost + return &a + }(), + DedicatedHost: []aws.DedicatedHost{ + {ID: "h-1234567890abcdef0"}, + }, + }, + }, + }, + }, + }, + }, + expectedFeatureGates: []configv1.FeatureGateName{features.FeatureGateAWSDedicatedHosts}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gatedFeatures := GatedFeatures(tc.installConfig) + + // Extract feature gate names from the result + var actualFeatureGates []configv1.FeatureGateName + for _, gf := range gatedFeatures { + if gf.Condition { + actualFeatureGates = append(actualFeatureGates, gf.FeatureGateName) + } + } + + assert.ElementsMatch(t, tc.expectedFeatureGates, actualFeatureGates, + "Expected feature gates %v but got %v", tc.expectedFeatureGates, actualFeatureGates) + }) + } +} + +func TestGatedFeatures_DedicatedHostsFieldPath(t *testing.T) { + 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, + Platform: types.MachinePoolPlatform{ + AWS: &aws.MachinePool{ + HostPlacement: &aws.HostPlacement{ + Affinity: func() *aws.HostAffinity { + a := aws.HostAffinityDedicatedHost + return &a + }(), + DedicatedHost: []aws.DedicatedHost{ + {ID: "h-1234567890abcdef0"}, + }, + }, + }, + }, + }, + }, + } + + gatedFeatures := GatedFeatures(installConfig) + + // Find the dedicated hosts feature + var dedicatedHostFeature *field.Path + for _, gf := range gatedFeatures { + if gf.FeatureGateName == features.FeatureGateAWSDedicatedHosts { + dedicatedHostFeature = gf.Field + break + } + } + + assert.NotNil(t, dedicatedHostFeature, "Expected to find dedicated hosts feature gate") + expectedPath := field.NewPath("compute").Index(0).Child("platform", "aws", "hostPlacement") + assert.Equal(t, expectedPath.String(), dedicatedHostFeature.String(), + "Field path should point to the hostPlacement field") +}