-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
volume ls: fix race that caused it to fail #24479
Conversation
If volume ls was called while another volume was removed at the right time it could have failed with "no such volume" as we did not ignore such error during listing. As we list things and this no longer exists the correct thing is to ignore the error and continue like we do with containers, pods, etc... This was pretty easy to reproduce with these two commands running in different terminals: while :; do bin/podman volume create test && bin/podman volume rm test || break; done while :; do bin/podman volume ls || break ; done I have a slight feeling that this might solve containers#23913 but I am not to sure there so I am not adding a Fixes here. Signed-off-by: Paul Holzinger <[email protected]>
@edsantiago PTAL, I think this fixes a very obvious volume ls flake. But I am still a bit confused about #23913 so I am not going to claim this closes it but I think it is possible it does |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM regardless of whether or not it fixes the volume eater. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, giuseppe, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
If volume ls was called while another volume was removed at the right time it could have failed with "no such volume" as we did not ignore such error during listing. As we list things and this no longer exists the correct thing is to ignore the error and continue like we do with containers, pods, etc...
This was pretty easy to reproduce with these two commands running in different terminals:
I have a slight feeling that this might solve #23913 but I am not to sure there so I am not adding a Fixes here.
Does this PR introduce a user-facing change?