Skip to content

ipams: Re-enable legacy remote plugins support#51009

Merged
robmry merged 1 commit intomoby:masterfrom
olljanat:legacy-remote-ipam-support
Sep 19, 2025
Merged

ipams: Re-enable legacy remote plugins support#51009
robmry merged 1 commit intomoby:masterfrom
olljanat:legacy-remote-ipam-support

Conversation

@olljanat
Copy link
Contributor

@olljanat olljanat commented Sep 19, 2025

- What I did
3c97181 which was part of big refactoring PR #47727 disabled remote IPAM plugins loading in case of plugin getter is nil.

However, in Windows that is always the case so this logic needs to be revisitted.

Closes #50596

- How I did it
Removed plugin getter is nil check competely. However, it can be also done just for Windows in case it causes issues like this.

- How to verify it
Windows side tested like it is described in linked issue so if CI passes this should be fine.

- Human readable description for the release notes

- Fix a bug causing IPAM plugins to not be loaded on Windows

if err := remoteIpam.Register(r, pg); err != nil {
return err
}
if err := remoteIpam.Register(r, pg); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

If I get it correctly, pg is nil on Windows because it doesn't support 'managed' plugins, right? Would you mind adding a comment telling that, so we don't reintroduce this issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akerouanton Yes, managed plugins are not implemented for Windows at the moment. Comment added.

Signed-off-by: Olli Janatuinen <olli.janatuinen@gmail.com>
@olljanat olljanat force-pushed the legacy-remote-ipam-support branch from 2f5df20 to aa49231 Compare September 19, 2025 10:01
Copy link
Contributor

@robmry robmry left a comment

Choose a reason for hiding this comment

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

LGTM - thank you.

(Will merge it when the tests finish.)

@robmry
Copy link
Contributor

robmry commented Sep 19, 2025

This one flaked again, but it's unrelated ...

=== FAIL: amd64.docker.docker.integration.networking TestBridgeICC/IPv6_link-local_address_on_internal_network (4.57s)
    firewall.go:69: Firewalld reload completed at 2025-09-19T13:57:54Z
    bridge_linux_test.go:219: assertion failed: 1 (res.ExitCode int) != 0 (int)
    bridge_linux_test.go:221: assertion failed: string "PING fe80::2%eth0 (fe80::2): 56 data bytes\n\n--- fe80::2%eth0 ping statistics ---\n1 packets transmitted, 0 packets received, 100% packet loss\n" does not contain "1 packets transmitted, 1 packets received"
    --- FAIL: TestBridgeICC/IPv6_link-local_address_on_internal_network (4.57s)

@robmry robmry merged commit 2f87bb4 into moby:master Sep 19, 2025
266 of 268 checks passed
@vvoland
Copy link
Contributor

vvoland commented Sep 24, 2025

@robmry @akerouanton @olljanat should this one have a changelog entry?

@thaJeztah
Copy link
Member

Oh! I should mention that I marked this for backporting because it was a regression (at least, from what I understood from the conversation above) and it looked like a small enough change to include.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IPAM plugin loading is broken in Windows starting from Docker 27.0.1

5 participants