fix(storage): roll back unmounted volumes on getBlockVolumes error - #860
Open
Anand-240 wants to merge 1 commit into
Open
fix(storage): roll back unmounted volumes on getBlockVolumes error#860Anand-240 wants to merge 1 commit into
Anand-240 wants to merge 1 commit into
Conversation
getBlockVolumes unmounts each supported bind-mounted block volume from the host one at a time. If any mount after the first failed to process (a later getMountInfo error, a failed setLoopAutoclear, or a failed mount.Unmount), the function returned immediately and dropped the list of volumes it had already unmounted. Those earlier host mounts were never restored, and since create failed before state.json was written, delete had no record of them either. The result was host bind mounts left permanently detached after a single transient failure during container creation. Add rollbackBlockVolumes, which restores any already-unmounted volumes via the existing restoreBlockVolumes helper before an error is returned from getBlockVolumes. Also handle the case where the autoclear flag was cleared on a device but its unmount still failed, by restoring the flag directly for that entry. Fixes: urunc-dev#859 Signed-off-by: Anand-240 <srivastavaanandprakash16@gmail.com>
✅ Deploy Preview for urunc canceled.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
getBlockVolumesunmounts host bind mounts one at a time so they can be handed to the guest as block devices. The problem is that if any mount after the first one fails during that process (bad mountinfo, autoclear flag change fails, or the unmount itself fails), the function bails out immediately and throws away the list of volumes it had already unmounted. Those earlier mounts just stay detached on the host, and sincecreatefails beforestate.jsongets written,deletedoesn't even know they exist to restore them.This PR adds
rollbackBlockVolumes, which restores whatever was already unmounted (reusing the existingrestoreBlockVolumeslogic) before the error is returned. It also covers the case where the autoclear flag got cleared on a device but the unmount call itself failed, by restoring that flag directly.Related issues
getBlockVolumesare silently discarded on error, permanently orphaning already-unmounted block volumes #859How was this tested?
gofmt -lcleanGOOS=linux GOARCH=amd64(no build/vet issues)rollbackBlockVolumescovering the no-op case and volumes without a host mountpointmake test_unikontainers/ e2e suite, so leaving that to CILLM usage
Used Claude to help track down the bug and put this fix together, reviewed and tested everything myself before opening this.
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).