ping @shykes @unclejack @vieux @crosbymichael

reping @unclejack @vieux @crosbymichael

+1

+1 This is fantastic and nicely fits the Hadoop usecase. How hard would it be to extend this to specify the CIDR per bridge rather than globally (assuming #6155 is implemented)?

@subhraveti ~ 4 hours with tests for "per-bridge" I think, it is mostly IP allocator feature.

rebased
ping @unclejack @vieux @crosbymichael

I think this seems sane, since we're controlling IP address assignment, providing more control over how they get assigned is generally good, although the UI could end up getting kind of cumbersome to specify via command-line flags, but that's inevitable and a much larger discussion than what's proposed here. :)

Also, benchmarks result:

benchmark              old ns/op     new ns/op     delta       
BenchmarkRequestIP     434104        275999        -36.42%     

benchmark              old allocs     new allocs     delta       
BenchmarkRequestIP     1278           771            -39.67%     

benchmark              old bytes     new bytes     delta       
BenchmarkRequestIP     31426         18648         -40.66%

@LK4D4 Looks like there are some conflicts. Can you rebase this?

@fkautz Rebased :) I hope that this rebase will bring PR a lucky charm for review.

Assigning this to myself so that I can review it.

@fkautz Cool, thank you!

We will need some input from @shykes to make sure the changes are acceptable, since they change the UI. I've asked @tianon to add it to Thursday's meeting, contingent on @shykes being online.

Assuming he agrees with the UI approach, we'll also need additional documentation in docs. I have a few use cases to test out as well, I'll get back to you on how those go.

It will be sixth meeting for this PR. I glad to see, that someone interested in this apart from me :)

+1 on the design, as long as it doesn't break links. Just a superficial note on the syntax (--ip feels simpler and more natural).

Sorry for being slow.

@shykes I don't think that ip is more natural for this feature :) I expect that --ip sets one fixed IP within bridge range and possibly this will be useful feature. But here we specify not ip, but exactly the cidr.
--fixed-cidr notation already very popular in openstack nova.

I agree, fixed CIDR is more descriptive. An IP should also be assignable
with /32.
On Jul 10, 2014 9:23 PM, "Alexandr Morozov" [email protected]
wrote:

@shykes https://github.com/shykes I don't think that ip is more natural
for this feature :) I expect that --ip sets one fixed IP within bridge
range and possibly this will be useful feature. But here we specify not ip,
but exactly the cidr.
--fixed-cidr notation already very popular in openstack nova.

—
Reply to this email directly or view it on GitHub
#6101 (comment).

ping @shykes
What are you think about --fixed-cidr now?

--fixed-cidr only provides half of the story. I think --fixed-cidr should not set the subnet, only the ip range allocaiton. If we are following openstack's approach, we should use --fixed-range to set the subnet separately from the allocation range. These two combined allow for a great degree of flexibility.

[edit] --fixed-cidr works as i was hoping, no change is necessary.

Just to keep the conversation together:

https://botbot.me/freenode/docker-dev/msg/18199269/

<shykes> I'm ok to merge 6101

Had a talk with @LK4D4. The subnet isn't touched, so we should be good to go. Just need to add some documentation.

LGTM

I think just needs docs review from @SvenDowideit

Docs LGTM - @jamtur01 @fredlf @ostezer

While I like having examples, giving the users an idea of situations when they might use an option, I can't really think of one that is interesting - its the kind of thing an admin will use because they know they need it.

Okay, now we have bunch of lgtms here :) We need the one who brave enough to merge it or close it forever. ping @crosbymichael

I don't have time to test right now but if someone else reviews and tests I'm good with this

maybe @tiborvass or @unclejack tomorrow?

Fixed article text.

LGTM

The docs need to be approved as well. Please don't merge until @fredlf @jamtur01 and @ostezer have approved the docs and the help text.

pint @fredlf @ostezer
Guys, review docs pls.

@fredlf Thanks for review. Could you help me with that big sentence about MTU? I'm not very experienced English language user :/

how about something like

The host machine can both receive and send packets to
containers connected to the bridge.
 The *maximum transmission unit* (MTU) or largest packet length that the interface will
allow will be copied from the Docker host's default route interface.

I've ignored the 1500 bytes - do we need it?

LGTM. We'll fix the docs later.

ping @unclejack
can we merge this now?

Will this fix docker changing the container ip on container or deamon restart?

@DwayneBull Nope

@LK4D4 I assume you could get round it with something like --fixed-cidr="172.168.0.1/32"

@DwayneBull Problem that this is daemon-wide option. There is another PR for per-container IP control.

@jamtur01 Is it OK if we merge this PR with the current docs?

LGTM