-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Add 'tags' support to aws_spot_fleet_request #2042
Add 'tags' support to aws_spot_fleet_request #2042
Conversation
@@ -377,6 +382,21 @@ func buildSpotFleetLaunchSpecification(d map[string]interface{}, meta interface{ | |||
} | |||
} | |||
|
|||
if v, ok := d["tags"]; ok { |
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.
Do you mind adding an extra check here? i.e.
if m, ok := d["tags"].(map[string]interface{}); ok && len(m) > 0 {
otherwise this would break all configs without tags, see output from relevant acceptance tests:
=== RUN TestAccAWSSpotFleetRequest_associatePublicIpAddress
--- FAIL: TestAccAWSSpotFleetRequest_associatePublicIpAddress (682.03s)
testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:
* aws_spot_fleet_request.foo: 1 error(s) occurred:
* aws_spot_fleet_request.foo: Error requesting spot fleet: [WARN] Error creating Spot fleet request, retrying: InvalidSpotFleetRequestConfig: Tag value cannot be null. Use empty string instead.
status code: 400, request id: 42b4061d-3214-4bae-9fba-0751e13458ab
Done! Thanks, for some reason this is one of the test cases I didn't test. That's what the acceptance tests are for, I suppose. Please let me know if there are any other issues you see with the change. |
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.
Thanks for the changes, I just added an acceptance test to make sure we don't break related functionality in the future & pushed that into your branch.
🚀
@radeksimko in which TF version will this be available? |
This was previously released in terraform-provider-aws version 1.2.0 and has been available in all releases since. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
You could also search the issue number 2042 in CHANGELOG. |
Sorry if I misuderstood but it seems to me that "tags" are possible only in the Indeed, if I apply the example in the documentation by using a tag like this :
the tag is in fact only on the spot instance created by the request but not on the request itself (whereas its possible to add tags in AWS conole for this resource) And if I try to add a
I have the following error :
I thought this PR would be to add tag on the resource directly...Did I misunderstood the PR here ? |
Hi, we encounter the same problem as @yogeek.
I upgraded the AWS provider to version 1.42 and this still not work. Do you know what's the problem ? |
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! |
This adds support for tags on spot fleet requests. I tried to follow the pattern of the nearly-identical
tags
block inresource_aws_instance.go
(the difference being Spot Fleet only supportsinstance
, notvolume
, as a resource type).Since the documentation for
aws_spot_fleet_request
simply references theaws_instance
documentation, I didn't add any additional info abouttags
, but I did include atags
block in one of the examples.This should resolve feature request #1232.
Closes #1232