[17.06] backport Skip inspect of built-in networks on stack deploy

[andrewhsu]

To backport fix:

• Skip inspect of built-in networks on stack deploy docker/cli#362 Skip inspect of built-in networks on stack deploy

Attempted to cherry-pick the only commit of that PR, docker/cli@7f53c99, and had a conflict. Chose to resolve it by bringing in one more older commit docker/cli@657457e from PR docker/cli/pull/184

Cherry pick command for docker/cli@657457e (to prevent conflict) and docker/cli@7f53c99 (to bring in originally intended fix):

$ git cherry-pick -s -x -Xsubtree=components/cli 657457e 7f53c99

This then resulted in no conflicts.

cc @alexmavr @mavenugo @dnephin @thaJeztah may want to double check if the extra commit is ok to bring in to avoid the cherry-pick conflict

[andrewhsu]

Hmm...just noticed the one git commit i pulled in from docker/cli#184 may require the vendor of docker/docker to get the moby/moby/pull/33630 change.

[alexmavr]

It looks like the network.Scope field was added to the network API type after 17.03, and the entire feature around swarm-scoped networks shouldn't be applicable to the 17.03 CLI client.

Instead of a clean cherry-pick of docker/cli#362, we could consider a backport PR that omits lines 185 and 186 entirely: https://github.com/docker/cli/pull/362/files#diff-f45e59dc9b7f6a7c5aafbeab6a677260R185

PTAL codeowners @dnephin and @vdemeester for a confirmation on whether that's a good approach

Disclaimer: This is the first time I'm navigating the new cross-linked repository model.

[andrewhsu]

@dnephin @vdemeester @alexmavr Hmm...the lines 185 and 186 are outside of the conflict area. Here is how I created the cherry pick and arrived at the conflict with just docker/cli@7f53c99:

$ git clone git@github.com:docker/docker-ce # fresh clone
$ cd docker-ce # enter the working dir
$ git checkout 17.06 # checkout the branch
$ git fetch git@github.com:docker/cli master:cli # get the git commits from upstream cli
$ git cherry-pick -s -x -Xsubtree=components/cli 7f53c99 # pick just the one commit
$ git diff --diff-filter=U # show the conflicting lines
diff --cc components/cli/cli/command/stack/deploy_composefile.go
index 1cafca58e7,1f50ae1a1d..0000000000
--- a/components/cli/cli/command/stack/deploy_composefile.go
+++ b/components/cli/cli/command/stack/deploy_composefile.go
@@@ -174,7 -171,12 +174,16 @@@ func validateExternalNetworks
        externalNetworks []string,
  ) error {
        for _, networkName := range externalNetworks {
++<<<<<<< HEAD
 +              network, err := client.NetworkInspect(ctx, networkName, false)
++=======
+               if !container.NetworkMode(networkName).IsUserDefined() {
+                       // Networks that are not user defined always exist on all nodes as
+                       // local-scoped networks, so there's no need to inspect them.
+                       continue
+               }
+               network, err := client.NetworkInspect(ctx, networkName, types.NetworkInspectOptions{})
++>>>>>>> 7f53c99dfe... Skip inspects of built-in networks on stack deploy
                switch {
                case dockerclient.IsErrNotFound(err):
                        return errors.Errorf("network %q is declared as external, but could not be found. You need to create a swarm-scoped network before the stack is deployed", networkName)
diff --cc components/cli/cli/command/stack/deploy_composefile_test.go
index 6d811ed208,f303fc8865..0000000000
--- a/components/cli/cli/command/stack/deploy_composefile_test.go
+++ b/components/cli/cli/command/stack/deploy_composefile_test.go
@@@ -70,7 -73,8 +73,12 @@@ func TestValidateExternalNetworks(t *te
  
        for _, testcase := range testcases {
                fakeClient := &network.FakeClient{
++<<<<<<< HEAD
 +                      NetworkInspectFunc: func(_ context.Context, _ string, _ bool) (types.NetworkResource, error) {
++=======
+                       NetworkInspectFunc: func(_ context.Context, _ string, _ types.NetworkInspectOptions) (types.NetworkResource, error) {
+                               testcase.inspected = true
++>>>>>>> 7f53c99dfe... Skip inspects of built-in networks on stack deploy
                                return testcase.inspectResponse, testcase.inspectError
                        },
                }

[alexmavr]

Ah, I'm sure lines 185 and 186 would cause a build failure then.

To resolve the conflict, I believe the best approach is to just switch the invocations of the NetworkInspect functions to their previous signatures of (context.Context, string, bool).

Since this is definitely not a clean cherry-pick, how can I file a backport PR? Is there any such precedent yet?

[andrewhsu]

@alexmavr You can create a separate PR to the docker-ce 17.06 branch with the cherry pick if you follow the same procedure I documented to arrive at the conflict: #146 (comment)

And I can have a look and close this PR if it is suitable.

[alexmavr]

@andrewhsu filed #161 with a manual conflict resolution. PTAL and close this one if needed

[andrewhsu]

Closing this PR in favour of #161 because @alexmavr got it going on.