terraform-aws-alb icon indicating copy to clipboard operation
terraform-aws-alb copied to clipboard

Changing target groups order results in destroy and re-creation

Open aminclip opened this issue 1 year ago β€’ 6 comments

Description

Following the principles of terraform that one of them is defining the final state, order of target_groups definition shouldn't change their state because their state is independent of the definition order.

Imagine having this target group defined:

target_groups = [
  {
    backend_protocol = "HTTP"
    backend_port     = 80
  },
  {
    backend_protocol = "HTTPS"
    backend_port     = 443
  }
]

if you change the order and have it like this:

target_groups = [
  {
    backend_protocol = "HTTPS"
    backend_port     = 443
  },
  {
    backend_protocol = "HTTP"
    backend_port     = 80
  }
]

it will destroy and recreate target groups, thus causing service outages.

This is the output of such change in "examples/complete-alb":

Plan: 7 to add, 6 to change, 7 to destroy.

Changes to Outputs:
  ~ target_group_arn_suffixes = [
        "targetgroup/h120220915105414049800000003/9cb701bfc0c637e1",
      - "targetgroup/l1-20220915105414046600000001/336c0218a37ed199",
      - "targetgroup/l2-20220915105414047500000002/5a6490249a9a9603",
      + (known after apply),
      + (known after apply),
    ]
  ~ target_group_arns         = [
        "arn:aws:elasticloadbalancing:eu-west-1:___account__id__:targetgroup/h120220915105414049800000003/9cb701bfc0c637e1",
      - "arn:aws:elasticloadbalancing:eu-west-1:___account__id__:targetgroup/l1-20220915105414046600000001/336c0218a37ed199",
      - "arn:aws:elasticloadbalancing:eu-west-1:___account__id__:targetgroup/l2-20220915105414047500000002/5a6490249a9a9603",
      + (known after apply),
      + (known after apply),
    ]
  ~ target_group_attachments  = {
      - "1.lambda_with_allowed_triggers"    = "arn:aws:elasticloadbalancing:eu-west-1:___account__id__:targetgroup/l1-20220915105414046600000001/336c0218a37ed199-20220915105415373900000005" -> null
      + "1.lambda_without_allowed_triggers" = (known after apply)
      + "2.lambda_with_allowed_triggers"    = (known after apply)
      - "2.lambda_without_allowed_triggers" = "arn:aws:elasticloadbalancing:eu-west-1:___account__id__:targetgroup/l2-20220915105414047500000002/5a6490249a9a9603-20220915105415372200000004" -> null
        # (2 unchanged elements hidden)
    }
  ~ target_group_names        = [
        "h120220915105414049800000003",
      - "l1-20220915105414046600000001",
      - "l2-20220915105414047500000002",
      + (known after apply),
      + (known after apply),
    ]

If your request is for a new feature, please use the Feature request template.

  • [x] βœ‹ I have searched the open/closed issues and my issue is not listed.

Versions

  • Module version [Required]: 7.0.0

  • Terraform version:

Terraform v1.2.8
on darwin_amd64
  • Provider version(s):
+ provider registry.terraform.io/hashicorp/aws v4.30.0
+ provider registry.terraform.io/hashicorp/external v2.2.2
+ provider registry.terraform.io/hashicorp/local v2.2.3
+ provider registry.terraform.io/hashicorp/null v3.1.1
+ provider registry.terraform.io/hashicorp/random v3.4.3

Reproduction Code [Required]

Please use examples/complete-alb.

Steps to reproduce the behavior:

  1. Run terraform apply to create infrastructure.
  2. Change the order of target groups defined at line 370 and run terraform apply again.

Expected behavior

The module doesn't change anything, because defined state is matching the live state.

Actual behavior

Target groups are being deleted and re-created, because the order has changed.

Terminal Output Screenshot(s)

Provided at the top of the issue.

Additional context

None.

aminclip avatar Sep 15 '22 11:09 aminclip

Hi @aminclip !

target_groups is a list, and the module refers internally to target_groups_index to map listeners/rules (see this line).

We can probably improve this a bit by introducing an alias and map resources using it, but it will be a breaking change. I am not sure we need to do it now.

antonbabenko avatar Sep 15 '22 11:09 antonbabenko

hey @antonbabenko,

I understand. What do you think on creating a separate branch to be merged to the next major (8.x) ?

aminclip avatar Sep 15 '22 12:09 aminclip

Sure, once we work on another major release, but as I say I don't know when we will we working on it.

antonbabenko avatar Sep 15 '22 12:09 antonbabenko

A question regarding the "breaking change", isn't it only changing internals of the module? so the module interface will stay the same.

aminclip avatar Sep 15 '22 12:09 aminclip

Not really, if we want to make it properly and forward compatible for further improvements down the road (like we have done in many other modules already). I don't know how to make such a change backward compatible and bump a minor release.

antonbabenko avatar Sep 15 '22 13:09 antonbabenko

ok, thank you and good job.

peace.

aminclip avatar Sep 15 '22 13:09 aminclip

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions[bot] avatar Oct 16 '22 00:10 github-actions[bot]

This issue was automatically closed because of stale in 10 days

github-actions[bot] avatar Oct 27 '22 00:10 github-actions[bot]

I'm not sure if passing time solves the issue.

aminclip avatar Oct 27 '22 10:10 aminclip

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.

github-actions[bot] avatar Nov 27 '22 02:11 github-actions[bot]