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

Tag on-demand instances at creation time #496

Merged
merged 8 commits into from
Jul 2, 2020

Conversation

clintoncwolfe
Copy link
Contributor

@clintoncwolfe clintoncwolfe commented Jun 17, 2020

Description

This is started as an adoption of #414 by @sumitag, adding tests and rebasing. #414 applies tags at instance creation time to on-demand instances.

This PR then goes on to convert spot instance requests to use the create_instances() API call, which allows tagging at creation time. This allows all tagging to happen at creation time, so all post-hoc tagging code is removed.

There are some fallout effects from using create_instances() instead of spot_instance_request():

  • Since it is not a spot instance request, the is no Spot Instance Request ID to report to the user.
  • Spot Instance Requests internally retry the request until it succeeds or the request expires. create_instances() is one-shot, so kitchen-ec2 must explicitly retry the call.
  • There is a feature that allows the user to set the spot price to "on-demand" to get the on-demand price; internally this sets the bid price to an empty string. This is not allowed in the create_instances() API. Currently this is not solved, and the feature is broken pending discussion. Update: When on-demand is specified, the max_price field may simply be omitted, causing the bid price to be set to the on-demand price.

Issues Resolved

Fixes #360
Fixes #480
Fixes #464

I suggest PRs #364 and #414 be closed.

Check List

  • All tests pass. See TESTING.md for details.
  • New functionality includes testing.
  • New functionality has been documented in the README if applicable.

sumitag and others added 3 commits June 16, 2020 16:59
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
@clintoncwolfe
Copy link
Contributor Author

clintoncwolfe commented Jun 18, 2020

After some conversation with @tyler-ball, we think we may be able to use the AWS run_instances() call to tag spot instances at creation time as well. I'm placing this into draft status while I do a rewrite.

@clintoncwolfe clintoncwolfe marked this pull request as draft June 18, 2020 18:20
This allows spot instances to be tagged at creation time.
The tag-after-create code still needs to be removed, and there
is still some refactoring to be done.
Also, the ability to use "ondemand" as the spot price is
currently broken.

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
@clintoncwolfe clintoncwolfe marked this pull request as ready for review June 25, 2020 18:34
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Copy link
Contributor

@tyler-ball tyler-ball left a comment

Choose a reason for hiding this comment

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

1 small update then :shipit:

Nice work @clintoncwolfe! This also removes about 2x as many lines as it adds 💪

README.md Outdated Show resolved Hide resolved
Co-authored-by: Tyler Ball <tball@chef.io>
@tas50 tas50 merged commit 6e5d64d into test-kitchen:master Jul 2, 2020
@tas50
Copy link
Member

tas50 commented Jul 2, 2020

Way cool. Thanks @clintoncwolfe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants