Services: use ServiceStatus on API v1.41 and up

API v1.41 adds a new option to get the number of desired
and running tasks when listing services. This patch enables
this functionality, and provides a fallback mechanism when
the ServiceStatus is not available, which would be when
using an older API version.

Now that the swarm.Service struct captures this information,
the `ListInfo` type is no longer needed, so it is removed,
and the related list- and formatting functions have been
modified accordingly.

To reduce repetition, sorting the services has been moved
to the formatter. This is a slight change in behavior, but
all calls to the formatter performed this sort first, so
the change will not lead to user-facing changes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

cli/command/stack/kubernetes/conversion.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/docker/cli/cli/command/service"
 	"github.com/docker/compose-on-kubernetes/api/labels"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/swarm"
@@ -154,35 +153,65 @@ const (
 	publishedOnRandomPortSuffix = "-random-ports"
 )
 
-func convertToServices(replicas *appsv1beta2.ReplicaSetList, daemons *appsv1beta2.DaemonSetList, services *apiv1.ServiceList) ([]swarm.Service, map[string]service.ListInfo, error) {
+func convertToServices(replicas *appsv1beta2.ReplicaSetList, daemons *appsv1beta2.DaemonSetList, services *apiv1.ServiceList) ([]swarm.Service, error) {
 	result := make([]swarm.Service, len(replicas.Items))
-	infos := make(map[string]service.ListInfo, len(replicas.Items)+len(daemons.Items))
+
 	for i, r := range replicas.Items {
-		s, err := convertToService(r.Labels[labels.ForServiceName], services, r.Spec.Template.Spec.Containers)
+		s, err := replicatedService(r, services)
 		if err != nil {
-			return nil, nil, err
+			return nil, err
 		}
 		result[i] = *s
-		infos[s.ID] = service.ListInfo{
-			Mode:     "replicated",
-			Replicas: fmt.Sprintf("%d/%d", r.Status.AvailableReplicas, r.Status.Replicas),
-		}
 	}
 	for _, d := range daemons.Items {
-		s, err := convertToService(d.Labels[labels.ForServiceName], services, d.Spec.Template.Spec.Containers)
+		s, err := globalService(d, services)
 		if err != nil {
-			return nil, nil, err
+			return nil, err
 		}
 		result = append(result, *s)
-		infos[s.ID] = service.ListInfo{
-			Mode:     "global",
-			Replicas: fmt.Sprintf("%d/%d", d.Status.NumberReady, d.Status.DesiredNumberScheduled),
-		}
 	}
 	sort.Slice(result, func(i, j int) bool {
 		return result[i].ID < result[j].ID
 	})
-	return result, infos, nil
+	return result, nil
+}
+
+func uint64ptr(i int32) *uint64 {
+	var o uint64
+	if i > 0 {
+		o = uint64(i)
+	}
+	return &o
+}
+
+func replicatedService(r appsv1beta2.ReplicaSet, services *apiv1.ServiceList) (*swarm.Service, error) {
+	s, err := convertToService(r.Labels[labels.ForServiceName], services, r.Spec.Template.Spec.Containers)
+	if err != nil {
+		return nil, err
+	}
+	s.Spec.Mode = swarm.ServiceMode{
+		Replicated: &swarm.ReplicatedService{Replicas: uint64ptr(r.Status.Replicas)},
+	}
+	s.ServiceStatus = &swarm.ServiceStatus{
+		RunningTasks: uint64(r.Status.AvailableReplicas),
+		DesiredTasks: uint64(r.Status.Replicas),
+	}
+	return s, nil
+}
+
+func globalService(d appsv1beta2.DaemonSet, services *apiv1.ServiceList) (*swarm.Service, error) {
+	s, err := convertToService(d.Labels[labels.ForServiceName], services, d.Spec.Template.Spec.Containers)
+	if err != nil {
+		return nil, err
+	}
+	s.Spec.Mode = swarm.ServiceMode{
+		Global: &swarm.GlobalService{},
+	}
+	s.ServiceStatus = &swarm.ServiceStatus{
+		RunningTasks: uint64(d.Status.NumberReady),
+		DesiredTasks: uint64(d.Status.DesiredNumberScheduled),
+	}
+	return s, nil
 }
 
 func convertToService(serviceName string, services *apiv1.ServiceList, containers []apiv1.Container) (*swarm.Service, error) {

cli/command/stack/kubernetes/conversion_test.go

@@ -3,7 +3,6 @@ package kubernetes
 import (
 	"testing"
 
-	"github.com/docker/cli/cli/command/service"
 	"github.com/docker/compose-on-kubernetes/api/labels"
 	"github.com/docker/docker/api/types/swarm"
 	"gotest.tools/assert"
@@ -19,49 +18,45 @@ func TestReplicasConversionNeedsAService(t *testing.T) {
 		Items: []appsv1beta2.ReplicaSet{makeReplicaSet("unknown", 0, 0)},
 	}
 	services := apiv1.ServiceList{}
-	_, _, err := convertToServices(&replicas, &appsv1beta2.DaemonSetList{}, &services)
+	_, err := convertToServices(&replicas, &appsv1beta2.DaemonSetList{}, &services)
 	assert.ErrorContains(t, err, "could not find service")
 }
 
 func TestKubernetesServiceToSwarmServiceConversion(t *testing.T) {
 	testCases := []struct {
+		doc              string
 		replicas         *appsv1beta2.ReplicaSetList
 		services         *apiv1.ServiceList
 		expectedServices []swarm.Service
-		expectedListInfo map[string]service.ListInfo
 	}{
-		// Match replicas with headless stack services
 		{
-			&appsv1beta2.ReplicaSetList{
+			doc: "Match replicas with headless stack services",
+			replicas: &appsv1beta2.ReplicaSetList{
 				Items: []appsv1beta2.ReplicaSet{
 					makeReplicaSet("service1", 2, 5),
 					makeReplicaSet("service2", 3, 3),
 				},
 			},
-			&apiv1.ServiceList{
+			services: &apiv1.ServiceList{
 				Items: []apiv1.Service{
 					makeKubeService("service1", "stack", "uid1", apiv1.ServiceTypeClusterIP, nil),
 					makeKubeService("service2", "stack", "uid2", apiv1.ServiceTypeClusterIP, nil),
 					makeKubeService("service3", "other-stack", "uid2", apiv1.ServiceTypeClusterIP, nil),
 				},
 			},
-			[]swarm.Service{
-				makeSwarmService("stack_service1", "uid1", nil),
-				makeSwarmService("stack_service2", "uid2", nil),
-			},
-			map[string]service.ListInfo{
-				"uid1": {Mode: "replicated", Replicas: "2/5"},
-				"uid2": {Mode: "replicated", Replicas: "3/3"},
+			expectedServices: []swarm.Service{
+				makeSwarmService(t, "stack_service1", "uid1", withMode("replicated", 5), withStatus(2, 5)),
+				makeSwarmService(t, "stack_service2", "uid2", withMode("replicated", 3), withStatus(3, 3)),
 			},
 		},
-		// Headless service and LoadBalancer Service are tied to the same Swarm service
 		{
-			&appsv1beta2.ReplicaSetList{
+			doc: "Headless service and LoadBalancer Service are tied to the same Swarm service",
+			replicas: &appsv1beta2.ReplicaSetList{
 				Items: []appsv1beta2.ReplicaSet{
 					makeReplicaSet("service", 1, 1),
 				},
 			},
-			&apiv1.ServiceList{
+			services: &apiv1.ServiceList{
 				Items: []apiv1.Service{
 					makeKubeService("service", "stack", "uid1", apiv1.ServiceTypeClusterIP, nil),
 					makeKubeService("service-published", "stack", "uid2", apiv1.ServiceTypeLoadBalancer, []apiv1.ServicePort{
@@ -73,29 +68,26 @@ func TestKubernetesServiceToSwarmServiceConversion(t *testing.T) {
 					}),
 				},
 			},
-			[]swarm.Service{
-				makeSwarmService("stack_service", "uid1", []swarm.PortConfig{
-					{
+			expectedServices: []swarm.Service{
+				makeSwarmService(t, "stack_service", "uid1",
+					withMode("replicated", 1),
+					withStatus(1, 1), withPort(swarm.PortConfig{
 						PublishMode:   swarm.PortConfigPublishModeIngress,
 						PublishedPort: 80,
 						TargetPort:    80,
 						Protocol:      swarm.PortConfigProtocolTCP,
-					},
-				}),
-			},
-			map[string]service.ListInfo{
-				"uid1": {Mode: "replicated", Replicas: "1/1"},
+					}),
+				),
 			},
 		},
-		// Headless service and NodePort Service are tied to the same Swarm service
-
 		{
-			&appsv1beta2.ReplicaSetList{
+			doc: "Headless service and NodePort Service are tied to the same Swarm service",
+			replicas: &appsv1beta2.ReplicaSetList{
 				Items: []appsv1beta2.ReplicaSet{
 					makeReplicaSet("service", 1, 1),
 				},
 			},
-			&apiv1.ServiceList{
+			services: &apiv1.ServiceList{
 				Items: []apiv1.Service{
 					makeKubeService("service", "stack", "uid1", apiv1.ServiceTypeClusterIP, nil),
 					makeKubeService("service-random-ports", "stack", "uid2", apiv1.ServiceTypeNodePort, []apiv1.ServicePort{
@@ -107,27 +99,28 @@ func TestKubernetesServiceToSwarmServiceConversion(t *testing.T) {
 					}),
 				},
 			},
-			[]swarm.Service{
-				makeSwarmService("stack_service", "uid1", []swarm.PortConfig{
-					{
+			expectedServices: []swarm.Service{
+				makeSwarmService(t, "stack_service", "uid1",
+					withMode("replicated", 1),
+					withStatus(1, 1),
+					withPort(swarm.PortConfig{
 						PublishMode:   swarm.PortConfigPublishModeHost,
 						PublishedPort: 35666,
 						TargetPort:    80,
 						Protocol:      swarm.PortConfigProtocolTCP,
-					},
-				}),
-			},
-			map[string]service.ListInfo{
-				"uid1": {Mode: "replicated", Replicas: "1/1"},
+					}),
+				),
 			},
 		},
 	}
 
 	for _, tc := range testCases {
-		swarmServices, listInfo, err := convertToServices(tc.replicas, &appsv1beta2.DaemonSetList{}, tc.services)
-		assert.NilError(t, err)
-		assert.DeepEqual(t, tc.expectedServices, swarmServices)
-		assert.DeepEqual(t, tc.expectedListInfo, listInfo)
+		tc := tc
+		t.Run(tc.doc, func(t *testing.T) {
+			swarmServices, err := convertToServices(tc.replicas, &appsv1beta2.DaemonSetList{}, tc.services)
+			assert.NilError(t, err)
+			assert.DeepEqual(t, tc.expectedServices, swarmServices)
+		})
 	}
 }
 
@@ -172,8 +165,46 @@ func makeKubeService(service, stack, uid string, serviceType apiv1.ServiceType,
 	}
 }
 
-func makeSwarmService(service, id string, ports []swarm.PortConfig) swarm.Service {
-	return swarm.Service{
+func withMode(mode string, replicas uint64) func(*swarm.Service) {
+	return func(service *swarm.Service) {
+		switch mode {
+		case "global":
+			service.Spec.Mode = swarm.ServiceMode{
+				Global: &swarm.GlobalService{},
+			}
+		case "replicated":
+			service.Spec.Mode = swarm.ServiceMode{
+				Replicated: &swarm.ReplicatedService{Replicas: &replicas},
+			}
+			withStatus(0, replicas)
+		default:
+			service.Spec.Mode = swarm.ServiceMode{}
+			withStatus(0, 0)
+		}
+	}
+}
+
+func withPort(port swarm.PortConfig) func(*swarm.Service) {
+	return func(service *swarm.Service) {
+		if service.Endpoint.Ports == nil {
+			service.Endpoint.Ports = make([]swarm.PortConfig, 0)
+		}
+		service.Endpoint.Ports = append(service.Endpoint.Ports, port)
+	}
+}
+
+func withStatus(running, desired uint64) func(*swarm.Service) {
+	return func(service *swarm.Service) {
+		service.ServiceStatus = &swarm.ServiceStatus{
+			RunningTasks: running,
+			DesiredTasks: desired,
+		}
+	}
+}
+
+func makeSwarmService(t *testing.T, service, id string, opts ...func(*swarm.Service)) swarm.Service {
+	t.Helper()
+	s := swarm.Service{
 		ID: id,
 		Spec: swarm.ServiceSpec{
 			Annotations: swarm.Annotations{
@@ -185,8 +216,9 @@ func makeSwarmService(service, id string, ports []swarm.PortConfig) swarm.Servic
 				},
 			},
 		},
-		Endpoint: swarm.Endpoint{
-			Ports: ports,
-		},
 	}
+	for _, o := range opts {
+		o(&s)
+	}
+	return s
 }

cli/command/stack/kubernetes/services.go

@@ -109,16 +109,12 @@ func RunServices(dockerCli *KubeCli, opts options.Services) error {
 	}
 
 	// Convert Replicas sets and kubernetes services to swarm services and formatter information
-	services, info, err := convertToServices(replicasList, daemonsList, servicesList)
+	services, err := convertToServices(replicasList, daemonsList, servicesList)
 	if err != nil {
 		return err
 	}
 	services = filterServicesByName(services, filters.Get("name"), stackName)
 
-	if opts.Quiet {
-		info = map[string]service.ListInfo{}
-	}
-
 	format := opts.Format
 	if len(format) == 0 {
 		if len(dockerCli.ConfigFile().ServicesFormat) > 0 && !opts.Quiet {
@@ -132,7 +128,7 @@ func RunServices(dockerCli *KubeCli, opts options.Services) error {
 		Output: dockerCli.Out(),
 		Format: service.NewListFormat(format, opts.Quiet),
 	}
-	return service.ListFormatWrite(servicesCtx, services, info)
+	return service.ListFormatWrite(servicesCtx, services)
 }
 
 func filterServicesByName(services []swarm.Service, names []string, stackName string) []swarm.Service {

cli/command/stack/swarm/services.go

@@ -3,55 +3,59 @@ package swarm
 import (
 	"context"
 	"fmt"
-	"sort"
 
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/service"
 	"github.com/docker/cli/cli/command/stack/formatter"
 	"github.com/docker/cli/cli/command/stack/options"
 	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/api/types/filters"
-	"vbom.ml/util/sortorder"
 )
 
 // RunServices is the swarm implementation of docker stack services
 func RunServices(dockerCli command.Cli, opts options.Services) error {
-	ctx := context.Background()
-	client := dockerCli.Client()
+	var (
+		err    error
+		ctx    = context.Background()
+		client = dockerCli.Client()
+	)
 
-	filter := getStackFilterFromOpt(opts.Namespace, opts.Filter)
-	services, err := client.ServiceList(ctx, types.ServiceListOptions{Filters: filter})
+	listOpts := types.ServiceListOptions{
+		Filters: getStackFilterFromOpt(opts.Namespace, opts.Filter),
+		// When not running "quiet", also get service status (number of running
+		// and desired tasks). Note that this is only supported on API v1.41 and
+		// up; older API versions ignore this option, and we will have to collect
+		// the information manually below.
+		Status: !opts.Quiet,
+	}
+
+	services, err := client.ServiceList(ctx, listOpts)
 	if err != nil {
 		return err
 	}
 
 	// if no services in this stack, print message and exit 0
 	if len(services) == 0 {
-		fmt.Fprintf(dockerCli.Err(), "Nothing found in stack: %s\n", opts.Namespace)
+		_, _ = fmt.Fprintf(dockerCli.Err(), "Nothing found in stack: %s\n", opts.Namespace)
 		return nil
 	}
 
-	sort.Slice(services, func(i, j int) bool {
-		return sortorder.NaturalLess(services[i].Spec.Name, services[j].Spec.Name)
-	})
-	info := map[string]service.ListInfo{}
-	if !opts.Quiet {
-		taskFilter := filters.NewArgs()
-		for _, service := range services {
-			taskFilter.Add("service", service.ID)
-		}
-
-		tasks, err := client.TaskList(ctx, types.TaskListOptions{Filters: taskFilter})
+	if listOpts.Status {
+		// Now that a request was made, we know what API version was used (either
+		// through configuration, or after client and daemon negotiated a version).
+		// If API version v1.41 or up was used; the daemon should already have done
+		// the legwork for us, and we don't have to calculate the number of desired
+		// and running tasks. On older API versions, we need to do some extra requests
+		// to get that information.
+		//
+		// So theoretically, this step can be skipped based on API version, however,
+		// some of our unit tests don't set the API version, and there may be other
+		// situations where the client uses the "default" version. To account for
+		// these situations, we do a quick check for services that do not have
+		// a ServiceStatus set, and perform a lookup for those.
+		services, err = service.AppendServiceStatus(ctx, client, services)
 		if err != nil {
 			return err
 		}
-
-		nodes, err := client.NodeList(ctx, types.NodeListOptions{})
-		if err != nil {
-			return err
-		}
-
-		info = service.GetServicesStatus(services, nodes, tasks)
 	}
 
 	format := opts.Format
@@ -67,5 +71,5 @@ func RunServices(dockerCli command.Cli, opts options.Services) error {
 		Output: dockerCli.Out(),
 		Format: service.NewListFormat(format, opts.Quiet),
 	}
-	return service.ListFormatWrite(servicesCtx, services, info)
+	return service.ListFormatWrite(servicesCtx, services)
 }