p4u1/docker-cli
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cli/command/service: remove AppendServiceStatus (API <v1.41)

Browse Source 
This function was added in 7405ac5c2d as
a fallback for API < v1.41, which did not include the service status
in the response. Current API versions return this information, so there's
no need to fetch it manually.

It was not gated by API version for some tests (which didn't set API
version), but should not be needed for non-test situations.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn
2025-10-10 20:39:37 +02:00
parent f046bd371a
commit b8f2b7c678
4 changed files with 6 additions and 195 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
cli/command/image/opts.go
View File

@ -1,17 +0,0 @@
package image
import (
"os"
"github.com/spf13/pflag"
)
// addPlatformFlag adds "--platform" to a set of flags for API version 1.32 and
// later, using the value of "DOCKER_DEFAULT_PLATFORM" (if set) as a default.
//
// It should not be used for new uses, which may have a different API version
// requirement.
func addPlatformFlag(flags *pflag.FlagSet, target *string) {
flags.StringVar(target, "platform", os.Getenv("DOCKER_DEFAULT_PLATFORM"), "Set platform if server is multi-platform capable")
_ = flags.SetAnnotation("platform", "version", []string{"1.32"})
}

123
cli/command/service/list.go
View File

@ -8,7 +8,6 @@ import (
"github.com/docker/cli/cli/command/formatter"
flagsHelper "github.com/docker/cli/cli/flags"
"github.com/docker/cli/opts"
"github.com/moby/moby/api/types/swarm"
"github.com/moby/moby/client"
"github.com/spf13/cobra"
)
@ -43,44 +42,19 @@ func newListCommand(dockerCLI command.Cli) *cobra.Command {
}
func runList(ctx context.Context, dockerCLI command.Cli, options listOptions) error {
var (
apiClient = dockerCLI.Client()
err error
)
listOpts := client.ServiceListOptions{
apiClient := dockerCLI.Client()
services, err := apiClient.ServiceList(ctx, client.ServiceListOptions{
Filters: options.filter.Value(),
// 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: !options.quiet,
}
services, err := apiClient.ServiceList(ctx, listOpts)
})
if err != nil {
return err
}
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 = AppendServiceStatus(ctx, apiClient, services)
if err != nil {
return err
}
}
format := options.format
if len(format) == 0 {
if len(dockerCLI.ConfigFile().ServicesFormat) > 0 && !options.quiet {
@ -96,94 +70,3 @@ func runList(ctx context.Context, dockerCLI command.Cli, options listOptions) er
}
return ListFormatWrite(servicesCtx, services)
}
// AppendServiceStatus propagates the ServiceStatus field for "services".
//
// If API version v1.41 or up is used, this information is already set by the
// daemon. On older API versions, we need to do some extra requests to get
// that information. Theoretically, this function 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 take these situations into account, we do a quick check for services
// that don't have ServiceStatus set, and perform a lookup for those.
func AppendServiceStatus(ctx context.Context, c client.APIClient, services []swarm.Service) ([]swarm.Service, error) {
status := map[string]*swarm.ServiceStatus{}
taskFilter := make(client.Filters)
for i, s := range services {
// there is no need in this switch to check for job modes. jobs are not
// supported until after ServiceStatus was introduced.
switch {
case s.ServiceStatus != nil:
// Server already returned service-status, so we don't
// have to look-up tasks for this service.
continue
case s.Spec.Mode.Replicated != nil:
// For replicated services, set the desired number of tasks;
// that way we can present this information in case we're unable
// to get a list of tasks from the server.
services[i].ServiceStatus = &swarm.ServiceStatus{DesiredTasks: *s.Spec.Mode.Replicated.Replicas}
status[s.ID] = &swarm.ServiceStatus{}
taskFilter.Add("service", s.ID)
case s.Spec.Mode.Global != nil:
// No such thing as number of desired tasks for global services
services[i].ServiceStatus = &swarm.ServiceStatus{}
status[s.ID] = &swarm.ServiceStatus{}
taskFilter.Add("service", s.ID)
default:
// Unknown task type
}
}
if len(status) == 0 {
// All services have their ServiceStatus set, so we're done
return services, nil
}
tasks, err := c.TaskList(ctx, client.TaskListOptions{Filters: taskFilter})
if err != nil {
return nil, err
}
if len(tasks) == 0 {
return services, nil
}
activeNodes, err := getActiveNodes(ctx, c)
if err != nil {
return nil, err
}
for _, task := range tasks {
if status[task.ServiceID] == nil {
// This should not happen in practice; either all services have
// a ServiceStatus set, or none of them.
continue
}
// TODO: this should only be needed for "global" services. Replicated
// services have `Spec.Mode.Replicated.Replicas`, which should give this value.
if task.DesiredState != swarm.TaskStateShutdown {
status[task.ServiceID].DesiredTasks++
}
if _, nodeActive := activeNodes[task.NodeID]; nodeActive && task.Status.State == swarm.TaskStateRunning {
status[task.ServiceID].RunningTasks++
}
}
for i, service := range services {
if s := status[service.ID]; s != nil {
services[i].ServiceStatus = s
}
}
return services, nil
}
func getActiveNodes(ctx context.Context, c client.NodeAPIClient) (map[string]struct{}, error) {
nodes, err := c.NodeList(ctx, client.NodeListOptions{})
if err != nil {
return nil, err
}
activeNodes := make(map[string]struct{})
for _, n := range nodes {
if n.Status.State != swarm.NodeStateDown {
activeNodes[n.ID] = struct{}{}
}
}
return activeNodes, nil
}

27
cli/command/stack/services_test.go
View File

@ -21,8 +21,6 @@ func TestStackServicesErrors(t *testing.T) {
args []string
flags map[string]string
serviceListFunc func(options client.ServiceListOptions) ([]swarm.Service, error)
nodeListFunc func(options client.NodeListOptions) ([]swarm.Node, error)
taskListFunc func(options client.TaskListOptions) ([]swarm.Task, error)
expectedError string
}{
{
@ -32,29 +30,6 @@ func TestStackServicesErrors(t *testing.T) {
},
expectedError: "error getting services",
},
{
args: []string{"foo"},
serviceListFunc: func(options client.ServiceListOptions) ([]swarm.Service, error) {
return []swarm.Service{*builders.Service(builders.GlobalService())}, nil
},
nodeListFunc: func(options client.NodeListOptions) ([]swarm.Node, error) {
return nil, errors.New("error getting nodes")
},
taskListFunc: func(options client.TaskListOptions) ([]swarm.Task, error) {
return []swarm.Task{*builders.Task()}, nil
},
expectedError: "error getting nodes",
},
{
args: []string{"foo"},
serviceListFunc: func(options client.ServiceListOptions) ([]swarm.Service, error) {
return []swarm.Service{*builders.Service(builders.GlobalService())}, nil
},
taskListFunc: func(options client.TaskListOptions) ([]swarm.Task, error) {
return nil, errors.New("error getting tasks")
},
expectedError: "error getting tasks",
},
{
args: []string{"foo"},
flags: map[string]string{
@ -71,8 +46,6 @@ func TestStackServicesErrors(t *testing.T) {
t.Run(tc.expectedError, func(t *testing.T) {
cli := test.NewFakeCli(&fakeClient{
serviceListFunc: tc.serviceListFunc,
nodeListFunc: tc.nodeListFunc,
taskListFunc: tc.taskListFunc,
})
cmd := newServicesCommand(cli)
cmd.SetArgs(tc.args)

34
cli/command/stack/services_utils.go
View File

@ -3,44 +3,16 @@ package stack
import (
"context"
"github.com/docker/cli/cli/command/service"
"github.com/moby/moby/api/types/swarm"
"github.com/moby/moby/client"
)
// getServices is the swarm implementation of listing stack services
func getServices(ctx context.Context, apiClient client.APIClient, opts serviceListOptions) ([]swarm.Service, error) {
listOpts := client.ServiceListOptions{
return apiClient.ServiceList(ctx, client.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.
// and desired tasks).
Status: !opts.quiet,
}
services, err := apiClient.ServiceList(ctx, listOpts)
if err != nil {
return nil, err
}
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, apiClient, services)
if err != nil {
return nil, err
}
}
return services, nil
})
}