Skip to content

Move containerd.service from Requires= to Wants= #512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
tianon wants to merge 1 commit into from
Closed

Move containerd.service from Requires= to Wants= #512

tianon wants to merge 1 commit into from

Conversation

@tianon
Copy link
Contributor

@tianon tianon commented Dec 5, 2020

Per the systemd.unit documentation:

If this unit gets activated, the units listed will be activated as well. If one of the other units fails to activate, and an ordering dependency After= on the failing unit is set, this unit will not be started. Besides, with or without specifying After=, this unit will be stopped if one of the other units is explicitly stopped.

Often, it is a better choice to use Wants= instead of Requires= in order to achieve a system that is more robust when dealing with failing services.

This should also be generally "safe" given we added --containerd=/run/containerd/containerd.sock to the flags we pass to dockerd.

See #511 (comment).

@tianon tianon mentioned this pull request Dec 5, 2020
Closed
@thaJeztah
Copy link
Member

thaJeztah commented Dec 5, 2020

love the typo; its on theme 😇

@tianon tianon changed the title Move conatinerd.service from Requires= to Wants= Move containerd.service from Requires= to Wants= Dec 5, 2020
@tianon
Copy link
Contributor Author

tianon commented Dec 5, 2020

love the typo; its on theme

I have no idea what you're talking about!! (it's fixed 😇 ❤️)

@tianon
Copy link
Contributor Author

tianon commented Dec 8, 2020

I have verified that with this change, I'm able to systemctl restart containerd without Docker going down (and that it does successfully reconnect to containerd).

@thaJeztah
Copy link
Member

thaJeztah commented Dec 8, 2020

Thanks! I actually recalled I left this as a comment for follow up on #508

We may want to look into using Wants= instead of Requires=, because
that allows docker to continue running if containerd is restarted, quoting
the systemd documentation:

Often, it is a better choice to use Wants= instead of Requires= in order
to achieve a system that is more robust when dealing with failing services.

Given that docker will likely still fail if the containerd socket is not
present, startup will fail if containerd is not running, but if containerd
is restarted, the docker daemon may be able to try reconnecting.

thaJeztah
thaJeztah previously approved these changes Dec 8, 2020
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

thaJeztah commented Apr 2, 2021

@tianon looks like this needs a rebase 😅

@tianon
Move containerd.service from Requires= to Wants=
7aa6d8d 
Per the systemd.unit documentation:

> If this unit gets activated, the units listed will be activated as well. If one of the other units fails to activate, and an ordering dependency After= on the failing unit is set, this unit will not be started. Besides, with or without specifying After=, this unit will be stopped if one of the other units is explicitly stopped.
>
> Often, it is a better choice to use Wants= instead of Requires= in order to achieve a system that is more robust when dealing with failing services.

This should also be generally "safe" given we added `--containerd=/run/containerd/containerd.sock` to the flags we pass to `dockerd`.

Signed-off-by: Tianon Gravi <[email protected]>
@tianon tianon force-pushed the wants-containerd branch from 1e9d40a to 7aa6d8d Compare April 2, 2021 22:25
@thaJeztah
Copy link
Member

thaJeztah commented Jul 12, 2021

Closing here, because the file was moved back to the moby repository.
I asked @aiordache to carry this (move to moby), thanks!

@thaJeztah thaJeztah closed this Jul 12, 2021
@aiordache aiordache mentioned this pull request Jul 12, 2021
Merged
@tianon tianon deleted the wants-containerd branch July 12, 2021 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tianon @thaJeztah