From e88b1939f7adf90aa71c34048ce4725c2713c852 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 3 Feb 2025 15:35:26 +0100 Subject: [PATCH 1/2] service/progress: newReplicatedJobProgressUpdater: slight cleanup Use intermediate vars, so that the replicatedJobProgressUpdater can be created in one go intead of setting some fields after the fact. Signed-off-by: Sebastiaan van Stijn --- cli/command/service/progress/progress.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/cli/command/service/progress/progress.go b/cli/command/service/progress/progress.go index 6fa73d9331df..32be8804a624 100644 --- a/cli/command/service/progress/progress.go +++ b/cli/command/service/progress/progress.go @@ -573,16 +573,17 @@ type replicatedJobProgressUpdater struct { } func newReplicatedJobProgressUpdater(service swarm.Service, progressOut progress.Output) *replicatedJobProgressUpdater { - u := &replicatedJobProgressUpdater{ - progressOut: progressOut, - concurrent: int(*service.Spec.Mode.ReplicatedJob.MaxConcurrent), - total: int(*service.Spec.Mode.ReplicatedJob.TotalCompletions), - jobIteration: service.JobStatus.JobIteration.Index, - } - u.progressDigits = len(strconv.Itoa(u.total)) - u.activeDigits = len(strconv.Itoa(u.concurrent)) + concurrent := int(*service.Spec.Mode.ReplicatedJob.MaxConcurrent) + total := int(*service.Spec.Mode.ReplicatedJob.TotalCompletions) - return u + return &replicatedJobProgressUpdater{ + progressOut: progressOut, + concurrent: concurrent, + total: total, + jobIteration: service.JobStatus.JobIteration.Index, + progressDigits: len(strconv.Itoa(total)), + activeDigits: len(strconv.Itoa(concurrent)), + } } // update writes out the progress of the replicated job. From a4288003bd8da4844372a99220b9f9e5ef1c5a16 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 3 Feb 2025 15:42:23 +0100 Subject: [PATCH 2/2] service/progress: ServiceProgress: avoid fuzzy matching service ID in loop Tasks with a service filter will result in the daemon performing a lookup of the full service ID, then updating the provided filter with the actual ID: https://github.com/moby/moby/blob/96ded2a1badcc5cb99d6d2ab7904d6f9e527cc4b/daemon/cluster/tasks.go#L15-L30 The `getService()` helper has a fast-path for situations where the given filter is a full ID, before falling back to fuzzy-logic to search filters by service name or prefix, which would return an error if the result is ambiguous; https://github.com/moby/moby/blob/96ded2a1badcc5cb99d6d2ab7904d6f9e527cc4b/daemon/cluster/helpers.go#L62-L81 The loop executed here calls `client.ServiceInspectWithRaw()` to get info of the service, and that method is ultimately calling the exact same `getService()` helper on the daemon side, which means that we don't need to repeat the work; we can use the `Service.ID` resolved from that call, and use it to apply as filter for listing the tasks. Signed-off-by: Sebastiaan van Stijn --- cli/command/service/progress/progress.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/cli/command/service/progress/progress.go b/cli/command/service/progress/progress.go index 32be8804a624..edfabbd40dde 100644 --- a/cli/command/service/progress/progress.go +++ b/cli/command/service/progress/progress.go @@ -79,14 +79,6 @@ func ServiceProgress(ctx context.Context, apiClient client.APIClient, serviceID signal.Notify(sigint, os.Interrupt) defer signal.Stop(sigint) - taskFilter := filters.NewArgs() - taskFilter.Add("service", serviceID) - taskFilter.Add("_up-to-date", "true") - - getUpToDateTasks := func() ([]swarm.Task, error) { - return apiClient.TaskList(ctx, types.TaskListOptions{Filters: taskFilter}) - } - var ( updater progressUpdater converged bool @@ -151,7 +143,10 @@ func ServiceProgress(ctx context.Context, apiClient client.APIClient, serviceID return nil } - tasks, err := getUpToDateTasks() + tasks, err := apiClient.TaskList(ctx, types.TaskListOptions{Filters: filters.NewArgs( + filters.KeyValuePair{Key: "service", Value: service.ID}, + filters.KeyValuePair{Key: "_up-to-date", Value: "true"}, + )}) if err != nil { return err } @@ -218,7 +213,10 @@ func ServiceProgress(ctx context.Context, apiClient client.APIClient, serviceID } } -func getActiveNodes(ctx context.Context, apiClient client.APIClient) (map[string]struct{}, error) { +// getActiveNodes returns all nodes that are currently not in status [swarm.NodeStateDown]. +// +// TODO(thaJeztah): this should really be a filter on [apiClient.NodeList] instead of being filtered on the client side. +func getActiveNodes(ctx context.Context, apiClient client.NodeAPIClient) (map[string]struct{}, error) { nodes, err := apiClient.NodeList(ctx, types.NodeListOptions{}) if err != nil { return nil, err