Add restart policy for enhanced restart manager by junnplus · Pull Request #6744 · containerd/containerd

junnplus · 2022-03-29T04:11:13Z

Signed-off-by: Ye Sijun [email protected]

TL;DR: In order to be compatible with the restart policies of docker in nerdctl, we need enhance containerd's restart manager by adding a label:
containerd.io/restart.policy, containerd.io/restart.count.

Discussed in containerd/nerdctl#945

policy, _ := restart.NewPolicy("on-failure")
container, err := client.NewContainer(ctx, id,
    restart.WithStatus(containerd.Running),
    restart.WithPolicy(policy))

theopenlab-ci · 2022-03-29T05:19:20Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 35s (non-voting)
containerd-test-arm64 : SUCCESS in 5m 43s (non-voting)
containerd-integration-test-arm64 : SUCCESS in 24m 08s (non-voting)

theopenlab-ci · 2022-03-29T05:46:38Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 22s (non-voting)
containerd-test-arm64 : SUCCESS in 5m 46s (non-voting)
containerd-integration-test-arm64 : SUCCESS in 24m 02s (non-voting)

runtime/restart/restart_test.go

runtime/restart/restart.go

AkihiroSuda

Thank you! 👍

theopenlab-ci · 2022-03-29T07:52:07Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 29s (non-voting)
containerd-test-arm64 : SUCCESS in 6m 02s (non-voting)
containerd-integration-test-arm64 : SUCCESS in 25m 02s (non-voting)

theopenlab-ci · 2022-03-29T14:57:38Z

Build succeeded.

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

theopenlab-ci · 2022-03-29T15:41:15Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 28s (non-voting)
containerd-test-arm64 : SUCCESS in 5m 55s (non-voting)
containerd-integration-test-arm64 : SUCCESS in 26m 11s (non-voting)

junnplus · 2022-03-29T16:55:01Z

/retest

theopenlab-ci · 2022-03-29T18:06:27Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 22s (non-voting)
containerd-test-arm64 : SUCCESS in 5m 37s (non-voting)
containerd-integration-test-arm64 : SUCCESS in 25m 09s (non-voting)

theopenlab-ci · 2022-03-30T09:28:17Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 35s (non-voting)
containerd-test-arm64 : SUCCESS in 59m 14s (non-voting)
containerd-integration-test-arm64 : SUCCESS in 32m 40s (non-voting)

junnplus · 2022-04-01T16:33:48Z

@AkihiroSuda PTAL 🙏

kzys

I know that this Docker-compatible restart support is something nerdctl wants and cannot implement by itself by being a short-lived CLI. However would we make containerd Docker-compatible in this way?

There are other features that would need a persistent daemon such as health check (see #5609). I'm unsure that we'd like to have all of them in containerd.

integration/client/restart_monitor_test.go

runtime/restart/monitor/monitor.go

AkihiroSuda · 2022-04-05T06:58:46Z

However would we make containerd Docker-compatible in this way?

Yes, I don't see a problem, as the plugin can be turned off.

There are other features that would need a persistent daemon such as health check (see #5609). I'm unsure that we'd like to have all of them in containerd.

We can have a healthcheck plugin too.
Users can turn off when they don't need it.

runtime/restart/restart.go

AkihiroSuda

Thanks, but "unless-stopped" logic doesn't seem correct
https://github.com/containerd/containerd/pull/6744/files#r842432941

Other part looks good

theopenlab-ci · 2022-04-07T15:15:21Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 32s (non-voting)
containerd-test-arm64 : SUCCESS in 5m 46s (non-voting)
containerd-integration-test-arm64 : SUCCESS in 24m 48s (non-voting)

runtime/restart/monitor/monitor.go

Signed-off-by: Ye Sijun <[email protected]>

junnplus · 2022-04-08T17:08:01Z

@kzys I specially handled "" for count label and log the error from Atoi(). https://github.com/containerd/containerd/compare/bb54f7c05161887ffef1ce75026de97b335a3075..3df7674058301f290fc148719d483e47f4118494

Please take a look again, Thanks!

theopenlab-ci · 2022-04-08T18:30:57Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 4m 58s (non-voting)
containerd-test-arm64 : SUCCESS in 6m 22s (non-voting)
containerd-integration-test-arm64 : FAILURE in 3m 04s (non-voting)

junnplus · 2022-04-15T11:02:09Z

Ping @AkihiroSuda @kzys @dmcgowan

fuweid · 2022-04-19T15:09:59Z

@AkihiroSuda the change looks good to you?

junnplus force-pushed the restart-policy branch 3 times, most recently from d5549d4 to 87259b0 Compare March 29, 2022 04:53

junnplus force-pushed the restart-policy branch from 87259b0 to 5f7d90a Compare March 29, 2022 05:20

AkihiroSuda added impact/changelog kind/enhancement labels Mar 29, 2022

AkihiroSuda reviewed Mar 29, 2022

View reviewed changes

runtime/restart/restart_test.go Show resolved Hide resolved

AkihiroSuda reviewed Mar 29, 2022

View reviewed changes

runtime/restart/restart.go Show resolved Hide resolved

AkihiroSuda reviewed Mar 29, 2022

View reviewed changes

runtime/restart/restart.go Show resolved Hide resolved

AkihiroSuda reviewed Mar 29, 2022

View reviewed changes

junnplus force-pushed the restart-policy branch from 5f7d90a to 1536985 Compare March 29, 2022 07:22

junnplus force-pushed the restart-policy branch from 1536985 to 3bd541c Compare March 29, 2022 14:29

junnplus force-pushed the restart-policy branch from 3bd541c to dea6935 Compare March 29, 2022 15:12

junnplus force-pushed the restart-policy branch from dea6935 to e0b1fec Compare March 29, 2022 16:09

junnplus requested a review from AkihiroSuda March 29, 2022 16:57

junnplus force-pushed the restart-policy branch from e0b1fec to 6ed4d81 Compare March 30, 2022 08:27

kzys reviewed Apr 1, 2022

View reviewed changes

integration/client/restart_monitor_test.go Outdated Show resolved Hide resolved

runtime/restart/monitor/monitor.go Show resolved Hide resolved

AkihiroSuda reviewed Apr 5, 2022

View reviewed changes

runtime/restart/restart.go Show resolved Hide resolved

AkihiroSuda requested changes Apr 5, 2022

View reviewed changes

junnplus force-pushed the restart-policy branch from 6ed4d81 to 480528a Compare April 6, 2022 06:50

junnplus requested review from AkihiroSuda, fuweid and kzys April 7, 2022 15:55

fuweid approved these changes Apr 8, 2022

View reviewed changes

kzys requested changes Apr 8, 2022

View reviewed changes

runtime/restart/monitor/monitor.go Show resolved Hide resolved

add restart policy for enhanced restart manager

3df7674

Signed-off-by: Ye Sijun <[email protected]>

junnplus force-pushed the restart-policy branch from bb54f7c to 3df7674 Compare April 8, 2022 17:04

junnplus requested a review from kzys April 10, 2022 08:48

kzys approved these changes Apr 18, 2022

View reviewed changes

AkihiroSuda approved these changes Apr 19, 2022

View reviewed changes

fuweid merged commit 9503d72 into containerd:main Apr 19, 2022

AkihiroSuda mentioned this pull request Apr 20, 2022

[Regression on main] Segfaults at runtime/restart/monitor/monitor.go:226 #6831

Closed

PandaBlackAndWhitr mentioned this pull request Sep 3, 2022

Restart option "unless-stopped" and "on-failure" doesn't work for nerdctl compose containerd/nerdctl#1350

Closed

dmcgowan added this to the 1.7 milestone Oct 20, 2022

fahedouch mentioned this pull request Nov 4, 2022

Refactor nerdctl logs functionality to pkg/logging. containerd/nerdctl#1468

Merged

AkihiroSuda mentioned this pull request Dec 5, 2022

FATA[0040] unsupported restart policy "unless-stopped", supported policies are: [] containerd/nerdctl#1605

Closed

dmcgowan mentioned this pull request Dec 10, 2022

Prepare release notes for v1.7.0-beta.1 #7793

Merged

dmcgowan mentioned this pull request Jan 3, 2023

Prepare release notes for v1.7.0-beta.2 #7903

Merged

dmcgowan mentioned this pull request Jan 28, 2023

Prepare release notes for v1.7.0-beta.3 #8014

Merged

dmcgowan mentioned this pull request Feb 15, 2023

Prepare release notes for v1.7.0 beta.4 #8110

Merged

This was referenced Feb 25, 2023

Prepare release notes for v1.7.0-rc.0 #8162

Merged

Prepare release notes for v1.7.0-rc.1 #8192

Merged

This was referenced Mar 7, 2023

Add release notes for v1.7.0-rc.2 #8216

Merged

Prepare release notes for v1.7.0-rc.3 #8233

Merged

Prepare release notes for v1.7.0 #8242

Merged

Conversation

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

Uh oh!

Uh oh!

theopenlab-ci bot commented Mar 29, 2022

Uh oh!

theopenlab-ci bot commented Mar 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

theopenlab-ci bot commented Mar 29, 2022

Uh oh!

theopenlab-ci bot commented Mar 29, 2022

Uh oh!

theopenlab-ci bot commented Mar 29, 2022

Uh oh!

junnplus commented Mar 29, 2022

Uh oh!

theopenlab-ci bot commented Mar 29, 2022

Uh oh!

theopenlab-ci bot commented Mar 30, 2022

Uh oh!

junnplus commented Apr 1, 2022

Uh oh!

kzys left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AkihiroSuda commented Apr 5, 2022

Uh oh!

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

theopenlab-ci bot commented Apr 7, 2022

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

theopenlab-ci bot commented Apr 8, 2022

Uh oh!

junnplus commented Apr 15, 2022

Uh oh!

fuweid commented Apr 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

junnplus commented Mar 29, 2022 •

edited

Loading

junnplus commented Apr 8, 2022 •

edited

Loading