Merge pull request containerd#3918 from apostasie/ci-fix-flaky-signals

CI: rewrite signal tests

Author: AkihiroSuda

cmd/nerdctl/container/container_restart_linux_test.go

@@ -18,6 +18,7 @@ package container
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -26,6 +27,7 @@ import (
 
 	"github.com/containerd/nerdctl/v2/pkg/testutil"
 	"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
+	"github.com/containerd/nerdctl/v2/pkg/testutil/test"
 )
 
 func TestRestart(t *testing.T) {
@@ -123,30 +125,38 @@ func TestRestartWithTime(t *testing.T) {
 }
 
 func TestRestartWithSignal(t *testing.T) {
-	t.Parallel()
-	base := testutil.NewBase(t)
-	tID := testutil.Identifier(t)
-
-	base.Cmd("run", "-d", "--name", tID, testutil.AlpineImage, "sh", "-c", `
-		trap 'echo "Received SIGUSR1"; exit 0' SIGUSR1
-		echo "Starting"
-		while true; do
-			sleep 1
-		done
-	`).AssertOK()
-	defer base.Cmd("rm", "-f", tID).Run()
-
-	base.EnsureContainerStarted(tID)
-
-	inspect := base.InspectContainer(tID)
-	initialPid := inspect.State.Pid
-
-	base.Cmd("restart", "--signal", "SIGUSR1", tID).AssertOK()
-	base.EnsureContainerStarted(tID)
-
-	newInspect := base.InspectContainer(tID)
-	newPid := newInspect.State.Pid
-
-	assert.Assert(t, initialPid != newPid, "Container PID should have changed after restart")
-
+	testCase := nerdtest.Setup()
+
+	testCase.Cleanup = func(data test.Data, helpers test.Helpers) {
+		helpers.Anyhow("rm", "-f", data.Identifier())
+	}
+
+	testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand {
+		cmd := nerdtest.RunSigProxyContainer(nerdtest.SigUsr1, false, nil, data, helpers)
+		// Capture the current pid
+		data.Set("oldpid", strconv.Itoa(nerdtest.InspectContainer(helpers, data.Identifier()).State.Pid))
+		// Send the signal
+		helpers.Ensure("restart", "--signal", "SIGUSR1", data.Identifier())
+		return cmd
+	}
+
+	testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected {
+		return &test.Expected{
+			// Check the container did indeed exit
+			ExitCode: 137,
+			Output: test.All(
+				// Check that we saw SIGUSR1 inside the container
+				test.Contains(nerdtest.SignalCaught),
+				func(stdout string, info string, t *testing.T) {
+					// Ensure the container was restarted
+					nerdtest.EnsureContainerStarted(helpers, data.Identifier())
+					// Check the new pid is different
+					newpid := strconv.Itoa(nerdtest.InspectContainer(helpers, data.Identifier()).State.Pid)
+					assert.Assert(helpers.T(), newpid != data.Get("oldpid"), info)
+				},
+			),
+		}
+	}
+
+	testCase.Run(t)
 }

cmd/nerdctl/container/container_run_linux_test.go

@@ -28,7 +28,6 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
-	"syscall"
 	"testing"
 	"time"
 
@@ -366,78 +365,61 @@ func TestRunTTY(t *testing.T) {
 	}
 }
 
-func runSigProxy(t *testing.T, args ...string) (string, bool, bool) {
-	t.Parallel()
-	base := testutil.NewBase(t)
-	testContainerName := testutil.Identifier(t)
-	defer base.Cmd("rm", "-f", testContainerName).Run()
-
-	fullArgs := []string{"run"}
-	fullArgs = append(fullArgs, args...)
-	fullArgs = append(fullArgs,
-		"--name",
-		testContainerName,
-		testutil.CommonImage,
-		"sh",
-		"-c",
-		testutil.SigProxyTestScript,
-	)
+func TestRunSigProxy(t *testing.T) {
+	testCase := nerdtest.Setup()
 
-	result := base.Cmd(fullArgs...).Start()
-	process := result.Cmd.Process
+	testCase.SubTests = []*test.Case{
+		{
+			Description: "SigProxyDefault",
 
-	// Waits until we reach the trap command in the shell script, then sends SIGINT.
-	time.Sleep(3 * time.Second)
-	syscall.Kill(process.Pid, syscall.SIGINT)
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("rm", "-f", data.Identifier())
+			},
 
-	// Waits until SIGINT is sent and responded to, then kills process to avoid timeout
-	time.Sleep(3 * time.Second)
-	process.Kill()
+			Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
+				cmd := nerdtest.RunSigProxyContainer(os.Interrupt, true, nil, data, helpers)
+				err := cmd.Signal(os.Interrupt)
+				assert.NilError(helpers.T(), err)
+				return cmd
+			},
 
-	sigIntRecieved := strings.Contains(result.Stdout(), testutil.SigProxyTrueOut)
-	timedOut := strings.Contains(result.Stdout(), testutil.SigProxyTimeoutMsg)
+			Expected: test.Expects(0, nil, test.Contains(nerdtest.SignalCaught)),
+		},
+		{
+			Description: "SigProxyTrue",
 
-	return result.Stdout(), sigIntRecieved, timedOut
-}
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("rm", "-f", data.Identifier())
+			},
 
-func TestRunSigProxy(t *testing.T) {
+			Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
+				cmd := nerdtest.RunSigProxyContainer(os.Interrupt, true, []string{"--sig-proxy=true"}, data, helpers)
+				err := cmd.Signal(os.Interrupt)
+				assert.NilError(helpers.T(), err)
+				return cmd
+			},
 
-	type testCase struct {
-		name        string
-		args        []string
-		want        bool
-		expectedOut string
-	}
-	testCases := []testCase{
-		{
-			name:        "SigProxyDefault",
-			args:        []string{},
-			want:        true,
-			expectedOut: testutil.SigProxyTrueOut,
-		},
-		{
-			name:        "SigProxyTrue",
-			args:        []string{"--sig-proxy=true"},
-			want:        true,
-			expectedOut: testutil.SigProxyTrueOut,
+			Expected: test.Expects(0, nil, test.Contains(nerdtest.SignalCaught)),
 		},
 		{
-			name:        "SigProxyFalse",
-			args:        []string{"--sig-proxy=false"},
-			want:        false,
-			expectedOut: "",
+			Description: "SigProxyFalse",
+
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("rm", "-f", data.Identifier())
+			},
+
+			Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
+				cmd := nerdtest.RunSigProxyContainer(os.Interrupt, true, []string{"--sig-proxy=false"}, data, helpers)
+				err := cmd.Signal(os.Interrupt)
+				assert.NilError(helpers.T(), err)
+				return cmd
+			},
+
+			Expected: test.Expects(127, nil, test.DoesNotContain(nerdtest.SignalCaught)),
 		},
 	}
 
-	for _, tc := range testCases {
-		tc := tc
-		t.Run(tc.name, func(t *testing.T) {
-			stdout, sigIntRecieved, timedOut := runSigProxy(t, tc.args...)
-			errorMsg := fmt.Sprintf("%s failed;\nExpected: '%s'\nActual: '%s'", tc.name, tc.expectedOut, stdout)
-			assert.Equal(t, false, timedOut, errorMsg)
-			assert.Equal(t, tc.want, sigIntRecieved, errorMsg)
-		})
-	}
+	testCase.Run(t)
 }
 
 func TestRunWithFluentdLogDriver(t *testing.T) {

cmd/nerdctl/container/container_stop_linux_test.go

@@ -29,7 +29,9 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
 	"github.com/containerd/nerdctl/v2/pkg/testutil"
 	iptablesutil "github.com/containerd/nerdctl/v2/pkg/testutil/iptables"
+	"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
 	"github.com/containerd/nerdctl/v2/pkg/testutil/nettestutil"
+	"github.com/containerd/nerdctl/v2/pkg/testutil/test"
 )
 
 func TestStopStart(t *testing.T) {
@@ -73,31 +75,23 @@ func TestStopStart(t *testing.T) {
 }
 
 func TestStopWithStopSignal(t *testing.T) {
-	t.Parallel()
-	// There may be issues with logs in Docker.
-	// This test is flaky with Docker. Might be related to https://github.com/containerd/nerdctl/pull/3557
-	base := testutil.NewBase(t)
-	testContainerName := testutil.Identifier(t)
-	defer base.Cmd("rm", "-f", testContainerName).Run()
+	testCase := nerdtest.Setup()
 
-	base.Cmd("run", "-d", "--stop-signal", "SIGQUIT", "--name", testContainerName, testutil.CommonImage, "sh", "-euxc", `#!/bin/sh
-set -eu
-echo "Script started"
-quit=0
-trap 'echo "SIGQUIT received"; quit=1' QUIT
-echo "Trap set"
-while true; do
-    if [ $quit -eq 1 ]; then
-        echo "Quitting loop"
-        break
-    fi
-    echo "In loop"
-    sleep 1
-done
-echo "signal quit"
-sync`).AssertOK()
-	base.Cmd("stop", testContainerName).AssertOK()
-	base.Cmd("logs", "-f", testContainerName).AssertOutContains("signal quit")
+	testCase.Cleanup = func(data test.Data, helpers test.Helpers) {
+		helpers.Anyhow("rm", "-f", data.Identifier())
+	}
+
+	testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand {
+		cmd := nerdtest.RunSigProxyContainer(nerdtest.SigQuit, false,
+			[]string{"--stop-signal", nerdtest.SigQuit.String()}, data, helpers)
+		helpers.Ensure("stop", data.Identifier())
+		return cmd
+	}
+
+	// Verify that SIGQUIT was sent to the container AND that the container did forcefully exit
+	testCase.Expected = test.Expects(137, nil, test.Contains(nerdtest.SignalCaught))
+
+	testCase.Run(t)
 }
 
 func TestStopCleanupForwards(t *testing.T) {

pkg/testutil/nerdtest/utilities_linux.go

@@ -0,0 +1,69 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package nerdtest
+
+import (
+	"os"
+	"strconv"
+	"strings"
+	"syscall"
+	"time"
+
+	"github.com/containerd/nerdctl/v2/pkg/testutil"
+	"github.com/containerd/nerdctl/v2/pkg/testutil/test"
+)
+
+const SignalCaught = "received"
+
+var SigQuit os.Signal = syscall.SIGQUIT
+var SigUsr1 os.Signal = syscall.SIGUSR1
+
+func RunSigProxyContainer(signal os.Signal, exitOnSignal bool, args []string, data test.Data, helpers test.Helpers) test.TestableCommand {
+	sig := strconv.Itoa(int(signal.(syscall.Signal)))
+	ready := "trap ready"
+	script := `#!/bin/sh
+	set -eu
+
+	sig_msg () {
+		printf "` + SignalCaught + `\n"
+		[ "` + strconv.FormatBool(exitOnSignal) + `" != true ] || exit 0
+	}
+
+	trap sig_msg ` + sig + `
+	printf "` + ready + `\n"
+	while true; do
+		sleep 0.1
+	done
+`
+
+	args = append(args, "--name", data.Identifier(), testutil.CommonImage, "sh", "-c", script)
+	args = append([]string{"run"}, args...)
+
+	cmd := helpers.Command(args...)
+	cmd.Background(10 * time.Second)
+	EnsureContainerStarted(helpers, data.Identifier())
+
+	for {
+		out := helpers.Capture("logs", data.Identifier())
+		if strings.Contains(out, ready) {
+			break
+		}
+		time.Sleep(100 * time.Millisecond)
+	}
+
+	return cmd
+}

pkg/testutil/test/command.go

@@ -185,6 +185,10 @@ func (gc *GenericCommand) Background(timeout time.Duration) {
 	gc.result = icmd.StartCmd(i)
 }
 
+func (gc *GenericCommand) Signal(sig os.Signal) error {
+	return gc.result.Cmd.Process.Signal(sig)
+}
+
 func (gc *GenericCommand) withEnv(env map[string]string) {
 	if gc.Env == nil {
 		gc.Env = map[string]string{}

pkg/testutil/test/test.go

@@ -114,6 +114,8 @@ type TestableCommand interface {
 	Run(expect *Expected)
 	// Background allows starting a command in the background
 	Background(timeout time.Duration)
+	// Signal sends a signal to a backgrounded command
+	Signal(sig os.Signal) error
 	// Stderr allows retrieving the raw stderr output of the command
 	Stderr() string
 }

pkg/testutil/testutil_linux.go

@@ -54,25 +54,6 @@ var (
 	// It should be "connection refused" as per the TCP RFC.
 	// https://www.rfc-editor.org/rfc/rfc793
 	ExpectedConnectionRefusedError = "connection refused"
-
-	SigProxyTrueOut    = "received SIGINT"
-	SigProxyTimeoutMsg = "Timed Out; No signal received"
-	SigProxyTestScript = `#!/bin/sh
-	set -eu
-
-	sig_msg () {
-		printf "` + SigProxyTrueOut + `"
-		end
-	}
-
-	trap sig_msg INT
-	timeout=0
-	while [ $timeout -ne 10 ]; do
-		timeout=$((timeout+1))
-		sleep 1
-	done
-	printf "` + SigProxyTimeoutMsg + `"
-	end`
 )
 
 const (