Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel spot requests when associated instances are terminated #48

Merged
merged 3 commits into from Jul 19, 2021

Conversation

henryzxu
Copy link
Contributor

@henryzxu henryzxu commented Jul 16, 2021

When deleting proxies filled by one-time spot requests, the instances are terminated, but the spot request itself is not cancelled, leaving the door open to being filled again in the future when not associated with cloudproxy. This PR deletes the spot request if it exists in the delete_proxy() function. Related to #42, but unclear if applicable to persistent spot requests.

When using one-time spot requests, the instances are terminated, but the spot request is not cancelled, leaving the door open to being filled again in the future. This PR deletes the spot request if it exists in the aws delete_proxy() function. Related to claffin#42, but unclear if applicable to persistent spot requests.
@claffin claffin linked an issue Jul 18, 2021 that may be closed by this pull request
@henryzxu
Copy link
Contributor Author

Looking into this a little further, per https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/spot-requests.html, it looks like one-time requests are closed if the instance is terminated, and this pr proactively cancels the request instead of waiting for ec2 to do it eventually.

Additional testing seems to indicate we can also cancel persistent spot requests and thus prevent future undesired spin ups of associated instances per #42. I've updated the condition to be applicable to both spot requests accordingly.

@claffin
Copy link
Owner

claffin commented Jul 19, 2021

Excellent PR, thank you for your contribution.

I've tested this now, it does what's described and fixes issue #42. I will merge.

@claffin claffin merged commit 44272eb into claffin:main Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ghost proxies when destroying using spot in AWS
2 participants