Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

Ensure proper cleanup of dangling resources in case of failure #4317

Merged
merged 1 commit into from Jan 28, 2018

Conversation

nolith
Copy link
Contributor

@nolith nolith commented Nov 23, 2017

In AWS EC2 driver we start creating resources that simply get lost in case
of failure during instance creation.

The worst case scenario is when a SpotInstance price is too low to be fulfilled
before the timeout.

In such case we already created a Key Pair, but we are not going to persist
config.json due to the failure, and we will never be able to delete such key.

Moreover the SpotInstanceRequest will remain in AWS and if it will be fulfilled
later a new instance will be created outside of docker-machine control.

This patch ensures that a proper cleanup is done in case of failure in
amazonec2.Create()

Furthermore docker-machine rm is now able to handle properly AWS machines
failed during creation thanks to this two new conditions:

  1. Key Pair deletion will not fail in case of missing KeyPairName
  2. Instance termination will not fail in case of missing InstanceId

Closes #3555

In AWS EC2 driver we start creating resources that simply get lost in case
of failure during instance creation.

The worst case scenario is when a SpotInstance price is too low to be fulfilled
before the timeout.

In such case we already created a Key Pair, but we are not going to persist
`config.json` due to the failure, and we will never be able to delete such key.

Moreover the SpotInstanceRequest will remain in AWS and if it will be fulfilled
later a new instance will be created outside of docker-machine control.

This patch ensures that a proper cleanup is done in case of failure in
`amazonec2.Create()`

Furthermore `docker-machine rm` is now able to handle properly AWS machines
failed during creation thanks to this two new conditions:

1. Key Pair deletion will not fail in case of missing `KeyPairName`
2. Instance termination will not fail in case of missing `InstanceId`

Signed-off-by: Alessio Caiazza <acaiazza@gitlab.com>
Copy link
Member

@dgageot dgageot left a comment

Choose a reason for hiding this comment

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

LGTM

@kostyrev
Copy link

kostyrev commented Dec 8, 2017

When will this be merged?

@dgageot
Copy link
Member

dgageot commented Dec 12, 2017

ping @shin-

@tpresa
Copy link

tpresa commented Jan 2, 2018

Any updates on when this will be merged?
ping @shin-

@dgageot dgageot merged commit 83d186e into docker:master Jan 28, 2018
@shin- shin- added this to the 0.14.0 milestone Feb 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants