Add subpath support to volumes in --mount option
#24532
Conversation
openshift-ci
bot
added
release-note
approved
mheon
force-pushed
the
subpath
branch
2 times, most recently
from
f7bdadf to
61e5275
Compare
pkg/specgenutil/volumes.go
Outdated
| safeOpts := make([]string, 0, len(mnt.Options)) | ||
| for _, opt := range mnt.Options { | ||
| splitOpt := strings.SplitN(opt, "=", 2) | ||
| if splitOpt[0] == "subpath" { | ||
| if len(splitOpt) != 2 { | ||
| return nil, fmt.Errorf("subpath was not provided a path") | ||
| } | ||
| newVolume.SubPath = splitOpt[1] | ||
| } else { | ||
| safeOpts = append(safeOpts, opt) | ||
| } | ||
| } |
There was a problem hiding this comment.
Please not another "random" place to parse options. Can't we just fix parseMountOptions() to return a suitable type? We already have to many different places where we pass mount options
pkg/specgenutil/volumes.go
Outdated
| ) | ||
|
|
||
| type universalMount struct { | ||
| mount spec.Mount |
There was a problem hiding this comment.
if we embed the struct we would not need to add the extra .mount everywhere
There was a problem hiding this comment.
I'd prefer it that way if we could just cast the universalMount to a spec.Mount after, but we can't. So it saves us a few changes in parseMountOptions but hurts more than helps in the callers.
All the backend work was done a while back for image volumes, so this is effectively just plumbing the option in for volumes in the parser logic. We do need to change the return type of the volume parser as it only worked on spec.Mount before (which does not have subpath support, so we'd have to pass it as an option and parse it again) but that is cleaner than the alternative. Fixes containers#20661 Signed-off-by: Matt Heon <mheon@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, mheon 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 |
|
/lgtm |
|
Can we expect this in the next minor version? |
|
Yes. Per our release schedule that is 5.4 in February. |
stale-locking-app
bot
added
the
locked - please file new issue/PR
4 participants
All the backend work was done a while back for image volumes, so this is effectively just plumbing the option in for volumes in the parser logic. It's a little uglier than I'd like because our volume mount parser produces an OCI Spec mount struct, which doesn't have a field for subpath, so we have to pass through the options array and parse it back out after, which is a little gross. But not gross enough to rewrite that parsing logic.
Fixes #20661
Does this PR introduce a user-facing change?