OCPBUGS-92817: delete MAPI MachineSets before CAPI in e2e cleanup by pmeida · Pull Request #611 · openshift/cluster-capi-operator

pmeida · 2026-06-26T10:22:02Z

Summary

Fixes a deadlock in cleanupMachineSetTestResources that causes e2e-aws-capi-techpreview to fail with a 15-minute timeout.

When a test creates both a CAPI MachineSet and a MAPI MachineSet with the same name and authoritativeAPI: ClusterAPI, deleting CAPI first causes the sync controller to loop in reconcileCAPItoMAPIMachineSetDeletionNormal - it waits for the CAPI-specific finalizer (cluster.x-k8s.io/machineset) to be removed, but its own constant requeues conflict with the CAPI controller's finalizer removal patch, deadlocking cleanup.

Deleting MAPI first instead triggers reconcileCAPItoMAPIMachineSetDeletionCAPINotDeleting, which removes the sync finalizer from CAPI immediately. The CAPI MachineSet can then be deleted cleanly with no sync interference.

Test plan

e2e-aws-capi-techpreview passes without the Should have deleted MachineSet openshift-cluster-api/capi-ms-auth-capi-* timeout

Fixes: https://issues.redhat.com/browse/OCPBUGS-92817

Summary by CodeRabbit

Bug Fixes
- Adjusted test cleanup order for machine set migrations so managed resources are removed in the correct sequence.
- Improved cleanup reliability by deleting MAPI machine sets before CAPI machine sets, helping avoid leftover resources during end-to-end runs.

openshift-merge-bot · 2026-06-26T10:22:05Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

openshift-ci-robot · 2026-06-26T10:22:09Z

@pmeida: This pull request references Jira Issue OCPBUGS-92817, 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)

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

Details

In response to this:

Summary

Fixes a deadlock in cleanupMachineSetTestResources that causes e2e-aws-capi-techpreview to consistently fail with a 15-minute timeout.

When a test creates both a CAPI MachineSet and a MAPI MachineSet with the same name and authoritativeAPI: ClusterAPI, deleting CAPI first causes the sync controller to loop in reconcileCAPItoMAPIMachineSetDeletionNormal — it waits for the CAPI-specific finalizer (cluster.x-k8s.io/machineset) to be removed, but its own constant requeues conflict with the CAPI controller's finalizer removal patch, deadlocking cleanup.

Deleting MAPI first instead triggers reconcileCAPItoMAPIMachineSetDeletionCAPINotDeleting, which removes the sync finalizer from CAPI immediately. The CAPI MachineSet can then be deleted cleanly with no sync interference.

Test plan

e2e-aws-capi-techpreview passes without the Should have deleted MachineSet openshift-cluster-api/capi-ms-auth-capi-* timeout

Fixes: https://issues.redhat.com/browse/OCPBUGS-92817

🤖 Generated with Claude Code

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-robot · 2026-06-26T10:22:46Z

@pmeida: This pull request references Jira Issue OCPBUGS-92817, which is valid.

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 POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

Summary

Fixes a deadlock in cleanupMachineSetTestResources that causes e2e-aws-capi-techpreview to consistently fail with a 15-minute timeout.

When a test creates both a CAPI MachineSet and a MAPI MachineSet with the same name and authoritativeAPI: ClusterAPI, deleting CAPI first causes the sync controller to loop in reconcileCAPItoMAPIMachineSetDeletionNormal — it waits for the CAPI-specific finalizer (cluster.x-k8s.io/machineset) to be removed, but its own constant requeues conflict with the CAPI controller's finalizer removal patch, deadlocking cleanup.

Deleting MAPI first instead triggers reconcileCAPItoMAPIMachineSetDeletionCAPINotDeleting, which removes the sync finalizer from CAPI immediately. The CAPI MachineSet can then be deleted cleanly with no sync interference.

Test plan

e2e-aws-capi-techpreview passes without the Should have deleted MachineSet openshift-cluster-api/capi-ms-auth-capi-* timeout

Fixes: https://issues.redhat.com/browse/OCPBUGS-92817

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 · 2026-06-26T10:23:52Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: a913ec79-d4dc-4182-803e-93cf673241d9

📥 Commits

Reviewing files that changed from the base of the PR and between 3dffe24 and f0ca2bb.

📒 Files selected for processing (1)

e2e/machineset_migration_helpers.go

Walkthrough

Modifies cleanupMachineSetTestResources in the e2e test helper to reorder cleanup operations so MAPI MachineSets are deleted before CAPI MachineSets, updating the associated function comment. No signature or behavioral scope changes occur.

Changes

Cleanup ordering change

Layer / File(s)	Summary
Reorder deletion sequence in cleanup helper `e2e/machineset_migration_helpers.go`	Updates the function comment describing deletion order and relocates the CAPI MachineSet deletion loop to execute after the MAPI MachineSet deletion loop.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested labels: verified

Suggested reviewers: mdbooth, theobarberbany

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely matches the main change: deleting MAPI MachineSets before CAPI during e2e cleanup.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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	The PR only reorders cleanup in a helper; no Ginkgo It/Describe/Context/When titles were added or made dynamic.
Test Structure And Quality	✅ Passed	Cleanup order change is bounded by existing timeouts, uses DeferCleanup/BeforeAll patterns, and doesn’t add any multi-behavior or weak-assertion test code.
Microshift Test Compatibility	✅ Passed	This PR only reorders cleanup in a helper; it adds no new Ginkgo tests or test bodies, so the MicroShift test-compatibility check is not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo tests were added; the only change is cleanup order in a helper, with no SNO-specific node assumptions.
Topology-Aware Scheduling Compatibility	✅ Passed	Only e2e cleanup order changed in the helper; no affinity, nodeSelector, topology spread, or replica/scheduling logic was added.
Ote Binary Stdout Contract	✅ Passed	PR only reorders cleanup in a test helper; suite entrypoints/init have no stdout writes, and the only report warning writes to os.Stderr.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Only a helper cleanup reorder changed; the file has no It/Describe/Context blocks and no IPv4 or external-connectivity assumptions.
No-Weak-Crypto	✅ Passed	Scanned all PR-touched files; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons were added.
Container-Privileges	✅ Passed	The PR only changes cleanup order in e2e/machineset_migration_helpers.go; no touched manifest sets privileged, hostPID, hostNetwork, hostIPC, or allowPrivilegeEscalation.
No-Sensitive-Data-In-Logs	✅ Passed	The only cleanup change reorders deletions; the file adds no new log statements exposing secrets or PII.

✨ 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

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

pmeida · 2026-06-26T10:51:11Z

This fixes the test's cleanup order to use the supported deletion path (MAPI first). When MAPI is deleted first, reconcileCAPItoMAPIMachineSetDeletionCAPINotDeleting removes the sync finalizer from CAPI immediately and steps aside, allowing clean deletion.

The root cause of the deadlock is in reconcileCAPItoMAPIMachineSetDeletionNormal: when CAPI is deleted first, the sync controller requeues indefinitely waiting for the CAPI-specific finalizer to be removed, and those requeues can conflict with the CAPI controller's own finalizer removal patch.
This fix avoids triggering that path in tests but it doesnt solve the core issue.

pmeida · 2026-06-26T12:23:41Z

/test e2e-aws-capi-techpreview

damdo · 2026-06-26T13:13:20Z

/assign @theobarberbany

openshift-ci-robot · 2026-06-26T14:32:05Z

@pmeida: This pull request references Jira Issue OCPBUGS-92817, which is valid.

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 POST, which is one of the valid states (NEW, ASSIGNED, POST)

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

Details

In response to this:

Summary

Fixes a deadlock in cleanupMachineSetTestResources that causes e2e-aws-capi-techpreview to fail with a 15-minute timeout.

When a test creates both a CAPI MachineSet and a MAPI MachineSet with the same name and authoritativeAPI: ClusterAPI, deleting CAPI first causes the sync controller to loop in reconcileCAPItoMAPIMachineSetDeletionNormal - it waits for the CAPI-specific finalizer (cluster.x-k8s.io/machineset) to be removed, but its own constant requeues conflict with the CAPI controller's finalizer removal patch, deadlocking cleanup.

Deleting MAPI first instead triggers reconcileCAPItoMAPIMachineSetDeletionCAPINotDeleting, which removes the sync finalizer from CAPI immediately. The CAPI MachineSet can then be deleted cleanly with no sync interference.

Test plan

e2e-aws-capi-techpreview passes without the Should have deleted MachineSet openshift-cluster-api/capi-ms-auth-capi-* timeout

Fixes: https://issues.redhat.com/browse/OCPBUGS-92817

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.

pmeida · 2026-06-30T11:04:22Z

/verified by e2e-aws-capi-techpreview

openshift-ci-robot · 2026-06-30T11:04:35Z

@pmeida: This PR has been marked as verified by e2e-aws-capi-techpreview.

Details

In response to this:

/verified by e2e-aws-capi-techpreview

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.

theobarberbany · 2026-06-30T11:54:00Z

/lgtm
/approve

nice one :)

openshift-merge-bot · 2026-06-30T11:54:14Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-capi-disconnected-techpreview
/test e2e-aws-capi-techpreview
/test e2e-aws-capi-techpreview-post-install
/test e2e-aws-ovn-techpreview
/test e2e-aws-ovn-techpreview-upgrade
/test e2e-azure-capi-techpreview
/test e2e-azure-ovn-techpreview
/test e2e-azure-ovn-techpreview-upgrade
/test e2e-gcp-capi-techpreview
/test e2e-gcp-ovn-techpreview
/test e2e-metal3-capi-techpreview
/test e2e-openstack-capi-techpreview
/test e2e-vsphere-capi-techpreview
/test regression-clusterinfra-aws-ipi-techpreview-capi

openshift-ci · 2026-06-30T11:54:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: theobarberbany

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~~ [theobarberbany]

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 · 2026-06-30T15:33:48Z

/retest-required

Remaining retests: 0 against base HEAD 3dffe24 and 2 for PR HEAD b2e2670 in total

When a test creates both a CAPI MachineSet and a MAPI MachineSet with the same name and authoritativeAPI: ClusterAPI, the sync controller manages deletion through reconcileCAPItoMAPIMachineSetDeletionNormal. Deleting CAPI first causes the sync controller to issue deletion to MAPI and then loop waiting for the CAPI-specific finalizer (cluster.x-k8s.io/machineset) to be removed. The sync controller's constant requeues conflict with the CAPI controller's finalizer removal patch, causing a deadlock. Deleting MAPI first triggers reconcileCAPItoMAPIMachineSetDeletionCAPINotDeleting which removes the sync finalizer from CAPI immediately. The CAPI MachineSet can then be deleted cleanly with only its own finalizer to manage. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

openshift-ci · 2026-07-01T07:30:24Z

New changes are detected. LGTM label has been removed.

openshift-ci-robot · 2026-07-01T07:35:39Z

@pmeida: This pull request references Jira Issue OCPBUGS-92817, which is valid.

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 POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

Summary

Fixes a deadlock in cleanupMachineSetTestResources that causes e2e-aws-capi-techpreview to fail with a 15-minute timeout.

When a test creates both a CAPI MachineSet and a MAPI MachineSet with the same name and authoritativeAPI: ClusterAPI, deleting CAPI first causes the sync controller to loop in reconcileCAPItoMAPIMachineSetDeletionNormal - it waits for the CAPI-specific finalizer (cluster.x-k8s.io/machineset) to be removed, but its own constant requeues conflict with the CAPI controller's finalizer removal patch, deadlocking cleanup.

Deleting MAPI first instead triggers reconcileCAPItoMAPIMachineSetDeletionCAPINotDeleting, which removes the sync finalizer from CAPI immediately. The CAPI MachineSet can then be deleted cleanly with no sync interference.

Test plan

e2e-aws-capi-techpreview passes without the Should have deleted MachineSet openshift-cluster-api/capi-ms-auth-capi-* timeout

Fixes: https://issues.redhat.com/browse/OCPBUGS-92817

Summary by CodeRabbit

Bug Fixes

Adjusted test cleanup order for machine set migrations so managed resources are removed in the correct sequence.

Improved cleanup reliability by deleting MAPI machine sets before CAPI machine sets, helping avoid leftover resources during end-to-end runs.

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 · 2026-07-01T08:01:16Z

@pmeida: 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/regression-clusterinfra-aws-ipi-techpreview-capi	`b2e2670`	link	false	`/test regression-clusterinfra-aws-ipi-techpreview-capi`
ci/prow/e2e-aws-capi-techpreview-post-install	`b2e2670`	link	true	`/test e2e-aws-capi-techpreview-post-install`
ci/prow/e2e-gcp-ovn-techpreview	`b2e2670`	link	true	`/test e2e-gcp-ovn-techpreview`
ci/prow/e2e-aws-capi-techpreview	`b2e2670`	link	true	`/test e2e-aws-capi-techpreview`

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.

pmeida · 2026-07-01T08:03:14Z

/retest-required

damdo · 2026-07-01T08:17:20Z

/pipeline required

openshift-merge-bot · 2026-07-01T08:17:23Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-capi-disconnected-techpreview
/test e2e-aws-capi-techpreview
/test e2e-aws-capi-techpreview-post-install
/test e2e-aws-ovn-techpreview
/test e2e-aws-ovn-techpreview-upgrade
/test e2e-azure-capi-techpreview
/test e2e-azure-ovn-techpreview
/test e2e-azure-ovn-techpreview-upgrade
/test e2e-gcp-capi-techpreview
/test e2e-gcp-ovn-techpreview
/test e2e-metal3-capi-techpreview
/test e2e-openstack-capi-techpreview
/test e2e-vsphere-capi-techpreview
/test regression-clusterinfra-aws-ipi-techpreview-capi

openshift-ci-robot added jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Jun 26, 2026

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Jun 26, 2026

openshift-ci Bot requested review from racheljpg and theobarberbany June 26, 2026 10:22

pmeida changed the title ~~OCPBUGS-92817: delete MAPI MachineSets before CAPI in cleanup~~ OCPBUGS-92817: delete MAPI MachineSets before CAPI in e2e cleanup Jun 26, 2026

pmeida mentioned this pull request Jun 26, 2026

OCPCLOUD-3507: Add OTE e2e tests for ClusterAPIMachineManagement feature gate #606

Open

openshift-ci Bot assigned theobarberbany Jun 26, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jun 30, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 30, 2026

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 30, 2026

pmeida force-pushed the OCPBUGS-92817 branch from b2e2670 to f0ca2bb Compare July 1, 2026 07:30

openshift-ci-robot removed the verified Signifies that the PR passed pre-merge verification criteria label Jul 1, 2026

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 1, 2026

Uh oh!

Conversation

pmeida commented Jun 26, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented Jun 26, 2026

Uh oh!

openshift-ci-robot commented Jun 26, 2026

Summary

Test plan

Uh oh!

openshift-ci-robot commented Jun 26, 2026

Summary

Test plan

Uh oh!

coderabbitai Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

pmeida commented Jun 26, 2026

Uh oh!

pmeida commented Jun 26, 2026

Uh oh!

damdo commented Jun 26, 2026

Uh oh!

openshift-ci-robot commented Jun 26, 2026

Summary

Test plan

Uh oh!

pmeida commented Jun 30, 2026

Uh oh!

openshift-ci-robot commented Jun 30, 2026

Uh oh!

theobarberbany commented Jun 30, 2026

Uh oh!

openshift-merge-bot Bot commented Jun 30, 2026

Uh oh!

openshift-ci Bot commented Jun 30, 2026

Uh oh!

openshift-merge-bot Bot commented Jun 30, 2026

Uh oh!

openshift-ci Bot commented Jul 1, 2026

Uh oh!

openshift-ci-robot commented Jul 1, 2026

Summary

Test plan

Summary by CodeRabbit

Uh oh!

openshift-ci Bot commented Jul 1, 2026

Uh oh!

pmeida commented Jul 1, 2026

Uh oh!

damdo commented Jul 1, 2026

Uh oh!

openshift-merge-bot Bot commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pmeida commented Jun 26, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 26, 2026 •

edited

Loading