fix(network): pass IPAM options when creating networks - #14094
Conversation
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The consolidation of the IPAM creation block is correct and complete. The original bug — where a second block silently overwrote createOpts.IPAM with a fresh empty struct, discarding any Options — is properly fixed by merging everything into a single block guarded by n.Ipam.Driver != "" || len(n.Ipam.Config) > 0 || len(n.Ipam.Options) > 0. Iterating a nil n.Ipam.Config slice is a Go no-op, and passing nil for createOpts.IPAM when no IPAM fields are set is valid. The new test TestExecutePlanCreateNetworkWithIPAMOptions directly exercises the previously-broken code path and uses assert.DeepEqual to confirm the daemon receives the correct options.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
createNetwork built the IPAM object in two separate blocks; the second silently overwrote createOpts.IPAM with a fresh empty struct and never forwarded n.Ipam.Options to the daemon. Consolidate into a single block covering driver, config pool, and options. Fixes docker#13785 Supersedes docker#13936 Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
9398cb8 to
33962b0
Compare
3 participants
What I did
createNetworkbuilt the IPAM object in two separate blocks; the second silently overwrotecreateOpts.IPAMwith a fresh empty struct and never forwardedn.Ipam.Optionsto the daemon.Consolidate into a single block covering driver, config pool, and options.
Related issue
Fixes #13785
Supersedes #13936
(not mandatory) A picture of a cute animal, if possible in relation to what you did
