Skip to content

Attempt to deflake several tests - #5218

Merged
AkihiroSuda merged 5 commits into
containerd:mainfrom
AkihiroSuda:flaky
Sep 17, 2026
Merged

AkihiroSuda merged 5 commits into
containerd:mainfrom
AkihiroSuda:flaky

Conversation

@AkihiroSuda

Copy link
Copy Markdown
Member

Attempt to deflake:

  • TestSave, TestLoadStdinFromPipe, and TestBuildContextWithOCILayout
  • TestIPFSCompNoBuild
  • TestRunWithLogBinary
  • TestAttachForAutoRemovedContainer
  • TestComposeStart

Relates to:

@AkihiroSuda AkihiroSuda added the area/ci e.g., CI failure label Sep 17, 2026
@AkihiroSuda AkihiroSuda added this to the v2.4.0 milestone Sep 17, 2026
`nerdctl save` occasionally wrote a truncated - sometimes empty - archive, which
then blew up at the other end:

    failed to ingest "blobs/sha256/ca3cd4...": short read: expected 2811947 bytes
    but got 2785280: unexpected EOF
    untar manifest "manifest.json": unexpected EOF
    tar: Unexpected EOF in archive
    unrecognized image format

The archive comes back from the transfer service over a stream, and
`ImageExportStream.MarshalAny` copies that stream into our writer from a
goroutine of its own. `client.Transfer` returns when the daemon is done, not
when that goroutine has drained what is still in flight, so `saveAction` would
close the output file - or the process would simply exit - with the tail of the
archive still unwritten. A quiet machine hides this; a busy CI runner does not.

That goroutine closes the writer once the copy is over, which is the only signal
available, so hand the export stream a writer that reports being closed and wait
for it.

Measured against containerd v2.4.0-beta.0, with a writer slow enough to lose the
race every time: `Transfer` returns after 2621440 of 2723840 bytes, and the
remaining 102400 arrive afterwards.

Fixes the `TestSave`, `TestLoadStdinFromPipe` and `TestBuildContextWithOCILayout`
entries of the flaky test dashboard (issue 5202): all three save an image and
read the archive back.

Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
TestIPFSCompNoBuild kept failing to get a kubo registry up:

    kubo.go:78: assertion failed: error is not nil: error after 5 attempts:
    Get "http://10.1.0.151:5001/api/v0": context deadline exceeded

with, in the container logs:

    Error: lock /data/ipfs/repo.lock: someone else has the lock

The kubo image declares a HEALTHCHECK, `ipfs dag stat /ipfs/Qm...`, and
`healthcheck.StartTimer` runs the first probe right away, when the container
starts - which is exactly when the container command is running `ipfs init &&
ipfs config && ipfs daemon`. The probe and those commands both open the IPFS
repo, only one of them gets /data/ipfs/repo.lock, and whichever loses exits
non-zero. When that is `ipfs config`, the && chain never reaches `ipfs daemon`
and the API is never served, so the test polls a port that will never answer.

Reproduced, outside of any test, by running the probe command concurrently with
`ipfs config` in that image: the same error, every couple of iterations.

The probe is of no use to us - the helper already waits for the API to answer
before handing the registry over - so turn it off.

Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
TestRunWithLogBinary failed with:

    container_run_test.go:627: assertion failed: expression is false:
    strings.Contains(log, "foo")

The test starts `run -d ... sh -euxc "echo foo; echo bar"` with a
`binary://` log driver, and reads the file that the log binary writes as soon
as `run -d` hands the container id back. At that point the container has not
necessarily said anything yet, and the log binary - a process of its own,
draining the container's stdout pipe on its own schedule - has possibly not even
been scheduled. The file exists, because the binary creates it on startup, so
the read succeeds and returns nothing.

Wait for the container to be over, then for the binary to have written down what
it said.

Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
TestAttachForAutoRemovedContainer failed with:

    container_attach_linux_test.go:207: assertion failed:
    strings.Contains(helpers.Capture("ps", "-a"), data.Identifier()) is true

and, right after, in the cleanup:

    Error response from daemon: removal of container
    testattachforautoremovedcontainer-23f097dc is already in progress

The container is started with `--rm`, and the test asserts that it is gone from
`ps -a` the moment the `attach` it was running under returns. Removal is not
done at that point - it is triggered by the container process being over and
proceeds on its own - so `ps -a` may still list it, as the daemon itself said.

Give it the same treatment as the other container states we wait on, with a new
EnsureContainerRemoved next to EnsureContainerStarted and EnsureContainerExited.

Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
TestComposeStart failed with one of the two services missing from the listing
entirely:

    🖊️ Inspecting output (match)
    👀 testing:  `NAME      IMAGE     COMMAND   SERVICE   CREATED   STATUS    PORTS
    `
    ❌ FAILED!   `Up|running`

while the other one was reported as `Up Less than a second`, which says how tight
this was. `compose start` returns once it has asked for the containers to be
started, and `compose ps` only lists the ones that are actually running, so the
service that is slowest to come up is simply not there yet.

Wait for both containers to be running before asking for the listing. That needs
the project name to be known rather than derived from the temp directory, so pin
it with -p, as the compose down tests already do.

Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda
AkihiroSuda marked this pull request as ready for review September 17, 2026 13:37
@AkihiroSuda

Copy link
Copy Markdown
Member Author

CI all green, without restarting 🎉

@haytok haytok left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

@AkihiroSuda
AkihiroSuda merged commit e5d5ba6 into containerd:main Sep 17, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci e.g., CI failure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants