Browse code

Merge pull request #51317 from austinvazquez/refactor-client-container-logs

client: refactor `ContainerLogs` to wrap result

Austin Vazquez authored on 2025/10/29 19:06:22
Showing 5 changed files
... ...
@@ -63,7 +63,7 @@ type ContainerAPIClient interface {
63 63
 	ContainerInspect(ctx context.Context, container string, options ContainerInspectOptions) (ContainerInspectResult, error)
64 64
 	ContainerKill(ctx context.Context, container string, options ContainerKillOptions) (ContainerKillResult, error)
65 65
 	ContainerList(ctx context.Context, options ContainerListOptions) (ContainerListResult, error)
66
-	ContainerLogs(ctx context.Context, container string, options ContainerLogsOptions) (io.ReadCloser, error)
66
+	ContainerLogs(ctx context.Context, container string, options ContainerLogsOptions) (ContainerLogsResult, error)
67 67
 	ContainerPause(ctx context.Context, container string, options ContainerPauseOptions) (ContainerPauseResult, error)
68 68
 	ContainerRemove(ctx context.Context, container string, options ContainerRemoveOptions) (ContainerRemoveResult, error)
69 69
 	ContainerRename(ctx context.Context, container, newContainerName string) error
... ...
@@ -5,6 +5,7 @@ import (
5 5
 	"fmt"
6 6
 	"io"
7 7
 	"net/url"
8
+	"sync"
8 9
 	"time"
9 10
 
10 11
 	"github.com/moby/moby/client/internal/timestamp"
... ...
@@ -22,6 +23,12 @@ type ContainerLogsOptions struct {
22 22
 	Details    bool
23 23
 }
24 24
 
25
+// ContainerLogsResult is the result of a container logs operation.
26
+type ContainerLogsResult struct {
27
+	rc    io.ReadCloser
28
+	close func() error
29
+}
30
+
25 31
 // ContainerLogs returns the logs generated by a container in an [io.ReadCloser].
26 32
 // It's up to the caller to close the stream.
27 33
 //
... ...
@@ -48,10 +55,10 @@ type ContainerLogsOptions struct {
48 48
 // [stdcopy.StdType]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#StdType
49 49
 // [Stdout]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#Stdout
50 50
 // [Stderr]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#Stderr
51
-func (cli *Client) ContainerLogs(ctx context.Context, containerID string, options ContainerLogsOptions) (io.ReadCloser, error) {
51
+func (cli *Client) ContainerLogs(ctx context.Context, containerID string, options ContainerLogsOptions) (ContainerLogsResult, error) {
52 52
 	containerID, err := trimID("container", containerID)
53 53
 	if err != nil {
54
-		return nil, err
54
+		return ContainerLogsResult{}, err
55 55
 	}
56 56
 
57 57
 	query := url.Values{}
... ...
@@ -66,7 +73,7 @@ func (cli *Client) ContainerLogs(ctx context.Context, containerID string, option
66 66
 	if options.Since != "" {
67 67
 		ts, err := timestamp.GetTimestamp(options.Since, time.Now())
68 68
 		if err != nil {
69
-			return nil, fmt.Errorf(`invalid value for "since": %w`, err)
69
+			return ContainerLogsResult{}, fmt.Errorf(`invalid value for "since": %w`, err)
70 70
 		}
71 71
 		query.Set("since", ts)
72 72
 	}
... ...
@@ -74,7 +81,7 @@ func (cli *Client) ContainerLogs(ctx context.Context, containerID string, option
74 74
 	if options.Until != "" {
75 75
 		ts, err := timestamp.GetTimestamp(options.Until, time.Now())
76 76
 		if err != nil {
77
-			return nil, fmt.Errorf(`invalid value for "until": %w`, err)
77
+			return ContainerLogsResult{}, fmt.Errorf(`invalid value for "until": %w`, err)
78 78
 		}
79 79
 		query.Set("until", ts)
80 80
 	}
... ...
@@ -94,7 +101,33 @@ func (cli *Client) ContainerLogs(ctx context.Context, containerID string, option
94 94
 
95 95
 	resp, err := cli.get(ctx, "/containers/"+containerID+"/logs", query, nil)
96 96
 	if err != nil {
97
-		return nil, err
97
+		return ContainerLogsResult{}, err
98
+	}
99
+	return newContainerLogsResult(resp.Body), nil
100
+}
101
+
102
+func newContainerLogsResult(rc io.ReadCloser) ContainerLogsResult {
103
+	if rc == nil {
104
+		panic("rc cannot be nil")
105
+	}
106
+	return ContainerLogsResult{
107
+		rc:    rc,
108
+		close: sync.OnceValue(rc.Close),
109
+	}
110
+}
111
+
112
+// Read implements the io.Reader interface.
113
+func (r ContainerLogsResult) Read(p []byte) (n int, err error) {
114
+	if r.rc == nil {
115
+		return 0, io.EOF
116
+	}
117
+	return r.rc.Read(p)
118
+}
119
+
120
+// Close closes the underlying reader.
121
+func (r ContainerLogsResult) Close() error {
122
+	if r.close == nil {
123
+		return nil
98 124
 	}
99
-	return resp.Body, nil
125
+	return r.close()
100 126
 }
... ...
@@ -30,10 +30,8 @@ func TestLogsFollowTailEmpty(t *testing.T) {
30 30
 	id := container.Run(ctx, t, apiClient, container.WithCmd("sleep", "100000"))
31 31
 
32 32
 	logs, err := apiClient.ContainerLogs(ctx, id, client.ContainerLogsOptions{ShowStdout: true, Tail: "2"})
33
-	if logs != nil {
34
-		defer logs.Close()
35
-	}
36 33
 	assert.Check(t, err)
34
+	defer logs.Close()
37 35
 
38 36
 	_, err = stdcopy.StdCopy(io.Discard, io.Discard, logs)
39 37
 	assert.Check(t, err)
... ...
@@ -63,7 +63,7 @@ type ContainerAPIClient interface {
63 63
 	ContainerInspect(ctx context.Context, container string, options ContainerInspectOptions) (ContainerInspectResult, error)
64 64
 	ContainerKill(ctx context.Context, container string, options ContainerKillOptions) (ContainerKillResult, error)
65 65
 	ContainerList(ctx context.Context, options ContainerListOptions) (ContainerListResult, error)
66
-	ContainerLogs(ctx context.Context, container string, options ContainerLogsOptions) (io.ReadCloser, error)
66
+	ContainerLogs(ctx context.Context, container string, options ContainerLogsOptions) (ContainerLogsResult, error)
67 67
 	ContainerPause(ctx context.Context, container string, options ContainerPauseOptions) (ContainerPauseResult, error)
68 68
 	ContainerRemove(ctx context.Context, container string, options ContainerRemoveOptions) (ContainerRemoveResult, error)
69 69
 	ContainerRename(ctx context.Context, container, newContainerName string) error
... ...
@@ -5,6 +5,7 @@ import (
5 5
 	"fmt"
6 6
 	"io"
7 7
 	"net/url"
8
+	"sync"
8 9
 	"time"
9 10
 
10 11
 	"github.com/moby/moby/client/internal/timestamp"
... ...
@@ -22,6 +23,12 @@ type ContainerLogsOptions struct {
22 22
 	Details    bool
23 23
 }
24 24
 
25
+// ContainerLogsResult is the result of a container logs operation.
26
+type ContainerLogsResult struct {
27
+	rc    io.ReadCloser
28
+	close func() error
29
+}
30
+
25 31
 // ContainerLogs returns the logs generated by a container in an [io.ReadCloser].
26 32
 // It's up to the caller to close the stream.
27 33
 //
... ...
@@ -48,10 +55,10 @@ type ContainerLogsOptions struct {
48 48
 // [stdcopy.StdType]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#StdType
49 49
 // [Stdout]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#Stdout
50 50
 // [Stderr]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#Stderr
51
-func (cli *Client) ContainerLogs(ctx context.Context, containerID string, options ContainerLogsOptions) (io.ReadCloser, error) {
51
+func (cli *Client) ContainerLogs(ctx context.Context, containerID string, options ContainerLogsOptions) (ContainerLogsResult, error) {
52 52
 	containerID, err := trimID("container", containerID)
53 53
 	if err != nil {
54
-		return nil, err
54
+		return ContainerLogsResult{}, err
55 55
 	}
56 56
 
57 57
 	query := url.Values{}
... ...
@@ -66,7 +73,7 @@ func (cli *Client) ContainerLogs(ctx context.Context, containerID string, option
66 66
 	if options.Since != "" {
67 67
 		ts, err := timestamp.GetTimestamp(options.Since, time.Now())
68 68
 		if err != nil {
69
-			return nil, fmt.Errorf(`invalid value for "since": %w`, err)
69
+			return ContainerLogsResult{}, fmt.Errorf(`invalid value for "since": %w`, err)
70 70
 		}
71 71
 		query.Set("since", ts)
72 72
 	}
... ...
@@ -74,7 +81,7 @@ func (cli *Client) ContainerLogs(ctx context.Context, containerID string, option
74 74
 	if options.Until != "" {
75 75
 		ts, err := timestamp.GetTimestamp(options.Until, time.Now())
76 76
 		if err != nil {
77
-			return nil, fmt.Errorf(`invalid value for "until": %w`, err)
77
+			return ContainerLogsResult{}, fmt.Errorf(`invalid value for "until": %w`, err)
78 78
 		}
79 79
 		query.Set("until", ts)
80 80
 	}
... ...
@@ -94,7 +101,33 @@ func (cli *Client) ContainerLogs(ctx context.Context, containerID string, option
94 94
 
95 95
 	resp, err := cli.get(ctx, "/containers/"+containerID+"/logs", query, nil)
96 96
 	if err != nil {
97
-		return nil, err
97
+		return ContainerLogsResult{}, err
98
+	}
99
+	return newContainerLogsResult(resp.Body), nil
100
+}
101
+
102
+func newContainerLogsResult(rc io.ReadCloser) ContainerLogsResult {
103
+	if rc == nil {
104
+		panic("rc cannot be nil")
105
+	}
106
+	return ContainerLogsResult{
107
+		rc:    rc,
108
+		close: sync.OnceValue(rc.Close),
109
+	}
110
+}
111
+
112
+// Read implements the io.Reader interface.
113
+func (r ContainerLogsResult) Read(p []byte) (n int, err error) {
114
+	if r.rc == nil {
115
+		return 0, io.EOF
116
+	}
117
+	return r.rc.Read(p)
118
+}
119
+
120
+// Close closes the underlying reader.
121
+func (r ContainerLogsResult) Close() error {
122
+	if r.close == nil {
123
+		return nil
98 124
 	}
99
-	return resp.Body, nil
125
+	return r.close()
100 126
 }