Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

service/progress: ServiceProgress: avoid fuzzy matching service ID in loop #5788

Merged
merged 2 commits into from
Feb 3, 2025

Conversation

thaJeztah
Copy link
Member

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.

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.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

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 <github@gone.nl>
… 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 <github@gone.nl>
@thaJeztah thaJeztah added status/2-code-review area/swarm kind/refactor PR's that refactor, or clean-up code labels Feb 3, 2025
@thaJeztah thaJeztah added this to the 28.0.0 milestone Feb 3, 2025
@thaJeztah thaJeztah self-assigned this Feb 3, 2025
@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 59.43%. Comparing base (4771aba) to head (a428800).
Report is 7 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5788   +/-   ##
=======================================
  Coverage   59.42%   59.43%           
=======================================
  Files         347      347           
  Lines       29395    29392    -3     
=======================================
+ Hits        17469    17470    +1     
+ Misses      10954    10950    -4     
  Partials      972      972           

@thaJeztah thaJeztah merged commit 795b7d5 into docker:master Feb 3, 2025
104 checks passed
@thaJeztah thaJeztah deleted the progress_clean branch February 3, 2025 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/swarm kind/refactor PR's that refactor, or clean-up code status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants