[Volume Plugins] Specifying options resets Volume Driver to local. #9650
Comments
openshift-ci-robot
added
the
kind/bug
|
I checked the state of and it is correct: So need to investigate Then... In function ( Entered gets printed in both cases, but SURVIVED only when options are blank. And finally, I got here: Line 1574 in d92b946 This function gets called for every driver, but actually local-specific one. This must be the reason of the bug. By doing so: func WithVolumeOptions(options map[string]string) VolumeCreateOption {
return func(volume *Volume) error {
if volume.valid {
return define.ErrVolumeFinalized
}
volume.config.Options = make(map[string]string)
for key, value := range options {
// switch key {
//case "type", "device", "o", "UID", "GID":
// volume.config.Options[key] = value
//default:
// return errors.Wrapf(define.ErrInvalidArg, "unrecognized volume option %q is not supported with local driver", key)
//}
volume.config.Options[key] = value
}
return nil
}
}Bug is fixed. Create a PR? Deleting this checks here seems to be ok, because we have: podman/libpod/runtime_volume_linux.go Line 69 in d92b946 the same checks here. Considering podman/libpod/runtime_volume_linux.go Line 60 in d92b946 better solution would be refactoring local into the kind-of-plugin with the same interface. As such this interface could be extended with checkCreateVolumeOptions method, checkNeedCreate, actuallyCreateVolume and so on. This will greatly reduce complexity of current implementation.
|
|
Can you verify that |
|
It does have an issue. Actually I first encountered it in cli, but tracking down is simpler using REST API. One of the important thing here is that option must have different option key than ones that local driver supports. So, you won't have that exact bug with plugin that only supports "type", "device", "o", "UID" and/or "GID". But the following issue will arise instead: "cannot mount in rootless mode" . I think I figured out this one too: I will update this message with details ASAP. Here is the patch that allowed me to use rootlessly the volume driver plugin: diff --git a/libpod/options.go b/libpod/options.go
index 6344e1acc..9e09a2164 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -1580,12 +1580,12 @@ func WithVolumeOptions(options map[string]string) VolumeCreateOption {
volume.config.Options = make(map[string]string)
for key, value := range options {
- switch key {
- case "type", "device", "o", "UID", "GID":
- volume.config.Options[key] = value
- default:
- return errors.Wrapf(define.ErrInvalidArg, "unrecognized volume option %q is not supported with local driver", key)
- }
+ //switch key {
+ //case "type", "device", "o", "UID", "GID":
+ // volume.config.Options[key] = value
+ //default:
+ // return errors.Wrapf(define.ErrInvalidArg, "unrecognized volume option %q is not supported with local driver", key)
+ //}
volume.config.Options[key] = value
}
diff --git a/libpod/volume_internal_linux.go b/libpod/volume_internal_linux.go
index 67ac41874..bd6c5650b 100644
--- a/libpod/volume_internal_linux.go
+++ b/libpod/volume_internal_linux.go
@@ -33,7 +33,7 @@ func (v *Volume) mount() error {
}
// We cannot mount volumes as rootless.
- if rootless.IsRootless() {
+ if !v.UsesVolumeDriver() && rootless.IsRootless() {
return errors.Wrapf(define.ErrRootless, "cannot mount volumes without root privileges")
}
@@ -147,8 +147,9 @@ func (v *Volume) unmount(force bool) error {
v.state.MountCount = 0
return v.save()
}
-
- return errors.Wrapf(define.ErrRootless, "cannot mount or unmount volumes without root privileges")
+ if !v.UsesVolumeDriver() {
+ return errors.Wrapf(define.ErrRootless, "cannot mount or unmount volumes without root privileges")
+ }
}
if !force {I need some more research into this to confirm that It resolves all issues and doesn't break anything. The second fix (cannot mount without root error) has very solid foundation behind it: plugins are the way of delegation the hard work to something external with well defined API. Being a black box from podman point of view, this service might or might not require root to function. We don't know this information and should not apply a local-specific heuristics to try to detect that. |
|
Here is latest code of my ongoing rework of famous docker plugin for podman: https://gist.github.com/thephoenixofthevoid/2ced5a66e94777ad60d6a0b3b54d66c5 It depended on like 3-6 packages like docker-volume-sdk and so on. I examined all of them and inlined only relevant part of these (both relevant for such application and relevant for podman). The goal is to factor out common framework to build arbitrary plugins rapidly. The structure is already clearly visible. Example:
|
|
If you're willing to submit it as a PR, I would accept the above patch if it was changed to still validate options if the driver was set to |
|
It happens here: podman/libpod/runtime_volume_linux.go Lines 66 to 77 in d92b946 And there is the check you are talking about right now. It looks like there are 2 checks of the same thing before normalization of arguments and right after that. |
|
Alright, this looks fine, then. |
|
A friendly reminder that this issue had no activity for 30 days. |
|
I'm going to go ahead and close, since #9808 merged with the fix. |
github-actions
bot
added
the
locked - please file new issue/PR
Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)
/kind bug
If using volume plugin with specified options, Driver (--driver) argument is completely ignored.
Steps to reproduce the issue:
Forked
docker-volume-sshfsand changed paths to socket (it has been hardcoded).Added a record in
containers.confStarted a plugin. Then tried to create a volume without options and specifying them.
Describe the results you received:
For
podman volume create --driver testpluginas well as using API endpoint/libpod/volumes/createwith{ Driver: "testplugin" }got:API Endpoint response:
Which is correct actually. But.
For
podman volume create --driver testplugin --opt sshcmd=<whatever>as well as using API endpoint/libpod/volumes/createwith{ Driver: "testplugin", Options: { sshcmd: <whatever> } }got:and the response:
Describe the results you expected:
Expected to create a volume that corresponds to a remote sshfs.
Additional information you deem important (e.g. issue happens only occasionally):
This might be related:
podman/cmd/podman/volumes/create.go
Line 77 in d92b946
Output of
podman version:Output of
podman info --debug:Package info (e.g. output of
rpm -q podmanorapt list podman):Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide?
Yes/No
Additional environment details (AWS, VirtualBox, physical, etc.):
The text was updated successfully, but these errors were encountered: