Skip to content
This repository was archived by the owner on Oct 13, 2023. It is now read-only.

[18.09] backport default-addr-pool-mask-length param max value check#73

Merged
tiborvass merged 2 commits intodocker-archive:18.09from
thaJeztah:18.09_backport_addr_pool
Oct 11, 2018
Merged

[18.09] backport default-addr-pool-mask-length param max value check#73
tiborvass merged 2 commits intodocker-archive:18.09from
thaJeztah:18.09_backport_addr_pool

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

backport of moby#37736 and moby#37949 for 18.09

git checkout -b 18.09_backport_addr_pool ce-engine/18.09
git cherry-pick -s -S -x 148ff00a0a800fad99de11ee3021d4c5d4869157
git cherry-pick -s -S -x d25c5df80e60cdbdc23fe3d0e2a6808123643dc7

conflict in daemon/cluster/noderunner.go and vendor.conf, due to #60, which is not in upstream

diff --cc daemon/cluster/noderunner.go
index 071d8af5a7,aa905d6780..0000000000
--- a/daemon/cluster/noderunner.go
+++ b/daemon/cluster/noderunner.go
@@@ -13,7 -13,7 +13,11 @@@ import 
        "github.com/docker/docker/daemon/cluster/executor/container"
        lncluster "github.com/docker/libnetwork/cluster"
        swarmapi "github.com/docker/swarmkit/api"
++<<<<<<< HEAD
 +      "github.com/docker/swarmkit/manager/allocator/cnmallocator"
++=======
+       swarmallocator "github.com/docker/swarmkit/manager/allocator/cnmallocator"
++>>>>>>> 148ff00a0a... Global Default AddressPool - Update
        swarmnode "github.com/docker/swarmkit/node"
        "github.com/pkg/errors"
        "github.com/sirupsen/logrus"
@@@ -122,7 -122,7 +126,11 @@@ func (n *nodeRunner) start(conf nodeSta
                ListenControlAPI:   control,
                ListenRemoteAPI:    conf.ListenAddr,
                AdvertiseRemoteAPI: conf.AdvertiseAddr,
++<<<<<<< HEAD
 +              NetworkConfig: &cnmallocator.NetworkConfig{
++=======
+               NetworkConfig: &swarmallocator.NetworkConfig{
++>>>>>>> 148ff00a0a... Global Default AddressPool - Update
                        DefaultAddrPool: conf.DefaultAddressPool,
                        SubnetSize:      conf.SubnetSize,
                },
diff --cc vendor.conf
index d6a41a3759,6070edbac6..0000000000
--- a/vendor.conf
+++ b/vendor.conf

Addressing few review comments as part of code refactoring.
Also moved validation logic from CLI to Moby.

Signed-off-by: selansen <elango.siva@docker.com>
(cherry picked from commit 148ff00)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We check for max value for -default-addr-pool-mask-length param as 32.
But There won't be enough addresses on the  overlay network. Hence we are
keeping it 29 so that we would be having atleast 8 addresses in /29 network.

Signed-off-by: selansen <elango.siva@docker.com>
(cherry picked from commit d25c5df)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added this to the 18.09.0 milestone Oct 4, 2018
@thaJeztah
Copy link
Copy Markdown
Member Author

ping @selansen PTAL

@selansen
Copy link
Copy Markdown

selansen commented Oct 4, 2018

Little confused here. half of moby#37736 changes already in due to #60 but few changes are missing.
When I looked into the code, I noticed most of my changes are present and notified @andrewhsu that my changes are already present in 18.09. wondering how did rest of the changes got missed out. Anyways thanks for porting these PRs.

@selansen
Copy link
Copy Markdown

selansen commented Oct 4, 2018

LGTM

@thaJeztah
Copy link
Copy Markdown
Member Author

yes it would've been easier if #60 cherry-picked the upstream changes, but it was merged already, so I decided to just fix the conflicts

Copy link
Copy Markdown

@tiborvass tiborvass left a comment

Choose a reason for hiding this comment

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

LGTM

@tiborvass tiborvass merged commit b38d454 into docker-archive:18.09 Oct 11, 2018
@thaJeztah thaJeztah deleted the 18.09_backport_addr_pool branch October 11, 2018 20:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants