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.

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

Already on GitHub? Sign in to your account

Ensure health probe is stopped when a container exits #32274

Merged
merged 1 commit into from Apr 4, 2017

Conversation

Projects
None yet
6 participants
@mlaventure
Contributor

mlaventure commented Mar 31, 2017

Signed-off-by: Kenfe-Mickael Laventure mickael.laventure@gmail.com

--

- What I did

Fixes #30107 by preventing an health check to keep on running after a container has been removed

- How I did it

By always stopping health checks on container exits. Health checks are always started if present on container start (see daemon.StateChanged()), so there's no fear of missing any and it avoid unneeded exec while the container is still down.

- How to verify it

Given it's a race, it has to be manually testing unfortunately. Instructions provided by @esbite were used (https://gist.github.com/esbite/d0c639e72435b4721d44445b0972dff8)

- Description for the changelog

Ensure health probe is stopped when a container exits

- A picture of a cute animal (not mandatory but encouraged)

Not Today 馃檧

@vdemeester

LGTM 馃

Show outdated Hide outdated daemon/delete.go
Ensure health probe is stopped when a container exits
Signed-off-by: Kenfe-Mickael Laventure <mickael.laventure@gmail.com>
@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure Apr 3, 2017

Contributor

@vdemeester @cpuguy83 changed the fix (and updated the PR description) PTAL

Contributor

mlaventure commented Apr 3, 2017

@vdemeester @cpuguy83 changed the fix (and updated the PR description) PTAL

@mlaventure mlaventure changed the title from Ensure health probe is stopped when a container is removed to Ensure health probe is stopped when a container exits Apr 3, 2017

@tonistiigi

LGTM

@cpuguy83

LGTM

@vdemeester

This comment has been minimized.

Show comment
Hide comment
@vdemeester

vdemeester Apr 4, 2017

Member

powerpc failure known flaky, let's merge this 馃懠

Member

vdemeester commented Apr 4, 2017

powerpc failure known flaky, let's merge this 馃懠

@vdemeester vdemeester merged commit d7e7aa6 into moby:master Apr 4, 2017

5 of 6 checks passed

powerpc Jenkins build Docker-PRs-powerpc 1289 has encountered an error
Details
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 32448 has succeeded
Details
janky Jenkins build Docker-PRs 41094 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 12219 has succeeded
Details
z Jenkins build Docker-PRs-s390x 1076 has succeeded
Details

@GordonTheTurtle GordonTheTurtle added this to the 17.05.0 milestone Apr 4, 2017

@mlaventure mlaventure deleted the mlaventure:fix-healthcheck-rm-race branch Apr 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment