Allow user to specify default address pools for docker networks#36054
Closed
selansen wants to merge 0 commit intomoby:masterfrom
Closed
Allow user to specify default address pools for docker networks#36054selansen wants to merge 0 commit intomoby:masterfrom
selansen wants to merge 0 commit intomoby:masterfrom
Conversation
Contributor
Author
|
Note : This PR was already reviewed by many a while ago and additional changes on top of old PR need to be reviewed. |
|
So happy to see this coming in!! (hopefully). Keep up the great work! +a billion |
Contributor
Author
|
Thanks :) Will try to push it into upstream ASAP.
…On Thu, Jan 18, 2018 at 6:14 PM, Dustin Oberloh ***@***.***> wrote:
So happy to see this coming in!! (hopefully). Keep up the great work!
+a billion
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#36054 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKEKnw-lRsTP9JhbbbPoFFusRn6-vbY0ks5tL9BFgaJpZM4Rjk-f>
.
|
thaJeztah
added
status/needs-vendoring
status/1-design-review
area/networking
|
Failure message: |
Contributor
Author
|
We are still having lot of code review discussion on libnetwork. once I
freeze libnetwork code, I will come back to fixing moby CI.
Working with our team to close all the review comments now.
…On Fri, Jan 26, 2018 at 9:14 PM, John Yani ***@***.***> wrote:
Failure message:
05:09:54 The result of vndr differs
05:09:54
05:09:54 M vendor/github.com/docker/libnetwork/ipamutils/utils.go
05:09:54
05:09:54 Please vendor your package with github.com/LK4D4/vndr.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#36054 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKEKn22VugyLWBVvcKzKlLwap2x6xwdQks5tOoaBgaJpZM4Rjk-f>
.
|
selansen
force-pushed
the
master
branch
2 times, most recently
from
cbc75df to
e687e58
Compare
dnephin
reviewed
Jan 29, 2018
Member
There was a problem hiding this comment.
This can still be part of the API suite, and use the API to do the inspect instead of the docker cli.
selansen
force-pushed
the
master
branch
6 times, most recently
from
9b800b3 to
0de683e
Compare
fcrisciani
reviewed
Feb 1, 2018
Contributor
There was a problem hiding this comment.
we should avoid this method and pass the Pool (if any) as a network option and not change the ipam init code path
Contributor
|
AFAIK new test cases needs to be written using the machinery from |
Contributor
Author
|
I discussed with @dnephin and moved my API from docker_cli_daemon_test.go
<https://github.com/moby/moby/pull/29376/files#diff-6d9e7586edc05102d4ad66e54659c8c5>
to docker_api_network_test.go
<https://github.com/moby/moby/pull/36054/files#diff-d27869c471324416cf8914063c3ae8c3>
.
my CI passed only after my changes.
…On Thu, Feb 1, 2018 at 5:14 PM, Kirill Kolyshkin ***@***.***> wrote:
AFAIK new test cases needs to be written using the machinery from
integration (rather than integration-cli). This mostly means using API
calls rather than invoking docker cli.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#36054 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKEKn7Cprd-LoQR6Icwa-UToVOJFMfMOks5tQjc0gaJpZM4Rjk-f>
.
|
Member
|
@selansen looks like this needs a rebase |
selansen
force-pushed
the
master
branch
2 times, most recently
from
d96d456 to
99e4ae8
Compare
Member
|
Continued in #36396 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the work done by aboch (PR #29376) a while ago. I have made changes on top of it and tested it. I will be creating two more PRs, one for Libnetwork and one for CLI documentation. Will update PR details here soon.
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}]}