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

adds the ability to apply tags to spot request instances #3481

Closed

Conversation

richardbowden
Copy link
Contributor

@richardbowden richardbowden commented Feb 22, 2018

I have ported over PR hashicorp/terraform#8515.

This PR allows the tagging of instances that have been requested via spot. Tags are applied to the spot_instance_request are only applied to the instance when wait_for_fulfillment = true is set so we can get the instance ID and tag it. If wait_for_fulfillment = false then normal behaviour if followed.

I also ran tests locally.

❯ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSpotInstanceRequest_SirAndInstanceTags' ⏎
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSSpotInstanceRequest_SirAndInstanceTags -timeout 120m
=== RUN TestAccAWSSpotInstanceRequest_SirAndInstanceTags
--- PASS: TestAccAWSSpotInstanceRequest_SirAndInstanceTags (161.57s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 22, 2018
@Ninir Ninir added enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. labels Feb 22, 2018
@richardbowden
Copy link
Contributor Author

noted that changes to the base code has resulted in conflicts, will take a look at this.

@aeschright aeschright requested a review from a team June 25, 2019 18:50
@ThinkBriK
Copy link

Any news on this ?

@richardbowden
Copy link
Contributor Author

I don’t think I am going to have time any time soon, if someone else wants to take a look, go for it.

@bflad
Copy link
Contributor

bflad commented Jan 23, 2020

Hi @richardbowden 👋 Thank you for submitting this and sorry for the delayed review.

Since this pull request was introduced, there have been quite a few changes to how resource testing is performed and how resource tagging is implemented (newer Contributing Guide section). We would also prefer to see this tagging implementation occur as a separate argument (e.g. instance_tags) to prevent any confusion about the source of the tagging and allow operators to apply differing tags between the Spot Instance Request and the Instance itself.

Given those and the existing merge conflicts, we are going to close this pull request for now. If you or anyone has the motivation to continue with this functionality, please submit a new pull request against the latest codebase, following the guidance above, and we will be happy to take a fresh look. 👍

@bflad bflad closed this Jan 23, 2020
@ghost
Copy link

ghost commented Mar 27, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants