Skip to content

Commit

Permalink
run/tests: fix flaky RunAttachTermination test
Browse files Browse the repository at this point in the history
During an attached `docker run`, the CLI starts capturing signals so
that they can be forwarded to the container. The CLI stops capturing
signals after container is no longer running/it's streams are closed.

This test had two issues:
- it would close the streams early, causing the CLI to think the
  container had exited, and stop signal handling (causing the SIGINT
  to not be captured and interrupt the test instead), and
- it would send immediately on the status channel returned by WaitFunc,
  which would also signal the container has exited and caused the CLI to
  stop signal handling

This patch addresses both of these issues and makes this test less
flaky.

Signed-off-by: Laura Brehm <[email protected]>
  • Loading branch information
laurazard committed Dec 16, 2024
1 parent a0cd512 commit ad29724
Showing 1 changed file with 24 additions and 5 deletions.
29 changes: 24 additions & 5 deletions cli/command/container/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ func TestRunAttachTermination(t *testing.T) {
var conn net.Conn
killCh := make(chan struct{})
attachCh := make(chan struct{})
startCh := make(chan struct{})
containerExitC := make(chan struct{})
fakeCLI := test.NewFakeCli(&fakeClient{
createContainerFunc: func(_ *container.Config, _ *container.HostConfig, _ *network.NetworkingConfig, _ *specs.Platform, _ string) (container.CreateResponse, error) {
return container.CreateResponse{
Expand All @@ -158,6 +160,7 @@ func TestRunAttachTermination(t *testing.T) {
},
containerKillFunc: func(ctx context.Context, containerID, signal string) error {
killCh <- struct{}{}
containerExitC <- struct{}{}
return nil
},
containerAttachFunc: func(ctx context.Context, containerID string, options container.AttachOptions) (types.HijackedResponse, error) {
Expand All @@ -173,11 +176,19 @@ func TestRunAttachTermination(t *testing.T) {
responseChan := make(chan container.WaitResponse, 1)
errChan := make(chan error)

responseChan <- container.WaitResponse{
StatusCode: 130,
}
go func() {
<-containerExitC
responseChan <- container.WaitResponse{
StatusCode: 130,
}
}()
return responseChan, errChan
},
containerStartFunc: func(containerID string, options container.StartOptions) error {
startCh <- struct{}{}
return nil
},

// use new (non-legacy) wait API
// see: 38591f20d07795aaef45d400df89ca12f29c603b
Version: "1.30",
Expand All @@ -201,16 +212,24 @@ func TestRunAttachTermination(t *testing.T) {
case <-attachCh:
}

// run command should attempt to start the container
select {
case <-time.After(5 * time.Second):
t.Fatal("containerStartCh was not called before the timeout")
case <-startCh:
}

assert.NilError(t, syscall.Kill(syscall.Getpid(), syscall.SIGINT))
// end stream from "container" so that we'll detach
conn.Close()

select {
case <-killCh:
case <-time.After(5 * time.Second):
t.Fatal("containerKillFunc was not called before the timeout")
}

// end stream from "container" so that we'll detach
conn.Close()

select {
case cmdErr := <-cmdErrC:
assert.Equal(t, cmdErr, cli.StatusError{
Expand Down

0 comments on commit ad29724

Please sign in to comment.