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
Support persistent spot #1102
Support persistent spot #1102
Conversation
80847e2
to
fd9080a
Compare
34950b4
to
5d447d4
Compare
d89e469
to
7184a99
Compare
f2aea25
to
c59d142
Compare
I’ve just tested to deploy my staging environment with this branch and it worked fine for me. Note I haven’t tried out any of the new features added, only whether what I’m using right now still works (I also use a spot instance, but not yet the new flags you’ve added). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's one odd thing:
https://github.com/NixOS/nixops/pull/1102/commits shows the first commit appear twice, and when I try to rebase this PR on top of something else, it complains about an empty commit.
Why is that / can it be fixed?
@nh2 Thanks for the review. i did get @Chakerbh branch and added o ntop of it so i may did something wrong there to cause the commit message to appear twice Ill check and fix that. |
0bca858
to
c685543
Compare
@nh2 So i fixed the latter commits but i was not able to do for the duplicates ones i got a lot of conflicts when rebasing :( . |
@PsyanticY maybe try rebasing and squashing any extra commit + the 2 first commits |
1c85af6
to
587686c
Compare
This means when the instance gets interrupted, the request is still valid, and if the termination behaviour is set to "stop", the instance will start automatically when the request get fulfilled again by AWS. There's no way to achieve this with Boto2, so I changed the call that create the spot request to Boto3. I did some refactoring, so the same call is used when creating both On demand and spot instances.
587686c
to
8f4713e
Compare
@AmineChikhaoui Done squashing the commits, also did re-base the pr on current master. (man that was a pain) |
@PsyanticY thanks ! |
Ok interesting, so here is what happens: I create a persistent spot instance request So, I believe that this PR needs more work to teach nixops to recover cleanly after a termination behavior of stop. The good part is at least it's possible to fake a spot instance termination :) |
(experimenting with some labels to make filtering PRs easy) |
I think the way you did test the persistent spot is wrong since persistent spot relies on stopping the instance when we are out of capacity and restart it when we have. since the instance is stopped the instance id is not lost and we can recover cleanly given that. Terminating a persistent spot is not supported since if you even try to deploy a a persistent spot with
|
Ok I think you're right the annoucement mentioned:
|
8f4713e
to
a1cbeb8
Compare
a1cbeb8
to
28eef04
Compare
This a followup and some fixes to #1011
cc @Chakerbh