Skip to content

Commit d362cbb

Browse files
CopilotJunkern
Copilot
and
Junkern
authored
Fix docker_container.container_logs to return demultiplexed log content (#899)
* Initial plan * fix(container): strip docker log headers Agent-Logs-Url: https://github.com/kreuzwerker/terraform-provider-docker/sessions/95ec3303-bdee-4023-900b-ee2b1f1f034f Co-authored-by: Junkern <3775779+Junkern@users.noreply.github.com> * fix(container): return log read errors Agent-Logs-Url: https://github.com/kreuzwerker/terraform-provider-docker/sessions/95ec3303-bdee-4023-900b-ee2b1f1f034f Co-authored-by: Junkern <3775779+Junkern@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Junkern <3775779+Junkern@users.noreply.github.com> Co-authored-by: Martin <Junkern@users.noreply.github.com>
1 parent 23a8c92 commit d362cbb

3 files changed

Lines changed: 61 additions & 19 deletions

File tree

‎internal/provider/resource_docker_container_funcs.go‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ package provider
22

33
import (
44
"archive/tar"
5-
"bufio"
65
"bytes"
76
"context"
87
"encoding/base64"
98
"encoding/json"
109
"errors"
1110
"fmt"
11+
"io"
1212
"log"
1313
"os"
1414
"strconv"
@@ -22,6 +22,7 @@ import (
2222
"github.com/docker/docker/api/types/mount"
2323
"github.com/docker/docker/api/types/network"
2424
"github.com/docker/docker/client"
25+
"github.com/docker/docker/pkg/stdcopy"
2526
"github.com/docker/go-connections/nat"
2627
"github.com/docker/go-units"
2728
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -629,32 +630,27 @@ func resourceDockerContainerCreate(ctx context.Context, d *schema.ResourceData,
629630

630631
if d.Get("attach").(bool) {
631632
var b bytes.Buffer
632-
logsRead := make(chan bool)
633+
logsDone := make(chan error, 1)
633634
if d.Get("logs").(bool) {
634635
go func() {
635-
defer func() { logsRead <- true }()
636636
reader, err := client.ContainerLogs(ctx, retContainer.ID, container.LogsOptions{
637637
ShowStdout: true,
638638
ShowStderr: true,
639639
Follow: true,
640640
Timestamps: false,
641641
})
642642
if err != nil {
643-
log.Panic(err)
643+
logsDone <- fmt.Errorf("unable to read container logs: %w", err)
644+
return
644645
}
645646
defer reader.Close() //nolint:errcheck
646647

647-
scanner := bufio.NewScanner(reader)
648-
for scanner.Scan() {
649-
line := scanner.Text()
650-
b.WriteString(line)
651-
b.WriteString("\n")
652-
653-
log.Printf("[DEBUG] container logs: %s", line)
654-
}
655-
if err := scanner.Err(); err != nil {
656-
log.Fatal(err)
648+
if err := copyContainerLogs(&b, reader, d.Get("tty").(bool)); err != nil {
649+
logsDone <- fmt.Errorf("unable to copy container logs: %w", err)
650+
return
657651
}
652+
653+
logsDone <- nil
658654
}()
659655
}
660656

@@ -666,10 +662,9 @@ func resourceDockerContainerCreate(ctx context.Context, d *schema.ResourceData,
666662
}
667663
case <-attachCh:
668664
if d.Get("logs").(bool) {
669-
// There is a race condition here.
670-
// If the goroutine does not finish writing into the buffer by this line, we will have no logs.
671-
// Thus, waiting for the goroutine to finish
672-
<-logsRead
665+
if err := <-logsDone; err != nil {
666+
return diag.FromErr(err)
667+
}
673668
d.Set("container_logs", b.String())
674669
}
675670
}
@@ -678,6 +673,16 @@ func resourceDockerContainerCreate(ctx context.Context, d *schema.ResourceData,
678673
return resourceDockerContainerRead(ctx, d, meta)
679674
}
680675

676+
func copyContainerLogs(dst io.Writer, reader io.Reader, tty bool) error {
677+
if tty {
678+
_, err := io.Copy(dst, reader)
679+
return err
680+
}
681+
682+
_, err := stdcopy.StdCopy(dst, dst, reader)
683+
return err
684+
}
685+
681686
// parseSystemPaths checks if `systempaths=unconfined` security option is set,
682687
// and returns the `MaskedPaths` and `ReadonlyPaths` accordingly. An updated
683688
// list of security options is returned with this option removed, because the

‎internal/provider/resource_docker_container_funcs_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package provider
22

33
import (
4+
"bytes"
45
"context"
56
"testing"
67

8+
"github.com/docker/docker/pkg/stdcopy"
79
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
810
)
911

@@ -32,3 +34,38 @@ func TestDockerContainer_DeleteMissingContainer(t *testing.T) {
3234
t.Fatalf("expected no error deleting missing container, got: %v", diags)
3335
}
3436
}
37+
38+
func TestCopyContainerLogs_Demultiplex(t *testing.T) {
39+
var input bytes.Buffer
40+
stdoutWriter := stdcopy.NewStdWriter(&input, stdcopy.Stdout)
41+
stderrWriter := stdcopy.NewStdWriter(&input, stdcopy.Stderr)
42+
43+
if _, err := stdoutWriter.Write([]byte("stdout\n")); err != nil {
44+
t.Fatalf("failed to write stdout log frame: %v", err)
45+
}
46+
if _, err := stderrWriter.Write([]byte("stderr\n")); err != nil {
47+
t.Fatalf("failed to write stderr log frame: %v", err)
48+
}
49+
50+
var output bytes.Buffer
51+
if err := copyContainerLogs(&output, &input, false); err != nil {
52+
t.Fatalf("expected logs to be copied, got error: %v", err)
53+
}
54+
55+
if got, want := output.String(), "stdout\nstderr\n"; got != want {
56+
t.Fatalf("unexpected logs output: got %q, want %q", got, want)
57+
}
58+
}
59+
60+
func TestCopyContainerLogs_TTY(t *testing.T) {
61+
input := bytes.NewBufferString("tty-output\n")
62+
var output bytes.Buffer
63+
64+
if err := copyContainerLogs(&output, input, true); err != nil {
65+
t.Fatalf("expected logs to be copied, got error: %v", err)
66+
}
67+
68+
if got, want := output.String(), "tty-output\n"; got != want {
69+
t.Fatalf("unexpected logs output: got %q, want %q", got, want)
70+
}
71+
}

‎internal/provider/resource_docker_container_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1541,7 +1541,7 @@ func TestAccDockerContainer_logs(t *testing.T) {
15411541
resource.TestCheckResourceAttr("docker_container.foo", "attach", "true"),
15421542
resource.TestCheckResourceAttr("docker_container.foo", "logs", "true"),
15431543
resource.TestCheckResourceAttr("docker_container.foo", "must_run", "false"),
1544-
resource.TestCheckResourceAttr("docker_container.foo", "container_logs", "\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u00021\n\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u00022\n\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u00023\n\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u00024\n\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u00025\n\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u00026\n\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u00027\n\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u00028\n\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u00029\n\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u000310\n"),
1544+
resource.TestCheckResourceAttr("docker_container.foo", "container_logs", "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"),
15451545
),
15461546
},
15471547
},

0 commit comments

Comments
 (0)