SPLAT-2172: AWS dedicate host support by vr4manta · Pull Request #10079 · openshift/installer

vr4manta · 2025-11-13T18:42:28Z

Changes

Bumped openshift/api
Added logic to allow HostPlacement of dedicated hosts

Dependencies

Notes

MAO and CAO changes are needed for it to fully work. For now, this PR is adding the ability to generate the needed outputs for bootstrapping.

openshift-ci-robot · 2025-11-13T18:42:32Z

@vr4manta: This pull request references SPLAT-2172 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

SPLAT-2172

Changes

Bumped openshift/api

Added logic to allow HostPlacement of dedicated hosts

Dependencies

SPLAT-2206: Added AWS dedicated host support api#2484

SPLAT-2167: Added dedicated hosts support for AWS machine-api-operator#1419

SPLAT-2167: Added dedicated host support for AWS cluster-capi-operator#374

SPLAT-2540: Added test support for AWS dedicated hosts cluster-api-actuator-pkg#446

Notes

MAO and CAO changes are needed for it to fully work. For now, this PR is adding the ability to generate the needed outputs for bootstrapping.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

tthvo · 2025-11-14T00:35:26Z

/cc

tthvo · 2025-11-18T02:10:05Z

/retest-required

tthvo

Cool feature 👍 🚀

I have quite some comments 😅 Sorry if I review a little too soon...

pkg/types/aws/validation/machinepool.go

tthvo · 2025-11-18T05:56:02Z

pkg/types/aws/validation/machinepool.go

+		switch *p.HostPlacement.Affinity {
+		case aws.HostAffinityAnyAvailable:
+			if p.HostPlacement.DedicatedHost != nil {
+				allErrs = append(allErrs, field.Required(fldPath.Child("dedicatedHost"), "dedicatedHost is required when 'affinity' is set to DedicatedHost, and forbidden otherwise"))


Did you mean hostPlacement.dedicatedHost is forbidden if affinity is AnyAvailable?

allErrs = append(allErrs, field.Forbidden(fldPath.Child("dedicatedHost"), "dedicatedHost must not be set when affinity is AnyAvailable"))

@tthvo So this was an interesting one. In openshift/api and openshift/machine-api-operator, it was suggested to error this way for this scenario. I was doing this to keep it consistent.

https://github.com/openshift/api/blob/0598ae66682433c49adc29b1ff14a201487f3afe/machine/v1beta1/types_awsprovider.go#L406

openshift/machine-api-operator@f669914#diff-30c686f7a9dbd2aa917e5e410401d720a2a4bf4a11163495f5610c8f4b98edaeR917

Now that said, I am happy to make your suggestion if you prefer installer say it this way. Just let me know.

pkg/types/aws/validation/machinepool.go

pkg/types/aws/validation/machinepool_test.go

pkg/asset/installconfig/aws/validation.go

tthvo · 2025-11-18T10:21:46Z

pkg/asset/installconfig/aws/validation.go

+			if err != nil {
+				allErrs = append(allErrs, field.InternalError(placementPath.Child("dedicatedHost"), err))
+			} else {
+				// Check the returned configured hosts to see if the dedicated hosts defined in install-config exists.


💡 ❓ I got 2 questions here:

Do the zones of dedicated hosts need to match machinepool zone field if defined?

installer/pkg/types/aws/machinepool.go

Lines 5 to 9 in 7c9b71a

type MachinePool struct {

// Zones is list of availability zones that can be used.

//

// +optional

Zones []string `json:"zones,omitempty"`

Do the user-input zones for dedicated hosts need to match with the actual zones returned from AWS?

// If user specified a zone, validate it matches AWS if host.Zone != "" && host.Zone != hostDetails.Zone { allErrs = append(allErrs, field.Invalid( fldPath.Child("hostPlacement", "dedicatedHost").Index(i).Child("zone"), host.Zone, fmt.Sprintf("specified zone %s does not match actual host zone %s", host.Zone, hostDetails.Zone))) }

Do the user-input zones for dedicated hosts need to match with the actual zones returned from AWS?

On that point, I'm curious if users should need to specify the zone for the dedicated host? It seems like we can just look up the dedicated host by id and then use that to determine the zone (which the pr is already doing IIUC). Seems like we might be able to shed zone from the install config.

pkg/asset/machines/worker.go

pkg/asset/machines/aws/awsmachines.go

pkg/asset/machines/aws/machinesets.go

vr4manta · 2025-11-18T18:45:04Z

/hold
Need to rebase and add validation logic to make sure defined dedicated hosts are in zones based on the machinepool config.

vr4manta · 2025-11-21T20:17:31Z

/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-aws-ovn-dedicated

openshift-ci · 2025-11-21T20:17:46Z

@vr4manta: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-nightly-4.21-e2e-aws-ovn-dedicated

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/1c5c6d80-c717-11f0-93e9-d918c346504c-0

vr4manta · 2025-11-24T13:30:33Z

/unhold
/retest

vr4manta · 2025-11-26T12:45:36Z

/retest

vr4manta · 2025-12-02T18:51:18Z

/hold
based on discussions with @patrickdillon and @JoelSpeed , I am adding MAPA support and then will enhance this PR to leverage that code path without being dependent on CAPI.

vr4manta · 2026-02-17T15:58:42Z

/retest

vr4manta · 2026-02-23T12:58:21Z

@patrickdillon this PR is now ready for re-review. MAPA is now used for the creation of nodes on dedicated host. I also added a little logic to clean up dynamic dedicated hosts. Thanks!

openshift-ci · 2026-02-24T18:50:36Z

@vr4manta: This PR was included in a payload test run from openshift/machine-api-provider-aws#174
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-ovn-dedicated

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/af501260-11b1-11f1-8355-45eb72a0dd0e-0

openshift-ci · 2026-02-24T20:37:23Z

@vr4manta: This PR was included in a payload test run from openshift/machine-api-provider-aws#174
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-ovn-dedicated

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/8fbbd240-11c0-11f1-9976-41485f7c73c3-0

openshift-ci · 2026-02-25T12:08:44Z

@vr4manta: This PR was included in a payload test run from openshift/machine-api-provider-aws#174
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-ovn-dedicated

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/aee21670-1242-11f1-9543-785cee1a4298-0

pkg/asset/installconfig/aws/dedicatedhosts.go

patrickdillon · 2026-02-26T20:12:18Z

pkg/asset/installconfig/aws/validation.go

+			if err != nil {
+				allErrs = append(allErrs, field.InternalError(placementPath.Child("dedicatedHost"), err))
+			} else {
+				// Check the returned configured hosts to see if the dedicated hosts defined in install-config exists.


Do the user-input zones for dedicated hosts need to match with the actual zones returned from AWS?

On that point, I'm curious if users should need to specify the zone for the dedicated host? It seems like we can just look up the dedicated host by id and then use that to determine the zone (which the pr is already doing IIUC). Seems like we might be able to shed zone from the install config.

pkg/types/aws/validation/machinepool.go

pkg/types/aws/machinepool.go

vr4manta · 2026-02-27T15:50:59Z

/hold
After making changes based on reviews, getting error on checking dedicated hosts against defined zones

pkg/asset/installconfig/aws/permissions.go

patrickdillon · 2026-02-27T17:14:29Z

pkg/asset/installconfig/aws/validation.go

+		// 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"


nit: it might be more user friendly to specify that they're only supported on compute machine pools, but NBD

patrickdillon · 2026-02-27T17:20:26Z

pkg/asset/installconfig/aws/validation.go

+	// Dedicated hosts are only supported on worker (compute) pools
+	if poolName != "" && poolName != types.MachinePoolComputeRoleName {
+		placementPath := fldPath.Child("hostPlacement")
+		errMsg := fmt.Sprintf("dedicated hosts are only supported on %s pools, not on %s pools", types.MachinePoolComputeRoleName, poolName)
+		allErrs = append(allErrs, field.Invalid(placementPath, pool.HostPlacement, errMsg))
+		return allErrs
+	}
+
+	// Control plane pools cannot use dedicated hosts
+	if poolName == "" {
+		placementPath := fldPath.Child("hostPlacement")
+		errMsg := "dedicated hosts are not supported on control plane pools"
+		allErrs = append(allErrs, field.Invalid(placementPath, pool.HostPlacement, errMsg))
+		return allErrs
+	}


nit: I would have expected these validations in pkg/types (because they don't require aws sdk)

pkg/asset/installconfig/aws/validation.go

patrickdillon · 2026-02-27T17:29:00Z

pkg/asset/installconfig/aws/validation.go

+						allErrs = append(allErrs, field.Invalid(dhPath, host, errMsg))
+					}
+
+					// Check for multiple hosts in the same zone


Shouldn't this use case be ok? For general compute nodes, we support provisioning more than one compute node per zone (e.g. replica count > zone count)

patrickdillon · 2026-02-27T17:35:15Z

pkg/destroy/aws/ec2helpers.go

+	describeOutput, err := client.DescribeHosts(ctx, &ec2v2.DescribeHostsInput{
+		HostIds: []string{id},
+	})
+	if err != nil {
+		errCode := HandleErrorCode(err)
+		if errCode == "InvalidHostID.NotFound" {
+			// Host doesn't exist, nothing to do
+			return nil


This call to DescribeHosts is always made but permissions.go is listing the permission as optional. We need to either make the permission required, or gracefully handle the permission error (i think handling the permission error is the way to go).

pkg/destroy/aws/ec2helpers.go

patrickdillon · 2026-02-27T17:37:52Z

pkg/destroy/aws/ec2helpers.go

+	}
+
+	// Release the host
+	_, err = client.ReleaseHosts(ctx, &ec2v2.ReleaseHostsInput{


This probably also needs to be included in the permissions list?

openshift-ci · 2026-03-02T21:48:45Z

@vr4manta: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-gcp-ovn-xpn	`9364945`	link	false	`/test e2e-gcp-ovn-xpn`
ci/prow/e2e-metal-ovn-two-node-fencing	`9364945`	link	false	`/test e2e-metal-ovn-two-node-fencing`
ci/prow/gcp-private	`9364945`	link	false	`/test gcp-private`
ci/prow/e2e-gcp-secureboot	`9364945`	link	false	`/test e2e-gcp-secureboot`
ci/prow/gcp-custom-endpoints-proxy-wif	`9364945`	link	false	`/test gcp-custom-endpoints-proxy-wif`
ci/prow/e2e-metal-assisted	`9364945`	link	false	`/test e2e-metal-assisted`
ci/prow/e2e-metal-ipi-ovn-virtualmedia	`9364945`	link	false	`/test e2e-metal-ipi-ovn-virtualmedia`
ci/prow/e2e-metal-ipi-ovn	`9364945`	link	false	`/test e2e-metal-ipi-ovn`
ci/prow/e2e-gcp-ovn	`9364945`	link	true	`/test e2e-gcp-ovn`
ci/prow/e2e-metal-single-node-live-iso	`9364945`	link	false	`/test e2e-metal-single-node-live-iso`
ci/prow/e2e-gcp-custom-dns	`9364945`	link	false	`/test e2e-gcp-custom-dns`
ci/prow/e2e-azurestack	`9364945`	link	false	`/test e2e-azurestack`
ci/prow/e2e-gcp-default-config	`9364945`	link	false	`/test e2e-gcp-default-config`
ci/prow/e2e-gcp-xpn-dedicated-dns-project	`9364945`	link	false	`/test e2e-gcp-xpn-dedicated-dns-project`
ci/prow/e2e-openstack-ovn	`9364945`	link	true	`/test e2e-openstack-ovn`
ci/prow/e2e-gcp-custom-endpoints	`9364945`	link	false	`/test e2e-gcp-custom-endpoints`
ci/prow/e2e-aws-ovn-dualstack-ipv6-primary-techpreview	`37b76cb`	link	false	`/test e2e-aws-ovn-dualstack-ipv6-primary-techpreview`
ci/prow/e2e-aws-ovn-dualstack-ipv4-primary-techpreview	`37b76cb`	link	false	`/test e2e-aws-ovn-dualstack-ipv4-primary-techpreview`
ci/prow/e2e-aws-ovn-heterogeneous	`4b1d68a`	link	false	`/test e2e-aws-ovn-heterogeneous`
ci/prow/e2e-aws-ovn-imdsv2	`4b1d68a`	link	false	`/test e2e-aws-ovn-imdsv2`
ci/prow/e2e-aws-ovn-shared-vpc-custom-security-groups	`4b1d68a`	link	false	`/test e2e-aws-ovn-shared-vpc-custom-security-groups`
ci/prow/okd-scos-images	`4b1d68a`	link	true	`/test okd-scos-images`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 13, 2025

openshift-ci bot requested review from mtulio and patrickdillon November 13, 2025 18:43

openshift-ci bot requested a review from tthvo November 14, 2025 00:35

vr4manta force-pushed the SPLAT-2172 branch 3 times, most recently from 24500b6 to 342ab2d Compare November 17, 2025 17:44

tthvo reviewed Nov 18, 2025

View reviewed changes

vr4manta force-pushed the SPLAT-2172 branch from 342ab2d to f624e2f Compare November 18, 2025 18:51

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2025

vr4manta force-pushed the SPLAT-2172 branch 5 times, most recently from 82915c2 to 8ef8679 Compare November 21, 2025 13:54

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2025

vr4manta force-pushed the SPLAT-2172 branch from 8ef8679 to b21797f Compare November 21, 2025 19:37

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2025

vr4manta force-pushed the SPLAT-2172 branch from b21797f to eae7731 Compare November 21, 2025 19:40

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 24, 2025

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2025

vr4manta mentioned this pull request Feb 24, 2026

SPLAT-2627: Created OTE Tests for dedicated hosts openshift/machine-api-provider-aws#174

Draft

patrickdillon reviewed Feb 26, 2026

View reviewed changes

vr4manta force-pushed the SPLAT-2172 branch 3 times, most recently from c873bb3 to 5a397fb Compare February 27, 2026 15:50

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 27, 2026

patrickdillon reviewed Feb 27, 2026

View reviewed changes

pkg/asset/installconfig/aws/permissions.go Outdated Show resolved Hide resolved

patrickdillon reviewed Feb 27, 2026

View reviewed changes

vr4manta force-pushed the SPLAT-2172 branch from 5a397fb to dbd72f6 Compare February 27, 2026 17:22

patrickdillon reviewed Feb 27, 2026

View reviewed changes

pkg/asset/installconfig/aws/validation.go Outdated Show resolved Hide resolved

patrickdillon reviewed Feb 27, 2026

View reviewed changes

pkg/asset/installconfig/aws/validation.go Outdated Show resolved Hide resolved

patrickdillon reviewed Feb 27, 2026

View reviewed changes

pkg/destroy/aws/ec2helpers.go Outdated Show resolved Hide resolved

patrickdillon reviewed Feb 27, 2026

View reviewed changes

vr4manta added 3 commits March 2, 2026 10:56

Add IPI installation on AWS dedicated hosts

8375a83

Added permission checking for dedicated hosts

a2cce94

Added logic to handle cleaning up dynamic dedicated hosts

769c12d

vr4manta force-pushed the SPLAT-2172 branch from dbd72f6 to cc8a0b9 Compare March 2, 2026 15:57

vr4manta added 2 commits March 2, 2026 11:40

Added featuregate check for BYO DH

38c03fa

Fix lint issues w/ legacy tests

4b1d68a

vr4manta force-pushed the SPLAT-2172 branch from cc8a0b9 to 4b1d68a Compare March 2, 2026 16:41

	type MachinePool struct {
	// Zones is list of availability zones that can be used.
	//
	// +optional
	Zones []string `json:"zones,omitempty"`

Conversation

vr4manta commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Dependencies

Notes

Uh oh!

openshift-ci-robot commented Nov 13, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Dependencies

Notes

Uh oh!

tthvo commented Nov 14, 2025

Uh oh!

tthvo commented Nov 18, 2025

Uh oh!

tthvo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vr4manta commented Nov 18, 2025

Uh oh!

vr4manta commented Nov 21, 2025

Uh oh!

openshift-ci bot commented Nov 21, 2025

Uh oh!

vr4manta commented Nov 24, 2025

Uh oh!

vr4manta commented Nov 26, 2025

Uh oh!

vr4manta commented Dec 2, 2025

Uh oh!

vr4manta commented Feb 17, 2026

Uh oh!

vr4manta commented Feb 23, 2026

Uh oh!

openshift-ci bot commented Feb 24, 2026

Uh oh!

openshift-ci bot commented Feb 24, 2026

Uh oh!

openshift-ci bot commented Feb 25, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vr4manta commented Feb 27, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

vr4manta commented Nov 13, 2025 •

edited

Loading

openshift-ci-robot commented Nov 13, 2025 •

edited by openshift-ci bot

Loading