-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add nocreate option for named volumes #27867
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
base: main
Are you sure you want to change the base?
Conversation
Add a per-volume 'nocreate' option that prevents automatic creation of
named volumes when they don't exist. When specified, Podman will fail
if the volume is not found instead of creating it automatically.
Usage: -v myvolume:/data:nocreate
--mount type=volume,src=myvolume,dst=/data,nocreate
See: containers#27862
Signed-off-by: Ygal Blum <ygal.blum@gmail.com>
11d80db to
2efb9ae
Compare
Luap99
left a comment
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.
Thanks, I prefer this one. One comment on the implementation code but otherwise this seem fine.
| // Extract nocreate option before validation since it's not a standard mount option | ||
| rawOpts := strings.Split(splitVol[2], ",") | ||
| filteredOpts := make([]string, 0, len(rawOpts)) | ||
| for _, opt := range rawOpts { | ||
| if opt == "nocreate" { | ||
| noCreate = true | ||
| } else { | ||
| filteredOpts = append(filteredOpts, opt) | ||
| } | ||
| } | ||
| if len(filteredOpts) > 0 { | ||
| if options, err = parse.ValidateVolumeOpts(filteredOpts); err != nil { | ||
| return nil, nil, nil, err | ||
| } |
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.
I think this needs to be moved into ValidateVolumeOpts it already has options there that are not real mount options anyway.
Doing parsing in several places is never good otherwise, we should filter that option out later I think for consistent, i.e.
#26945
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.
The reason I chose this way was that ValidateVolumeOpts is implemented in the common repo and this option was only for Podman. So, I wasn't sure if putting a usage specific option in the common code would be a better practice. But, I guess this is not the only case. So, I'll open a PR there and update this PR once its merged.
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.
I've added it to the valid list: containers/container-libs#585
Once merged, I will move the actual parsing into
Line 35 in fd90d33
| func processOptionsInternal(options []string, isTmpfs bool, sourcePath string, getDefaultMountOptions getDefaultMountOptionsFn) ([]string, error) { |
Add a per-volume 'nocreate' option that prevents automatic creation of named volumes when they don't exist. When specified, Podman will fail if the volume is not found instead of creating it automatically.
Usage: -v myvolume:/data:nocreate
--mount type=volume,src=myvolume,dst=/data,nocreate
See: #27862
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?