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

Allow non-persistent spot requests #3311

Merged
merged 2 commits into from
Oct 15, 2015
Merged

Allow non-persistent spot requests #3311

merged 2 commits into from
Oct 15, 2015

Conversation

caarlos0
Copy link
Contributor

Where I work we are using terraform to scale our CI workers, with spot instances. The workers will shutdown after some time of inactivity, and the instance shutdown behavior is set to terminate.

But, since terraform forces us to create the spot request as persistent, when we shut the instance down and terminate it, another instance will be launched, so, we can't automatically scale down.

Note that we don't use terraform to manage those instances, just to launch them.

This change should enable us to do that.

I also updated docs.

Thanks!

@jonathanbeber
Copy link

+1 👏

@apparentlymart
Copy link
Contributor

Cool!

If I'm understanding this correctly, then requests created with a spot_type of "one-time" will be created by Terraform, and then they can later close of their own accord. terraform refresh (and thus plan) will then see the spot_bid_status change to show it's closed, but no action will be taken other than that. If you wanted to create a new spot instances it would then be necessary to taint the request to force Terraform to create a new one.

For your CI use-case, are you just using Terraform to create a fresh request and then discarding the terraform state and letting the resources clean up themselves?

Does CancelSpotInstanceRequests complete successfully even if the spot request is already closed?

@caarlos0
Copy link
Contributor Author

@apparentlymart yes, for example, if I terminate the machine created from a spot request of type one-time, the spot request will be set to closed.

So, what I want to do is:

  • create a spot request with shutdown behavior set to terminate and with this new property set to one-time
  • worker is started up and work for some time
  • after some time without nothing to do, a script in the crontab of the worker will shut itself down, thus, closing the spot request

Every time I run terraform for this manner, I remove the tf state file so it would not mess with the last resource.

This way, I can easily scale the workers not spending too much money.

For your CI use-case, are you just using Terraform to create a fresh request and then discarding the terraform state and letting the resources clean up themselves?

Exactly.

Does CancelSpotInstanceRequests complete successfully even if the spot request is already closed?

I don't know, I'm just testing all this stuff right now, so I closed the spot request myself.

@apparentlymart
Copy link
Contributor

Could you test the behavior of CancelSpotInstanceRequests by spinning up a spot instance, letting it terminate as you said, and then running terraform destroy to see if it completes successfully?

Of course this doesn't really matter for your workflow since you're never going to try to destroy a spot request, but it'd be good for it to behave in a consistent manner so that users with other usage patterns don't find themselves stuck with an un-destroyable resource if their instance happens to terminate before they destroy it with Terraform.

@caarlos0
Copy link
Contributor Author

@apparentlymart OK, I did two tests:

  1. Created a one-time spot request and, while it still alive, execute terraform destroy;
  2. Created a one-time spot request, terminated the instance and execute terraform destroy.

Both cases worked as expected.

@apparentlymart
Copy link
Contributor

Awesome! Thanks for indulging me. 😄

@caarlos0
Copy link
Contributor Author

@apparentlymart haha, no problem 🍻

@caarlos0
Copy link
Contributor Author

@radeksimko any updates on this?

@apparentlymart
Copy link
Contributor

LGTM 🚢

apparentlymart added a commit that referenced this pull request Oct 15, 2015
@apparentlymart apparentlymart merged commit 1300f16 into hashicorp:master Oct 15, 2015
@apparentlymart
Copy link
Contributor

Merging this resulted in a failed build but it appears to be caused by Azure/azure-sdk-for-go#225 rather than anything in this changeset.

@caarlos0 caarlos0 deleted the allow-non-persistent-spot-requests branch October 15, 2015 12:15
@caarlos0
Copy link
Contributor Author

@apparentlymart thanks!

@ghost
Copy link

ghost commented Apr 30, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 30, 2020
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.

4 participants