Skip to content

Force endpoint delete#862

Merged
mrjana merged 2 commits intomoby:masterfrom
mavenugo:epcleanup
Jan 12, 2016
Merged

Force endpoint delete#862
mrjana merged 2 commits intomoby:masterfrom
mavenugo:epcleanup

Conversation

@mavenugo
Copy link
Copy Markdown
Contributor

@mavenugo mavenugo commented Jan 9, 2016

There are cases in multi-host networking where an ungraceful host shutdown (and the docker daemon in that host never comes back up again) could result in endpoint resources (such as ip-address) being locked up. This PR provides an ability for higher level orchestration systems to force an endpoint cleanup from the datastore and hence the resources can be reused.

Signed-off-by: Madhu Venugopal madhu@docker.com

@mavenugo
Copy link
Copy Markdown
Contributor Author

mavenugo commented Jan 9, 2016

@mrjana @vieux as discussed, {with this PR + appropriate changes in docker/docker} swarm or other higher level orchestration systems will be able to reclaim the endpoint resources.
This along with Preferred IP will satisfy the container rescheduling needs as described in docker-archive/classicswarm#1578.

@vieux
Copy link
Copy Markdown
Contributor

vieux commented Jan 11, 2016

LGTM

@mavenugo mavenugo force-pushed the epcleanup branch 2 times, most recently from 6782564 to 67bc1f9 Compare January 12, 2016 03:15
@mavenugo mavenugo changed the title WIP : Forced endpoint delete Force endpoint delete Jan 12, 2016
@mavenugo mavenugo force-pushed the epcleanup branch 2 times, most recently from 8a2d892 to b291b59 Compare January 12, 2016 03:33
@mavenugo
Copy link
Copy Markdown
Contributor Author

@mrjana PTAL. Took care of all the changes as we discussed. (including the IT)

endpoint.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please make this error message more clearer for the user? Some thing like the remote host %s where the connection is active is alive? Also please remove the : and add it next to where you say host

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@vieux based on the design-review comments from @mrjana we decided to add this check to prevent the -f from cleaning up the endpoint when the remote host holding the endpoint is still alive. At the engine level, the node liveness is a factor of discovery timer (by default 20 * 3 sec). This means, swarm cannot push a force delete till the engine catches up with the node failure.
We need this check in order for protecting the engine from invalid forced delete which can lead to inconsistent states and the user can get confused.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I disagree with this change, force is force.

In engine you can docker rmi -f <image> even if the engine has containers using this image.
I agree it can lead to inconsistent states, but the user did force it.

Signed-off-by: Madhu Venugopal <madhu@docker.com>
Signed-off-by: Madhu Venugopal <madhu@docker.com>
@mrjana
Copy link
Copy Markdown
Contributor

mrjana commented Jan 12, 2016

LGTM

mrjana added a commit that referenced this pull request Jan 12, 2016
@mrjana mrjana merged commit 8407851 into moby:master Jan 12, 2016
@mavenugo mavenugo deleted the epcleanup branch July 5, 2016 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants