API: add "Swarm" header to _ping endpoint #42064
Conversation
|
/cc @djs55 |
djs55
left a comment
There was a problem hiding this comment.
Nice, thanks! I'd like to use this from the Docker Desktop diagnostics code. Currently we call a swarm endpoint which logs an error if swarm is not enabled, leading users to think there has been some kind of swarm error...
b531f1b to
6e096f0
Compare
|
@cpuguy83 made some changes; let me know if this is closer to what you had in mind |
| if s.IsActiveManager() || s.IsManager() { | ||
| state += " (manager)" | ||
| } else { | ||
| state += " (worker)" | ||
| } |
There was a problem hiding this comment.
Slightly thinking if we should keep manager/worker separate from node state (as other status/role combinations would be possible)
client/ping.go
Outdated
| parts := strings.SplitN(si, " ", 2) | ||
| ping.SwarmStatus = &swarm.Status{ | ||
| NodeState: swarm.LocalNodeState(parts[0]), | ||
| ControlAvailable: len(parts) == 2 && parts[1] == "(manager)", |
There was a problem hiding this comment.
This seems a little bit fiddly -- we're essentially parsing what's effectively a human-readable string back into separate parts? 😬
There was a problem hiding this comment.
Yes, agreed. I've been going back-and-forth on what the best approach would be; we could make the format itself more "parseable" (<status>/<role>) or two separate headers (I wasn't sure about the separate headers, because we won't be able to set the role until the status is also available (IIRC).
This adds an additional "Swarm" header to the _ping endpoint response, which allows a client to detect if Swarm is enabled on the daemon, without having to call additional endpoints. This change is not versioned in the API, and will be returned irregardless of the API version that is used. Clients should fall back to using other endpoints to get this information if the header is not present. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
|
@tianon changed it to |
|
Thanks, everyone for reviewing; let's get this one in 👍 |
| ctx := context.TODO() | ||
|
|
||
| t.Run("before swarm init", func(t *testing.T) { | ||
| res, _, err := request.Get("/_ping") |
There was a problem hiding this comment.
This should've been:
| res, _, err := request.Get("/_ping") | |
| res, _, err := request.Get("/_ping", request.Host(d.Sock())) |
Fixing in #43658
depends on #42063
This adds an additional "Swarm" header to the _ping endpoint response, which allows a client to detect if Swarm is enabled on the daemon, without having to call additional endpoints.
This change is not versioned in the API, and will be returned irregardless of the API version that is used. Clients should fall back to using other endpoints to get this information if the header is not present.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)