From cc9969bfed84523a6a8370d7ad85422eb0d4c00c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 30 Oct 2025 10:08:51 +0100 Subject: [PATCH] client: Client.ServiceLogs: close reader on context cancellation Use a cancelReadCloser to automatically close the reader when the context is cancelled. Consumers are still recommended to manually close the reader, but the cancelReadCloser makes the Close idempotent. Signed-off-by: Sebastiaan van Stijn --- client/service_logs.go | 27 +++++++------------ integration/service/create_test.go | 12 ++++----- .../moby/moby/client/service_logs.go | 27 +++++++------------ 3 files changed, 24 insertions(+), 42 deletions(-) diff --git a/client/service_logs.go b/client/service_logs.go index ab19adc4aa..fd565db5a0 100644 --- a/client/service_logs.go +++ b/client/service_logs.go @@ -29,8 +29,14 @@ type ServiceLogsResult interface { io.ReadCloser } -// ServiceLogs returns the logs generated by a service in an [ServiceLogsResult]. +// ServiceLogs returns the logs generated by a service in a [ServiceLogsResult]. +// as an [io.ReadCloser]. Callers should close the stream. +// +// The underlying [io.ReadCloser] is automatically closed if the context is canceled, func (cli *Client) ServiceLogs(ctx context.Context, serviceID string, options ServiceLogsOptions) (ServiceLogsResult, error) { + // TODO(thaJeztah): this function needs documentation about the format of ths stream (similar to for container logs) + // TODO(thaJeztah): migrate CLI utilities to the client where suitable; https://github.com/docker/cli/blob/v29.0.0-rc.1/cli/command/service/logs.go#L73-L348 + serviceID, err := trimID("service", serviceID) if err != nil { return nil, err @@ -71,30 +77,15 @@ func (cli *Client) ServiceLogs(ctx context.Context, serviceID string, options Se return nil, err } return &serviceLogsResult{ - body: resp.Body, + ReadCloser: newCancelReadCloser(ctx, resp.Body), }, nil } type serviceLogsResult struct { - // body must be closed to avoid a resource leak - body io.ReadCloser + io.ReadCloser } var ( _ io.ReadCloser = (*serviceLogsResult)(nil) _ ServiceLogsResult = (*serviceLogsResult)(nil) ) - -func (r *serviceLogsResult) Read(p []byte) (int, error) { - if r == nil || r.body == nil { - return 0, io.EOF - } - return r.body.Read(p) -} - -func (r *serviceLogsResult) Close() error { - if r == nil || r.body == nil { - return nil - } - return r.body.Close() -} diff --git a/integration/service/create_test.go b/integration/service/create_test.go index faf213a4f1..a8d0eb06c4 100644 --- a/integration/service/create_test.go +++ b/integration/service/create_test.go @@ -232,14 +232,14 @@ func TestCreateServiceSecretFileMode(t *testing.T) { poll.WaitOn(t, swarm.RunningTasksCount(ctx, apiClient, serviceID, instances), swarm.ServicePoll) - body, err := apiClient.ServiceLogs(ctx, serviceID, client.ServiceLogsOptions{ + res, err := apiClient.ServiceLogs(ctx, serviceID, client.ServiceLogsOptions{ Tail: "1", ShowStdout: true, }) assert.NilError(t, err) - defer body.Close() + defer func() { _ = res.Close() }() - content, err := io.ReadAll(body) + content, err := io.ReadAll(res) assert.NilError(t, err) assert.Check(t, is.Contains(string(content), "-rwxrwxrwx")) @@ -291,14 +291,14 @@ func TestCreateServiceConfigFileMode(t *testing.T) { poll.WaitOn(t, swarm.RunningTasksCount(ctx, apiClient, serviceID, instances)) - body, err := apiClient.ServiceLogs(ctx, serviceID, client.ServiceLogsOptions{ + res, err := apiClient.ServiceLogs(ctx, serviceID, client.ServiceLogsOptions{ Tail: "1", ShowStdout: true, }) assert.NilError(t, err) - defer body.Close() + defer func() { _ = res.Close() }() - content, err := io.ReadAll(body) + content, err := io.ReadAll(res) assert.NilError(t, err) assert.Check(t, is.Contains(string(content), "-rwxrwxrwx")) diff --git a/vendor/github.com/moby/moby/client/service_logs.go b/vendor/github.com/moby/moby/client/service_logs.go index ab19adc4aa..fd565db5a0 100644 --- a/vendor/github.com/moby/moby/client/service_logs.go +++ b/vendor/github.com/moby/moby/client/service_logs.go @@ -29,8 +29,14 @@ type ServiceLogsResult interface { io.ReadCloser } -// ServiceLogs returns the logs generated by a service in an [ServiceLogsResult]. +// ServiceLogs returns the logs generated by a service in a [ServiceLogsResult]. +// as an [io.ReadCloser]. Callers should close the stream. +// +// The underlying [io.ReadCloser] is automatically closed if the context is canceled, func (cli *Client) ServiceLogs(ctx context.Context, serviceID string, options ServiceLogsOptions) (ServiceLogsResult, error) { + // TODO(thaJeztah): this function needs documentation about the format of ths stream (similar to for container logs) + // TODO(thaJeztah): migrate CLI utilities to the client where suitable; https://github.com/docker/cli/blob/v29.0.0-rc.1/cli/command/service/logs.go#L73-L348 + serviceID, err := trimID("service", serviceID) if err != nil { return nil, err @@ -71,30 +77,15 @@ func (cli *Client) ServiceLogs(ctx context.Context, serviceID string, options Se return nil, err } return &serviceLogsResult{ - body: resp.Body, + ReadCloser: newCancelReadCloser(ctx, resp.Body), }, nil } type serviceLogsResult struct { - // body must be closed to avoid a resource leak - body io.ReadCloser + io.ReadCloser } var ( _ io.ReadCloser = (*serviceLogsResult)(nil) _ ServiceLogsResult = (*serviceLogsResult)(nil) ) - -func (r *serviceLogsResult) Read(p []byte) (int, error) { - if r == nil || r.body == nil { - return 0, io.EOF - } - return r.body.Read(p) -} - -func (r *serviceLogsResult) Close() error { - if r == nil || r.body == nil { - return nil - } - return r.body.Close() -}