OCPBUGS-47508: Add max-concurrent-reconciles flag to machine actuator

[RadekManak]

Summary

Add --max-concurrent-reconciles (default 1) and pass it through machine.AddWithActuatorOpts into controller-runtime MaxConcurrentReconciles, matching Azure and GCP.

What was checked

Thread-safety / concurrency skim of MAPA paths that matter at MaxConcurrentReconciles=10:

• controller-runtime still serializes reconciles per Machine key; concurrency only parallelizes across different Machines
• regionCache is sync.RWMutex-guarded; shared-credentials file construction is mutexed
• AWS clients/sessions are built per reconcile (no long-lived shared EC2 client across Machines)
• Create path looks up existing instances before RunInstances (idempotency guard); same Machine cannot double-create under key serialization
• MachineSet instance-type cache is separate and also mutexed; drain controller is unaffected (still default options)
• No unlocked process-global mutable maps found on the machine actuator path

Residual ops risk is normal EC2 API pressure/throttling under large waves (same class of risk Azure/GCP already accept at concurrency 10), not memory unsafety.

Summary by CodeRabbit

• New Features
  • Added a command-line option to configure the maximum number of concurrent reconciliation operations.
  • Improved control over controller processing concurrency during manager startup.

[openshift-ci-robot]

@RadekManak: This pull request references Jira Issue OCPBUGS-47508, which is invalid:

• expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

• Add --max-concurrent-reconciles (default 1) and wire it through machine.AddWithActuatorOpts, matching the Azure/GCP pattern.
• Required companion so machine-api-operator can pass --max-concurrent-reconciles=10 on AWS without the controller rejecting the unknown flag.

This enables higher machine-controller concurrency during large AWS scale-ups (address population catch-up for CSR approval); see OCPBUGS-47508.

Test plan

• GOFLAGS="-mod=vendor -buildvcs=false" NO_DOCKER=1 make machine-controller-manager
• Binary accepts --max-concurrent-reconciles=10 --version
• go test ./cmd/manager/ -count=1
• CI

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.

[coderabbitai]

Walkthrough

The manager adds a --max-concurrent-reconciles flag, defaulting to 1, and passes its value to AWS machine actuator registration through controller.Options.

Changes

Controller concurrency configuration

Layer / File(s)	Summary
Manager concurrency wiring cmd/manager/main.go	Adds the concurrency flag and configures actuator registration with MaxConcurrentReconciles.

Estimated code review effort: 2 (Simple) | ~5 minutes

Suggested reviewers: nrb, theobarberbany

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	Only cmd/manager/main.go changed; no Ginkgo test titles or test files were added or modified, so there are no unstable test names to flag.
Test Structure And Quality	✅ Passed	No Ginkgo tests were added or changed; the diff only touches cmd/manager/main.go, so this checklist is not applicable.
Microshift Test Compatibility	✅ Passed	Only cmd/manager/main.go changed; it adds a CLI flag and controller options, with no new Ginkgo/e2e tests or MicroShift-sensitive APIs.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Only cmd/manager/main.go changed; no new or modified Ginkgo e2e tests were added, so SNO compatibility is not implicated.
Topology-Aware Scheduling Compatibility	✅ Passed	Only added a max-concurrency flag and controller option; no affinity, nodeSelector, spread, PDB, or topology-dependent replica logic was introduced.
Ote Binary Stdout Contract	✅ Passed	PASS: Patch only adds a flag and wires MaxConcurrentReconciles into controller options; no new process-level stdout writes were introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Only cmd/manager/main.go changed; no new Ginkgo/e2e tests or external connectivity assumptions were added.
No-Weak-Crypto	✅ Passed	The PR only adds a concurrency flag in cmd/manager/main.go; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons were introduced.
Container-Privileges	✅ Passed	Only cmd/manager/main.go changed; no container/K8s manifests or securityContext fields were added.
No-Sensitive-Data-In-Logs	✅ Passed	Diff only adds a concurrency flag and wires it into controller options; no new logs or sensitive fields are introduced.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main change: adding the max-concurrent-reconciles flag to the machine actuator.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/manager/main.go`:
- Around line 113-117: After flag.Parse(), validate the value referenced by
maxConcurrentReconciles and fail fast unless it is at least 1. Reject zero and
negative --max-concurrent-reconciles inputs before starting the manager, rather
than relying on controller-runtime’s coercion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: fb8d805f-97c3-4e6d-b94d-f24676b1f1d5

📥 Commits

Reviewing files that changed from the base of the PR and between 1071858 and 910a271.

📒 Files selected for processing (1)

• cmd/manager/main.go

[RadekManak]

/jira refresh

[openshift-ci-robot]

@RadekManak: This pull request references Jira Issue OCPBUGS-47508, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.0.0) matches configured target version for branch (5.0.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @sunzhaohua2

Details

In response to this:

/jira refresh

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.

[openshift-ci]

@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: sunzhaohua2.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

@RadekManak: This pull request references Jira Issue OCPBUGS-47508, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.0.0) matches configured target version for branch (5.0.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @sunzhaohua2

In response to this:

/jira refresh

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.

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.

[RadekManak]

/retest

[damdo]

/approve

@RadekManak have we ran some testing with higher concurrency count in CI?

[RadekManak]

@damdo I want to request perfscale team to rerun the 2000 machine scale-up after all 3 PR linked to this bug merge. I am confident this change is safe. I am proposing we merge this with e2e only.

openshift/machine-api-operator#1521 PR actually sets this to concurrency=10
openshift/cluster-machine-approver#310 Core fix to avoid the issue even if concurrency=10 does not help.

[RadekManak]

This PR adds only a flag that is not set yet and defaults to 1.
/override ci/prow/e2e-aws-serial-2of2
/verified by e2e

[openshift-ci-robot]

@RadekManak: This PR has been marked as verified by e2e.

Details

In response to this:

This PR adds only a flag that is not set yet and defaults to 1.
/override ci/prow/e2e-aws-serial-2of2
/verified by e2e

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.

[openshift-ci]

@RadekManak: Overrode contexts on behalf of RadekManak: ci/prow/e2e-aws-serial-2of2

Details

In response to this:

This PR adds only a flag that is not set yet and defaults to 1.
/override ci/prow/e2e-aws-serial-2of2
/verified by e2e

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.

[damdo]

@damdo I want to request perfscale team to rerun the 2000 machine scale-up after all 3 PR linked to this bug merge. I am confident this change is safe. I am proposing we merge this with e2e only.

openshift/machine-api-operator#1521 PR actually sets this to concurrency=10 openshift/cluster-machine-approver#310 Core fix to avoid the issue even if concurrency=10 does not help.

Fine with that, but if the perfscale tests report issues WRT races I expect this will get reverted before branching. Thanks

[damdo]

/lgtm

[mdbooth]

/approve
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damdo, mdbooth

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [damdo]

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

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD 13cbf8a and 2 for PR HEAD 1dccc61 in total

[openshift-ci]

@RadekManak: all tests passed!

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]

@RadekManak: Jira Issue OCPBUGS-47508: Some pull requests linked via external trackers have merged:

• openshift/machine-api-provider-aws#195

The following pull request, linked via external tracker, has not merged:

• openshift/machine-api-operator#1521 is open

All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

Jira Issue OCPBUGS-47508 has not been moved to the MODIFIED state.

This PR is marked as verified. If the remaining PRs listed above are marked as verified before merging, the issue will automatically be moved to VERIFIED after all of the changes from the PRs are available in an accepted nightly payload.

Details

In response to this:

Summary

Add --max-concurrent-reconciles (default 1) and pass it through machine.AddWithActuatorOpts into controller-runtime MaxConcurrentReconciles, matching Azure and GCP.

What was checked

Thread-safety / concurrency skim of MAPA paths that matter at MaxConcurrentReconciles=10:

• controller-runtime still serializes reconciles per Machine key; concurrency only parallelizes across different Machines
• regionCache is sync.RWMutex-guarded; shared-credentials file construction is mutexed
• AWS clients/sessions are built per reconcile (no long-lived shared EC2 client across Machines)
• Create path looks up existing instances before RunInstances (idempotency guard); same Machine cannot double-create under key serialization
• MachineSet instance-type cache is separate and also mutexed; drain controller is unaffected (still default options)
• No unlocked process-global mutable maps found on the machine actuator path

Residual ops risk is normal EC2 API pressure/throttling under large waves (same class of risk Azure/GCP already accept at concurrency 10), not memory unsafety.

Summary by CodeRabbit

• New Features
• Added a command-line option to configure the maximum number of concurrent reconciliation operations.
• Improved control over controller processing concurrency during manager startup.

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.