ROX-37314: Release the cluster deletion lock before cleanup by vikin91 · Pull Request #23210 · stackrox/stackrox

vikin91 · 2026-10-02T14:29:10Z

Description

Cluster deletion takes a lifecycle write lock so in-flight sensor writes finish before cleanup starts. BeginDeletion held that lock until postRemoveCluster returned.

Dev builds use a mutex watchdog (pkg/sync, //go:build !release). If an exclusive lock is held longer than 10 seconds, unlock aborts the process with SIGABRT. Deleting a cluster's deployments, nodes, secrets, and the rest takes longer than that, so Central dies in the middle of TestClusterDeletion. The test's next API call fails with connection refused. Release builds use the standard library mutex and do not abort, which is why this shows up on CI dev images. On this run the Central log is RWMutex.Unlock in BeginDeletion taking more than 10s, then SIGABRT, about 14 seconds after delete.

Enter already refuses a new lease once deleting is set, and it checks that flag before taking a read lock. The write lock only has to wait for leases that are already held. BeginDeletion now waits for those, then drops the lock before returning. The function it returns still clears deleting when cleanup finishes, so new writes stay out for the whole cleanup without holding the lock across it.

User-facing documentation

CHANGELOG.md is updated OR update is not needed
documentation PR is created and is linked above OR is not needed

Testing and quality

the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
CI results are inspected

Automated testing

How I validated my change

CI ocp-4-12-nongroovy-e2e-tests

AI-Assisted: cursor, gate change and unit test drafted by the agent, logic reviewed in chat.

Cluster deletion held the lifecycle write lock until postRemoveCluster finished. On dev builds the mutex watchdog SIGABRTs when an exclusive lock is held longer than 10s, which killed Central during TestClusterDeletion. BeginDeletion still waits for in-flight writers, then drops the lock. The deleting flag keeps new writes out until cleanup finishes. Request: scratch the previous branch contents and attempt a proper fix. Partially generated by AI.

openshift-ci · 2026-10-02T14:29:14Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

vikin91 · 2026-10-02T14:30:04Z

/retest-times 5 ocp-4-12-nongroovy-e2e-tests

coderabbitai · 2026-10-02T14:32:16Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: d886acd7-ceb6-4e75-ae86-49556df86233

📥 Commits

Reviewing files that changed from the base of the PR and between 21230a6 and e8f1a31.

📒 Files selected for processing (2)

central/cluster/lifecycle/gate.go
central/cluster/lifecycle/gate_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Summary

Summary by CodeRabbit

Bug Fixes
- Improved reliability during extended deletion cleanup, preventing long-running operations from triggering development-time timeout warnings.
- Deletion remains active during cleanup, and new entries are still rejected until cleanup finishes.
Tests
- Added coverage to verify deletion behavior while cleanup is in progress.

Walkthrough

Gate.BeginDeletion now drains active readers and releases the entry’s write lock before returning its cleanup function. The cleanup function still clears the deletion flag and releases the entry. A test checks lock access and rejection of a later Enter while deletion is active.

Changes

Deletion gate

Layer / File(s)	Summary
Deletion lock and validation `central/cluster/lifecycle/gate.go`, `central/cluster/lifecycle/gate_test.go`	`BeginDeletion` drains active readers, then releases the write lock before cleanup. The test checks that the lock can be acquired while deletion remains active and that a later `Enter` is rejected.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: stringy

Merge Risk: ⚪ Minimal · up to e8f1a

The change releases the lifecycle lock before slow cluster cleanup. This avoids the dev-build 10-second lock watchdog abort and keeps new writes blocked during deletion. No concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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.
Title check	✅ Passed	The title clearly and concisely describes the main change: releasing the cluster deletion lock before cleanup.
Description check	✅ Passed	The description is detailed and covers the problem, implementation, testing, and documentation sections. It clearly notes that CI inspection and validation are still incomplete.

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

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

codecov · 2026-10-02T14:36:28Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.93%. Comparing base (6599284) to head (e8f1a31).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #23210      +/-   ##
==========================================
- Coverage   51.96%   51.93%   -0.04%     
==========================================
  Files        2904     2904              
  Lines      182959   182958       -1     
==========================================
- Hits        95081    95014      -67     
- Misses      79564    79613      +49     
- Partials     8314     8331      +17

Flag	Coverage Δ
go-unit-tests	`51.93% <100.00%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2026-10-02T14:37:37Z

🚀 Build Images Ready

Images are ready for commit e8f1a31. To use with deploy scripts:

export MAIN_IMAGE_TAG=5.1.x-155-ge8f1a31708

rhacs-bot · 2026-10-02T14:42:58Z

/test ocp-4-12-nongroovy-e2e-tests

rhacs-bot · 2026-10-02T17:33:52Z

/test ocp-4-12-nongroovy-e2e-tests

rhacs-bot · 2026-10-02T20:23:08Z

/test ocp-4-12-nongroovy-e2e-tests

vikin91 added the ai-assisted label Oct 2, 2026

openshift-ci Bot added the do-not-merge/work-in-progress label Oct 2, 2026

github-actions Bot added the area/central label Oct 2, 2026

vikin91 added the auto-retest PRs with this label will be automatically retested if prow checks fails label Oct 2, 2026

vikin91 changed the title ~~fix: release the cluster deletion lock before cleanup~~ ROX-37314: Release the cluster deletion lock before cleanup Oct 2, 2026

Navigation Menu

ROX-37314: Release the cluster deletion lock before cleanup - #23210

ROX-37314: Release the cluster deletion lock before cleanup#23210
vikin91 wants to merge 1 commit into
masterfrom
piotr/cluster-deletion-stay-up

vikin91 commented Oct 2, 2026

Uh oh!

openshift-ci Bot commented Oct 2, 2026

Uh oh!

vikin91 commented Oct 2, 2026

Uh oh!

coderabbitai Bot commented Oct 2, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 2, 2026

Uh oh!

rhacs-bot commented Oct 2, 2026

Uh oh!

rhacs-bot commented Oct 2, 2026

Uh oh!

rhacs-bot commented Oct 2, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

vikin91 commented Oct 2, 2026

Description

User-facing documentation

Testing and quality

Automated testing

How I validated my change

Uh oh!

openshift-ci Bot commented Oct 2, 2026

Uh oh!

vikin91 commented Oct 2, 2026

Uh oh!

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

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Oct 2, 2026

🚀 Build Images Ready

Uh oh!

rhacs-bot commented Oct 2, 2026

Uh oh!

rhacs-bot commented Oct 2, 2026

Uh oh!

rhacs-bot commented Oct 2, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Oct 2, 2026 •

edited

Loading

codecov Bot commented Oct 2, 2026 •

edited

Loading