From b04586627791dcdc16a347f9d593ba61d8733128 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 2 Jan 2024 11:17:08 +0900 Subject: [PATCH] logging: respect ctx The ctx is cancelled on SIGTERM Fix issue 2726 (regression in PR 2683) Signed-off-by: Akihiro Suda --- .github/workflows/test.yml | 2 +- Dockerfile | 2 +- cmd/nerdctl/container_run_test.go | 14 ++++++++++++++ go.mod | 1 + go.sum | 2 ++ pkg/logging/logging.go | 26 +++++++++++++++++++++----- 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a9a22221d01..2e6ac21683b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -70,7 +70,7 @@ jobs: test-integration: runs-on: "ubuntu-${{ matrix.ubuntu }}" - timeout-minutes: 60 + timeout-minutes: 40 strategy: fail-fast: false matrix: diff --git a/Dockerfile b/Dockerfile index 1e00f459ebb..c36a5ac26f5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -313,7 +313,7 @@ RUN curl -L -o nydus-static.tgz "https://github.com/dragonflyoss/image-service/r mv nydus-static/nydus-image nydus-static/nydusd nydus-static/nydusify /usr/bin/ && \ rm nydus-static.tgz CMD ["gotestsum", "--format=testname", "--rerun-fails=2", "--packages=github.com/containerd/nerdctl/cmd/nerdctl/...", \ - "--", "-timeout=60m", "-args", "-test.kill-daemon"] + "--", "-timeout=30m", "-args", "-test.kill-daemon"] FROM test-integration AS test-integration-rootless # Install SSH for creating systemd user session. diff --git a/cmd/nerdctl/container_run_test.go b/cmd/nerdctl/container_run_test.go index b543c8811ae..e18aa7ac8e6 100644 --- a/cmd/nerdctl/container_run_test.go +++ b/cmd/nerdctl/container_run_test.go @@ -510,3 +510,17 @@ func TestRunAddHostRemainsWhenAnotherContainerCreated(t *testing.T) { base.Cmd("exec", containerName, "cat", "/etc/hosts").AssertOutWithFunc(checkEtcHosts) } + +// https://github.com/containerd/nerdctl/issues/2726 +func TestRunRmTime(t *testing.T) { + base := testutil.NewBase(t) + base.Cmd("pull", testutil.CommonImage) + t0 := time.Now() + base.Cmd("run", "--rm", testutil.CommonImage, "true").AssertOK() + t1 := time.Now() + took := t1.Sub(t0) + const deadline = 3 * time.Second + if took > deadline { + t.Fatalf("expected to have completed in %v, took %v", deadline, took) + } +} diff --git a/go.mod b/go.mod index 13fdefa3577..df6cc4b6c0c 100644 --- a/go.mod +++ b/go.mod @@ -40,6 +40,7 @@ require ( github.com/moby/sys/mount v0.3.3 github.com/moby/sys/signal v0.7.0 github.com/moby/term v0.5.0 + github.com/muesli/cancelreader v0.2.2 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.0-rc5 github.com/opencontainers/runtime-spec v1.1.0 diff --git a/go.sum b/go.sum index ab9c981260b..18f02d3fc39 100644 --- a/go.sum +++ b/go.sum @@ -214,6 +214,8 @@ github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0= github.com/moby/term v0.5.0/go.mod h1:8FzsFHVUBGZdbDsJw/ot+X+d5HLUbvklYLJ9uGfcI3Y= github.com/mr-tron/base58 v1.2.0 h1:T/HDJBh4ZCPbU39/+c3rRvE0uKBQlU27+QI8LJ4t64o= github.com/mr-tron/base58 v1.2.0/go.mod h1:BinMc/sQntlIE1frQmRFPUoPA1Zkr8VRgBdjWI2mNwc= +github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= +github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= github.com/multiformats/go-base32 v0.1.0 h1:pVx9xoSPqEIQG8o+UbAe7DNi51oej1NtK+aGkbLYxPE= github.com/multiformats/go-base32 v0.1.0/go.mod h1:Kj3tFY6zNr+ABYMqeUNeGvkIC/UYgtWibDcT0rExnbI= github.com/multiformats/go-base36 v0.1.0 h1:JR6TyF7JjGd3m6FbLU2cOxhC0Li8z8dLNGQ89tUg4F4= diff --git a/pkg/logging/logging.go b/pkg/logging/logging.go index f59936f8f18..9339868f2a7 100644 --- a/pkg/logging/logging.go +++ b/pkg/logging/logging.go @@ -31,6 +31,7 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/runtime/v2/logging" "github.com/containerd/log" + "github.com/muesli/cancelreader" ) const ( @@ -140,10 +141,25 @@ func LoadLogConfig(dataStore, ns, id string) (LogConfig, error) { return logConfig, nil } -func loggingProcessAdapter(driver Driver, dataStore string, config *logging.Config) error { +func loggingProcessAdapter(ctx context.Context, driver Driver, dataStore string, config *logging.Config) error { if err := driver.PreProcess(dataStore, config); err != nil { return err } + + stdoutR, err := cancelreader.NewReader(config.Stdout) + if err != nil { + return err + } + stderrR, err := cancelreader.NewReader(config.Stderr) + if err != nil { + return err + } + go func() { + <-ctx.Done() // delivered on SIGTERM + stdoutR.Cancel() + stderrR.Cancel() + }() + var wg sync.WaitGroup wg.Add(3) stdout := make(chan string, 10000) @@ -161,8 +177,8 @@ func loggingProcessAdapter(driver Driver, dataStore string, config *logging.Conf } } - go processLogFunc(config.Stdout, stdout) - go processLogFunc(config.Stderr, stderr) + go processLogFunc(stdoutR, stdout) + go processLogFunc(stderrR, stderr) go func() { defer wg.Done() driver.Process(stdout, stderr) @@ -175,7 +191,7 @@ func loggerFunc(dataStore string) (logging.LoggerFunc, error) { if dataStore == "" { return nil, errors.New("got empty data store") } - return func(_ context.Context, config *logging.Config, ready func() error) error { + return func(ctx context.Context, config *logging.Config, ready func() error) error { if config.Namespace == "" || config.ID == "" { return errors.New("got invalid config") } @@ -193,7 +209,7 @@ func loggerFunc(dataStore string) (logging.LoggerFunc, error) { return err } - return loggingProcessAdapter(driver, dataStore, config) + return loggingProcessAdapter(ctx, driver, dataStore, config) } else if !errors.Is(err, os.ErrNotExist) { // the file does not exist if the container was created with nerdctl < 0.20 return err