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

provider/aws: AWS SpotFleet Requests now works with Subnets and AZs #8320

Merged
merged 2 commits into from
Aug 24, 2016

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Aug 19, 2016

Fixes #8150
Fixes #8203

Also added a complete set of tests that reflect all of the use cases
that Amazon document
http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/spot-fleet-examples.html

It is important to note there that Terraform will be suggesting that
users create multiple launch configurations rather than AWS's version of
combing values into CSV based parameters. This will ensure that we are
able to enforce the correct state

Also note that associate_public_ip_address now defaults to false - a migration has been
included in this PR to migration users of this functionality. This needs
to be noted in the changelog. The last part of changing functionality
here is waiting for the state of the request to become active. Before
we get to this state, we cannot guarantee that Amazon have accepted the
request or it could have failed validation.

% make testacc TEST=./builtin/providers/aws
% TESTARGS='-run=TestAccAWSSpotFleetRequest_'
% 2 ↵
==> Checking that code complies with gofmt requirements...
/Users/stacko/Code/go/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/22 15:44:21 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v
-run=TestAccAWSSpotFleetRequest_ -timeout 120m
=== RUN   TestAccAWSSpotFleetRequest_changePriceForcesNewRequest
--- PASS: TestAccAWSSpotFleetRequest_changePriceForcesNewRequest (133.90s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion (76.67s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList (75.22s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList (96.95s)
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz (74.44s)
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet (97.82s)
=== RUN   TestAccAWSSpotFleetRequest_overriddingSpotPrice
--- PASS: TestAccAWSSpotFleetRequest_overriddingSpotPrice (76.22s)
=== RUN   TestAccAWSSpotFleetRequest_diversifiedAllocation
--- PASS: TestAccAWSSpotFleetRequest_diversifiedAllocation (79.81s)
=== RUN   TestAccAWSSpotFleetRequest_withWeightedCapacity
--- PASS: TestAccAWSSpotFleetRequest_withWeightedCapacity (77.15s)
=== RUN   TestAccAWSSpotFleetRequest_CannotUseEmptyKeyName
--- PASS: TestAccAWSSpotFleetRequest_CannotUseEmptyKeyName (0.00s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    788.184s

@stack72 stack72 changed the title Initial commit [WIP] Making AWS SpotFleet Requests work with Subnets and AZs Aug 19, 2016
@stack72 stack72 force-pushed the aws-spot-fleet-request branch 4 times, most recently from 7bac93a to 5626f66 Compare August 22, 2016 14:59
@stack72 stack72 changed the title [WIP] Making AWS SpotFleet Requests work with Subnets and AZs provider/aws: AWS SpotFleet Requests now works with Subnets and AZs Aug 22, 2016
@kwilczynski
Copy link
Contributor

This is some amazing work!

subnet_id and availability_zone

Also added a complete set of tests that reflect all of the use cases
that Amazon document
http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/spot-fleet-examples.html

It is important to note there that Terraform will be suggesting that
users create multiple launch configurations rather than AWS's version of
combing values into CSV based parameters. This will ensure that we are
able to enforce the correct state

Also note that `associate_public_ip_address` now defaults to `false` - a migration has been
included in this PR to migration users of this functionality. This needs
to be noted in the changelog. The last part of changing functionality
here is waiting for the state of the request to become `active`. Before
we get to this state, we cannot guarantee that Amazon have accepted the
request or it could have failed validation.

```
% make testacc TEST=./builtin/providers/aws
% TESTARGS='-run=TestAccAWSSpotFleetRequest_'
% 2 ↵
==> Checking that code complies with gofmt requirements...
/Users/stacko/Code/go/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/22 15:44:21 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v
-run=TestAccAWSSpotFleetRequest_ -timeout 120m
=== RUN   TestAccAWSSpotFleetRequest_changePriceForcesNewRequest
--- PASS: TestAccAWSSpotFleetRequest_changePriceForcesNewRequest (133.90s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion (76.67s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList (75.22s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList (96.95s)
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz (74.44s)
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet (97.82s)
=== RUN   TestAccAWSSpotFleetRequest_overriddingSpotPrice
--- PASS: TestAccAWSSpotFleetRequest_overriddingSpotPrice (76.22s)
=== RUN   TestAccAWSSpotFleetRequest_diversifiedAllocation
--- PASS: TestAccAWSSpotFleetRequest_diversifiedAllocation (79.81s)
=== RUN   TestAccAWSSpotFleetRequest_withWeightedCapacity
--- PASS: TestAccAWSSpotFleetRequest_withWeightedCapacity (77.15s)
=== RUN   TestAccAWSSpotFleetRequest_CannotUseEmptyKeyName
--- PASS: TestAccAWSSpotFleetRequest_CannotUseEmptyKeyName (0.00s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    788.184s
```
@stack72 stack72 force-pushed the aws-spot-fleet-request branch from 5626f66 to dcce17b Compare August 22, 2016 15:32
@@ -49,7 +52,7 @@ func resourceAwsSpotFleetRequest() *schema.Resource {
"associate_public_ip_address": &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Default: true,
Default: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 probably will need to document this as breaking?

@jen20
Copy link
Contributor

jen20 commented Aug 24, 2016

Minor question but LGTM other than that. Certainly makes this resource a lot more useful!

@stack72
Copy link
Contributor Author

stack72 commented Aug 24, 2016

@jen20 you are correct - no need for a 1 line helper :) Clutter --

@stack72 stack72 merged commit e524603 into master Aug 24, 2016
@stack72 stack72 deleted the aws-spot-fleet-request branch August 24, 2016 10:08
richardbowden pushed a commit to richardbowden/terraform that referenced this pull request Aug 27, 2016
…ashicorp#8320)

* provider/aws: Change Spot Fleet Request to allow a combination of
subnet_id and availability_zone

Also added a complete set of tests that reflect all of the use cases
that Amazon document
http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/spot-fleet-examples.html

It is important to note there that Terraform will be suggesting that
users create multiple launch configurations rather than AWS's version of
combing values into CSV based parameters. This will ensure that we are
able to enforce the correct state

Also note that `associate_public_ip_address` now defaults to `false` - a migration has been
included in this PR to migration users of this functionality. This needs
to be noted in the changelog. The last part of changing functionality
here is waiting for the state of the request to become `active`. Before
we get to this state, we cannot guarantee that Amazon have accepted the
request or it could have failed validation.

```
% make testacc TEST=./builtin/providers/aws
% TESTARGS='-run=TestAccAWSSpotFleetRequest_'
% 2 ↵
==> Checking that code complies with gofmt requirements...
/Users/stacko/Code/go/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/22 15:44:21 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v
-run=TestAccAWSSpotFleetRequest_ -timeout 120m
=== RUN   TestAccAWSSpotFleetRequest_changePriceForcesNewRequest
--- PASS: TestAccAWSSpotFleetRequest_changePriceForcesNewRequest (133.90s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion (76.67s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList (75.22s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList (96.95s)
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz (74.44s)
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet (97.82s)
=== RUN   TestAccAWSSpotFleetRequest_overriddingSpotPrice
--- PASS: TestAccAWSSpotFleetRequest_overriddingSpotPrice (76.22s)
=== RUN   TestAccAWSSpotFleetRequest_diversifiedAllocation
--- PASS: TestAccAWSSpotFleetRequest_diversifiedAllocation (79.81s)
=== RUN   TestAccAWSSpotFleetRequest_withWeightedCapacity
--- PASS: TestAccAWSSpotFleetRequest_withWeightedCapacity (77.15s)
=== RUN   TestAccAWSSpotFleetRequest_CannotUseEmptyKeyName
--- PASS: TestAccAWSSpotFleetRequest_CannotUseEmptyKeyName (0.00s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    788.184s
```

* Update resource_aws_spot_fleet_request.go
@ghost
Copy link

ghost commented Apr 23, 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 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.

@ghost ghost locked and limited conversation to collaborators Apr 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants