Merge pull request #32110 from adshmh/30977-stack-rm-should-accept-multiple-labels

stack rm should accept multiple arguments

command/stack/client_test.go

@@ -0,0 +1,153 @@
+package stack
+
+import (
+	"strings"
+
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/api/types/swarm"
+	"github.com/docker/docker/cli/compose/convert"
+	"github.com/docker/docker/client"
+	"golang.org/x/net/context"
+)
+
+type fakeClient struct {
+	client.Client
+
+	services []string
+	networks []string
+	secrets  []string
+
+	removedServices []string
+	removedNetworks []string
+	removedSecrets  []string
+
+	serviceListFunc   func(options types.ServiceListOptions) ([]swarm.Service, error)
+	networkListFunc   func(options types.NetworkListOptions) ([]types.NetworkResource, error)
+	secretListFunc    func(options types.SecretListOptions) ([]swarm.Secret, error)
+	serviceRemoveFunc func(serviceID string) error
+	networkRemoveFunc func(networkID string) error
+	secretRemoveFunc  func(secretID string) error
+}
+
+func (cli *fakeClient) ServiceList(ctx context.Context, options types.ServiceListOptions) ([]swarm.Service, error) {
+	if cli.serviceListFunc != nil {
+		return cli.serviceListFunc(options)
+	}
+
+	namespace := namespaceFromFilters(options.Filters)
+	servicesList := []swarm.Service{}
+	for _, name := range cli.services {
+		if belongToNamespace(name, namespace) {
+			servicesList = append(servicesList, serviceFromName(name))
+		}
+	}
+	return servicesList, nil
+}
+
+func (cli *fakeClient) NetworkList(ctx context.Context, options types.NetworkListOptions) ([]types.NetworkResource, error) {
+	if cli.networkListFunc != nil {
+		return cli.networkListFunc(options)
+	}
+
+	namespace := namespaceFromFilters(options.Filters)
+	networksList := []types.NetworkResource{}
+	for _, name := range cli.networks {
+		if belongToNamespace(name, namespace) {
+			networksList = append(networksList, networkFromName(name))
+		}
+	}
+	return networksList, nil
+}
+
+func (cli *fakeClient) SecretList(ctx context.Context, options types.SecretListOptions) ([]swarm.Secret, error) {
+	if cli.secretListFunc != nil {
+		return cli.secretListFunc(options)
+	}
+
+	namespace := namespaceFromFilters(options.Filters)
+	secretsList := []swarm.Secret{}
+	for _, name := range cli.secrets {
+		if belongToNamespace(name, namespace) {
+			secretsList = append(secretsList, secretFromName(name))
+		}
+	}
+	return secretsList, nil
+}
+
+func (cli *fakeClient) ServiceRemove(ctx context.Context, serviceID string) error {
+	if cli.serviceRemoveFunc != nil {
+		return cli.serviceRemoveFunc(serviceID)
+	}
+
+	cli.removedServices = append(cli.removedServices, serviceID)
+	return nil
+}
+
+func (cli *fakeClient) NetworkRemove(ctx context.Context, networkID string) error {
+	if cli.networkRemoveFunc != nil {
+		return cli.networkRemoveFunc(networkID)
+	}
+
+	cli.removedNetworks = append(cli.removedNetworks, networkID)
+	return nil
+}
+
+func (cli *fakeClient) SecretRemove(ctx context.Context, secretID string) error {
+	if cli.secretRemoveFunc != nil {
+		return cli.secretRemoveFunc(secretID)
+	}
+
+	cli.removedSecrets = append(cli.removedSecrets, secretID)
+	return nil
+}
+
+func serviceFromName(name string) swarm.Service {
+	return swarm.Service{
+		ID: "ID-" + name,
+		Spec: swarm.ServiceSpec{
+			Annotations: swarm.Annotations{Name: name},
+		},
+	}
+}
+
+func networkFromName(name string) types.NetworkResource {
+	return types.NetworkResource{
+		ID:   "ID-" + name,
+		Name: name,
+	}
+}
+
+func secretFromName(name string) swarm.Secret {
+	return swarm.Secret{
+		ID: "ID-" + name,
+		Spec: swarm.SecretSpec{
+			Annotations: swarm.Annotations{Name: name},
+		},
+	}
+}
+
+func namespaceFromFilters(filters filters.Args) string {
+	label := filters.Get("label")[0]
+	return strings.TrimPrefix(label, convert.LabelNamespace+"=")
+}
+
+func belongToNamespace(id, namespace string) bool {
+	return strings.HasPrefix(id, namespace+"_")
+}
+
+func objectName(namespace, name string) string {
+	return namespace + "_" + name
+}
+
+func objectID(name string) string {
+	return "ID-" + name
+}
+
+func buildObjectIDs(objectNames []string) []string {
+	IDs := make([]string, len(objectNames))
+	for i, name := range objectNames {
+		IDs[i] = objectID(name)
+	}
+	return IDs
+}

command/stack/deploy_test.go

@@ -4,39 +4,12 @@ import (
 	"bytes"
 	"testing"
 
-	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/cli/compose/convert"
 	"github.com/docker/docker/cli/internal/test"
-	"github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/testutil/assert"
 	"golang.org/x/net/context"
 )
 
-type fakeClient struct {
-	client.Client
-	serviceList []string
-	removedIDs  []string
-}
-
-func (cli *fakeClient) ServiceList(ctx context.Context, options types.ServiceListOptions) ([]swarm.Service, error) {
-	services := []swarm.Service{}
-	for _, name := range cli.serviceList {
-		services = append(services, swarm.Service{
-			ID: name,
-			Spec: swarm.ServiceSpec{
-				Annotations: swarm.Annotations{Name: name},
-			},
-		})
-	}
-	return services, nil
-}
-
-func (cli *fakeClient) ServiceRemove(ctx context.Context, serviceID string) error {
-	cli.removedIDs = append(cli.removedIDs, serviceID)
-	return nil
-}
-
 func TestPruneServices(t *testing.T) {
 	ctx := context.Background()
 	namespace := convert.NewNamespace("foo")
@@ -44,11 +17,11 @@ func TestPruneServices(t *testing.T) {
 		"new":  {},
 		"keep": {},
 	}
-	client := &fakeClient{serviceList: []string{"foo_keep", "foo_remove"}}
+	client := &fakeClient{services: []string{objectName("foo", "keep"), objectName("foo", "remove")}}
 	dockerCli := test.NewFakeCli(client, &bytes.Buffer{})
 	dockerCli.SetErr(&bytes.Buffer{})
 
 	pruneServices(ctx, dockerCli, namespace, services)
 
-	assert.DeepEqual(t, client.removedIDs, []string{"foo_remove"})
+	assert.DeepEqual(t, client.removedServices, buildObjectIDs([]string{objectName("foo", "remove")}))
 }

command/stack/remove.go

@@ -2,6 +2,7 @@ package stack
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/swarm"
@@ -13,56 +14,63 @@ import (
 )
 
 type removeOptions struct {
-	namespace string
+	namespaces []string
 }
 
-func newRemoveCommand(dockerCli *command.DockerCli) *cobra.Command {
+func newRemoveCommand(dockerCli command.Cli) *cobra.Command {
 	var opts removeOptions
 
 	cmd := &cobra.Command{
-		Use:     "rm STACK",
+		Use:     "rm STACK [STACK...]",
 		Aliases: []string{"remove", "down"},
-		Short:   "Remove the stack",
+		Short:   "Remove one or more stacks",
-		Args:    cli.ExactArgs(1),
+		Args:    cli.RequiresMinArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			opts.namespace = args[0]
+			opts.namespaces = args
 			return runRemove(dockerCli, opts)
 		},
 	}
 	return cmd
 }
 
-func runRemove(dockerCli *command.DockerCli, opts removeOptions) error {
+func runRemove(dockerCli command.Cli, opts removeOptions) error {
-	namespace := opts.namespace
+	namespaces := opts.namespaces
 	client := dockerCli.Client()
 	ctx := context.Background()
 
-	services, err := getServices(ctx, client, namespace)
+	var errs []string
-	if err != nil {
+	for _, namespace := range namespaces {
-		return err
+		services, err := getServices(ctx, client, namespace)
+		if err != nil {
+			return err
+		}
+
+		networks, err := getStackNetworks(ctx, client, namespace)
+		if err != nil {
+			return err
+		}
+
+		secrets, err := getStackSecrets(ctx, client, namespace)
+		if err != nil {
+			return err
+		}
+
+		if len(services)+len(networks)+len(secrets) == 0 {
+			fmt.Fprintf(dockerCli.Out(), "Nothing found in stack: %s\n", namespace)
+			continue
+		}
+
+		hasError := removeServices(ctx, dockerCli, services)
+		hasError = removeSecrets(ctx, dockerCli, secrets) || hasError
+		hasError = removeNetworks(ctx, dockerCli, networks) || hasError
+
+		if hasError {
+			errs = append(errs, fmt.Sprintf("Failed to remove some resources from stack: %s", namespace))
+		}
 	}
 
-	networks, err := getStackNetworks(ctx, client, namespace)
+	if len(errs) > 0 {
-	if err != nil {
+		return errors.Errorf(strings.Join(errs, "\n"))
-		return err
-	}
-
-	secrets, err := getStackSecrets(ctx, client, namespace)
-	if err != nil {
-		return err
-	}
-
-	if len(services)+len(networks)+len(secrets) == 0 {
-		fmt.Fprintf(dockerCli.Out(), "Nothing found in stack: %s\n", namespace)
-		return nil
-	}
-
-	hasError := removeServices(ctx, dockerCli, services)
-	hasError = removeSecrets(ctx, dockerCli, secrets) || hasError
-	hasError = removeNetworks(ctx, dockerCli, networks) || hasError
-
-	if hasError {
-		return errors.Errorf("Failed to remove some resources")
 	}
 	return nil
 }

command/stack/remove_test.go

@@ -0,0 +1,107 @@
+package stack
+
+import (
+	"bytes"
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/docker/docker/cli/internal/test"
+	"github.com/docker/docker/pkg/testutil/assert"
+)
+
+func TestRemoveStack(t *testing.T) {
+	allServices := []string{
+		objectName("foo", "service1"),
+		objectName("foo", "service2"),
+		objectName("bar", "service1"),
+		objectName("bar", "service2"),
+	}
+	allServicesIDs := buildObjectIDs(allServices)
+
+	allNetworks := []string{
+		objectName("foo", "network1"),
+		objectName("bar", "network1"),
+	}
+	allNetworksIDs := buildObjectIDs(allNetworks)
+
+	allSecrets := []string{
+		objectName("foo", "secret1"),
+		objectName("foo", "secret2"),
+		objectName("bar", "secret1"),
+	}
+	allSecretsIDs := buildObjectIDs(allSecrets)
+
+	cli := &fakeClient{
+		services: allServices,
+		networks: allNetworks,
+		secrets:  allSecrets,
+	}
+	cmd := newRemoveCommand(test.NewFakeCli(cli, &bytes.Buffer{}))
+	cmd.SetArgs([]string{"foo", "bar"})
+
+	assert.NilError(t, cmd.Execute())
+	assert.DeepEqual(t, cli.removedServices, allServicesIDs)
+	assert.DeepEqual(t, cli.removedNetworks, allNetworksIDs)
+	assert.DeepEqual(t, cli.removedSecrets, allSecretsIDs)
+}
+
+func TestSkipEmptyStack(t *testing.T) {
+	buf := new(bytes.Buffer)
+	allServices := []string{objectName("bar", "service1"), objectName("bar", "service2")}
+	allServicesIDs := buildObjectIDs(allServices)
+
+	allNetworks := []string{objectName("bar", "network1")}
+	allNetworksIDs := buildObjectIDs(allNetworks)
+
+	allSecrets := []string{objectName("bar", "secret1")}
+	allSecretsIDs := buildObjectIDs(allSecrets)
+
+	cli := &fakeClient{
+		services: allServices,
+		networks: allNetworks,
+		secrets:  allSecrets,
+	}
+	cmd := newRemoveCommand(test.NewFakeCli(cli, buf))
+	cmd.SetArgs([]string{"foo", "bar"})
+
+	assert.NilError(t, cmd.Execute())
+	assert.Contains(t, buf.String(), "Nothing found in stack: foo")
+	assert.DeepEqual(t, cli.removedServices, allServicesIDs)
+	assert.DeepEqual(t, cli.removedNetworks, allNetworksIDs)
+	assert.DeepEqual(t, cli.removedSecrets, allSecretsIDs)
+}
+
+func TestContinueAfterError(t *testing.T) {
+	allServices := []string{objectName("foo", "service1"), objectName("bar", "service1")}
+	allServicesIDs := buildObjectIDs(allServices)
+
+	allNetworks := []string{objectName("foo", "network1"), objectName("bar", "network1")}
+	allNetworksIDs := buildObjectIDs(allNetworks)
+
+	allSecrets := []string{objectName("foo", "secret1"), objectName("bar", "secret1")}
+	allSecretsIDs := buildObjectIDs(allSecrets)
+
+	removedServices := []string{}
+	cli := &fakeClient{
+		services: allServices,
+		networks: allNetworks,
+		secrets:  allSecrets,
+
+		serviceRemoveFunc: func(serviceID string) error {
+			removedServices = append(removedServices, serviceID)
+
+			if strings.Contains(serviceID, "foo") {
+				return errors.New("")
+			}
+			return nil
+		},
+	}
+	cmd := newRemoveCommand(test.NewFakeCli(cli, &bytes.Buffer{}))
+	cmd.SetArgs([]string{"foo", "bar"})
+
+	assert.Error(t, cmd.Execute(), "Failed to remove some resources from stack: foo")
+	assert.DeepEqual(t, removedServices, allServicesIDs)
+	assert.DeepEqual(t, cli.removedNetworks, allNetworksIDs)
+	assert.DeepEqual(t, cli.removedSecrets, allSecretsIDs)
+}