Add IPAM Config Options to match libnetwork #17316

rmb938 · 2015-10-23T19:31:34Z

This should allow docker to pass through ipam config driver options to libnetwork.

~~I can't get my docker development environment working so I can't test this but it seems like it should work without breaking anything.~~ Fully tested and works with the API. Still needs discussion on how to implement on the cli.

Signed-off-by: Ryan Belgrave rmb1993@gmail.com

rmb938 · 2015-10-23T21:54:25Z

Can someone run the userns build again. It timed out.

Thanks

thaJeztah · 2015-10-23T21:58:59Z

@rmb938 done!

rmb938 · 2015-10-23T21:59:58Z

Thanks :)

thaJeztah · 2015-10-23T22:47:03Z

ping @mrjana @mavenugo

rmb938 · 2015-10-24T02:25:25Z

Just got my dev environment working properly and the change works with the API but more discussion is needed on how to implement in the cli.

rmb938 · 2015-10-28T23:30:20Z

Anyone care to look at this and possibly getting it merged?

thaJeztah · 2015-10-29T00:51:52Z

@rmb938 it's currently quite busy with preparations for the coming 1.9 release, so reviews will take longer than usual. Apologies for that, but hope you'll understand ❤️

LK4D4 · 2015-11-13T19:57:53Z

Dunno what's going on, but ping @mavenugo @mrjana @aboch

mavenugo · 2015-11-13T20:14:00Z

@rmb938 the -d option was meant for the network drivers. am not too sure if we should conflate that to ipam drivers as well. Or maybe, we should namespace the key to include the driver identifier to indicate which driver is the option is meant for. WDYT ?

rmb938 · 2015-11-15T00:55:33Z

I think implementing a --ipam-options or something would be better so it is more obvious to the user.

rmb938 · 2015-12-04T19:00:49Z

@mavenugo Would --ipam-options be reasonable to add?

mavenugo · 2015-12-04T19:12:26Z

@rmb938 I think so.

rmb938 · 2015-12-04T22:01:57Z

@mavenugo any idea why two of the tests failed? It doesn't seem like it is directly related to my changes.

thaJeztah · 2015-12-09T14:08:53Z

@mavenugo is this ready to move forward? I'm ok with adding an option to specify IPAM-options if the network team thinks these options should be available from the docker side

rmb938 · 2015-12-12T19:14:32Z

Fixed the merge conflicts

thaJeztah · 2015-12-12T23:55:32Z

Thanks @rmb938!

cpuguy83 · 2016-01-14T16:33:56Z

integration-cli/docker_cli_network_unix_test.go

+	c.Assert(opts["opt1"], checker.Equals, "drv1")
+	c.Assert(opts["opt2"], checker.Equals, "drv2")
+
+	// remove network


No need for this cleanup.

I don't fully understand what you mean. Do you want to get rid of the test?

No, just this removal bit, the test suite will take care of removing the network.

Oh ok will do.

cpuguy83 · 2016-01-14T16:34:26Z

One nit, LGTM if janky is happy.

cpuguy83 · 2016-01-14T16:44:49Z

ping @thaJeztah @vdemeester for docs

thaJeztah · 2016-01-14T17:32:51Z

api/client/network.go


 	cmd.Var(&flIpamSubnet, []string{"-subnet"}, "subnet in CIDR format that represents a network segment")
 	cmd.Var(&flIpamIPRange, []string{"-ip-range"}, "allocate container ip from a sub-range")
 	cmd.Var(&flIpamGateway, []string{"-gateway"}, "ipv4 or ipv6 Gateway for the master subnet")
 	cmd.Var(flIpamAux, []string{"-aux-address"}, "auxiliary ipv4 or ipv6 addresses used by Network driver")
 	cmd.Var(flOpts, []string{"o", "-opt"}, "set driver specific options")
+	cmd.Var(flIpamOpt, []string{"-ipam-opt"}, "set ipam driver specific options")


s/ipam/IPAM/

vdemeester · 2016-01-14T17:56:37Z

docs LGTM 🐰

thaJeztah · 2016-01-14T18:01:16Z

Oh, sorry, got called away for a bit; @rmb938 can you also update the example response/request in;

/~https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api_v1.22.md#inspect-network

and /~https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api_v1.22.md#create-a-network ?

Looks like this needs a rebase as well now

rmb938 · 2016-01-14T18:10:31Z

@thaJeztah No problem. Since there are no default options what would you like me to put in there? Or just leave it empty?

thaJeztah · 2016-01-14T18:17:12Z

@rmb938 I think leaving it empty for now should be okay then; just to make people aware of its existence. Alternatively, you can add some "fake" options to illustrate what it looks like (e.g. foo=bar)

rmb938 · 2016-01-14T18:22:39Z

@thaJeztah Sounds good. How does that look.

thaJeztah · 2016-01-14T18:59:41Z

Awesome! LGTM

calavera · 2016-01-14T19:02:25Z

man/docker-network-create.1.md

@@ -14,6 +14,7 @@ docker-network-create - create a new network
 [**--ip-range**=*[]*]
 [**--ipam-driver**=*default*]
 [**-o**|**--opt**=*map[]*]
+[**--ipam-opt**=*map[]*]


this list should be ordered alphabetically. So this option needs to go after ipam-driver.

Signed-off-by: Ryan Belgrave <rmb1993@gmail.com>

calavera · 2016-01-14T21:03:01Z

docs LGTM

Add IPAM Config Options to match libnetwork

GordonTheTurtle added the status/0-triage label Oct 23, 2015

rmb938 force-pushed the ipam_conf_options branch from fcae0ea to f11620e Compare October 23, 2015 19:34

rmb938 mentioned this pull request Oct 23, 2015

add ipam driver specific options #17282

Closed

rmb938 force-pushed the ipam_conf_options branch from f11620e to ebace0a Compare October 23, 2015 19:38

cpuguy83 added area/networking status/1-design-review and removed status/0-triage labels Nov 6, 2015

rmb938 force-pushed the ipam_conf_options branch from 4d795df to 1da5f30 Compare December 4, 2015 20:13

rmb938 force-pushed the ipam_conf_options branch from 1da5f30 to fbab56b Compare December 12, 2015 19:06

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Dec 12, 2015

rmb938 force-pushed the ipam_conf_options branch from de75f2b to 786a1f1 Compare December 12, 2015 19:11

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Dec 12, 2015

tiborvass added status/2-code-review and removed status/1-design-review labels Dec 14, 2015

cpuguy83 reviewed Jan 14, 2016
View reviewed changes

cpuguy83 added status/3-docs-review and removed status/2-code-review labels Jan 14, 2016

rmb938 force-pushed the ipam_conf_options branch from d39bdd4 to 8a093e0 Compare January 14, 2016 16:47

thaJeztah reviewed Jan 14, 2016
View reviewed changes

rmb938 force-pushed the ipam_conf_options branch from 8a093e0 to bbe3999 Compare January 14, 2016 17:40

rmb938 force-pushed the ipam_conf_options branch from bbe3999 to 6fb74e1 Compare January 14, 2016 18:22

calavera reviewed Jan 14, 2016
View reviewed changes

Add IPAM Config Options to match libnetwork

662cac0

Signed-off-by: Ryan Belgrave <rmb1993@gmail.com>

rmb938 force-pushed the ipam_conf_options branch from 6fb74e1 to 662cac0 Compare January 14, 2016 19:32

calavera added status/4-merge and removed status/3-docs-review labels Jan 14, 2016

tiborvass added a commit that referenced this pull request Jan 14, 2016

Merge pull request #17316 from rmb938/ipam_conf_options

e35f5a4

Add IPAM Config Options to match libnetwork

tiborvass merged commit e35f5a4 into moby:master Jan 14, 2016

albers mentioned this pull request Jan 18, 2016

Update bash completion for docker network create #19411

Merged

sdurrheimer mentioned this pull request Jan 19, 2016

Add zsh completion for 'docker network create --ipam-opt' #19459

Merged

smueller18 mentioned this pull request Oct 23, 2018

[docker_network] add ipv6 support ansible/ansible#47492

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add IPAM Config Options to match libnetwork #17316

Add IPAM Config Options to match libnetwork #17316

rmb938 commented Oct 23, 2015

rmb938 commented Oct 23, 2015

thaJeztah commented Oct 23, 2015

rmb938 commented Oct 23, 2015

thaJeztah commented Oct 23, 2015

rmb938 commented Oct 24, 2015

rmb938 commented Oct 28, 2015

thaJeztah commented Oct 29, 2015

LK4D4 commented Nov 13, 2015

mavenugo commented Nov 13, 2015

rmb938 commented Nov 15, 2015

rmb938 commented Dec 4, 2015

mavenugo commented Dec 4, 2015

rmb938 commented Dec 4, 2015

thaJeztah commented Dec 9, 2015

rmb938 commented Dec 12, 2015

thaJeztah commented Dec 12, 2015

cpuguy83 Jan 14, 2016

rmb938 Jan 14, 2016

cpuguy83 Jan 14, 2016

rmb938 Jan 14, 2016

cpuguy83 commented Jan 14, 2016

cpuguy83 commented Jan 14, 2016

thaJeztah Jan 14, 2016

vdemeester commented Jan 14, 2016

thaJeztah commented Jan 14, 2016

rmb938 commented Jan 14, 2016

thaJeztah commented Jan 14, 2016

rmb938 commented Jan 14, 2016

thaJeztah commented Jan 14, 2016

calavera Jan 14, 2016

calavera commented Jan 14, 2016

Add IPAM Config Options to match libnetwork #17316

Add IPAM Config Options to match libnetwork #17316

Conversation

rmb938 commented Oct 23, 2015

rmb938 commented Oct 23, 2015

thaJeztah commented Oct 23, 2015

rmb938 commented Oct 23, 2015

thaJeztah commented Oct 23, 2015

rmb938 commented Oct 24, 2015

rmb938 commented Oct 28, 2015

thaJeztah commented Oct 29, 2015

LK4D4 commented Nov 13, 2015

mavenugo commented Nov 13, 2015

rmb938 commented Nov 15, 2015

rmb938 commented Dec 4, 2015

mavenugo commented Dec 4, 2015

rmb938 commented Dec 4, 2015

thaJeztah commented Dec 9, 2015

rmb938 commented Dec 12, 2015

thaJeztah commented Dec 12, 2015

cpuguy83 Jan 14, 2016

Choose a reason for hiding this comment

rmb938 Jan 14, 2016

Choose a reason for hiding this comment

cpuguy83 Jan 14, 2016

Choose a reason for hiding this comment

rmb938 Jan 14, 2016

Choose a reason for hiding this comment

cpuguy83 commented Jan 14, 2016

cpuguy83 commented Jan 14, 2016

thaJeztah Jan 14, 2016

Choose a reason for hiding this comment

vdemeester commented Jan 14, 2016

thaJeztah commented Jan 14, 2016

rmb938 commented Jan 14, 2016

thaJeztah commented Jan 14, 2016

rmb938 commented Jan 14, 2016

thaJeztah commented Jan 14, 2016

calavera Jan 14, 2016

Choose a reason for hiding this comment

calavera commented Jan 14, 2016