mirror of
https://github.com/moby/moby.git
synced 2026-07-13 19:12:11 +00:00
Looking in history to learn why this struct existed, shows that this type was mostly the result of tech-debt accumulating over time; - originally ([moby@1aa7f13]) most of the request handling was internal; the [`call()` function][1] would make a request, read the `response.Body`, and return it as a `[]byte` (or an error if one happened). - some features needed the statuscode, so [moby@a4bcf7e] added an extra output variable to return the `response.StatusCode`. - some new features required streaming, so [moby@fdd8d4b] changed the function to return the `response.Body` as a `io.ReadCloser`, instead of a `[]byte`. - some features needed access to the content-type header, so a new `clientRequest` method was introduced in [moby@6b2eeaf] to read the `Content-Type` header from `response.Headers` and return it as a string. - of course, `Content-Type` may not be the only header needed, so [moby@0cdc3b7] changed the signature to return `response.Headers` as a whole as a `http.Header` - things became a bit unwieldy now, with the function having four (4) output variables, so [moby@126529c] chose to refactor this code, introducing a `serverResponse` struct to wrap them all, not realizing that all these values were effectively deconstructed from the `url.Response`, so now re-assembling them into our own "URL response", only preserving a subset of the information available. - now that we had a custom struct, it was possible to add more information to it without changing the signature. When there was a need to know the URL of the request that initiated the response, [moby@27ef09a] introduced a `reqURL` field to hold the `request.URL` which notably also is available in `response.Request.URL`. In short; - The original implementation tried to (pre-maturely) abstract the underlying response to provide a simplified interface. - While initially not needed, abstracting caused relevant information from the response (and request) to be unavailable to callers. - As a result, we ended up in a situation where we are deconstructing the original `url.Response`, only to re-assemble it into our own, custom struct (`serverResponsee`) with only a subset of the information preserved. This patch removes the `serverResponse` struct, instead returning the `url.Response` as-is, so that all information is preserved, allowing callers to use the information they need. There is one follow-up change to consider; commit [moby@589df17] introduced a `ensureReaderClosed` utility. Before that commit, the response body would be closed in a more idiomatic way through a [`defer serverResp.body.Close()`][2]. A later change in [docker/engine-api@5dd6452] added an optimization to that utility, draining the response to allow connections to be reused. While skipping that utility (and not draining the response) would not be a critical issue, it may be easy to overlook that utility, and to close the response body in the "idiomatic" way, resulting in a possible performance regression. We need to check if that optimization is still relevant or if later changes in Go itself already take care of this; we should also look if context cancellation is handled correctly for these. If it's still relevant, we could - Wrap the the `url.Response` in a custom struct ("drainCloser") to provide a `Close()` function handling the draining and closing; this would re- introduce a custom type to be returned, so perhaps not what we want. - Wrap the `url.Response.Body` in the response returned (so, calling) `response.Body.Close()` would call the wrapped closer. - Change the signature of `Client.sendRequest()` (and related) to return a `close()` func to handle this; doing so would more strongly encourage callers to close the response body. [1]:1aa7f1392d/commands.go (L1008-L1027)[2]:589df17a1a/api/client/ps.go (L84-L89)[moby@1aa7f13]:1aa7f1392d[moby@a4bcf7e]:a4bcf7e1ac[moby@fdd8d4b]:fdd8d4b7d9[moby@6b2eeaf]:6b2eeaf896[moby@0cdc3b7]:0cdc3b7539[moby@126529c]:126529c6d0[moby@27ef09a]:27ef09a46f[moby@589df17]:589df17a1a[docker/engine-api@5dd6452]:5dd6452d4dSigned-off-by: Sebastiaan van Stijn <github@gone.nl>
123 lines
3.7 KiB
Go
123 lines
3.7 KiB
Go
package client // import "github.com/docker/docker/client"
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"io"
|
|
"net/url"
|
|
|
|
"github.com/docker/docker/api/types/container"
|
|
"github.com/docker/docker/api/types/versions"
|
|
)
|
|
|
|
const containerWaitErrorMsgLimit = 2 * 1024 /* Max: 2KiB */
|
|
|
|
// ContainerWait waits until the specified container is in a certain state
|
|
// indicated by the given condition, either "not-running" (default),
|
|
// "next-exit", or "removed".
|
|
//
|
|
// If this client's API version is before 1.30, condition is ignored and
|
|
// ContainerWait will return immediately with the two channels, as the server
|
|
// will wait as if the condition were "not-running".
|
|
//
|
|
// If this client's API version is at least 1.30, ContainerWait blocks until
|
|
// the request has been acknowledged by the server (with a response header),
|
|
// then returns two channels on which the caller can wait for the exit status
|
|
// of the container or an error if there was a problem either beginning the
|
|
// wait request or in getting the response. This allows the caller to
|
|
// synchronize ContainerWait with other calls, such as specifying a
|
|
// "next-exit" condition before issuing a ContainerStart request.
|
|
func (cli *Client) ContainerWait(ctx context.Context, containerID string, condition container.WaitCondition) (<-chan container.WaitResponse, <-chan error) {
|
|
resultC := make(chan container.WaitResponse)
|
|
errC := make(chan error, 1)
|
|
|
|
containerID, err := trimID("container", containerID)
|
|
if err != nil {
|
|
errC <- err
|
|
return resultC, errC
|
|
}
|
|
|
|
// Make sure we negotiated (if the client is configured to do so),
|
|
// as code below contains API-version specific handling of options.
|
|
//
|
|
// Normally, version-negotiation (if enabled) would not happen until
|
|
// the API request is made.
|
|
if err := cli.checkVersion(ctx); err != nil {
|
|
errC <- err
|
|
return resultC, errC
|
|
}
|
|
if versions.LessThan(cli.ClientVersion(), "1.30") {
|
|
return cli.legacyContainerWait(ctx, containerID)
|
|
}
|
|
|
|
query := url.Values{}
|
|
if condition != "" {
|
|
query.Set("condition", string(condition))
|
|
}
|
|
|
|
resp, err := cli.post(ctx, "/containers/"+containerID+"/wait", query, nil, nil)
|
|
if err != nil {
|
|
defer ensureReaderClosed(resp)
|
|
errC <- err
|
|
return resultC, errC
|
|
}
|
|
|
|
go func() {
|
|
defer ensureReaderClosed(resp)
|
|
|
|
responseText := bytes.NewBuffer(nil)
|
|
stream := io.TeeReader(resp.Body, responseText)
|
|
|
|
var res container.WaitResponse
|
|
if err := json.NewDecoder(stream).Decode(&res); err != nil {
|
|
// NOTE(nicks): The /wait API does not work well with HTTP proxies.
|
|
// At any time, the proxy could cut off the response stream.
|
|
//
|
|
// But because the HTTP status has already been written, the proxy's
|
|
// only option is to write a plaintext error message.
|
|
//
|
|
// If there's a JSON parsing error, read the real error message
|
|
// off the body and send it to the client.
|
|
if errors.As(err, new(*json.SyntaxError)) {
|
|
_, _ = io.ReadAll(io.LimitReader(stream, containerWaitErrorMsgLimit))
|
|
errC <- errors.New(responseText.String())
|
|
} else {
|
|
errC <- err
|
|
}
|
|
return
|
|
}
|
|
|
|
resultC <- res
|
|
}()
|
|
|
|
return resultC, errC
|
|
}
|
|
|
|
// legacyContainerWait returns immediately and doesn't have an option to wait
|
|
// until the container is removed.
|
|
func (cli *Client) legacyContainerWait(ctx context.Context, containerID string) (<-chan container.WaitResponse, <-chan error) {
|
|
resultC := make(chan container.WaitResponse)
|
|
errC := make(chan error)
|
|
|
|
go func() {
|
|
resp, err := cli.post(ctx, "/containers/"+containerID+"/wait", nil, nil, nil)
|
|
if err != nil {
|
|
errC <- err
|
|
return
|
|
}
|
|
defer ensureReaderClosed(resp)
|
|
|
|
var res container.WaitResponse
|
|
if err := json.NewDecoder(resp.Body).Decode(&res); err != nil {
|
|
errC <- err
|
|
return
|
|
}
|
|
|
|
resultC <- res
|
|
}()
|
|
|
|
return resultC, errC
|
|
}
|