fix: re-enqueue the snapshot CR for the snapshot deletion during upgrading the engine #4432
Conversation
during upgrading the engine. longhorn/longhorn 10274 Signed-off-by: James Lu <[email protected]>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR addresses issue longhorn/longhorn#10274 by preventing snapshot deletion operations from proceeding while the volume engine is upgrading. The change ensures that snapshot deletion is delayed until the engine upgrade completes, preventing potential issues during the upgrade process.
Changes:
- Added a check for engine upgrade status during snapshot deletion that delays the operation by re-enqueueing the snapshot after 3 seconds
- Sets an informative error message indicating why the deletion is delayed
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sc.enqueueSnapshotAfter(snapshot, 3*time.Second) | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
The Status.Error field is set when the engine is upgrading but is not explicitly cleared when the engine finishes upgrading and deletion proceeds. Unlike the snapshot creation path where syncSnapshotWithSnapshotInfo clears the error when ReadyToUse becomes true (line 723), during deletion the DeletionTimestamp prevents error clearing. This means the "snapshot deletion delayed" message may persist in the status until the snapshot is fully removed, potentially causing confusion for users monitoring the snapshot status. Consider adding snapshot.Status.Error = "" after the isEngineUpgrading check to clear the error when deletion can proceed.
| // Clear any previous snapshot deletion delay error now that the engine is no longer upgrading | |
| snapshot.Status.Error = "" |
|
@mergify backport v1.11.x |
✅ Backports have been createdDetails
|
Which issue(s) this PR fixes:
Issue # longhorn/longhorn#10274
What this PR does / why we need it:
Special notes for your reviewer:
Additional documentation or context
https://ci.longhorn.io/job/private/job/longhorn-tests-regression/10195
https://ci.longhorn.io/job/private/job/longhorn-tests-regression/10199
https://ci.longhorn.io/job/private/job/longhorn-tests-regression/10207