Adopt Docker's own container restarts

[thockin]

moby/moby#7414

As I understand, when the docker daemon dies, this only restarts containers with a policy of always which means we still have to handle the on failure case ourselves.

[crosbymichael]

I updated the PR to handle the policy of on-failure so you don't have to worry about doing that yourself ;)

Thanks!

[mikebrow]

@thockin was there anything else you wanted to cover with this issue?

[thockin]

I think this is a question for @kubernetes/sig-node and the relevant devs to discuss.

Could/should restarting be delegated to the container runtimes (because it is plural now) or do we need a hook to do stuff when containers need restart? I feel like we need to own it, at least for some cases - e.g. a restart of the infra container necessitates a full pod restart, and Docker just can't know that (AFAIK anyway). So even if we assert that restarts are delegated to container runtimes, we still can't use Docker's restarts because Docker is ignorant of pods (or generically restart dependencies).

@dchen1107 @yujuhong I propose this be closed. If you concur, please put a bullet in it and close.

[yujuhong]

There are a couple of issues that we need to deal with before we can (if we choose to) completely delegate the container restarts to either the container runtimes or other daemons (e.g., systemd):

1. The ability to execute k8s-defined lifecycle hooks, as @thockin mentioned
2. The infra container restarts, which may be handled through proper hooks(?). We may not always need the infra containers in the future though.
3. In-place container restarts vs creating a new container. We use the latter but both docker and systemd (at least the way rkt uses it now) supports the former. If we ever want to switch to in-place restarts, we'll need to eliminate the need of having the exited container around for logs and various status examination.
4. Other advanced restart mechanisms such as backoffs, etc.

I think we will want to explore the possibility of delegating this responsibility to something else while or after converging to a new runtime interface. However, I don't see we directly using docker restart policy due to the required features stated above. I'm okay with closing this one and will open a issue if we want to discuss the issues for all container runtimes.

[thockin]

To be clear, when I said:

if we assert that restarts are delegated to container runtimes, we still
can't use Docker's restarts because
Docker is ignorant of pods

I meant that restarts could be delegated to the kubelet runtime
abstraction, wherein we wrap Docker's API and do things like the infra
container that don't apply to non-docker runtimes. I still think we can't
use Docker's own restarts. Agree we should probably close this.

On Sun, Apr 3, 2016 at 11:45 AM, Yu-Ju Hong notifications@github.com
wrote:

There are a couple of issues that we need to deal with before we can (if
we choose to) completely delegate the container restarts to either the
container runtimes or other daemons (e.g., systemd):

1. The ability to execute k8s-defined lifecycle hooks, as @thockin
   https://github.com/thockin mentioned
2. The infra container restarts, which may be handled through proper
   hooks(?). We may not always need the infra containers in the future though.
3. In-place container restarts vs creating a new container. We use the
   latter and but both docker and systemd (at least the way rkt uses it now)
   supports the former. If we ever want to switch to in-place restarts, we'll
   need to eliminate the need of having the exited container around for logs
   and various status examination.
4. Other advanced restart mechanisms such as backoffs, etc.

I think we will want to explore the possibility of delegating this
responsibility to something else while or after converging to a new
runtime interface. However, I don't see we directly using docker restart
policy due to the required features stated above. I'm okay with closing
this one and will open a issue if we want to discuss the issues for all
container runtimes.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#816 (comment)

[yujuhong]

Closing based on the discussion above.