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

resource/aws_appautoscaling_policy: Match correct policy when multiple policies with same name #3012

Merged
merged 3 commits into from
Jan 17, 2018

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Jan 16, 2018

DescribeScalingPolicies allows using ResourceId and ScalableDimension for additional filtering.

Closes #751

make testacc TEST=./aws TESTARGS='-run=TestAccAWSAppautoScalingPolicy'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSAppautoScalingPolicy -timeout 120m
=== RUN   TestAccAWSAppautoScalingPolicy_basic
--- PASS: TestAccAWSAppautoScalingPolicy_basic (39.47s)
=== RUN   TestAccAWSAppautoScalingPolicy_nestedSchema
--- PASS: TestAccAWSAppautoScalingPolicy_nestedSchema (29.96s)
=== RUN   TestAccAWSAppautoScalingPolicy_spotFleetRequest
--- PASS: TestAccAWSAppautoScalingPolicy_spotFleetRequest (70.29s)
=== RUN   TestAccAWSAppautoScalingPolicy_dynamoDb
--- PASS: TestAccAWSAppautoScalingPolicy_dynamoDb (33.25s)
=== RUN   TestAccAWSAppautoScalingPolicy_multiplePolicies
--- PASS: TestAccAWSAppautoScalingPolicy_multiplePolicies (35.19s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	208.225s

@bflad bflad added bug Addresses a defect in current functionality. service/applicationautoscaling labels Jan 16, 2018
@bflad bflad added this to the v1.7.1 milestone Jan 16, 2018
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

LGTM. I assume the bug only appears intermittently, so there's no point in adding test for it?

@bflad
Copy link
Contributor Author

bflad commented Jan 17, 2018

I'll add a failing then passing acceptance test. It should currently be failing when there are duplicately named policies on different resources under the same service. 😄

@bflad
Copy link
Contributor Author

bflad commented Jan 17, 2018

New failing acceptance test (occasionally) on master:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSAppautoScalingPolicy_multiplePoliciesSameName'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSAppautoScalingPolicy_multiplePoliciesSameName -timeout 120m
=== RUN   TestAccAWSAppautoScalingPolicy_multiplePoliciesSameName
--- FAIL: TestAccAWSAppautoScalingPolicy_multiplePoliciesSameName (34.18s)
	testing.go:513: Step 0 error: Check failed: Check 8/10 error: aws_appautoscaling_policy.read2: Attribute 'resource_id' expected "table/tf-autoscaled-table-yehp3", got "table/tf-autoscaled-table-gn7x8"
	testing.go:573: Error destroying resource! WARNING: Dangling resources
		may exist. The full state and error is shown below.

		Error: Error applying: 1 error(s) occurred:

		* aws_appautoscaling_policy.read1 (destroy): 1 error(s) occurred:

		* aws_appautoscaling_policy.read1: Failed to delete autoscaling policy: ObjectNotFoundException: No scaling policy found for service namespace: dynamodb, resource ID: table/tf-autoscaled-table-gn7x8, scalable dimension: dynamodb:table:ReadCapacityUnits, policy name: tf-appautoscaling-policy-omank-read
			status code: 400, request id: 8e2a479d-fb8f-11e7-918f-ef45d8ac7d7c

		State: aws_appautoscaling_policy.read1:
		  ID = tf-appautoscaling-policy-omank-read
		  provider = provider.aws
		  arn = arn:aws:autoscaling:us-west-2:193075746082:scalingPolicy:307eb45b-60e3-4095-8d0e-68d8a8dcb13f:resource/dynamodb/table/tf-autoscaled-table-gn7x8:policyName/tf-appautoscaling-policy-omank-read
		  name = tf-appautoscaling-policy-omank-read
		  policy_type = TargetTrackingScaling
		  resource_id = table/tf-autoscaled-table-gn7x8
		  scalable_dimension = dynamodb:table:ReadCapacityUnits
		  service_namespace = dynamodb
		  step_scaling_policy_configuration.# = 0
		  target_tracking_scaling_policy_configuration.# = 1
		  target_tracking_scaling_policy_configuration.0.customized_metric_specification.# = 0
		  target_tracking_scaling_policy_configuration.0.disable_scale_in = false
		  target_tracking_scaling_policy_configuration.0.predefined_metric_specification.# = 1
		  target_tracking_scaling_policy_configuration.0.predefined_metric_specification.0.predefined_metric_type = DynamoDBReadCapacityUtilization
		  target_tracking_scaling_policy_configuration.0.predefined_metric_specification.0.resource_label =
		  target_tracking_scaling_policy_configuration.0.scale_in_cooldown = 10
		  target_tracking_scaling_policy_configuration.0.scale_out_cooldown = 10
		  target_tracking_scaling_policy_configuration.0.target_value = 70

		  Dependencies:
		    aws_appautoscaling_target.read1
		aws_appautoscaling_target.read1:
		  ID = table/tf-autoscaled-table-gn7x8
		  provider = provider.aws
		  max_capacity = 10
		  min_capacity = 1
		  resource_id = table/tf-autoscaled-table-gn7x8
		  role_arn = arn:aws:iam::193075746082:role/aws-service-role/dynamodb.application-autoscaling.amazonaws.com/AWSServiceRoleForApplicationAutoScaling_DynamoDBTable
		  scalable_dimension = dynamodb:table:ReadCapacityUnits
		  service_namespace = dynamodb

		  Dependencies:
		    aws_dynamodb_table.dynamodb_table_test1
		aws_dynamodb_table.dynamodb_table_test1:
		  ID = tf-autoscaled-table-gn7x8
		  provider = provider.aws
		  arn = arn:aws:dynamodb:us-west-2:193075746082:table/tf-autoscaled-table-gn7x8
		  attribute.# = 1
		  attribute.3780938259.name = FooKey
		  attribute.3780938259.type = S
		  global_secondary_index.# = 0
		  hash_key = FooKey
		  local_secondary_index.# = 0
		  name = tf-autoscaled-table-gn7x8
		  read_capacity = 1
		  write_capacity = 1
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	34.223s
make: *** [testacc] Error 1

With updated code:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSAppautoScalingPolicy'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSAppautoScalingPolicy -timeout 120m
=== RUN   TestAccAWSAppautoScalingPolicy_basic
--- PASS: TestAccAWSAppautoScalingPolicy_basic (49.44s)
=== RUN   TestAccAWSAppautoScalingPolicy_nestedSchema
--- PASS: TestAccAWSAppautoScalingPolicy_nestedSchema (52.14s)
=== RUN   TestAccAWSAppautoScalingPolicy_spotFleetRequest
--- PASS: TestAccAWSAppautoScalingPolicy_spotFleetRequest (71.79s)
=== RUN   TestAccAWSAppautoScalingPolicy_dynamoDb
--- PASS: TestAccAWSAppautoScalingPolicy_dynamoDb (36.82s)
=== RUN   TestAccAWSAppautoScalingPolicy_multiplePoliciesSameName
--- PASS: TestAccAWSAppautoScalingPolicy_multiplePoliciesSameName (38.36s)
=== RUN   TestAccAWSAppautoScalingPolicy_multiplePoliciesSameResource
--- PASS: TestAccAWSAppautoScalingPolicy_multiplePoliciesSameResource (35.88s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	284.484s

@bflad bflad merged commit ef8f776 into master Jan 17, 2018
@bflad bflad deleted the b-aws_appautoscaling_policy-precise-get branch January 17, 2018 14:13
bflad added a commit that referenced this pull request Jan 17, 2018
@bflad
Copy link
Contributor Author

bflad commented Jan 22, 2018

This has been released in terraform-provider-aws version 1.7.1. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

drewsonne pushed a commit to drewsonne/terraform-provider-aws that referenced this pull request Mar 3, 2018
@ghost
Copy link

ghost commented Apr 8, 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 Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terraform assumes aws_appautoscaling_policy names are globally unique
2 participants