Add nocreate option for named volumes#27867
Conversation
ygalblum
added
the
do-not-merge/hold
ygalblum
force-pushed
the
volume-nocreate
branch
from
11d80db to
2efb9ae
Compare
Luap99
left a comment
Luap99
left a comment
There was a problem hiding this comment.
Thanks, I prefer this one. One comment on the implementation code but otherwise this seem fine.
pkg/specgen/volumes.go
Outdated
| // 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.
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.
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.
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
ygalblum
force-pushed
the
volume-nocreate
branch
from
2efb9ae to
2af27cf
Compare
|
@Luap99 I've updated the implementation following the change in the common code. I think I got lost a bit in the abstractions. So, I'm not sure if there is a better way to address this (better being not changing the signature of |
ygalblum
removed
the
do-not-merge/hold
|
@containers/podman-maintainers PTAL |
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>
ygalblum
force-pushed
the
volume-nocreate
branch
from
2af27cf to
64ec31a
Compare
|
I've rebased on |
4 participants
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?