fix: logs updated on starting of a stopped container

[Shubhranshu153]

Issue:

When we start a container that was running and stopped or exited, logs are not updated.

Steps to Reproduce:

step 1: Run and exit container

sudo nerdctl run -it --name test-1 alpine:latest
/ # echo hello
hello
/ # exit

2. start the same container

 sudo nerdctl start -a test-1
/ # echo bar
bar
/ # exit

3. Check logs. Bar is missing.

 sudo nerdctl logs test-1
/ # echo hello
hello
/ # exit

fix:

The pr fixes it by adding multiwriters to start of containers.

[Shubhranshu153]

@AkihiroSuda
PTAL when you get a chance.
Thank You.

[apostasie]

Docker failure is #3908

Rootful failure is #3513

[apostasie]

@Shubhranshu153 I just have a batch of nits on the test.

Btw, feedback on the test framework is highly welcome.

Ideally, if we get #3591 in first, we can get rid of the added DelayOnceReaders here (and unbuffer).

But anyhow, don't sweat it - I can clean that up afterwards if needed.

[apostasie]

Two last small nits.

@AkihiroSuda suggesting we merge this here before #3890 and I will take care of the rebase over there instead of putting @Shubhranshu153 through another round of test tweaks.

[Shubhranshu153]

@apostasie can we rerun this failed test, seems unrelated to the change.

[apostasie]

@apostasie can we rerun this failed test, seems unrelated to the change.

I don't have rights to do that.
Suggesting you slightly amend your commit message and then force push to trigger a full rebuild.
Alternatively @AkihiroSuda or another maintainer could poke the CI and just restart the failed ones.

About the failures:

• docker one is CI: tests sending signals to be caught by in-container shell scripts are racy #3908 (tentatively fixed by CI: rewrite signal tests #3918)
• rootful 1.7 is content digest not found #3513

The last one, jury is out.
Either it is a side-effect of what you do here, or it's on me and I will have to eat my hat first then fix it...

[apostasie]

About the last failure, suggesting you try this on your local environment:

while true; do gotestsum --debug "./cmd/nerdctl/container" -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY; sleep 1; done

Give it a few minutes.

Let me know if it flakes out with / without your patch.

[AkihiroSuda]

Suggesting you slightly amend your commit message and then force push to trigger a full rebuild.
Alternatively @AkihiroSuda or another maintainer could poke the CI and just restart the failed ones.

Alternatively you can also just close and reopen the PR to trigger a full rebuild

[Shubhranshu153]

@apostasie
Locally this test seem to pass

Exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 68282
go: downloading lukechampine.com/blake3 v1.3.0
go: downloading github.com/klauspost/cpuid/v2 v2.2.8
✓  cmd/nerdctl/container (2.767s)

DONE 5 tests in 96.857s
exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 79840
✓  cmd/nerdctl/container (1.629s)

DONE 5 tests in 5.596s
exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 80389
✓  cmd/nerdctl/container (1.074s)

DONE 5 tests in 4.698s
exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 80950
✓  cmd/nerdctl/container (1.823s)

DONE 5 tests in 4.764s
exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 81504
✓  cmd/nerdctl/container (1.733s)

DONE 5 tests in 4.969s
exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 82042
✓  cmd/nerdctl/container (1.166s)

DONE 5 tests in 5.910s
exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 82594
✓  cmd/nerdctl/container (1.042s)

DONE 5 tests in 4.549s
exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 83139
✓  cmd/nerdctl/container (1.038s)

DONE 5 tests in 4.683s
exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 83704
✓  cmd/nerdctl/container (1.206s)

DONE 5 tests in 4.603s
exec: [go test -json ./cmd/nerdctl/container -args -test.only-flaky=false -test.count 1 -test.run TestExecTTY]
go test pid: 84260
✓  cmd/nerdctl/container (1.25s)

DONE 5 tests in 4.535s

[Shubhranshu153]

Thanks the retry of the test passed.
@fahedouch @AkihiroSuda PTAL.

[AkihiroSuda]

Could you add a comment to explain the reason

[apostasie]

@Shubhranshu153 testing framework PR got merged.

Here is the TL;DR for you to update your PR:

• package got moved to:
  • github.com/containerd/nerdctl/mod/tigron/test
  • github.com/containerd/nerdctl/mod/tigron/require
  • github.com/containerd/nerdctl/mod/tigron/expect
• test.Contains should be replaced with expect.Contains
• test.Require is now require.All
• test.Windows is now require.Windows
• test.Not is now require.Not