From fcb924712bffa9adf144d4a488b09796b56b2215 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Feb 2025 16:07:04 +0100 Subject: [PATCH 1/7] integration/internal: JobComplete: require shallower interface Signed-off-by: Sebastiaan van Stijn --- integration/internal/swarm/states.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/internal/swarm/states.go b/integration/internal/swarm/states.go index 0800a75c84e43..4e1822a3ae555 100644 --- a/integration/internal/swarm/states.go +++ b/integration/internal/swarm/states.go @@ -87,7 +87,7 @@ func RunningTasksCount(ctx context.Context, client client.ServiceAPIClient, serv // JobComplete is a poll function for determining that a ReplicatedJob is // completed additionally, while polling, it verifies that the job never // exceeds MaxConcurrent running tasks -func JobComplete(ctx context.Context, client client.CommonAPIClient, service swarmtypes.Service) func(log poll.LogT) poll.Result { +func JobComplete(ctx context.Context, client client.ServiceAPIClient, service swarmtypes.Service) func(log poll.LogT) poll.Result { filter := filters.NewArgs(filters.Arg("service", service.ID)) var jobIteration swarmtypes.Version From b0e206b80711221b81d6537a8c50449e20f66085 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Feb 2025 16:18:41 +0100 Subject: [PATCH 2/7] client: separate Dialer() implementation from public API Signed-off-by: Sebastiaan van Stijn --- client/client.go | 4 ++++ client/hijack.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/client/client.go b/client/client.go index 97dbbf70b928c..d4544f1f1991c 100644 --- a/client/client.go +++ b/client/client.go @@ -449,6 +449,10 @@ func (cli *Client) dialerFromTransport() func(context.Context, string, string) ( // // ["docker dial-stdio"]: https://github.com/docker/cli/pull/1014 func (cli *Client) Dialer() func(context.Context) (net.Conn, error) { + return cli.dialer() +} + +func (cli *Client) dialer() func(context.Context) (net.Conn, error) { return func(ctx context.Context) (net.Conn, error) { if dialFn := cli.dialerFromTransport(); dialFn != nil { return dialFn(ctx, cli.proto, cli.addr) diff --git a/client/hijack.go b/client/hijack.go index 839d4c5cd6baa..2ddf15ea206bb 100644 --- a/client/hijack.go +++ b/client/hijack.go @@ -50,7 +50,7 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (_ net.Conn, req.Header.Set("Connection", "Upgrade") req.Header.Set("Upgrade", proto) - dialer := cli.Dialer() + dialer := cli.dialer() conn, err := dialer(ctx) if err != nil { return nil, "", errors.Wrap(err, "cannot connect to the Docker daemon. Is 'docker daemon' running on this host?") From e6dabfa3b1f213808756213ff32dc9188d8004ae Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Feb 2025 16:24:01 +0100 Subject: [PATCH 3/7] client: move resetting mediaType for hijack to where applicable The mediaType is only used in a single location; reset it in that location. Signed-off-by: Sebastiaan van Stijn --- client/hijack.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/client/hijack.go b/client/hijack.go index 2ddf15ea206bb..64d1c1f1d7084 100644 --- a/client/hijack.go +++ b/client/hijack.go @@ -30,6 +30,11 @@ func (cli *Client) postHijacked(ctx context.Context, path string, query url.Valu return types.HijackedResponse{}, err } + if versions.LessThan(cli.ClientVersion(), "1.42") { + // Prior to 1.42, Content-Type is always set to raw-stream and not relevant + mediaType = "" + } + return types.NewHijackedResponse(conn, mediaType), err } @@ -96,13 +101,7 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (_ net.Conn, hc.r.Reset(nil) } - var mediaType string - if versions.GreaterThanOrEqualTo(cli.ClientVersion(), "1.42") { - // Prior to 1.42, Content-Type is always set to raw-stream and not relevant - mediaType = resp.Header.Get("Content-Type") - } - - return conn, mediaType, nil + return conn, resp.Header.Get("Content-Type"), nil } // hijackedConn wraps a net.Conn and is returned by setupHijackConn in the case From 902c06fdf0d9d4dcb42d34414e34673b8d33d270 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Feb 2025 16:38:32 +0100 Subject: [PATCH 4/7] client: make setupHijackConn a regular function pass the dialer as argument Signed-off-by: Sebastiaan van Stijn --- client/hijack.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/client/hijack.go b/client/hijack.go index 64d1c1f1d7084..2c78fad002d05 100644 --- a/client/hijack.go +++ b/client/hijack.go @@ -25,7 +25,7 @@ func (cli *Client) postHijacked(ctx context.Context, path string, query url.Valu if err != nil { return types.HijackedResponse{}, err } - conn, mediaType, err := cli.setupHijackConn(req, "tcp") + conn, mediaType, err := setupHijackConn(cli.dialer(), req, "tcp") if err != nil { return types.HijackedResponse{}, err } @@ -35,7 +35,7 @@ func (cli *Client) postHijacked(ctx context.Context, path string, query url.Valu mediaType = "" } - return types.NewHijackedResponse(conn, mediaType), err + return types.NewHijackedResponse(conn, mediaType), nil } // DialHijack returns a hijacked connection with negotiated protocol proto. @@ -46,16 +46,15 @@ func (cli *Client) DialHijack(ctx context.Context, url, proto string, meta map[s } req = cli.addHeaders(req, meta) - conn, _, err := cli.setupHijackConn(req, proto) + conn, _, err := setupHijackConn(cli.Dialer(), req, proto) return conn, err } -func (cli *Client) setupHijackConn(req *http.Request, proto string) (_ net.Conn, _ string, retErr error) { +func setupHijackConn(dialer func(context.Context) (net.Conn, error), req *http.Request, proto string) (_ net.Conn, _ string, retErr error) { ctx := req.Context() req.Header.Set("Connection", "Upgrade") req.Header.Set("Upgrade", proto) - dialer := cli.dialer() conn, err := dialer(ctx) if err != nil { return nil, "", errors.Wrap(err, "cannot connect to the Docker daemon. Is 'docker daemon' running on this host?") From a57d737a86ab2c9739909a4fd23d77d492182b26 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Feb 2025 16:44:12 +0100 Subject: [PATCH 5/7] client: define separate interface for HijackDialer Signed-off-by: Sebastiaan van Stijn --- client/buildkit/buildkit.go | 2 +- client/interface.go | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/client/buildkit/buildkit.go b/client/buildkit/buildkit.go index 70961bb26b40b..4c18ca5086fad 100644 --- a/client/buildkit/buildkit.go +++ b/client/buildkit/buildkit.go @@ -15,7 +15,7 @@ import ( // Example: // // bkclient.New(ctx, "", ClientOpts(c)...) -func ClientOpts(c client.CommonAPIClient) []bkclient.ClientOpt { +func ClientOpts(c client.HijackDialer) []bkclient.ClientOpt { return []bkclient.ClientOpt{ bkclient.WithSessionDialer(func(ctx context.Context, proto string, meta map[string][]string) (net.Conn, error) { return c.DialHijack(ctx, "/session", proto, meta) diff --git a/client/interface.go b/client/interface.go index 470923a243d3c..56823d1f5ce3f 100644 --- a/client/interface.go +++ b/client/interface.go @@ -39,11 +39,16 @@ type CommonAPIClient interface { ServerVersion(ctx context.Context) (types.Version, error) NegotiateAPIVersion(ctx context.Context) NegotiateAPIVersionPing(types.Ping) - DialHijack(ctx context.Context, url, proto string, meta map[string][]string) (net.Conn, error) + HijackDialer Dialer() func(context.Context) (net.Conn, error) Close() error } +// HijackDialer defines methods for a hijack dialer. +type HijackDialer interface { + DialHijack(ctx context.Context, url, proto string, meta map[string][]string) (net.Conn, error) +} + // ContainerAPIClient defines API client methods for the containers type ContainerAPIClient interface { ContainerAttach(ctx context.Context, container string, options container.AttachOptions) (types.HijackedResponse, error) From 3725998e7d4144e9af36ce5cd7597045a6501fa2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Feb 2025 16:46:32 +0100 Subject: [PATCH 6/7] client: define interface for all Swarm-specific methods Introduce a SwarmManagementAPIClient interface that captures all swarm-specific methods on the API client. Signed-off-by: Sebastiaan van Stijn --- client/interface.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/client/interface.go b/client/interface.go index 56823d1f5ce3f..1245a1c2b7f39 100644 --- a/client/interface.go +++ b/client/interface.go @@ -25,12 +25,8 @@ type CommonAPIClient interface { ContainerAPIClient DistributionAPIClient ImageAPIClient - NodeAPIClient NetworkAPIClient PluginAPIClient - ServiceAPIClient - SwarmAPIClient - SecretAPIClient SystemAPIClient VolumeAPIClient ClientVersion() string @@ -42,6 +38,17 @@ type CommonAPIClient interface { HijackDialer Dialer() func(context.Context) (net.Conn, error) Close() error + SwarmManagementAPIClient +} + +// SwarmManagementAPIClient defines all methods for managing Swarm-specific +// objects. +type SwarmManagementAPIClient interface { + SwarmAPIClient + NodeAPIClient + ServiceAPIClient + SecretAPIClient + ConfigAPIClient } // HijackDialer defines methods for a hijack dialer. From 2997c0ddc00bee43c18a67fb9e05a8f6069eb1fa Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Feb 2025 15:56:10 +0100 Subject: [PATCH 7/7] client: deprecate CommonAPIClient interface The CommonAPIClient was used to define all the stable interfaces, and combined with the experimental ones through APIClient. In theory, this would allow someone to make sure they only depended on non-experimental methods or to implement an alternative client that only implements the stable methods. While there are users currently using this interface, all those uses depend on the actual client implementation, not a custom one, so they should be able to switch to use APIClient instead. In the meantime, start with deprecating, but keeping the interface the same for now, scheduling it to become an alias, and removed in a future release. Signed-off-by: Sebastiaan van Stijn --- client/{interface_experimental.go => checkpoint.go} | 10 +++++----- client/client.go | 3 +++ client/{interface.go => client_interfaces.go} | 12 +++++++++++- client/interface_stable.go | 10 ---------- 4 files changed, 19 insertions(+), 16 deletions(-) rename client/{interface_experimental.go => checkpoint.go} (76%) rename client/{interface.go => client_interfaces.go} (97%) delete mode 100644 client/interface_stable.go diff --git a/client/interface_experimental.go b/client/checkpoint.go similarity index 76% rename from client/interface_experimental.go rename to client/checkpoint.go index c585c104590fd..f690f7c9524e5 100644 --- a/client/interface_experimental.go +++ b/client/checkpoint.go @@ -6,11 +6,11 @@ import ( "github.com/docker/docker/api/types/checkpoint" ) -type apiClientExperimental interface { - CheckpointAPIClient -} - -// CheckpointAPIClient defines API client methods for the checkpoints +// CheckpointAPIClient defines API client methods for the checkpoints. +// +// Experimental: checkpoint and restore is still an experimental feature, +// and only available if the daemon is running with experimental features +// enabled. type CheckpointAPIClient interface { CheckpointCreate(ctx context.Context, container string, options checkpoint.CreateOptions) error CheckpointDelete(ctx context.Context, container string, options checkpoint.DeleteOptions) error diff --git a/client/client.go b/client/client.go index d4544f1f1991c..c980b66a16aaf 100644 --- a/client/client.go +++ b/client/client.go @@ -99,6 +99,9 @@ const DummyHost = "api.moby.localhost" // recent version before negotiation was introduced. const fallbackAPIVersion = "1.24" +// Ensure that Client always implements APIClient. +var _ APIClient = &Client{} + // Client is the API client that performs all operations // against a docker server. type Client struct { diff --git a/client/interface.go b/client/client_interfaces.go similarity index 97% rename from client/interface.go rename to client/client_interfaces.go index 1245a1c2b7f39..a3ef622eb3e78 100644 --- a/client/interface.go +++ b/client/client_interfaces.go @@ -20,7 +20,17 @@ import ( ) // CommonAPIClient is the common methods between stable and experimental versions of APIClient. -type CommonAPIClient interface { +// +// Deprecated: use [APIClient] instead. This type will be an alias for [APIClient] in the next release, and removed after. +type CommonAPIClient = stableAPIClient + +// APIClient is an interface that clients that talk with a docker server must implement. +type APIClient interface { + stableAPIClient + CheckpointAPIClient // CheckpointAPIClient is still experimental. +} + +type stableAPIClient interface { ConfigAPIClient ContainerAPIClient DistributionAPIClient diff --git a/client/interface_stable.go b/client/interface_stable.go deleted file mode 100644 index 5502cd7426614..0000000000000 --- a/client/interface_stable.go +++ /dev/null @@ -1,10 +0,0 @@ -package client // import "github.com/docker/docker/client" - -// APIClient is an interface that clients that talk with a docker server must implement. -type APIClient interface { - CommonAPIClient - apiClientExperimental -} - -// Ensure that Client always implements APIClient. -var _ APIClient = &Client{}