-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
provider/aws: adds the ability to apply tags to spot request instances #8515
provider/aws: adds the ability to apply tags to spot request instances #8515
Conversation
builtin/providers/aws/tags.go
Outdated
id := d.Id() | ||
|
||
// Check for a Spot Instance Requests as it requires special | ||
// handeling, the tagging process will tag the spot request it self |
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.
Typo.
@richardbowden thank you! Would you be able to add acceptance tests? So that we can ensure that the tags are being set and things hold water? |
@kwilczynski will make the changes mentioned :), will also get some tests done |
900a654
to
c3a4636
Compare
tagging code paths are tested in two ways, the original path will be tested as before using the existing resource types that test tags, for example |
@stack72 hey, just checking to see if anything else needs to be done for this PR ? Happy to do more |
changes setTags to the entry point for tagging, this looks for a spot request, but cloud look for others if needed, extracts the instance id from the sport request and passes it to setTagsActual, if not a spot request things work as before
050a606
to
28e476f
Compare
I guess it needs to be redone due to it not being merged back when the PR was created and mergeable. does not seem to of been a feature that was needed.
… On 9 Oct 2017, at 20:04, David Ficociello ***@***.***> wrote:
Any update on this PR? Has this been solved by another PR? This has been in conflict mode for a year.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#8515 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATAv9xqq6ufOxE0Mh1J5c97OrB7mGNfks5sqm5MgaJpZM4Ju6vO>.
|
Hello @richardbowden, and thanks for working on this! As part of the the Terraform 0.10 release earlier this year, all of the Terraform providers were moved to their own repositories in the terraform-providers GitHub organization, and removed from the Terraform Core repository. Unfortunately due to the fact that new issues and pull requests are being opened constantly, it was not possible for the various provider maintainers to merge all outstanding pull requests before this split, and there is no automatic way to migrate a pull request to a new repository. As a result, this pull request can sadly no longer be applied as-is, and so I'm going to close it. If you or someone else has the time and motivation to apply same changes to the Thanks again for working on this, and sorry it was not able to be merged before the provider repository changes. |
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. |
Fixes #3263
adds the ability to tag instances that are created by spot instance requests