Remove hardcoded IPAM config subnet value for ingress network by arkodg · Pull Request #2890 · moby/swarmkit

arkodg · 2019-09-10T00:09:57Z

Fixes: https://docker.atlassian.net/browse/ENGORC-2651

arkodg · 2019-09-10T00:12:15Z

@dperny is there a simple way to add a test-case inside swarmkit

tested the above code via a integration test in moby

diff --git a/integration/network/service_test.go b/integration/network/service_test.go
index df2404c372..e6d417b0c9 100644
--- a/integration/network/service_test.go
+++ b/integration/network/service_test.go
@@ -417,6 +417,12 @@ func TestServiceWithDefaultAddressPoolInit(t *testing.T) {
        assert.NilError(t, err)
        t.Logf("%s: NetworkInspect: %+v", t.Name(), out)
        assert.Assert(t, len(out.IPAM.Config) > 0)
+       assert.Equal(t, out.IPAM.Config[0].Subnet, "20.20.1.0/24")
+
+       // Also inspect ingress network and make sure its in the same subnet
+       out, err = cli.NetworkInspect(ctx, "ingress", types.NetworkInspectOptions{Verbose: true})
+       assert.NilError(t, err)
+       assert.Assert(t, len(out.IPAM.Config) > 0)
        assert.Equal(t, out.IPAM.Config[0].Subnet, "20.20.0.0/24")
 
        err = cli.ServiceRemove(ctx, serviceID)
diff --git a/vendor/github.com/docker/swarmkit/manager/manager.go b/vendor/github.com/docker/swarmkit/manager/manager.go
index 9112fb2b40..cba72c232f 100644
--- a/vendor/github.com/docker/swarmkit/manager/manager.go
+++ b/vendor/github.com/docker/swarmkit/manager/manager.go
@@ -1224,12 +1224,8 @@ func newIngressNetwork() *api.Network {
                        },
                        DriverConfig: &api.Driver{},
                        IPAM: &api.IPAMOptions{
-                               Driver: &api.Driver{},
-                               Configs: []*api.IPAMConfig{
-                                       {
-                                               Subnet: "10.255.0.0/16",
-                                       },
-                               },
+                               Driver:  &api.Driver{},
+                               Configs: []*api.IPAMConfig{},
                        },
                },
        }

INFO: Testing against a local daemon
=== RUN   TestServiceWithDefaultAddressPoolInit
--- PASS: TestServiceWithDefaultAddressPoolInit (19.97s)
    service_test.go:388: Creating a new daemon at: /go/src/github.com/docker/docker/bundles/test-integration/TestServiceWithDefaultAddressPoolInit
    daemon.go:336: [d4a9620a3ce4f] waiting for daemon to start
    daemon.go:336: [d4a9620a3ce4f] waiting for daemon to start
    daemon.go:364: [d4a9620a3ce4f] daemon started
    service_test.go:418: TestServiceWithDefaultAddressPoolInit: NetworkInspect: {Name:sthiraTestServiceWithDefaultAddressPoolInit ID:olderttt3vm9bkk3tgxg7j7gg Created:2019-09-09 23:59:33.3673397 +0000 UTC Scope:swarm Driver:overlay EnableIPv6:false IPAM:{Driver:default Options:map[] Config:[{Subnet:20.20.1.0/24 IPRange: Gateway:20.20.1.1 AuxAddress:map[]}]} Internal:false Attachable:false Ingress:false ConfigFrom:{Network:} ConfigOnly:false Containers:map[2bc54d98097d2b92a1cece7e0cd5ad882538e3ea523fbb6907b4ce8be80794c2:{Name:TestServiceTestServiceWithDefaultAddressPoolInit.1.pjr1m4aurilg9riercnvspb15 EndpointID:43aa1eeab2312463b8499c565b0d2f3cdccc00853adee66a93b5050f3a42aed8 MacAddress:02:42:14:14:01:03 IPv4Address:20.20.1.3/24 IPv6Address:} lb-sthiraTestServiceWithDefaultAddressPoolInit:{Name:sthiraTestServiceWithDefaultAddressPoolInit-endpoint EndpointID:157075a50f3ff116a0448617f43f3836bbb30507c1786746ebbcc2d6fec02e40 MacAddress:02:42:14:14:01:04 IPv4Address:20.20.1.4/24 IPv6Address:}] Options:map[com.docker.network.driver.overlay.vxlanid_list:4097] Labels:map[] Peers:[{Name:905c0be6043c IP:127.0.0.1}] Services:map[TestServiceTestServiceWithDefaultAddressPoolInit:{VIP:20.20.1.2 Ports:[] LocalLBIndex:256 Tasks:[{Name:TestServiceTestServiceWithDefaultAddressPoolInit.1.pjr1m4aurilg9riercnvspb15 EndpointID:43aa1eeab2312463b8499c565b0d2f3cdccc00853adee66a93b5050f3a42aed8 EndpointIP:20.20.1.3 Info:map[Host IP:127.0.0.1]}]}]}
    daemon.go:472: [d4a9620a3ce4f] Stopping daemon
    daemon.go:307: [d4a9620a3ce4f] exiting daemon
    daemon.go:459: [d4a9620a3ce4f] Daemon stopped
PASS

Fixes: https://docker.atlassian.net/browse/ENGORC-2651 Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>

codecov · 2019-09-10T00:30:30Z

Codecov Report

Merging #2890 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2890      +/-   ##
==========================================
+ Coverage   61.52%   61.57%   +0.05%     
==========================================
  Files         139      139              
  Lines       22616    22613       -3     
==========================================
+ Hits        13914    13924      +10     
+ Misses       7229     7210      -19     
- Partials     1473     1479       +6

dperny · 2019-09-10T16:01:04Z

There's not going to be a simple way to add a test case. The code passes through too many places. I'm satisfied with the test case from Moby handling this if you are.

Otherwise, LGTM. Glad this is a simple fix.

arkodg · 2019-09-10T16:31:52Z

sg thanks @dperny
PTAL @mavenugo @thaJeztah @selansen

selansen · 2019-09-10T16:55:14Z

glad its simple fix. I was pretty sure I noticed we call createINgressnetwork before we do init IPAM. anyways if above test case works with ingress, then it should be good.

thaJeztah · 2019-09-10T17:13:06Z

@arkodg can you also cherry-pick this to the relevant bump_XX branches in this repository? (before I forget; make sure to use the -s (sign-off) and -x option when cherry-picking (cherry-pick -s -x ...) so that there's a reference to the original commit,

full diff: moby/swarmkit@bbe3418...f35d910 changes included: - moby/swarmkit#2891 [19.03 backport] Remove hardcoded IPAM config subnet value for ingress network - backport of moby/swarmkit#2890 Remove hardcoded IPAM config subnet value for ingress network - fixes [ENGORC-2651] Specifying --default-addr-pool for docker swarm init is not picked up by ingress network Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

full diff: moby/swarmkit@bbe3418...f35d910 changes included: - moby/swarmkit#2891 [19.03 backport] Remove hardcoded IPAM config subnet value for ingress network - backport of moby/swarmkit#2890 Remove hardcoded IPAM config subnet value for ingress network - fixes [ENGORC-2651] Specifying --default-addr-pool for docker swarm init is not picked up by ingress network Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 02465c9f9da4064211d0f7e56a8c93a567589713 Component: engine

full diff: moby/swarmkit@7dded76...a8bbe7d changes included: - moby/swarmkit#2867 Only update non-terminal tasks on node removal - related to moby/swarmkit#2806 Fix leaking task resources when nodes are deleted - moby/swarmkit#2880 Bump to golang 1.12.9 - moby/swarmkit#2886 Bump vendoring to match current docker/docker master - regenerates protobufs - moby/swarmkit#2890 Remove hardcoded IPAM config subnet value for ingress network - fixes [ENGORC-2651] Specifying --default-addr-pool for docker swarm init is not picked up by ingress network Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

…v18.09) full diff: moby/swarmkit@142a737...5c86095 - moby/swarmkit#2892 [18.09 backport] Remove hardcoded IPAM config subnet value for ingress network - backport of moby/swarmkit#2890 Remove hardcoded IPAM config subnet value for ingress network - fixes [ENGORC-2651](https://docker.atlassian.net/browse/ENGORC-2651) - moby/swarmkit#2836 [18.09 backport] Switch to go 1.11 - backport of moby/swarmkit#2752 Switch to go 1.11 - moby/swarmkit#2901 [18.09 backport] Bump to golang 1.12.9 - backport of moby/swarmkit#2880 Bump to golang 1.12.9 - moby/swarmkit#2900 [18.09 backport] Fix update out of sequence and increase max recv gRPC message size for nodes and secrets - backport of moby/swarmkit#2762 Increased wait time on test utils WaitForCluster and WatchTaskCreate - backport of moby/swarmkit#2771 Allow using Configs as CredentialSpecs - **second commit only** (attempt to fix weirdly broken tests) - backport of moby/swarmkit#2808 Fix flaky tests - backport of moby/swarmkit#2866 Swap gometalinter for golangci-lint - backport of moby/swarmkit#2869 Increase max recv gRPC message size to initialize connection broker - related / similar to moby#38103 / docker-archive#102 cluster: set bigger grpc limit for array requests - related / similar to moby#39306 Increase max recv gRPC message size for nodes and secrets - fixes moby/swarmkit#2733 Error generated when messages size is too big - backport of moby/swarmkit#2870 Fix update out of sequence Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

full diff: moby/swarmkit@7dded76...a8bbe7d changes included: - moby/swarmkit#2867 Only update non-terminal tasks on node removal - related to moby/swarmkit#2806 Fix leaking task resources when nodes are deleted - moby/swarmkit#2880 Bump to golang 1.12.9 - moby/swarmkit#2886 Bump vendoring to match current docker/docker master - regenerates protobufs - moby/swarmkit#2890 Remove hardcoded IPAM config subnet value for ingress network - fixes [ENGORC-2651] Specifying --default-addr-pool for docker swarm init is not picked up by ingress network Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

…v18.09) full diff: moby/swarmkit@142a737...5c86095 - moby/swarmkit#2892 [18.09 backport] Remove hardcoded IPAM config subnet value for ingress network - backport of moby/swarmkit#2890 Remove hardcoded IPAM config subnet value for ingress network - fixes [ENGORC-2651](https://docker.atlassian.net/browse/ENGORC-2651) - moby/swarmkit#2836 [18.09 backport] Switch to go 1.11 - backport of moby/swarmkit#2752 Switch to go 1.11 - moby/swarmkit#2901 [18.09 backport] Bump to golang 1.12.9 - backport of moby/swarmkit#2880 Bump to golang 1.12.9 - moby/swarmkit#2900 [18.09 backport] Fix update out of sequence and increase max recv gRPC message size for nodes and secrets - backport of moby/swarmkit#2762 Increased wait time on test utils WaitForCluster and WatchTaskCreate - backport of moby/swarmkit#2771 Allow using Configs as CredentialSpecs - **second commit only** (attempt to fix weirdly broken tests) - backport of moby/swarmkit#2808 Fix flaky tests - backport of moby/swarmkit#2866 Swap gometalinter for golangci-lint - backport of moby/swarmkit#2869 Increase max recv gRPC message size to initialize connection broker - related / similar to moby/moby#38103 / docker-archive/engine#102 cluster: set bigger grpc limit for array requests - related / similar to moby/moby#39306 Increase max recv gRPC message size for nodes and secrets - fixes moby/swarmkit#2733 Error generated when messages size is too big - backport of moby/swarmkit#2870 Fix update out of sequence Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: e06f07ef337ab890f211397d6b408b75a2512dc5 Component: engine

Remove hardcoded IPAM config subnet value for ingress network

9ccb20b

Fixes: https://docker.atlassian.net/browse/ENGORC-2651 Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>

arkodg force-pushed the remove-ingress-hardcoded-subnet branch from 5aa7d59 to 9ccb20b Compare September 10, 2019 00:14

dperny merged commit a8bbe7d into moby:master Sep 10, 2019

This was referenced Sep 17, 2019

[19.03 backport] Remove hardcoded IPAM config subnet value for ingress network #2891

Merged

[18.09 backport] Remove hardcoded IPAM config subnet value for ingress network #2892

Merged

thaJeztah mentioned this pull request Sep 18, 2019

bump swarmkit to a8bbe7de43b42fc3a8430819ed577e1218dc64aa moby/moby#39953

Closed

thaJeztah mentioned this pull request Sep 19, 2019

[19.03] bump swarmkit to f35d9100f2c6ac810cc8d7de6e8f93dcc7a42d29 ENGCORE-1028 docker-archive/engine#369

Merged

thaJeztah mentioned this pull request Oct 2, 2019

[19.03 backport] Add TC to check dynamic subnet for ingress network docker-archive/engine#374

Merged

3 tasks

thaJeztah mentioned this pull request Oct 2, 2019

[18.09 backport] integration-cli: fix swarm tests flakiness docker-archive/engine#346

Merged

thaJeztah mentioned this pull request Oct 23, 2019

[18.09 backport] assorted test-fixes docker-archive/engine#401

Merged

dperny mentioned this pull request Dec 13, 2019

Bump swarmkit to 24fb4cfe8af56803640180c5592bf32da732ced2 moby/moby#40309

Merged

thaJeztah mentioned this pull request Feb 18, 2020

All managers in a swarm terminating with segmentation fault every few minutes moby/moby#40537

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove hardcoded IPAM config subnet value for ingress network#2890

Remove hardcoded IPAM config subnet value for ingress network#2890
dperny merged 1 commit intomoby:masterfrom
arkodg:remove-ingress-hardcoded-subnet

arkodg commented Sep 10, 2019 •

edited

Loading

Uh oh!

arkodg commented Sep 10, 2019

Uh oh!

codecov bot commented Sep 10, 2019

Uh oh!

dperny commented Sep 10, 2019

Uh oh!

arkodg commented Sep 10, 2019

Uh oh!

selansen commented Sep 10, 2019

Uh oh!

thaJeztah commented Sep 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

arkodg commented Sep 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arkodg commented Sep 10, 2019

Uh oh!

codecov bot commented Sep 10, 2019

Codecov Report

Uh oh!

dperny commented Sep 10, 2019

Uh oh!

arkodg commented Sep 10, 2019

Uh oh!

selansen commented Sep 10, 2019

Uh oh!

thaJeztah commented Sep 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arkodg commented Sep 10, 2019 •

edited

Loading