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: Fix spurious aws_spot_fleet_request diffs #12437

Merged
merged 3 commits into from
Mar 7, 2017
Merged

provider/aws: Fix spurious aws_spot_fleet_request diffs #12437

merged 3 commits into from
Mar 7, 2017

Conversation

Bowbaq
Copy link
Contributor

@Bowbaq Bowbaq commented Mar 4, 2017

This fixes the following issues:

  • The default value for launch_specification.monitoring was "", which got translated to false by AWS, generating a spurious diff
  • The default value for launch_specification.ebs_optimized was "", which got translated to false by AWS, generating a spurious diff
  • launch_specification.vpc_security_group_ids were not read back from AWS, generating a spurious diff
  • launch_specification.vpc_security_group_ids were not handled properly when setting 'associate_public_ip_address = true'
  • launch_specification.user_data was not handled consistently (sometimes hashed, sometimes not), generating a spurious diff

I also cleaned up some dead code handling a security_groups property since there is no way for the user to define it.

@stack72 it seems you've been reviewing spot fleet related PRs, so tagging you here.

One final note: it may be a good idea to reuse the same hash function for root/ebs/ephemeral volumes across this resource, aws_instance and aws_launch_configuration?

@stack72
Copy link
Contributor

stack72 commented Mar 6, 2017

Hi @Bowbaq

Thanks for the work here - currently, this change breaks our acceptance tests due to the change of hash

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSpotFleetRequest_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/06 16:03:57 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 (116.72s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion (62.87s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList
--- FAIL: TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList (58.03s)
	testing.go:265: Step 0 error: Check failed: 2 error(s) occurred:

		* Check 4/5 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.1590006269.availability_zone' not found
		* Check 5/5 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.3809475891.availability_zone' not found
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList (85.96s)
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz
--- FAIL: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz (54.30s)
	testing.go:265: Step 0 error: Check failed: 4 error(s) occurred:

		* Check 4/7 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.1590006269.instance_type' not found
		* Check 5/7 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.1590006269.availability_zone' not found
		* Check 6/7 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.3079734941.instance_type' not found
		* Check 7/7 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.3079734941.availability_zone' not found
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet (93.65s)
=== RUN   TestAccAWSSpotFleetRequest_overriddingSpotPrice
--- FAIL: TestAccAWSSpotFleetRequest_overriddingSpotPrice (58.69s)
	testing.go:265: Step 0 error: Check failed: 4 error(s) occurred:

		* Check 5/8 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.522395050.spot_price' not found
		* Check 6/8 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.522395050.instance_type' not found
		* Check 7/8 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.1590006269.spot_price' not found
		* Check 8/8 error: aws_spot_fleet_request.foo: Attribute 'launch_specification.1590006269.instance_type' not found

I am going to try and fix this up before i can merge this

Hope this is all ok - if so, stick with me for a day or 2

Paul

@stack72
Copy link
Contributor

stack72 commented Mar 7, 2017

Hi @Bowbaq

I have fixed up the tests now and all looks good - I am going to manually merge this to get the commit for the test results in

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSpotFleetRequest_'                                                 2 ↵ ✹
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/07 15:11:16 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 (113.45s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion (60.39s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList (63.28s)
=== RUN   TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList (81.73s)
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz (65.56s)
=== RUN   TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet (82.80s)
=== RUN   TestAccAWSSpotFleetRequest_overriddingSpotPrice
--- PASS: TestAccAWSSpotFleetRequest_overriddingSpotPrice (61.54s)

Thanks for the work

Paul

@stack72 stack72 merged commit 0af10de into hashicorp:master Mar 7, 2017
@Bowbaq Bowbaq deleted the mb-fix-spot-fleet-request branch March 7, 2017 17:48
@Bowbaq
Copy link
Contributor Author

Bowbaq commented Mar 7, 2017

@stack72 forgot to check the tests, my bad, thanks for fixing

@stack72
Copy link
Contributor

stack72 commented Mar 8, 2017

👍

@ghost
Copy link

ghost commented Apr 16, 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 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants