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

Add ListServiceStatuses grpc method #2856

Merged
merged 1 commit into from
May 16, 2019

Conversation

dperny
Copy link
Collaborator

@dperny dperny commented May 15, 2019

- What I did

Added a gRPC method ListServiceStatuses to the control API. This performs the extremely common UI operation of calculating service actual vs desired tasks on the server side, which reduces the amount of work and number of network requests required by clients.

- How I did it

Counting up tasks, EZPZ. Global services are checked with the standard trick of looking at all tasks in desired state running.

Using a new gRPC method avoids having to change the Service proto to accommodate what is essentially fleeting UI data.

- How to test it

Includes a robust unit test

- Description for the changelog

swarmkit now supports querying service task actual and desired counts directly, allowing for optimizations in the engine as well.

@dperny dperny force-pushed the add-service-status-endpoint branch from 4c61164 to 560a8a6 Compare May 15, 2019 19:43
@codecov
Copy link

codecov bot commented May 15, 2019

Codecov Report

Merging #2856 into master will increase coverage by 0.19%.
The diff coverage is 84%.

@@            Coverage Diff             @@
##           master    #2856      +/-   ##
==========================================
+ Coverage   62.15%   62.34%   +0.19%     
==========================================
  Files         139      139              
  Lines       22316    22341      +25     
==========================================
+ Hits        13870    13929      +59     
+ Misses       6960     6929      -31     
+ Partials     1486     1483       -3

Copy link
Contributor

@jlhawn jlhawn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IANAM but LGTM

... and it's very well commented! 👍

@dperny dperny force-pushed the add-service-status-endpoint branch from 560a8a6 to cc935e4 Compare May 16, 2019 15:18
Adds a ListServiceStatuses method. This does server-side computation of
service desired vs running tasks. This is a very common operation on the
client, but performing it previously required getting a list of all
tasks for a service, which was both network and computationally
expensive.

Signed-off-by: Drew Erny <[email protected]>
@dperny dperny force-pushed the add-service-status-endpoint branch from cc935e4 to 251cbb7 Compare May 16, 2019 15:35
@dperny
Copy link
Collaborator Author

dperny commented May 16, 2019

The TestUpdaterRollback tests are known flaky, and I am going to merge if I get a test run that is green, unless I see an error unrelated to TestUpdaterRollback.

@dperny dperny merged commit 88dcc0f into moby:master May 16, 2019
dperny added a commit to dperny/docker that referenced this pull request May 24, 2019
Includes the following changes since last vendoring:

moby/swarmkit#2795 - Add capabilities list to container specification
moby/swarmkit#2845 - Fix linting error
moby/swarmkit#2848 - Bump fernet/fernet-go
moby/swarmkit#2856 - Add ListServiceStatuses grpc method
moby/swarmkit#2857 - Use Service Placement Constraints in Enforcer

Signed-off-by: Drew Erny <[email protected]>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request May 26, 2019
Includes the following changes since last vendoring:

moby/swarmkit#2795 - Add capabilities list to container specification
moby/swarmkit#2845 - Fix linting error
moby/swarmkit#2848 - Bump fernet/fernet-go
moby/swarmkit#2856 - Add ListServiceStatuses grpc method
moby/swarmkit#2857 - Use Service Placement Constraints in Enforcer

Signed-off-by: Drew Erny <[email protected]>
Upstream-commit: 67e25ec5ac568a893e444891a6a583fd2f996f76
Component: engine
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants