From b8f2b7c678d00b5617590e0c1900988d348c70ca Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 10 Oct 2025 20:39:37 +0200 Subject: [PATCH] cli/command/service: remove AppendServiceStatus (API --- cli/command/image/opts.go | 17 ---- cli/command/service/list.go | 123 +--------------------------- cli/command/stack/services_test.go | 27 ------ cli/command/stack/services_utils.go | 34 +------- 4 files changed, 6 insertions(+), 195 deletions(-) delete mode 100644 cli/command/image/opts.go diff --git a/cli/command/image/opts.go b/cli/command/image/opts.go deleted file mode 100644 index 37378b6ec..000000000 --- a/cli/command/image/opts.go +++ /dev/null @@ -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"}) -} diff --git a/cli/command/service/list.go b/cli/command/service/list.go index 95254fd9c..cac25d8e4 100644 --- a/cli/command/service/list.go +++ b/cli/command/service/list.go @@ -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 -} diff --git a/cli/command/stack/services_test.go b/cli/command/stack/services_test.go index 0fe5b5fc8..8326b335e 100644 --- a/cli/command/stack/services_test.go +++ b/cli/command/stack/services_test.go @@ -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) diff --git a/cli/command/stack/services_utils.go b/cli/command/stack/services_utils.go index 4fd10f2d3..57718f037 100644 --- a/cli/command/stack/services_utils.go +++ b/cli/command/stack/services_utils.go @@ -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 + }) }