Allow user to specify default address pools for docker networks #36396

selansen · 2018-02-23T23:11:07Z

This is continuation of PR #36054
. I updated all review comments and re-vendor libnetwork. While doing so , I accidentally deleted my commits. Hence I was not able to update into PR 36054.

Description
When user creates a network without specifying a --subnet, docker will pick a subnet for the network from the static set 172.[17-31].0.0/16 and 192.168.[0-240].0/20 for the local scope networks and from the static set 10.[0-255].[0-255].0/24 for the global scope networks.
For different reasons, several users have asked to be able to control these defaults.
This PR brings in a change to allow users to control the default address pools at daemon boot.

As an example,
dockerd --default-address-pools base=10.10.0.0/16,size=24
would allow user to set the 256 pools 10.10.[0-255].0/24 as default for the local scope networks.

Multiple --default-address-pools can be specified.
To specify the option in the config json file:

{"default-address-pools":[{"base":"172.80.0.0/16","size":24},{"base":"172.90.0.0/16","size":24}]}

codecov · 2018-02-24T03:18:34Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d4e48af). Click here to learn what that means.
The diff coverage is 45.45%.

@@            Coverage Diff            @@
##             master   #36396   +/-   ##
=========================================
  Coverage          ?   35.09%           
=========================================
  Files             ?      615           
  Lines             ?    45745           
  Branches          ?        0           
=========================================
  Hits              ?    16053           
  Misses            ?    27582           
  Partials          ?     2110

selansen · 2018-02-24T14:50:08Z

@thaJeztah , @dnephin , @vdemeester , Pls use this for #36054.
Final Revendor-ing is done. Libnetwork code is merged into upstream with all required changes.

dnephin · 2018-02-26T19:21:42Z

daemon/config/config_unix.go

+// NetworkConfig stores the daemon-wide networking configurations
+type NetworkConfig struct {
+	// Default address pools for docker networks
+	DefaultAddressPools []*ipamutils.NetworkToSplit `json:"default-node-local-address-pools,omitempty"`


It looks like this is identical in both unix and windows. Can this go into CommonConfig instead?

Currently we dont support it on windows.

I see you adding this same field in config_windows.go. Should it be removed then?

My CI for windows p *windowsRS1 * failed due to build issue. so I had to
modify to fix CI failure.

Either way there is a problem here.

Either the code needs to be changed so it's not referenced on windows, or this field needs to be moved into a non-platform specific file. It does not make sense to dupliacte it.

dnephin · 2018-02-26T19:30:43Z

opts/address_pools.go

+func (p *PoolsOpt) Value() []*types.NetworkToSplit {
+	var pd []*types.NetworkToSplit
+	pd = append(pd, *p.values...)
+	return pd


Why not return *p.values ? Might also be good to check for nil.

make sense. will modify

dnephin · 2018-02-26T19:31:32Z

opts/address_pools.go

+
+// PoolsOpt is a Value type for parsing the default address pools definitions
+type PoolsOpt struct {
+	values *[]*types.NetworkToSplit


I don't think this needs to be a pointer. The PoolsOpt receiver is already a pointer, so you should be able to p.values = append(p.values, ...)

dnephin · 2018-02-26T19:35:14Z

integration-cli/docker_api_network_test.go

+	//c.Assert(out, checker.Contains, "175.30.0.0/16")
+	c.Assert(out.IPAM.Config[0].Subnet, checker.Equals, "175.30.0.0/16")
+
+	// Create a bridge network and verify its subnet is the second default pool


It looks like these are separate cases? Can you split this into separate Test functions?

It is same test case. We are testing it by creating 2 different network and make sure they fall into same user defined subnet. thats all.

selansen · 2018-02-27T19:43:06Z

@dnephin , Addressed all the comments that we discussed yesterday. Pls take a look at it when you have time.

dnephin

I guess the tests will fail now. I see why you were using a pointer to a slice.

dnephin · 2018-02-27T20:22:30Z

daemon/config/config_windows.go

@@ -15,7 +15,7 @@ type BridgeConfig struct {
 // to the docker daemon when you launch it with say: `dockerd -e windows`
 type Config struct {
 	CommonConfig
-
+	NetworkConfig


Still not sure why this is being added on windows if you say it isn't used, but at least now the struct isn't duplicated.

missed this one. will remove it.

dnephin · 2018-02-27T20:23:28Z

cmd/dockerd/config_unix.go

@@ -44,4 +44,6 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) {
 	flags.Var(&conf.ShmSize, "default-shm-size", "Default shm size for containers")
 	flags.BoolVar(&conf.NoNewPrivileges, "no-new-privileges", false, "Set no-new-privileges by default for new containers")
 	flags.StringVar(&conf.IpcMode, "default-ipc-mode", config.DefaultIpcMode, `Default mode for containers ipc ("shareable" | "private")`)
+	flags.Var(opts.NewPoolsOpt(conf.NetworkConfig.DefaultAddressPools), "default-node-local-address-pools", "Set the default address pools for node specific local networks")


You can remove "Set the". I see we have it for one other flag, but it reads better without it.

corrected it.

dnephin · 2018-02-27T20:25:46Z

opts/address_pools.go

+func (p *PoolsOpt) String() string {
+	pools := []string{}
+	for _, pool := range p.values {
+		repr := fmt.Sprintf(" %s %s", pool.Base, pool.Size)


Why the leading space in the format? There's already a space added by strings.Join()

Corrected it.

dnephin · 2018-02-27T20:29:29Z

daemon/config/config.go

+// NetworkConfig stores the daemon-wide networking configurations
+type NetworkConfig struct {
+	// Default address pools for docker networks
+	DefaultAddressPools []*ipamutils.NetworkToSplit `json:"default-node-local-address-pools,omitempty"`


Now I see why you were using a pointer to a slice.

The type here can be PoolsOpt instead of []*ipamutils.NetworkToSplit.

That way you don't need to accept any value in NewPoolsOpt, and to get the value out you can use: DefaultAddressPools.Value()

thaJeztah · 2018-03-06T00:47:39Z

Yup, tests look to be failing;

20:44:12 ----------------------------------------------------------------------
20:44:12 FAIL: docker_api_network_test.go:182: DockerDaemonSuite.TestAPIDaemonNetworkPools
20:44:12 
20:44:12 [d4ac7cc665224] waiting for daemon to start
20:44:12 [d4ac7cc665224] daemon started
20:44:12 
20:44:12 docker_api_network_test.go:199:
20:44:12     //c.Assert(out, checker.Contains, "175.30.0.0/16")
20:44:12     c.Assert(out.IPAM.Config[0].Subnet, checker.Equals, "175.30.0.0/16")
20:44:12 ... obtained string = "172.18.0.0/16"
20:44:12 ... expected string = "175.30.0.0/16"
20:44:12 
20:44:12 [d4ac7cc665224] exiting daemon

selansen · 2018-03-06T04:30:46Z

I will start working on this tomorrow .

…

On Mon, Mar 5, 2018 at 7:48 PM, Sebastiaan van Stijn < ***@***.***> wrote: Yup, tests look to be failing; 20:44:12 ---------------------------------------------------------------------- 20:44:12 FAIL: docker_api_network_test.go:182: DockerDaemonSuite.TestAPIDaemonNetworkPools 20:44:12 20:44:12 [d4ac7cc665224] waiting for daemon to start 20:44:12 [d4ac7cc665224] daemon started 20:44:12 20:44:12 docker_api_network_test.go:199: 20:44:12 //c.Assert(out, checker.Contains, "175.30.0.0/16") 20:44:12 c.Assert(out.IPAM.Config[0].Subnet, checker.Equals, "175.30.0.0/16") 20:44:12 ... obtained string = "172.18.0.0/16" 20:44:12 ... expected string = "175.30.0.0/16" 20:44:12 20:44:12 [d4ac7cc665224] exiting daemon — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#36396 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKEKn059DfQk7Ig_UfdB7gqOcpksFVRdks5tbdzrgaJpZM4SRmqc> .

selansen · 2018-03-14T22:57:54Z

@dnephin , All your suggestions have been taken care. Both manual and integration tests are working fine.
@thaJeztah Failures are not due to this changes. I have committed Flaky test issue on separate PR.
Also , libnetwork changes are already merged into moby when you picked up libnetwork for latest fixes.

dnephin

Thanks for making these changes

LGTM

ghost · 2018-03-20T09:32:00Z

Looks great! I really can't wait to use it in my setup :)

@vdemeester Could you review this as well?

selansen · 2018-03-20T12:20:38Z

@vdemeester @thaJeztah , I think we have gone through multiple review cycle on this.
Could pls give final blessing and merge it ?

vdemeester

LGTM 🐸

ToonSpin · 2018-07-28T21:33:07Z

Note that Docker remembers its networks, and this setting is only applied to new networks. If, like me, you are using docker-compose but are still finding routes that do not conform to your settings in the JSON configuration, then executing docker network ls and then docker network rm NETWORK on any offending networks should fix things.

rca · 2018-08-06T20:15:47Z

@mavenugo in the thread above you mention, "given that you identified my earlier comment on this topic." I'm looking to customize the IPAM for swarm network creation without having to specify the subnet for every stack I deploy. Is there, by chance, some trick that can be used until this is implemented for swarm networks as well.

Thank you!

selansen · 2018-08-06T21:11:47Z

We are working on overlay network support at this moment. New feature will be available in the next release.
only viable option is using "--subnet" while creating network. Looks like you are using the option already

rca · 2018-08-07T03:40:05Z

Yep, that's how I've been getting around this, and I'm looking to get out of the subnet management business. ;)

Thanks for the response, @selansen !

selansen · 2018-08-07T15:30:14Z

Hopefully soon :)

vincentjorgensen · 2019-01-29T19:33:10Z

I'm using 18.09.1, but I get "unknown flag: --default-address-pools". Do we know which release this will be in? Is there a way I can check so I don't clutter the forum with inane questions? thank you!

thaJeztah · 2019-01-29T19:43:57Z

Should be there in 18.09.1;

dockerd --help

...
   --default-address-pool pool-options       Default address pools for node specific local networks
...

And for swarm;

docker swarm init --help

Usage:	docker swarm init [OPTIONS]

Initialize a swarm

Options:
      --advertise-addr string                  Advertised address (format: <ip|interface>[:port])
      --autolock                               Enable manager autolocking (requiring an unlock key to start a stopped manager)
      --availability string                    Availability of the node ("active"|"pause"|"drain") (default "active")
      --cert-expiry duration                   Validity period for node certificates (ns|us|ms|s|m|h) (default 2160h0m0s)
      --data-path-addr string                  Address or interface to use for data path traffic (format: <ip|interface>)
      --default-addr-pool ipNetSlice           default address pool in CIDR format (default [])
      --default-addr-pool-mask-length uint32   default address pool subnet mask length (default 24)
      --dispatcher-heartbeat duration          Dispatcher heartbeat period (ns|us|ms|s|m|h) (default 5s)
      --external-ca external-ca                Specifications of one or more certificate signing endpoints
      --force-new-cluster                      Force create a new cluster from current state
      --listen-addr node-addr                  Listen address (format: <ip|interface>[:port]) (default 0.0.0.0:2377)
      --max-snapshots uint                     Number of additional Raft snapshots to retain
      --snapshot-interval uint                 Number of log entries between Raft snapshots (default 10000)
      --task-history-limit int                 Task history retention limit (default 5)

thaJeztah · 2019-01-29T19:45:22Z

Ah; I see you're using --default-address-pools (plural); I think that changed to singular during review

leokun · 2019-01-29T19:50:09Z

For docker daemon create a /etc/docker/daemon.json and add :

{
    “bip”: “10.10.0.1/24”
}

Or whatever you need.
Some notice : I had to create a /24 to get /16. But I can’t explain why

JorisVanEijden · 2019-02-14T08:04:39Z

The option does not seem to be in 18.09.1 for windows:

$ ./dockerd.exe --version
Docker version 18.09.1, build 4c52b90

$ ./dockerd.exe --help

Usage:  dockerd [OPTIONS]

A self-sufficient runtime for containers.

Options:
      --allow-nondistributable-artifacts list   Allow push of
                                                nondistributable
                                                artifacts to registry
      --api-cors-header string                  Set CORS headers in the
                                                Engine API
      --authorization-plugin list               Authorization plugins to load
  -b, --bridge string                           Attach containers to a
                                                virtual switch
      --cluster-advertise string                Address or interface name
                                                to advertise
      --cluster-store string                    URL of the distributed
                                                storage backend
      --cluster-store-opt map                   Set cluster store options
                                                (default map[])
      --config-file string                      Daemon configuration file
      --containerd string                       containerd grpc address
      --cri-containerd                          start containerd with cri
      --data-root string                        Root directory of
                                                persistent Docker state
                                                (default
                                                "C:\\ProgramData\\docker")
  -D, --debug                                   Enable debug mode
      --dns list                                DNS server to use
      --dns-opt list                            DNS options to use
      --dns-search list                         DNS search domains to use
      --exec-opt list                           Runtime execution options
      --exec-root string                        Root directory for
                                                execution state files
                                                (default
                                                "C:\\ProgramData\\docker\\exec-root")
      --experimental                            Enable experimental features
      --fixed-cidr string                       IPv4 subnet for fixed IPs
  -G, --group string                            Users or groups that can
                                                access the named pipe
      --help                                    Print usage
  -H, --host list                               Daemon socket(s) to connect to
      --insecure-registry list                  Enable insecure registry
                                                communication
      --label list                              Set key=value labels to
                                                the daemon
      --log-driver string                       Default driver for
                                                container logs (default
                                                "json-file")
  -l, --log-level string                        Set the logging level
                                                ("debug"|"info"|"warn"|"error"|"fatal") (default "info")
      --log-opt map                             Default log driver
                                                options for containers
                                                (default map[])
      --max-concurrent-downloads int            Set the max concurrent
                                                downloads for each pull
                                                (default 3)
      --max-concurrent-uploads int              Set the max concurrent
                                                uploads for each push
                                                (default 5)
      --metrics-addr string                     Set default address and
                                                port to serve the metrics
                                                api on
      --mtu int                                 Set the containers network MTU
      --network-control-plane-mtu int           Network Control plane MTU
                                                (default 1500)
      --node-generic-resource list              Advertise user-defined
                                                resource
  -p, --pidfile string                          Path to use for daemon
                                                PID file
      --raw-logs                                Full timestamps without
                                                ANSI coloring
      --register-service                        Register the service and exit
      --registry-mirror list                    Preferred Docker registry
                                                mirror
      --service-name string                     Set the Windows service
                                                name (default "docker")
      --shutdown-timeout int                    Set the default shutdown
                                                timeout (default 15)
      --storage-opt list                        Storage driver options
      --swarm-default-advertise-addr string     Set default address or
                                                interface for swarm
                                                advertised address
      --tls                                     Use TLS; implied by
                                                --tlsverify
      --tlscacert string                        Trust certs signed only
                                                by this CA (default
                                                "C:\\Users\\joris.vaneijden\\.docker\\ca.pem")
      --tlscert string                          Path to TLS certificate
                                                file (default
                                                "C:\\Users\\joris.vaneijden\\.docker\\cert.pem")
      --tlskey string                           Path to TLS key file
                                                (default
                                                "C:\\Users\\joris.vaneijden\\.docker\\key.pem")
      --tlsverify                               Use TLS and verify the remote
      --unregister-service                      Unregister the service
                                                and exit
  -v, --version                                 Print version information
                                                and quit

Maybe the mac build is missing it too?

kjelle · 2019-02-14T12:07:06Z

Hello.
I just set --default-addr-pool 192.168.0.0/16 and my newly initiated docker swarm set the ingress network to 10.255.0.0/16.

Jamuz · 2019-05-16T13:17:48Z

Hello.
I just set --default-addr-pool 192.168.0.0/16 and my newly initiated docker swarm set the ingress network to 10.255.0.0/16.

I have the same. The automatically created ingress network gets the subnet 10.255.0.0/16, but all manually created overlay networks do get the correct value set by the --default-addr-pool option. By manually re-creating the ingress network I get the correct subnet.

Automatically created ingress network gets 10.255.0.0/16:

$ docker swarm init --default-addr-pool 10.20.0.0/16
Swarm initialized: current node (t1ukoao37jotrynkucxmtrnbp) is now a manager.
... ...
$ docker network inspect ingress | grep Subnet
                    "Subnet": "10.255.0.0/16",

Manually creating a network gets 10.20.0.0/24

$ docker network create --driver overlay mynet
k5rlfr5av7glior2bm78rfqjp
$ docker inspect mynet | grep Subnet
                    "Subnet": "10.20.1.0/24",
$

I can even re-create the ingress network with correct value without using the --subnet option:

$ docker network rm ingress
WARNING! Before removing the routing-mesh network, make sure all the nodes in your swarm run the same docker engine version. Otherwise, removal may not be effective and functionality of newly create ingress networks will be impaired.
Are you sure you want to continue? [y/N] y
ingress
$ docker network create --driver overlay --ingress ingress
ojm9xnk6g55kppdvkwpc8h92u
$ docker inspect ingress | grep Subnet
                    "Subnet": "10.20.0.0/24",
$

It would be nice if the automatically created ingress network would follow the --default-addr-pool option.

selansen · 2019-05-16T15:26:41Z

Ingress network gets created before default IP pool init happens. Will have to take a look at on how we can get around current design. will take a look at it.

thaJeztah · 2019-05-16T20:56:14Z

@Jamuz perhaps you can create a ticket for that so that it doesn't get lost?

EugenMayer · 2019-09-07T19:51:49Z

This merge seems to be incomplete by any means, especially docs

Beside this "help" does not help in a single way

dockerd --help | grep address-pool
      --default-address-pool pool-options       Default address pools for node specific local networks

it is even wrong, since the setting is default-address-poolS ( plural )

It is also listed here https://docs.docker.com/engine/reference/commandline/dockerd/ as singular ..
but in the example ( json ) https://docs.docker.com/engine/reference/commandline/dockerd/#daemon-configuration-file it is plural

And it does only work in plural needs to be done this way

  "default-address-pools": [ { "base": "172.30.0.0/16", "size": 24 }]

I'am running

docker --version
Docker version 19.03.2, build 6a30dfca0

Considering the existince of bip/fixed-cidr and now this and the crazyness of DNS difference between "default networks" and "docker-compose bridge based" things seem to go out of control or at least it seems to me, there is no "big plot" anymore.

Sorry for the rant. But those topics hindered me in the last 2 months and wasted way too much time considering how easy they are solved in the end - which does not remove any of it's inconsistency or hidden features / confusion after all

selansen requested review from dnephin and vdemeester as code owners February 23, 2018 23:11

GordonTheTurtle added the status/0-triage label Feb 23, 2018

selansen force-pushed the master branch 2 times, most recently from d6a1679 to b167edd Compare February 24, 2018 03:18

thaJeztah added status/1-design-review and removed status/0-triage labels Feb 24, 2018

dnephin reviewed Feb 26, 2018

View reviewed changes

selansen force-pushed the master branch from b167edd to 0b07fe1 Compare February 27, 2018 19:40

dnephin reviewed Feb 27, 2018

View reviewed changes

thaJeztah mentioned this pull request Mar 6, 2018

Allow user to specify default address pools for docker networks #36054

Closed

GordonTheTurtle assigned MHBauer Mar 10, 2018

thaJeztah mentioned this pull request Mar 13, 2018

Fix automatic removal of ingress sandbox when last service leaves #36538

Merged

selansen force-pushed the master branch 5 times, most recently from cbe247b to d853a4c Compare March 14, 2018 17:44

dnephin approved these changes Mar 15, 2018

View reviewed changes

dnephin added the rebuild/janky label Mar 15, 2018

GordonTheTurtle removed the rebuild/janky label Mar 15, 2018

vdemeester approved these changes Mar 21, 2018

View reviewed changes

m1L31s mentioned this pull request May 31, 2018

Running more than one instance of a workspace created from factory crashes Che eclipse-che/che#9855

Closed

lmakarov mentioned this pull request Jun 18, 2018

Expand the number of concurrently running projects docksal/docksal#621

Open

cpuguy83 mentioned this pull request Jun 21, 2018

[New feature] Ability to restrict connection between containers #37309

Closed

selansen mentioned this pull request Jul 2, 2018

Allow user to specify default address pools for docker networks docker/cli#818

Merged

andrewyager mentioned this pull request Jul 21, 2018

default-address-pools can not be specified in configuration docker/for-mac#3108

Open

thaJeztah mentioned this pull request Jul 25, 2018

Global Default Address Pool feature support docker/cli#1233

Merged

mahnunchik mentioned this pull request Aug 6, 2018

Option "default-address-pools" not affected addresses in "swarm" scope docker/for-linux#387

Open

3 tasks

thaJeztah mentioned this pull request Sep 12, 2018

Docker (compose?) suddenly creates bridge using 192.168.16.0/20 range #37823

Closed

benyanke mentioned this pull request Feb 27, 2019

Allow user to specify default address pools for docker networks #29376

Closed

cpuguy83 mentioned this pull request Mar 19, 2019

[openvpn] ERROR: could not find an available, non-overlapping IPv4 address pool among the defaults to assign to the network docker/for-linux#418

Closed

1 task

trapier mentioned this pull request Apr 17, 2019

Document the default address pool for local networks docker/docs#8663

Closed

robertgzr mentioned this pull request Apr 18, 2019

Change default network address pool balena-os/balena-libnetwork#6

Closed

Jamuz mentioned this pull request May 17, 2019

Automatically created ingress network does not follow --default-addr-pool option #39228

Open

jessesimpson36 mentioned this pull request Jan 6, 2022

--default-address-pool in dockerd's daemon.json doesn't work, --default-address-pool(s) does. #43128

Open

chriscasola mentioned this pull request Oct 14, 2022

Unable to connect to host VPN through rancher desktop on Macos rancher-sandbox/rancher-desktop#2776

Open

Allow user to specify default address pools for docker networks #36396

Allow user to specify default address pools for docker networks #36396

Conversation

selansen commented Feb 23, 2018

codecov bot commented Feb 24, 2018 • edited Loading

Codecov Report

selansen commented Feb 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

selansen commented Feb 27, 2018

dnephin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah commented Mar 6, 2018

selansen commented Mar 6, 2018 via email

selansen commented Mar 14, 2018

dnephin left a comment

Choose a reason for hiding this comment

ghost commented Mar 20, 2018

selansen commented Mar 20, 2018

vdemeester left a comment

Choose a reason for hiding this comment

ToonSpin commented Jul 28, 2018

rca commented Aug 6, 2018

selansen commented Aug 6, 2018 • edited Loading

rca commented Aug 7, 2018

selansen commented Aug 7, 2018

vincentjorgensen commented Jan 29, 2019

thaJeztah commented Jan 29, 2019 • edited Loading

thaJeztah commented Jan 29, 2019

leokun commented Jan 29, 2019 • edited Loading

JorisVanEijden commented Feb 14, 2019

kjelle commented Feb 14, 2019

Jamuz commented May 16, 2019

selansen commented May 16, 2019

thaJeztah commented May 16, 2019

EugenMayer commented Sep 7, 2019

codecov bot commented Feb 24, 2018 •

edited

Loading

selansen commented Aug 6, 2018 •

edited

Loading

thaJeztah commented Jan 29, 2019 •

edited

Loading

leokun commented Jan 29, 2019 •

edited

Loading