diff --git a/cmd/nerdctl/container/container_run_restart_linux_test.go b/cmd/nerdctl/container/container_run_restart_linux_test.go index 17761188e39..779b94d3cbe 100644 --- a/cmd/nerdctl/container/container_run_restart_linux_test.go +++ b/cmd/nerdctl/container/container_run_restart_linux_test.go @@ -382,3 +382,86 @@ func TestRunRestartStatusLabel(t *testing.T) { testCase.Run(t) } + +func TestRunRestartAlwaysKillSignal(t *testing.T) { + testCase := nerdtest.Setup() + // This asserts the label nerdctl itself writes, so it does not depend on the + // containerd restart monitor actually acting on it. It is containerd-only + // because docker has no equivalent label. + testCase.Require = require.Not(nerdtest.Docker) + + testCase.SubTests = []*test.Case{ + { + Description: "a non-stop signal does not mark the container explicitly stopped", + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "-d", "--restart=always", "--name", data.Identifier(), + testutil.CommonImage, "sleep", "infinity") + helpers.Ensure("kill", "--signal", "HUP", data.Identifier()) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("inspect", data.Identifier()) + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeSuccess, + Output: expect.JSON([]dockercompat.Container{}, func(dc []dockercompat.Container, t tig.T) { + assert.Equal(t, 1, len(dc)) + assert.Assert(t, dc[0].Config.Labels[restart.ExplicitlyStoppedLabel] != "true", + "a container killed with SIGHUP must stay eligible for restart") + }), + } + }, + }, + { + Description: "the container's own stop signal marks it explicitly stopped", + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "-d", "--restart=always", "--stop-signal", "SIGUSR1", + "--name", data.Identifier(), testutil.CommonImage, "sleep", "infinity") + helpers.Ensure("kill", "--signal", "USR1", data.Identifier()) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("inspect", data.Identifier()) + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeSuccess, + Output: expect.JSON([]dockercompat.Container{}, func(dc []dockercompat.Container, t tig.T) { + assert.Equal(t, 1, len(dc)) + assert.Equal(t, dc[0].Config.Labels[restart.ExplicitlyStoppedLabel], "true") + }), + } + }, + }, + { + Description: "the default signal marks the container explicitly stopped", + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "-d", "--restart=always", "--name", data.Identifier(), + testutil.CommonImage, "sleep", "infinity") + helpers.Ensure("kill", data.Identifier()) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("inspect", data.Identifier()) + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeSuccess, + Output: expect.JSON([]dockercompat.Container{}, func(dc []dockercompat.Container, t tig.T) { + assert.Equal(t, 1, len(dc)) + assert.Equal(t, dc[0].Config.Labels[restart.ExplicitlyStoppedLabel], "true") + }), + } + }, + }, + } + + testCase.Run(t) +} diff --git a/pkg/cmd/container/kill.go b/pkg/cmd/container/kill.go index d42a7cd8c82..7239bcce297 100644 --- a/pkg/cmd/container/kill.go +++ b/pkg/cmd/container/kill.go @@ -85,9 +85,22 @@ func killContainer(ctx context.Context, container containerd.Container, signal s containerutil.UpdateErrorLabel(ctx, container, err) } }() - if err := containerutil.UpdateExplicitlyStoppedLabel(ctx, container, true); err != nil { + containerLabels, err := container.Labels(ctx) + if err != nil { + return err + } + stops, err := containerutil.IsStopSignal(signal, containerLabels) + if err != nil { return err } + // Recording a non-stopping signal as an explicit stop would permanently + // suppress the restart policy of a container that is still running. + if stops { + if err := containerutil.UpdateExplicitlyStoppedLabel(ctx, container, true); err != nil { + return err + } + } + task, err := container.Task(ctx, cio.Load) if err != nil { return err diff --git a/pkg/containerutil/containerutil.go b/pkg/containerutil/containerutil.go index 7c16bc720a0..2f0c1bfa0bf 100644 --- a/pkg/containerutil/containerutil.go +++ b/pkg/containerutil/containerutil.go @@ -482,6 +482,22 @@ func getSignal(signalValue string, containerLabels map[string]string) (syscall.S return signal.ParseSignal("SIGTERM") } +// IsStopSignal reports whether sig is expected to stop the container, as +// opposed to being merely delivered to the running process (e.g. SIGHUP). +// Signals other than SIGKILL must match the container's own stop signal. +func IsStopSignal(sig syscall.Signal, containerLabels map[string]string) (bool, error) { + if sig == syscall.SIGKILL { + return true, nil + } + + stopSignal, err := getSignal("", containerLabels) + if err != nil { + return false, err + } + + return sig == stopSignal, nil +} + func waitContainerStop(ctx context.Context, task containerd.Task, exitCh <-chan containerd.ExitStatus, id string) error { select { case <-ctx.Done(): diff --git a/pkg/containerutil/containerutil_test.go b/pkg/containerutil/containerutil_test.go index e45b3bb9773..7212abd2f00 100644 --- a/pkg/containerutil/containerutil_test.go +++ b/pkg/containerutil/containerutil_test.go @@ -18,8 +18,11 @@ package containerutil import ( "reflect" + "syscall" "testing" + containerd "github.com/containerd/containerd/v2/client" + "github.com/containerd/nerdctl/v2/pkg/labels" ) @@ -120,3 +123,65 @@ func TestGetContainerVolumes_Indexed(t *testing.T) { t.Errorf("Expected third volume to be named 'vol-2', got '%s'", indexedResult[2].Name) } } + +func TestIsStopSignal(t *testing.T) { + tests := []struct { + name string + sig syscall.Signal + containerLabels map[string]string + expected bool + expectedErr bool + }{ + { + name: "SIGKILL always stops the container", + sig: syscall.SIGKILL, + expected: true, + }, + { + name: "without a stopSignal label the stop signal is SIGTERM", + sig: syscall.SIGTERM, + expected: true, + }, + { + name: "SIGHUP does not stop the container", + sig: syscall.SIGHUP, + expected: false, + }, + { + name: "a signal matching the stopSignal label stops the container", + sig: syscall.SIGQUIT, + containerLabels: map[string]string{containerd.StopSignalLabel: "SIGQUIT"}, + expected: true, + }, + { + name: "SIGTERM does not stop the container when stopSignal is overridden", + sig: syscall.SIGTERM, + containerLabels: map[string]string{containerd.StopSignalLabel: "SIGQUIT"}, + expected: false, + }, + { + name: "an unparsable stopSignal label is an error", + sig: syscall.SIGHUP, + containerLabels: map[string]string{containerd.StopSignalLabel: "NOT_A_SIGNAL"}, + expectedErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := IsStopSignal(tt.sig, tt.containerLabels) + if tt.expectedErr { + if err == nil { + t.Fatalf("expected an error, got none") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.expected { + t.Errorf("expected %v, got %v", tt.expected, got) + } + }) + } +}