Skip to content

Conversation

@arkodg
Copy link

@arkodg arkodg commented Sep 10, 2019

@arkodg
Copy link
Author

arkodg commented Sep 10, 2019

@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

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

codecov bot commented Sep 10, 2019

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
Copy link
Collaborator

dperny commented Sep 10, 2019

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
Copy link
Author

arkodg commented Sep 10, 2019

sg thanks @dperny
PTAL @mavenugo @thaJeztah @selansen

@selansen
Copy link
Contributor

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.

@dperny dperny merged commit a8bbe7d into moby:master Sep 10, 2019
@thaJeztah
Copy link
Member

@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,

thaJeztah added a commit to thaJeztah/docker that referenced this pull request Sep 19, 2019
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 <[email protected]>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Sep 20, 2019
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 <[email protected]>
Upstream-commit: 02465c9f9da4064211d0f7e56a8c93a567589713
Component: engine
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Oct 7, 2019
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 <[email protected]>
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Oct 9, 2019
…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 <[email protected]>
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Oct 21, 2019
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 <[email protected]>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Oct 23, 2019
…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 <[email protected]>
Upstream-commit: e06f07ef337ab890f211397d6b408b75a2512dc5
Component: engine
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants