Merge pull request #6393 from thaJeztah/cleanup_stacks

cli/command/stack: cleanups and optimizations

Author: thaJeztah

cli/command/stack/formatter/formatter.go

@@ -43,30 +43,28 @@ type Stack struct {
 // StackWrite writes formatted stacks using the Context
 //
 // Deprecated: this function was for internal use and will be removed in the next release.
-func StackWrite(ctx formatter.Context, stacks []*Stack) error {
-	render := func(format func(subContext formatter.SubContext) error) error {
+func StackWrite(ctx formatter.Context, stacks []Stack) error {
+	fmtCtx := &stackContext{
+		HeaderContext: formatter.HeaderContext{
+			Header: formatter.SubHeaderContext{
+				"Name":     formatter.NameHeader,
+				"Services": stackServicesHeader,
+			},
+		},
+	}
+	return ctx.Write(fmtCtx, func(format func(subContext formatter.SubContext) error) error {
 		for _, stack := range stacks {
 			if err := format(&stackContext{s: stack}); err != nil {
 				return err
 			}
 		}
 		return nil
-	}
-	return ctx.Write(newStackContext(), render)
+	})
 }
 
 type stackContext struct {
 	formatter.HeaderContext
-	s *Stack
-}
-
-func newStackContext() *stackContext {
-	stackCtx := stackContext{}
-	stackCtx.Header = formatter.SubHeaderContext{
-		"Name":     formatter.NameHeader,
-		"Services": stackServicesHeader,
-	}
-	return &stackCtx
+	s Stack
 }
 
 func (s *stackContext) MarshalJSON() ([]byte, error) {

cli/command/stack/formatter/formatter_test.go

@@ -9,53 +9,57 @@ import (
 )
 
 func TestStackContextWrite(t *testing.T) {
-	cases := []struct {
-		context  formatter.Context
+	tests := []struct {
+		name     string
+		format   formatter.Format
 		expected string
 	}{
-		// Errors
 		{
-			formatter.Context{Format: "{{InvalidFunction}}"},
-			`template parsing error: template: :1: function "InvalidFunction" not defined`,
+			name:     "invalid function",
+			format:   `{{InvalidFunction}}`,
+			expected: `template parsing error: template: :1: function "InvalidFunction" not defined`,
 		},
 		{
-			formatter.Context{Format: "{{nil}}"},
-			`template parsing error: template: :1:2: executing "" at <nil>: nil is not a command`,
+			name:     "invalid placeholder",
+			format:   `{{nil}}`,
+			expected: `template parsing error: template: :1:2: executing "" at <nil>: nil is not a command`,
 		},
-		// Table format
 		{
-			formatter.Context{Format: SwarmStackTableFormat},
-			`NAME      SERVICES
+			name:   "table format",
+			format: SwarmStackTableFormat,
+			expected: `NAME      SERVICES
 baz       2
 bar       1
 `,
 		},
 		{
-			formatter.Context{Format: formatter.Format("table {{.Name}}")},
-			`NAME
+			name:   "custom table format",
+			format: `table {{.Name}}`,
+			expected: `NAME
 baz
 bar
 `,
 		},
-		// Custom Format
 		{
-			formatter.Context{Format: formatter.Format("{{.Name}}")},
-			`baz
+			name:   "custom format",
+			format: `{{.Name}}`,
+			expected: `baz
 bar
 `,
 		},
 	}
 
-	stacks := []*Stack{
-		{Name: "baz", Services: 2},
-		{Name: "bar", Services: 1},
-	}
-	for _, tc := range cases {
-		t.Run(string(tc.context.Format), func(t *testing.T) {
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
 			var out bytes.Buffer
-			tc.context.Output = &out
-
-			if err := StackWrite(tc.context, stacks); err != nil {
+			fmtCtx := formatter.Context{
+				Format: tc.format,
+				Output: &out,
+			}
+			if err := StackWrite(fmtCtx, []Stack{
+				{Name: "baz", Services: 2},
+				{Name: "bar", Services: 1},
+			}); err != nil {
 				assert.Error(t, err, tc.expected)
 			} else {
 				assert.Equal(t, out.String(), tc.expected)

cli/command/stack/list.go

@@ -46,16 +46,14 @@ func RunList(ctx context.Context, dockerCLI command.Cli, opts options.List) erro
 
 // runList performs a stack list against the specified swarm cluster
 func runList(ctx context.Context, dockerCLI command.Cli, opts listOptions) error {
-	ss, err := swarm.GetStacks(ctx, dockerCLI.Client())
+	stacks, err := swarm.GetStacks(ctx, dockerCLI.Client())
 	if err != nil {
 		return err
 	}
-	stacks := make([]*formatter.Stack, 0, len(ss))
-	stacks = append(stacks, ss...)
 	return format(dockerCLI.Out(), opts, stacks)
 }
 
-func format(out io.Writer, opts listOptions, stacks []*formatter.Stack) error {
+func format(out io.Writer, opts listOptions, stacks []formatter.Stack) error {
 	fmt := formatter.Format(opts.Format)
 	if fmt == "" || fmt == formatter.TableFormatKey {
 		fmt = formatter.SwarmStackTableFormat

cli/command/stack/swarm/deploy.go

@@ -69,19 +69,19 @@ func checkDaemonIsSwarmManager(ctx context.Context, dockerCli command.Cli) error
 }
 
 // pruneServices removes services that are no longer referenced in the source
-func pruneServices(ctx context.Context, dockerCCLI command.Cli, namespace convert.Namespace, services map[string]struct{}) {
-	apiClient := dockerCCLI.Client()
+func pruneServices(ctx context.Context, dockerCLI command.Cli, namespace convert.Namespace, services map[string]struct{}) {
+	apiClient := dockerCLI.Client()
 
 	oldServices, err := getStackServices(ctx, apiClient, namespace.Name())
 	if err != nil {
-		_, _ = fmt.Fprintln(dockerCCLI.Err(), "Failed to list services:", err)
+		_, _ = fmt.Fprintln(dockerCLI.Err(), "Failed to list services:", err)
 	}
 
-	pruneServices := []swarm.Service{}
+	toRemove := make([]swarm.Service, 0, len(oldServices))
 	for _, service := range oldServices {
 		if _, exists := services[namespace.Descope(service.Spec.Name)]; !exists {
-			pruneServices = append(pruneServices, service)
+			toRemove = append(toRemove, service)
 		}
 	}
-	removeServices(ctx, dockerCCLI, pruneServices)
+	removeServices(ctx, dockerCLI, toRemove)
 }

cli/command/stack/swarm/list.go

@@ -9,37 +9,31 @@ import (
 	"github.com/pkg/errors"
 )
 
-// GetStacks lists the swarm stacks.
+// GetStacks lists the swarm stacks with the number of services they contain.
 //
 // Deprecated: this function was for internal use and will be removed in the next release.
-func GetStacks(ctx context.Context, apiClient client.ServiceAPIClient) ([]*formatter.Stack, error) {
-	services, err := apiClient.ServiceList(
-		ctx,
-		client.ServiceListOptions{Filters: getAllStacksFilter()})
+func GetStacks(ctx context.Context, apiClient client.ServiceAPIClient) ([]formatter.Stack, error) {
+	services, err := apiClient.ServiceList(ctx, client.ServiceListOptions{
+		Filters: getAllStacksFilter(),
+	})
 	if err != nil {
 		return nil, err
 	}
-	m := make(map[string]*formatter.Stack)
-	for _, service := range services {
-		labels := service.Spec.Labels
-		name, ok := labels[convert.LabelNamespace]
+
+	idx := make(map[string]int, len(services))
+	out := make([]formatter.Stack, 0, len(services))
+
+	for _, svc := range services {
+		name, ok := svc.Spec.Labels[convert.LabelNamespace]
 		if !ok {
-			return nil, errors.Errorf("cannot get label %s for service %s",
-				convert.LabelNamespace, service.ID)
+			return nil, errors.New("cannot get label " + convert.LabelNamespace + " for service " + svc.ID)
 		}
-		ztack, ok := m[name]
-		if !ok {
-			m[name] = &formatter.Stack{
-				Name:     name,
-				Services: 1,
-			}
-		} else {
-			ztack.Services++
+		if i, ok := idx[name]; ok {
+			out[i].Services++
+			continue
 		}
+		idx[name] = len(out)
+		out = append(out, formatter.Stack{Name: name, Services: 1})
 	}
-	stacks := make([]*formatter.Stack, 0, len(m))
-	for _, stack := range m {
-		stacks = append(stacks, stack)
-	}
-	return stacks, nil
+	return out, nil
 }