Skip to content

IPv6 only: gateway, IPAM and address configuration#48284

Merged
robmry merged 5 commits intomoby:masterfrom
robmry:v6only/libnetwork
Aug 6, 2024
Merged

IPv6 only: gateway, IPAM and address configuration#48284
robmry merged 5 commits intomoby:masterfrom
robmry:v6only/libnetwork

Conversation

@robmry
Copy link
Copy Markdown
Contributor

@robmry robmry commented Aug 1, 2024

- What I did

Top-level libnetwork changes for IPv6-only, apart from DNS (which is up-next!).

- How I did it

  • Make Sandbox.needDefaultGW check for an IPv6 gateway.
    • So that docker_gwbridge isn't added for an IPv6-only network.
  • Set enableIPv4 in the tests that need it.
    • Without this, after the following commits, most tests fail with ipv4 pool is empty because no IPv4 address pools are set up.
  • Only set up IPv4 IPAM if enableIPv4.
  • Only allocate IPv4 addresses if enableIPv4.

- How to verify it

Updated tests still pass.

- Description for the changelog

n/a

@robmry robmry added kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny area/networking Networking area/networking/ipv6 Networking labels Aug 1, 2024
@robmry robmry self-assigned this Aug 1, 2024
@robmry robmry requested review from akerouanton and corhere and removed request for corhere August 1, 2024 15:42
Comment thread libnetwork/network.go Outdated
}()

defer func() {
if err != nil {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if err != nil {
if retErr != nil {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So, unrelated to changes in this PR, you're suggesting refactoring this function to make it use a retErr - ok, I can do that ...

Copy link
Copy Markdown
Member

@akerouanton akerouanton left a comment

Choose a reason for hiding this comment

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

I'd prefer having ipamAllocate written in a more idiomatic way, but since the code isn't broken, my previous comment is only a minor nit and not a hard blocker.

Copy link
Copy Markdown
Member

@akerouanton akerouanton left a comment

Choose a reason for hiding this comment

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

Thanks!

@robmry robmry merged commit 265f0a7 into moby:master Aug 6, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Sep 18, 2024
@robmry robmry mentioned this pull request Nov 4, 2024
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/networking/ipv6 Networking area/networking Networking kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants