feat: Added option to ignore changes to GSIs #72

lobsterdore · 2023-05-25T08:02:03Z

Description

This relates to this issue - hashicorp/terraform-provider-aws#671.

When using autoscaling with a provisioned table that has a GSI applying a TF change whilst the indices are scaled will reset capacity, which can be dangerous. This change has an option to ignore changes to global_secondary_index, which seems to be the only way to deal with this issue at present.

Motivation and Context

Without this change, using this module in the context described above can be dangerous.

Breaking Changes

This change should be backwards compatible.

How Has This Been Tested?

I have updated at least one of the examples/* to demonstrate and validate my change(s)
I have tested and validated these changes using one or more of the provided examples/* projects

I have executed pre-commit run -a on my pull request

lobsterdore · 2023-05-25T08:02:35Z

I am going to test this properly, is this sort of change acceptable at all?

antonbabenko

I think this is a valid case since there has been no visible traction for this issue in the Terraform AWS provider for the last 5 years.

autoscaling.tf

examples/global-tables/main.tf

outputs.tf

autoscaling.tf

lobsterdore · 2023-05-25T12:51:41Z

I think this is a valid case since there has been no visible traction for this issue in the Terraform AWS provider for the last 5 years.

Yeah it does seem to be a long term problem! Thanks for this, I will clean up the PR and get it tested.

lobsterdore · 2023-05-25T14:27:26Z

I'll ping back once I've fully tested this.

lobsterdore · 2023-05-25T18:53:59Z

Tested the autoscaling example and it's all working as expected (output attached):

> terraform apply

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # random_pet.this will be created
  + resource "random_pet" "this" {
      + id        = (known after apply)
      + length    = 2
      + separator = "-"
    }

  # module.dynamodb_table.aws_appautoscaling_policy.index_read_policy["TitleIndex"] will be created
  + resource "aws_appautoscaling_policy" "index_read_policy" {
      + alarm_arns         = (known after apply)
      + arn                = (known after apply)
      + id                 = (known after apply)
      + name               = (known after apply)
      + policy_type        = "TargetTrackingScaling"
      + resource_id        = (known after apply)
      + scalable_dimension = "dynamodb:index:ReadCapacityUnits"
      + service_namespace  = "dynamodb"

      + target_tracking_scaling_policy_configuration {
          + disable_scale_in   = false
          + scale_in_cooldown  = 0
          + scale_out_cooldown = 0
          + target_value       = 70

          + predefined_metric_specification {
              + predefined_metric_type = "DynamoDBReadCapacityUtilization"
            }
        }
    }

  # module.dynamodb_table.aws_appautoscaling_policy.index_write_policy["TitleIndex"] will be created
  + resource "aws_appautoscaling_policy" "index_write_policy" {
      + alarm_arns         = (known after apply)
      + arn                = (known after apply)
      + id                 = (known after apply)
      + name               = (known after apply)
      + policy_type        = "TargetTrackingScaling"
      + resource_id        = (known after apply)
      + scalable_dimension = "dynamodb:index:WriteCapacityUnits"
      + service_namespace  = "dynamodb"

      + target_tracking_scaling_policy_configuration {
          + disable_scale_in   = false
          + scale_in_cooldown  = 0
          + scale_out_cooldown = 0
          + target_value       = 70

          + predefined_metric_specification {
              + predefined_metric_type = "DynamoDBWriteCapacityUtilization"
            }
        }
    }

  # module.dynamodb_table.aws_appautoscaling_policy.table_read_policy[0] will be created
  + resource "aws_appautoscaling_policy" "table_read_policy" {
      + alarm_arns         = (known after apply)
      + arn                = (known after apply)
      + id                 = (known after apply)
      + name               = (known after apply)
      + policy_type        = "TargetTrackingScaling"
      + resource_id        = (known after apply)
      + scalable_dimension = "dynamodb:table:ReadCapacityUnits"
      + service_namespace  = "dynamodb"

      + target_tracking_scaling_policy_configuration {
          + disable_scale_in   = false
          + scale_in_cooldown  = 50
          + scale_out_cooldown = 40
          + target_value       = 45

          + predefined_metric_specification {
              + predefined_metric_type = "DynamoDBReadCapacityUtilization"
            }
        }
    }

  # module.dynamodb_table.aws_appautoscaling_policy.table_write_policy[0] will be created
  + resource "aws_appautoscaling_policy" "table_write_policy" {
      + alarm_arns         = (known after apply)
      + arn                = (known after apply)
      + id                 = (known after apply)
      + name               = (known after apply)
      + policy_type        = "TargetTrackingScaling"
      + resource_id        = (known after apply)
      + scalable_dimension = "dynamodb:table:WriteCapacityUnits"
      + service_namespace  = "dynamodb"

      + target_tracking_scaling_policy_configuration {
          + disable_scale_in   = false
          + scale_in_cooldown  = 50
          + scale_out_cooldown = 40
          + target_value       = 45

          + predefined_metric_specification {
              + predefined_metric_type = "DynamoDBWriteCapacityUtilization"
            }
        }
    }

  # module.dynamodb_table.aws_appautoscaling_target.index_read["TitleIndex"] will be created
  + resource "aws_appautoscaling_target" "index_read" {
      + arn                = (known after apply)
      + id                 = (known after apply)
      + max_capacity       = 30
      + min_capacity       = 10
      + resource_id        = (known after apply)
      + role_arn           = (known after apply)
      + scalable_dimension = "dynamodb:index:ReadCapacityUnits"
      + service_namespace  = "dynamodb"
      + tags_all           = (known after apply)
    }

  # module.dynamodb_table.aws_appautoscaling_target.index_write["TitleIndex"] will be created
  + resource "aws_appautoscaling_target" "index_write" {
      + arn                = (known after apply)
      + id                 = (known after apply)
      + max_capacity       = 30
      + min_capacity       = 10
      + resource_id        = (known after apply)
      + role_arn           = (known after apply)
      + scalable_dimension = "dynamodb:index:WriteCapacityUnits"
      + service_namespace  = "dynamodb"
      + tags_all           = (known after apply)
    }

  # module.dynamodb_table.aws_appautoscaling_target.table_read[0] will be created
  + resource "aws_appautoscaling_target" "table_read" {
      + arn                = (known after apply)
      + id                 = (known after apply)
      + max_capacity       = 10
      + min_capacity       = 5
      + resource_id        = (known after apply)
      + role_arn           = (known after apply)
      + scalable_dimension = "dynamodb:table:ReadCapacityUnits"
      + service_namespace  = "dynamodb"
      + tags_all           = (known after apply)
    }

  # module.dynamodb_table.aws_appautoscaling_target.table_write[0] will be created
  + resource "aws_appautoscaling_target" "table_write" {
      + arn                = (known after apply)
      + id                 = (known after apply)
      + max_capacity       = 10
      + min_capacity       = 5
      + resource_id        = (known after apply)
      + role_arn           = (known after apply)
      + scalable_dimension = "dynamodb:table:WriteCapacityUnits"
      + service_namespace  = "dynamodb"
      + tags_all           = (known after apply)
    }

  # module.dynamodb_table.aws_dynamodb_table.autoscaled_gsi_ignore[0] will be created
  + resource "aws_dynamodb_table" "autoscaled_gsi_ignore" {
      + arn              = (known after apply)
      + billing_mode     = "PROVISIONED"
      + hash_key         = "id"
      + id               = (known after apply)
      + name             = (known after apply)
      + range_key        = "title"
      + read_capacity    = 5
      + stream_arn       = (known after apply)
      + stream_enabled   = false
      + stream_label     = (known after apply)
      + stream_view_type = (known after apply)
      + tags             = (known after apply)
      + tags_all         = (known after apply)
      + write_capacity   = 5

      + attribute {
          + name = "age"
          + type = "N"
        }
      + attribute {
          + name = "id"
          + type = "N"
        }
      + attribute {
          + name = "title"
          + type = "S"
        }

      + global_secondary_index {
          + hash_key           = "title"
          + name               = "TitleIndex"
          + non_key_attributes = [
              + "id",
            ]
          + projection_type    = "INCLUDE"
          + range_key          = "age"
          + read_capacity      = 10
          + write_capacity     = 10
        }

      + point_in_time_recovery {
          + enabled = false
        }

      + server_side_encryption {
          + enabled     = false
          + kms_key_arn = (known after apply)
        }

      + timeouts {
          + create = "10m"
          + delete = "10m"
          + update = "60m"
        }

      + ttl {
          + enabled = false
        }
    }

Plan: 10 to add, 0 to change, 0 to destroy.

Changes to Outputs:
  + dynamodb_table_arn = (known after apply)
  + dynamodb_table_id  = (known after apply)

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

random_pet.this: Creating...
random_pet.this: Creation complete after 0s [id=grown-chicken]
module.dynamodb_table.aws_dynamodb_table.autoscaled_gsi_ignore[0]: Creating...
module.dynamodb_table.aws_dynamodb_table.autoscaled_gsi_ignore[0]: Still creating... [10s elapsed]
module.dynamodb_table.aws_dynamodb_table.autoscaled_gsi_ignore[0]: Creation complete after 13s [id=my-table-grown-chicken]
module.dynamodb_table.aws_appautoscaling_target.table_read[0]: Creating...
module.dynamodb_table.aws_appautoscaling_target.table_write[0]: Creating...
module.dynamodb_table.aws_appautoscaling_target.index_write["TitleIndex"]: Creating...
module.dynamodb_table.aws_appautoscaling_target.index_read["TitleIndex"]: Creating...
module.dynamodb_table.aws_appautoscaling_target.index_write["TitleIndex"]: Creation complete after 1s [id=table/my-table-grown-chicken/index/TitleIndex]
module.dynamodb_table.aws_appautoscaling_policy.index_write_policy["TitleIndex"]: Creating...
module.dynamodb_table.aws_appautoscaling_target.table_write[0]: Creation complete after 1s [id=table/my-table-grown-chicken]
module.dynamodb_table.aws_appautoscaling_target.index_read["TitleIndex"]: Creation complete after 1s [id=table/my-table-grown-chicken/index/TitleIndex]
module.dynamodb_table.aws_appautoscaling_target.table_read[0]: Creation complete after 1s [id=table/my-table-grown-chicken]
module.dynamodb_table.aws_appautoscaling_policy.table_write_policy[0]: Creating...
module.dynamodb_table.aws_appautoscaling_policy.index_read_policy["TitleIndex"]: Creating...
module.dynamodb_table.aws_appautoscaling_policy.table_read_policy[0]: Creating...
module.dynamodb_table.aws_appautoscaling_policy.index_write_policy["TitleIndex"]: Creation complete after 0s [id=DynamoDBWriteCapacityUtilization:table/my-table-grown-chicken/index/TitleIndex]
module.dynamodb_table.aws_appautoscaling_policy.table_write_policy[0]: Creation complete after 0s [id=DynamoDBWriteCapacityUtilization:table/my-table-grown-chicken]
module.dynamodb_table.aws_appautoscaling_policy.table_read_policy[0]: Creation complete after 0s [id=DynamoDBReadCapacityUtilization:table/my-table-grown-chicken]
module.dynamodb_table.aws_appautoscaling_policy.index_read_policy["TitleIndex"]: Creation complete after 0s [id=DynamoDBReadCapacityUtilization:table/my-table-grown-chicken/index/TitleIndex]

Apply complete! Resources: 10 added, 0 changed, 0 destroyed.

Outputs:

dynamodb_table_arn = "arn:aws:dynamodb:eu-west-1:xxxxxxxxx:table/my-table-grown-chicken"
dynamodb_table_id = "my-table-grown-chicken"

lobsterdore · 2023-05-25T19:01:57Z

I've updated the README as well, let me know if it needs anymore detail @antonbabenko.

antonbabenko

Almost perfect!

antonbabenko · 2023-05-25T19:13:07Z

README.md

@@ -35,6 +35,13 @@ There are two separate Terraform resources used for the DynamoDB table: one is f
 terraform state mv module.dynamodb_table.aws_dynamodb_table.this module.dynamodb_table.aws_dynamodb_table.autoscaled
 ```

+**Warning: autoscaling with global secondary indexes**


Thanks for adding this note!

autoscaling.tf

examples/global-tables/main.tf

This relates to this issue - hashicorp/terraform-provider-aws#671. When using autoscaling with a provisioned table that has a GSI applying a TF change whilst the indices are scaled will reset capacity, which can be dangerous. This change has an option to ignore changes to global_secondary_index, which seems to be the only way to deal with this issue at present.

lobsterdore · 2023-05-25T20:38:39Z

@antonbabenko fixed those issues.

## [3.3.0](v3.2.0...v3.3.0) (2023-05-25) ### Features * Added option to ignore changes to GSIs ([#72](#72)) ([44187e9](44187e9))

antonbabenko · 2023-05-25T21:09:27Z

This PR is included in version 3.3.0 🎉

github-actions · 2023-06-25T02:27:47Z

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

antonbabenko requested changes May 25, 2023

View reviewed changes

autoscaling.tf Outdated Show resolved Hide resolved

examples/global-tables/main.tf Outdated Show resolved Hide resolved

outputs.tf Outdated Show resolved Hide resolved

lobsterdore commented May 25, 2023

View reviewed changes

autoscaling.tf Outdated Show resolved Hide resolved

lobsterdore force-pushed the add-ignore-changes-global-secondary-indices-option branch from 375f03d to 65b0b22 Compare May 25, 2023 14:25

lobsterdore force-pushed the add-ignore-changes-global-secondary-indices-option branch from f5b020b to 2f62e3d Compare May 25, 2023 19:02

antonbabenko requested changes May 25, 2023

View reviewed changes

lobsterdore force-pushed the add-ignore-changes-global-secondary-indices-option branch from 2f62e3d to af27940 Compare May 25, 2023 19:38

lobsterdore force-pushed the add-ignore-changes-global-secondary-indices-option branch from af27940 to b621b44 Compare May 25, 2023 20:06

antonbabenko changed the title ~~feat: Adds Option to Ignore Changes to GSIs~~ feat: Added option to ignore changes to GSIs May 25, 2023

antonbabenko approved these changes May 25, 2023

View reviewed changes

antonbabenko merged commit 44187e9 into terraform-aws-modules:master May 25, 2023

antonbabenko pushed a commit that referenced this pull request May 25, 2023

chore(release): version 3.3.0 [skip ci]

03b38ee

## [3.3.0](v3.2.0...v3.3.0) (2023-05-25) ### Features * Added option to ignore changes to GSIs ([#72](#72)) ([44187e9](44187e9))

github-actions bot locked as resolved and limited conversation to collaborators Jun 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: Added option to ignore changes to GSIs #72

feat: Added option to ignore changes to GSIs #72

Uh oh!

lobsterdore commented May 25, 2023 •

edited

Loading

Uh oh!

lobsterdore commented May 25, 2023 •

edited

Loading

Uh oh!

antonbabenko left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

antonbabenko left a comment

Uh oh!

antonbabenko May 25, 2023

Uh oh!

Uh oh!

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

antonbabenko commented May 25, 2023

Uh oh!

github-actions bot commented Jun 25, 2023

Uh oh!

Uh oh!

Uh oh!

feat: Added option to ignore changes to GSIs #72

feat: Added option to ignore changes to GSIs #72

Uh oh!

Conversation

lobsterdore commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Breaking Changes

How Has This Been Tested?

Uh oh!

lobsterdore commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antonbabenko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

antonbabenko left a comment

Choose a reason for hiding this comment

Uh oh!

antonbabenko May 25, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lobsterdore commented May 25, 2023

Uh oh!

antonbabenko commented May 25, 2023

Uh oh!

github-actions bot commented Jun 25, 2023

Uh oh!

Uh oh!

lobsterdore commented May 25, 2023 •

edited

Loading

lobsterdore commented May 25, 2023 •

edited

Loading