resource/aws_autoscaling_group: Fix issue preventing the removal of load_balancers when switching to target_group_arns
#9478
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
size/L
nywilken
changed the title
load_balancers when switching to target_group_arns
bflad
reviewed
Jul 24, 2019
| @@ -78,6 +78,7 @@ func resourceAwsAutoscalingGroup() *schema.Resource { | |||
| "version": { | |||
| Type: schema.TypeString, | |||
| Optional: true, | |||
| Default: "$$Default", | |||
There was a problem hiding this comment.
I believe this fix should be the in test configuration and not in the resource. 😉 If I recall its a difference in Terraform 0.12's configuration handling regarding $ escaping.
There was a problem hiding this comment.
I see so within the testing configuration we should not escape $ and instead render the configuration such as Version: "$Latest". Is that correct. I see that the mixed_instance_policy.launch_template.version has an attribute of Default: "$$Deault" should that be removed at some point? Not within this PR.
There was a problem hiding this comment.
I broke this into a separate PR #9488
nywilken
force-pushed
the
b-aws_autoscaling_group-lingering-loadbalancers
branch
2 times, most recently
from
f3467cb to
43669b4
Compare
nywilken
changed the title
load_balancers when switching to target_group_arnsload_balancers when switching to target_group_arns
…ns issue Closes: #5197 Upon configuring an ASG with load_balancers, it was found that swapping the load_balancers argument with a list of target_group_arns would result in no diff for the load_balancers despite them being removed. This issue being the computed property on `load_balancers`, which refreshes the configuration at plan time resulting in `load_balancers` being added back to the configuration as they are still part of the referenced infrastructure. This change fixes that be removing the `Computed` property on both `load_balancers` and `target_group_arns` to ensure Terraform always honors the arguments within a provided configuration. ``` --- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer (390.68s) --- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer_ToTargetGroup (468.74s) ```
nywilken
force-pushed
the
b-aws_autoscaling_group-lingering-loadbalancers
branch
from
43669b4 to
c40d455
Compare
bflad
approved these changes
Jul 25, 2019
There was a problem hiding this comment.
LGTM! 🚀
Test failure below unrelated:
--- PASS: TestAccAWSAutoScalingGroup_autoGeneratedName (54.94s)
--- PASS: TestAccAWSAutoScalingGroup_classicVpcZoneIdentifier (71.28s)
--- PASS: TestAccAWSAutoScalingGroup_namePrefix (76.66s)
--- PASS: TestAccAWSAutoScalingGroup_launchTemplate (80.73s)
--- PASS: TestAccAWSAutoScalingGroup_VpcUpdates (84.41s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy (46.57s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandAllocationStrategy (43.83s)
--- PASS: TestAccAWSAutoScalingGroup_withMetrics (123.25s)
--- PASS: TestAccAWSAutoScalingGroup_ALB_TargetGroups (128.02s)
--- PASS: TestAccAWSAutoScalingGroup_serviceLinkedRoleARN (133.93s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotAllocationStrategy (51.99s)
--- PASS: TestAccAWSAutoScalingGroup_LaunchTemplate_IAMInstanceProfile (117.82s)
--- PASS: TestAccAWSAutoScalingGroup_launchTemplate_update (175.06s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandPercentageAboveBaseCapacity (93.12s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_LaunchTemplateSpecification_Version (44.75s)
--- PASS: TestAccAWSAutoScalingGroup_withPlacementGroup (179.74s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_OnDemandBaseCapacity (106.10s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotInstancePools (75.88s)
--- PASS: TestAccAWSAutoScalingGroup_terminationPolicies (198.96s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_LaunchTemplateSpecification_LaunchTemplateName (72.22s)
--- PASS: TestAccAWSAutoScalingGroup_initialLifecycleHook (201.72s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_InstancesDistribution_SpotMaxPrice (80.84s)
--- FAIL: TestAccAWSAutoScalingGroup_emptyAvailabilityZones (205.95s)
--- PASS: TestAccAWSAutoScalingGroup_MixedInstancesPolicy_LaunchTemplate_Override_InstanceType (41.77s)
--- PASS: TestAccAWSAutoScalingGroup_suspendingProcesses (262.62s)
--- PASS: TestAccAWSAutoScalingGroup_enablingMetrics (272.32s)
--- PASS: TestAccAWSAutoScalingGroup_basic (277.33s)
--- PASS: TestAccAWSAutoScalingGroup_tags (308.56s)
--- PASS: TestAccAWSAutoScalingGroup_ALB_TargetGroups_ELBCapacity (437.15s)
--- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer_ToTargetGroup (473.15s)
--- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer (504.07s)
|
Please see #9513 for the unexpected behavior change with this pull request. I'm going to lock this pull request only to encourage following the new issue and consolidating any discussions/efforts there. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Community Note
Closes: #5197
Release note for CHANGELOG:
Upon configuring an ASG with load_balancers, it was found that swapping the load_balancers argument with a list of target_group_arns would result in no diff for the load_balancers despite them being removed.This issue being the computed property on
load_balancers, which refreshes the configuration at plan time resulting inload_balancersbeing added back to the configuration as they are still part of the referenced infrastructure. This change removes theComputedproperty on bothload_balancersandtarget_group_arnsto ensure Terraform always honors the arguments within a provided configuration.Output from acceptance testing: