Bug 86417: installer: sync MAPI master machine edits to CAPI AWSMachine manifests

[chdeshpa-hue]

Summary

• Adds a MasterMAPISync asset that detects drift between MAPI master machine manifests (openshift/) and CAPI AWSMachine manifests (cluster-api/machines/) during create cluster
• When drift is detected: emits a per-field warning and syncs MAPI values → CAPI objects before infrastructure provisioning begins
• Fixes silent misconfiguration where users edit openshift/ master manifests after create manifests but edits are ignored because CAPI machines are provisioned from cluster-api/machines/

Bug

https://redhat.atlassian.net/browse/OCPBUGS-86417

Changes

File	Change
pkg/asset/machines/mastermapisync.go	New: MasterMAPISync asset — compares MAPI and CAPI master specs, warns on drift, syncs fields
pkg/asset/machines/mastermapisync_test.go	New: 10 unit tests covering no-drift, per-field drift, multi-field drift, helpers
pkg/asset/cluster/cluster.go	Add &machines.MasterMAPISync{} to Cluster.Dependencies()

How it works

                    install-config.yaml
                     /              \
                    v                v
          machines.Master      machines.ClusterAPI
          (openshift/)         (cluster-api/machines/)
               \                      /
                v                    v
            MasterMAPISync
            (compare + sync MAPI → CAPI)
                        |
                        v
                cluster.Cluster → Provision()

The MasterMAPISync asset:

1. Depends on InstallConfig, Master, and ClusterAPI
2. Gates on CAPI-enabled + AWS platform (no-op otherwise)
3. Extracts AWSMachineProviderConfig from each MAPI machine
4. Matches to corresponding AWSMachine in CAPI by name/index
5. Compares each provisioning-relevant field
6. On drift: logs per-field warning, syncs MAPI value → CAPI object, re-serializes

Fields synced: instanceType, AMI, rootVolume (size/type/IOPS/throughput/encryptionKey), IAM instance profile, IMDS httpTokens, confidentialCompute, publicIP, subnet (ID or filters).

Platform scope: AWS only. Follow-up PRs extend to GCP/Azure.

Test plan

• Unit tests pass (10/10): go test ./pkg/asset/machines/ -run "TestSyncAWSFields|TestIndexAWSMachines|TestFindAWSMachine"
• E2E: fresh install on AWS eu-north-1 with MAPI edits (instanceType m6i.4xlarge, vol 200 io2, iops 5000, IMDS Required) — all synced correctly to EC2
• Control: 3 clusters on unpatched 4.22 confirmed MAPI edits are silently ignored
• CI: e2e-aws

Workaround (for unpatched versions)

Set values in install-config.yaml before create manifests, or after create manifests edit both openshift/ and cluster-api/machines/ manifests.

Made with Cursor

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5d9c276f-fbfa-465c-bea7-17b0a8338994

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

Hi @chdeshpa-hue. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign stephenfin for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[patrickdillon]

checking individual. fields like this does not seem maintainable

[chdeshpa-hue]

Agreed, this won't age well as fields are added.

For context — AWS is the one platform where MAPI and CAPI are generated independently from install-config with no shared builder between them. The platforms that already bridge MAPI→CAPI (IBM Cloud, vSphere, Nutanix) use the same imperative field mapping, which is where this approach came from. But AWS has ~12 fields vs their 4-5, so the verbosity is more apparent here.

I considered regenerating CAPI from the MAPI spec directly, but that would require either making ClusterAPI depend on Master or introducing a shared intermediate — both feel too invasive for a targeted bug fix.

Instead I'm thinking of reworking this to use a field descriptor table: a []fieldSync slice where each entry declares {name, get, equal, apply}. The sync logic becomes a simple loop over the table, and adding a new field is one entry rather than a new code block.

var awsFieldSyncs = []fieldSync{
    {name: "instanceType", ...},
    {name: "ami.id", ...},
    {name: "rootVolume.size", ...},
    // one entry per synced field
}

Should I push a v2 with this approach?

[tthvo]

I do like the idea, but wanted to say that we are gradually migrating machine provisioning to CAPI. Eventually, all machine manifests are CAPI-based; thus, this issue will naturally go away as long as we keep a single source of truth for these machine manifests.

[chdeshpa-hue]

Thanks @patrickdillon @tthvo — both valid points. Proposing a redesign.

Context — these two PRs are interlinked:

PR	Bug	What it prevents
#10568 (this)	OCPBUGS-86417	User edits to openshift/ master machines silently ignored at provisioning → wrong EC2 specs
#10569	OCPBUGS-86418	CPMS template retains defaults → post-install rolling update silently reverts the user's customization

Together they form a cascading failure: the user's edit is first ignored (wrong instance provisioned), then actively undone (CPMS rolls back to defaults). Neither bug emits any warning today.

The current-version gap: AWS is the only major platform where MAPI and CAPI are generated as independent siblings — IBM Cloud, vSphere, and Nutanix already have unified capimachines.go builders. The CAPI-only migration won't be backported to 4.22, so users on this release need some guardrail.

Proposed redesign (addressing both concerns):

• OCPBUGS-86418: sync CPMS template from master machine providerSpecs to prevent unintended rolling update #10569 (CPMS): Replace field-by-field with a whole-providerSpec copy — the same pattern initial generation already uses (aws/machines.go L89, L145-146): copy master[0]'s providerSpec, clear Subnet + Placement.AvailabilityZone. Zero field maintenance, ~30 lines.

• Bug 86417: installer: sync MAPI master machine edits to CAPI AWSMachine manifests #10568 (this PR): Reduce to warning-only — no sync, no field mapping. Just detect that openshift/ master machines differ from cluster-api/machines/ and emit an actionable warning pointing users to the correct files. Trivially removable when the unified AWS builder lands.

Additionally, will draft a KCS knowledge article documenting the workaround for immediate customer relief.

Question: @tthvo — is there a target release for the unified AWS builder (aws/capimachines.go)? That helps scope how long the warning needs to live.