From 2429f15672eaa617f3fbcf73692d660fe63315ba Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Fri, 30 Jun 2017 17:00:16 -0700
Subject: [PATCH] Don't attempt to remove unsupported resources on older daemon

When running `docker stack rm <some stack>` against an older daemon,
a warning was printed for "configs" being ignored;

    WARNING: ignoring "configs" (requires API version 1.30, but the Docker daemon API version is 1.26)

Given that an old daemon cannot _have_ configs, there should not be
a need to warn, or _attempt_ to remove these resources.

This patch removes the warning, and skips fetching (and removing)
configs.

A check if _secrets_ are supported by the daemon is also added,
given that this would result in an error when attempted against
an older (pre 1.13) daemon.

There is one situation where this could lead to secrets or
configs being left behind; if the client is connecting to a
daemon that _does_ support secrets, configs, but the API version
is overridden using `DOCKER_API_VERSION`, no warning is printed,
and secrets and configs are not attempted to be removed.

Given that `DOCKER_API_VERSION` is regarded a feature for
debugging / "power users", it should be ok to ignore this.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 cli/command/stack/client_test.go |  6 +++++
 cli/command/stack/remove.go      | 18 ++++++---------
 cli/command/stack/remove_test.go | 38 ++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/cli/command/stack/client_test.go b/cli/command/stack/client_test.go
index 50442783fa..d1d85193e7 100644
--- a/cli/command/stack/client_test.go
+++ b/cli/command/stack/client_test.go
@@ -15,6 +15,8 @@ import (
 type fakeClient struct {
 	client.Client
 
+	version string
+
 	services []string
 	networks []string
 	secrets  []string
@@ -45,6 +47,10 @@ func (cli *fakeClient) ServerVersion(ctx context.Context) (types.Version, error)
 	}, nil
 }
 
+func (cli *fakeClient) ClientVersion() string {
+	return cli.version
+}
+
 func (cli *fakeClient) ServiceList(ctx context.Context, options types.ServiceListOptions) ([]swarm.Service, error) {
 	if cli.serviceListFunc != nil {
 		return cli.serviceListFunc(options)
diff --git a/cli/command/stack/remove.go b/cli/command/stack/remove.go
index fc7752407e..d95171aabf 100644
--- a/cli/command/stack/remove.go
+++ b/cli/command/stack/remove.go
@@ -51,20 +51,16 @@ func runRemove(dockerCli command.Cli, opts removeOptions) error {
 			return err
 		}
 
-		secrets, err := getStackSecrets(ctx, client, namespace)
-		if err != nil {
-			return err
+		var secrets []swarm.Secret
+		if versions.GreaterThanOrEqualTo(client.ClientVersion(), "1.25") {
+			secrets, err = getStackSecrets(ctx, client, namespace)
+			if err != nil {
+				return err
+			}
 		}
 
 		var configs []swarm.Config
-
-		version, err := client.ServerVersion(ctx)
-		if err != nil {
-			return err
-		}
-		if versions.LessThan(version.APIVersion, "1.30") {
-			fmt.Fprintf(dockerCli.Err(), "WARNING: ignoring \"configs\" (requires API version 1.30, but the Docker daemon API version is %s)\n", version.APIVersion)
-		} else {
+		if versions.GreaterThanOrEqualTo(client.ClientVersion(), "1.30") {
 			configs, err = getStackConfigs(ctx, client, namespace)
 			if err != nil {
 				return err
diff --git a/cli/command/stack/remove_test.go b/cli/command/stack/remove_test.go
index a924407af0..b4e8cc2ba8 100644
--- a/cli/command/stack/remove_test.go
+++ b/cli/command/stack/remove_test.go
@@ -40,7 +40,9 @@ func TestRemoveStack(t *testing.T) {
 	}
 	allConfigIDs := buildObjectIDs(allConfigs)
 
+	// Using API 1.24; removes services, networks, but doesn't remove configs and secrets
 	cli := &fakeClient{
+		version:  "1.24",
 		services: allServices,
 		networks: allNetworks,
 		secrets:  allSecrets,
@@ -49,6 +51,40 @@ func TestRemoveStack(t *testing.T) {
 	cmd := newRemoveCommand(test.NewFakeCli(cli, &bytes.Buffer{}))
 	cmd.SetArgs([]string{"foo", "bar"})
 
+	assert.NoError(t, cmd.Execute())
+	assert.Equal(t, allServiceIDs, cli.removedServices)
+	assert.Equal(t, allNetworkIDs, cli.removedNetworks)
+	assert.Nil(t, cli.removedSecrets)
+	assert.Nil(t, cli.removedConfigs)
+
+	// Using API 1.25; removes services, networks, but doesn't remove configs
+	cli = &fakeClient{
+		version:  "1.25",
+		services: allServices,
+		networks: allNetworks,
+		secrets:  allSecrets,
+		configs:  allConfigs,
+	}
+	cmd = newRemoveCommand(test.NewFakeCli(cli, &bytes.Buffer{}))
+	cmd.SetArgs([]string{"foo", "bar"})
+
+	assert.NoError(t, cmd.Execute())
+	assert.Equal(t, allServiceIDs, cli.removedServices)
+	assert.Equal(t, allNetworkIDs, cli.removedNetworks)
+	assert.Equal(t, allSecretIDs, cli.removedSecrets)
+	assert.Nil(t, cli.removedConfigs)
+
+	// Using API 1.30; removes services, networks, configs, and secrets
+	cli = &fakeClient{
+		version:  "1.30",
+		services: allServices,
+		networks: allNetworks,
+		secrets:  allSecrets,
+		configs:  allConfigs,
+	}
+	cmd = newRemoveCommand(test.NewFakeCli(cli, &bytes.Buffer{}))
+	cmd.SetArgs([]string{"foo", "bar"})
+
 	assert.NoError(t, cmd.Execute())
 	assert.Equal(t, allServiceIDs, cli.removedServices)
 	assert.Equal(t, allNetworkIDs, cli.removedNetworks)
@@ -72,6 +108,7 @@ func TestRemoveStackSkipEmpty(t *testing.T) {
 	allConfigIDs := buildObjectIDs(allConfigs)
 
 	fakeClient := &fakeClient{
+		version:  "1.30",
 		services: allServices,
 		networks: allNetworks,
 		secrets:  allSecrets,
@@ -106,6 +143,7 @@ func TestRemoveContinueAfterError(t *testing.T) {
 
 	removedServices := []string{}
 	cli := &fakeClient{
+		version:  "1.30",
 		services: allServices,
 		networks: allNetworks,
 		secrets:  allSecrets,