fix the restart desired to running when task not found #6833

junnplus · 2022-04-20T10:13:30Z

Signed-off-by: Ye Sijun junnplus@gmail.com

I'm sorry for misunderstanding 9503d72#diff-a9d90e82d0fee932f0437ab217a8ef14e4a56b4be02321de5193de0ad98e20f9L239-L249

For the case when Task returns not found and the restart desired to running, we need to continue to restart the container.

junnplus · 2022-04-20T10:14:45Z

I tested TestRunRestart of nerdctl locally, and it works.

containerd --version
containerd github.com/containerd/containerd v1.6.0-281-g4fd32eeb5 4fd32ee

$ go test -exec sudo -v ./cmd/nerdctl/... -run TestRunRestart -args -test.kill-daemon
test target: "nerdctl"
=== RUN   TestRunRestart
    run_restart_linux_test.go:44: NOTE: this test may take a while
    run_restart_linux_test.go:71: killing "containerd.service"
    run_restart_linux_test.go:72: checking activity of "containerd.service"
    run_restart_linux_test.go:72: (retry=0) active
    run_restart_linux_test.go:72: daemon "containerd.service" is now running, checking whether the daemon can handle requests
    run_restart_linux_test.go:72: (retry=0) time="2022-04-20T10:00:42Z" level=fatal msg="failed to dial \"/run/containerd/containerd.sock\": connection error: desc = \"transport: error while dialing: dial unix /run/containerd/containerd.sock: connect: connection refused\""
    run_restart_linux_test.go:72: (retry=1) activating
    run_restart_linux_test.go:72: (retry=2) active
    run_restart_linux_test.go:72: daemon "containerd.service" is now running, checking whether the daemon can handle requests
    run_restart_linux_test.go:72: daemon "containerd.service" can now handle requests
    run_restart_linux_test.go:79: (retry 0) ps -a: "CONTAINER ID    IMAGE                                              COMMAND                   CREATED           STATUS     PORTS                     NAMES\n367bda156295    ghcr.io/stargz-containers/nginx:1.19-alpine-org    \"/docker-entrypoint.…\"    15 minutes ago    Created                              c1-in-n0                  \n3843c3975c6f    ghcr.io/stargz-containers/nginx:1.19-alpine-org    \"/docker-entrypoint.…\"    15 minutes ago    Created                              c2-in-n1                  \n6465b0b125f6    ghcr.io/stargz-containers/nginx:1.19-alpine-org    \"/docker-entrypoint.…\"    6 seconds ago     Up         127.0.0.1:8080->80/tcp    nerdctl-testrunrestart    \nd941b0eec3a5    ghcr.io/stargz-containers/nginx:1.19-alpine-org    \"/docker-entrypoint.…\"    15 minutes ago    Created                              c0-in-n0                  \nf0460d99d436    ghcr.io/stargz-containers/nginx:1.19-alpine-org    \"/docker-entrypoint.…\"    15 minutes ago    Created                              c3-in-bridge              \n"
    run_restart_linux_test.go:82: test is passing, after 0 retries
--- PASS: TestRunRestart (6.89s)
PASS
ok      github.com/containerd/nerdctl/cmd/nerdctl       6.906s

theopenlab-ci · 2022-04-20T10:40:01Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 31s (non-voting)
containerd-test-arm64 : SUCCESS in 5m 38s (non-voting)
containerd-integration-test-arm64 : SUCCESS in 23m 48s (non-voting)

fuweid · 2022-04-20T13:06:21Z

runtime/restart/monitor/monitor.go

 		if err != nil {
 			logrus.WithError(err).Error("monitor")
-			if desiredStatus != containerd.Stopped {
+			if desiredStatus == containerd.Stopped {


if desiredStatus is running, the "on-failure" case will get empty status in the Line 245.
I am not sure that there is a way to store the status after delete. Anyway, it is safe to use with nerdctl.
Maybe we can add the comment about known issue.

An empty status has no effect on on-failure, which expects a status whose ExitStatus is not 0.

I get it, I added the comment about known issue in 243-245 .

Signed-off-by: Ye Sijun <junnplus@gmail.com>

theopenlab-ci · 2022-04-20T14:11:26Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 25s (non-voting)
containerd-test-arm64 : SUCCESS in 5m 31s (non-voting)
containerd-integration-test-arm64 : SUCCESS in 23m 40s (non-voting)

fuweid · 2022-04-20T14:43:23Z

I think we can watch event to handle the reconcile if necessary ~

estesp

LGTM

AkihiroSuda approved these changes Apr 20, 2022

View reviewed changes

fuweid reviewed Apr 20, 2022

View reviewed changes

junnplus force-pushed the monitor-restart branch 2 times, most recently from 52ff683 to 8232c44 Compare April 20, 2022 13:42

fix monitor restart

35994a4

Signed-off-by: Ye Sijun <junnplus@gmail.com>

junnplus force-pushed the monitor-restart branch from 8232c44 to 35994a4 Compare April 20, 2022 13:44

junnplus requested a review from fuweid April 20, 2022 13:48

fuweid approved these changes Apr 20, 2022

View reviewed changes

estesp approved these changes Apr 20, 2022

View reviewed changes

estesp merged commit 1a23678 into containerd:main Apr 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix the restart desired to running when task not found #6833

fix the restart desired to running when task not found #6833

Uh oh!

junnplus commented Apr 20, 2022 •

edited

Loading

Uh oh!

junnplus commented Apr 20, 2022 •

edited

Loading

Uh oh!

theopenlab-ci bot commented Apr 20, 2022

Uh oh!

fuweid Apr 20, 2022

Uh oh!

junnplus Apr 20, 2022

Uh oh!

junnplus Apr 20, 2022

Uh oh!

theopenlab-ci bot commented Apr 20, 2022

Uh oh!

fuweid commented Apr 20, 2022

Uh oh!

estesp left a comment

Uh oh!

Uh oh!

fix the restart desired to running when task not found #6833

fix the restart desired to running when task not found #6833

Uh oh!

Conversation

junnplus commented Apr 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junnplus commented Apr 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theopenlab-ci bot commented Apr 20, 2022

Uh oh!

fuweid Apr 20, 2022

Choose a reason for hiding this comment

Uh oh!

junnplus Apr 20, 2022

Choose a reason for hiding this comment

Uh oh!

junnplus Apr 20, 2022

Choose a reason for hiding this comment

Uh oh!

theopenlab-ci bot commented Apr 20, 2022

Uh oh!

fuweid commented Apr 20, 2022

Uh oh!

estesp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

junnplus commented Apr 20, 2022 •

edited

Loading

junnplus commented Apr 20, 2022 •

edited

Loading