Skip to content

d/cluster/convert: expose Addr() on plugins #49961

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

Merged
merged 1 commit into from
May 15, 2025

Conversation

corhere
Copy link
Contributor

@corhere corhere commented May 13, 2025

- What I did

- How I did it
The swarmPlugin type does not implement the Swarm plugin.AddrPlugin interface because it embeds an interface value which does not include that method in its method set. (You can type-assert an interface value to another interface which the concrete type implements, but a struct embedding an interface value is not itself an interface value.) Wrap the plugin with a different adapter type which exposes the Addr() method if the concrete plugin implements it.

- How to verify it
Hopes and prayers. Perhaps @dperny could lend me a hand in testing it?

- Human readable description for the release notes

Fix the `plugin does not implement PluginAddr interface` error for Swarm CSI drivers.

- A picture of a cute animal (not mandatory but encouraged)

The swarmPlugin type does not implement the Swarm plugin.AddrPlugin
interface because it embeds an interface value which does not include
that method in its method set. (You can type-assert an interface value
to another interface which the concrete type implements, but a struct
embedding an interface value is not itself an interface value.) Wrap the
plugin with a different adapter type which exposes the Addr() method if
the concrete plugin implements it.

Signed-off-by: Cory Snider <[email protected]>
Copy link
Contributor

@olljanat olljanat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

- How to verify it Hopes and prayers. Perhaps @dperny could lend me a hand in testing it?

There is quite many examples how to test these in https://github.com/olljanat/csi-plugins-for-docker-swarm
Democratic CSI is simplified version of what I got from @dperny and others should be working too.

I quickly tested with SMB driver and here are results.

Earlier volume creation failed error like this

DEBU[2025-05-12T19:37:33.585179851Z] handling POST request                         form-data="{\"ClusterVolumeSpec\":{\"AccessMode\":{\"MountVolume\":{},\"Scope\":\"single\",\"Sharing\":\"none\"},\"AccessibilityRequirements\":{},\"Availability\":\"active\",\"CapacityRange\":{\"LimitBytes\":0,\"RequiredBytes\":0},\"Secrets\":[{\"Key\":\"password\",\"Secret\":\"*****\"},{\"Key\":\"username\",\"Secret\":\"*****\"}]},\"Driver\":\"csi-smb\",\"DriverOpts\":{\"source\":\"//192.168.8.50/temp2\"},\"Name\":\"my-csi-smb-volume\"}" method=POST module=api request-url=/v1.48/volumes/create vars="map[version:1.48]"
DEBU[2025-05-12T19:37:33.585546361Z] using cluster volume
INFO[2025-05-12T19:37:33.593342940Z] creating volume                               attempt=0 driver=csi-smb module=csi/manager node.id=y8g6wl6zvjgq1ozq88dkan7la volume.id=ew1t9iqskmvmuhhyac06d5lnl
ERRO[2025-05-12T19:37:33.593453269Z] volume creation failed: plugin for driver "csi-smb" does not implement PluginAddr  attempt=0 driver=csi-smb module=csi/manager node.id=y8g6wl6zvjgq1ozq88dkan7la volume.id=ew1t9iqskmvmuhhyac06d5lnl

Now it passes

DEBU[2025-05-13T05:42:03.188273051Z] handling POST request                         form-data="{\"ClusterVolumeSpec\":{\"AccessMode\":{\"MountVolume\":{},\"Scope\":\"single\",\"Sharing\":\"none\"},\"AccessibilityRequirements\":{},\"Availability\":\"active\",\"CapacityRange\":{\"LimitBytes\":0,\"RequiredBytes\":0},\"Secrets\":[{\"Key\":\"password\",\"Secret\":\"*****\"},{\"Key\":\"username\",\"Secret\":\"*****\"}]},\"Driver\":\"csi-smb\",\"DriverOpts\":{\"source\":\"//192.168.8.50/temp2\"},\"Name\":\"my-csi-smb-volume\"}" method=POST module=api request-url=/v1.48/volumes/create vars="map[version:1.48]"
DEBU[2025-05-13T05:42:03.188647192Z] using cluster volume
INFO[2025-05-13T05:42:03.197152001Z] creating volume                               attempt=0 driver=csi-smb module=csi/manager node.id=nzenlshu8v2kh5se1hb0pey1i volume.id=njv6g2s9gn6g9m2vr7uoqzhev
ERRO[0122] I0513 05:42:03.200046       8 utils.go:76] GRPC call: /csi.v1.Identity/GetPluginCapabilities  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.200094       8 utils.go:77] GRPC request: {}  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.200174       8 utils.go:83] GRPC response: {"capabilities":[{"Type":{"Service":{"type":1}}}]}  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.201768       8 utils.go:76] GRPC call: /csi.v1.Controller/ControllerGetCapabilities  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.201794       8 utils.go:77] GRPC request: {}  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.201899       8 utils.go:83] GRPC response: {"capabilities":[{"Type":{"Rpc":{"type":1}}},{"Type":{"Rpc":{"type":13}}},{"Type":{"Rpc":{"type":7}}}]}  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.203665       8 utils.go:76] GRPC call: /csi.v1.Controller/CreateVolume  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.203700       8 utils.go:77] GRPC request: {"accessibility_requirements":{},"capacity_range":{},"name":"my-csi-smb-volume","parameters":{"source":"//192.168.8.50/temp2"},"secrets":"***stripped***","volume_capabilities":[{"AccessType":{"Mount":{}},"access_mode":{"mode":1}}]}  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.204181       8 controllerserver.go:314] internally mounting //192.168.8.50/temp2 at /tmp/my-csi-smb-volume  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.204378       8 nodeserver.go:209] NodeStageVolume: targetPath(/tmp/my-csi-smb-volume) volumeID(192.168.8.50/temp2#my-csi-smb-volume##) context(map[source://192.168.8.50/temp2]) mountflags([]) mountOptions([])  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.204867       8 mount_linux.go:243] Detected OS without systemd  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.204898       8 mount_linux.go:218] Mounting cmd (mount) with arguments (-t cifs -o <masked> //192.168.8.50/temp2 /tmp/my-csi-smb-volume)  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.221589       8 nodeserver.go:241] volume(192.168.8.50/temp2#my-csi-smb-volume##) mount "//192.168.8.50/temp2" on "/tmp/my-csi-smb-volume" succeeded  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.224774       8 controllerserver.go:332] internally unmounting /tmp/my-csi-smb-volume  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.224810       8 nodeserver.go:264] NodeUnstageVolume: CleanupMountPoint on /tmp/my-csi-smb-volume with volume 192.168.8.50/temp2#my-csi-smb-volume##  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.225591       8 mount_helper_common.go:93] unmounting "/tmp/my-csi-smb-volume" (corruptedMount: false, mounterCanSkipMountPointChecks: true)  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.225621       8 mount_linux.go:360] Unmounting /tmp/my-csi-smb-volume  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.230035       8 mount_helper_common.go:150] Deleting path "/tmp/my-csi-smb-volume"  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.230154       8 nodeserver.go:273] NodeUnstageVolume: unmount volume 192.168.8.50/temp2#my-csi-smb-volume## on /tmp/my-csi-smb-volume successfully  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
ERRO[0122] I0513 05:42:03.230187       8 utils.go:83] GRPC response: {"volume":{"volume_context":{"source":"//192.168.8.50/temp2","subdir":"my-csi-smb-volume"},"volume_id":"192.168.8.50/temp2#my-csi-smb-volume##"}}  plugin=9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba
DEBU[2025-05-13T05:42:03.235868039Z] updated volume                                module=scheduler node.id=nzenlshu8v2kh5se1hb0pey1i volume.id=njv6g2s9gn6g9m2vr7uoqzhev

And it get mounted correctly when creating service which uses it

$ docker volume ls --cluster
VOLUME NAME         GROUP     DRIVER    AVAILABILITY   STATUS
my-csi-smb-volume             csi-smb   active         in use (1 node)

$ mount |grep smb
//192.168.8.50/temp2/my-csi-smb-volume on /var/lib/docker/plugins/9b26cb6024ada1caad4337812072e405eb0e9f98e767b3d113ca396da20581ba/propagated-mount/njv6g2s9gn6g9m2vr7uoqzhev type cifs (rw,relatime,vers=3.1.1,cache=strict,username=olli,uid=0,noforceuid,gid=0,noforcegid,addr=192.168.8.50,file_mode=0755,dir_mode=0755,soft,nounix,serverino,mapposix,rsize=4194304,wsize=4194304,bsize=1048576,echo_interval=60,actimeo=1)

PS. It would make sense to create integration tests for CSI plugins too. Most likely

func TestServicePlugin(t *testing.T) {
would be good starting point but I would leave that for future PR as this clearly fixes the issue.

EDIT: Integration test is available in PR #49983

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈 thought I left my LGTM

perhaps could use a double-check from @dperny but looks good, thanks!

@thaJeztah thaJeztah added this to the 28.2.0 milestone May 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

Swarm plugin-related error "plugin does not implement PluginAddr interface" fills disk
7 participants