hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.7k stars 9.07k forks source link

No method to ignore changes in DynamoDB GSI #671

Open hashibot opened 7 years ago

hashibot commented 7 years ago

This issue was originally opened by @cypai as hashicorp/terraform#13393. It was migrated here as part of the provider split. The original body of the issue is below.


Terraform Version

$ terraform -v Terraform v0.8.8

Affected Resource(s)

Terraform Configuration Files

resource "aws_dynamodb_table" "FooBar" {
    name = "FooBar"
    read_capacity = 10
    write_capacity = 10
    hash_key = "foo-id"
    attribute {
        name = "foo-id"
        type = "S"
    }
    attribute {
        name = "bar-id"
        type = "N"
    }
    global_secondary_index {
        name = "bar-index"
        hash_key = "bar-id"
        read_capacity = 10
        write_capacity = 10
        projection_type = "ALL"
    }
    lifecycle {
        ignore_changes = ["read_capacity", "write_capacity"]
    }
}

References

We would like to ignore read/write capacity for the GSI as well, but this configuration only ignores the read/write capacity of the top-level table. We weren't able to find any usable workaround, since we can't interpolate variables in the lifecycle, and wildcards are not supported.

Technically we could hardcode the randomly generated number like ignore_changes = ["global_secondary_index.3291674533.read_capacity"], but that's obviously a terrible solution since it would change every time we make a change.

lra commented 7 years ago

Ignoring the GSI with its ID is not a work-around, it does ignore the *_capacity but stiff diffs the whole GSI =(

mcwqy9 commented 7 years ago

I've got some time this week to take a stab at this. I've done some golang in the past, but this would be my first contribution to this project. Can anyone suggest what might need to be done, at a high level? Would a good approach be to remove a GSI's capacity from the hash function that determines the number in "global_secondary_index.3291674533.read_capacity"? That would make that number stable through capacity changes, and allow an ignore to be hardcoded. Stab in the dark...

playdoz commented 6 years ago

I've been testing with the following and it seems to be working quite well, terraform will ignore the GSI read and write capacity (and unfortunately projection_type) changes but will pick up other changes like name, hash_key etc

lifecycle {
    ignore_changes = [
      "global_secondary_index",
      "read_capacity",
      "write_capacity",
    ]
  }

Not an ideal solution.

mcwqy9 commented 6 years ago

Interesting. Let me try that on my end. What version of Terraform and what version of the AWS provider are you testing with?

On Nov 23, 2017 5:14 PM, "kimgov" notifications@github.com wrote:

I've been testing with the following and it seems to be working quite well, terraform will ignore the GSI read and write capacity changes but will pick up other changes like name, hash_key etc

lifecycle { ignore_changes = [ "global_secondary_index", "read_capacity", "write_capacity", ] }

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/terraform-providers/terraform-provider-aws/issues/671#issuecomment-346711738, or mute the thread https://github.com/notifications/unsubscribe-auth/ABV4JBMt-c1fQVEvg4hDVcWrtbo05m0Cks5s5gpvgaJpZM4N5B0u .

icybin commented 6 years ago

Any update on this topic? What is the best terraform version to get out of this issue? I'm ussing 0.9.6 and I always see dynamodb things updated when I run terraform apply :(

armanshan12 commented 6 years ago

Any updates on this issue?

rsalmond commented 6 years ago

So apparently the above PR fixed this issue, does anyone know how to reference the read/write capacity of a GSI in the ignore_changes field to make it happen?

cypai commented 6 years ago

The comments in the above PR say that it is just a refactoring, only that it makes it easier to address this issue. I don't think it's fixed yet.

rsalmond commented 6 years ago

@cypai ah you're right, thanks for pointing that out! I read the wrong issue number. I'm getting by with the workaround @kimgov provided above in the meantime.

philipl commented 6 years ago

@kimgov, I tried ignoring changes on "global_secondary_index" but it completely ignores everything. I can't modify a hash key, or even add a new GSI. Terraform ignores all those changes. You are saying that these kinds of changes are planned by terraform?

playdoz commented 6 years ago

Hi @philipl, I left my workaround in months ago and for me haven't had to make much changes to the underlying dynamodb table itself since. I do note your issue though that it ignores everything, to get around that in the early days I just commented out the 'global secondary index' line, terraform apply the change and then add that line back in.

I know it's not an ideal solution at all, and I really don't like the way terraform manages dynamodbs and in particular GSIs.

andresvia commented 6 years ago

Maybe global secondary indexes should be a separate terraform resource type:

mock:

resource "aws_dynamodb_table" "table" {
  name = "table"
  // .. etc
}

resource "aws_dynamodb_global_secondary_index" "gsi" {
  name = "gsi"
  table = "${aws_dynamodb_table.table.name}"
  read_capacity = "100"
  write_capacity = "100"
  lifecycle {
    ignore_changes = ["read_capacity", "write_capacity"]
  }
 // ... etc
}
emmekappa commented 5 years ago

Having the same issue and still not found any workaround.

davehowell commented 5 years ago

Maybe global secondary indexes should be a separate terraform resource type:

@andresvia Seems like a good idea, it would just have to depend on a dynamodb table given the prefix of a GSI arn includes the table's arn. Alternatively it might be more consistent to have its own lifecycle block inside the global_secondary_index block.

Either way, it is pretty standard to use autoscaling policies, and when you have them it is also good practice to apply it to both the table and the GSI, not just the table, especially for writes, given most table writes imply also writing to the GSI.

This big note at the top of the docs for aws_dynamodb_table resource seems to acknowledge half of that:

Note: It is recommended to use lifecycle ignore_changes for read_capacity and/or write_capacity if there's autoscaling policy attached to the table.

ben-bourdin451 commented 5 years ago

I have just started to work on making GSIs into a new resource to fix this issue but I can't say how long it's going to take since I don't have a huge amount of spare time. I thought I would mention it here in case anyone was thinking of doing the same.

I would greatly appreciate some guidance on how to manage this kind of change from a maintainer. I'm not sure if I should leave the existing GSI code in the table as it is and support both for now much like security group rules or deprecate it completely as a new version of the provider.

davehowell commented 5 years ago

I can't speak for the maintainers but supporting both options for an interim period is a friendly way of deprecating features. Raise a warning that the old way will not be supported in the future gives people a chance to update their code. I've seen another issue that claims the timeout settings don't work for GSIs either, although I can't verify it. I know that timeout settings do work for table resources though. I'm guessing that the lifecycle, timeout and all the other common attributes would then be supported and those issues fixed by your PR. Thank you for working on it, the community will very much appreciate it.

bevanbennett commented 5 years ago

Just chiming in that this is an active and annoying issue for us, as our secondary index autoscaling is constantly at war with terraform. Splitting out secondary indexes as a separate resource seems reasonable, if refactor-inducng. I think we would prefer if we could add ignore_changes lifecycle blocks inside the GSI blocks.

andresvia commented 5 years ago

For people wanting to workaround this issue changing the table billing mode to PAY_PER_REQUEST may help in some cases. (warning: you may get charged more money depending on your dynamodb usage pattern)

tinder-jlee commented 5 years ago

What's the current status on this issue? We're having a difficulty as well.

ben-bourdin451 commented 5 years ago

I opened a PR to add the concept of a GSI as a new resource however as it stands this would break existing table configuration states.

I was hoping that someone with more experience in updating existing resource configurations could point me in the right direction. Haven't had any feedback yet. Not sure what is the best way of chasing this up.

If anyone has any suggestions please let me know

shijiesheng commented 4 years ago

Same issue here and it's quite annoying.

red8888 commented 4 years ago

bump

It would not be as bad if I could at least set a variable to ignore_changes = ["global_secondary_index"] so switching that on and off during updates could be a temp workaround but that is not possible: There is no support for conditionally modifying ignore_changes either https://github.com/hashicorp/terraform/issues/3116

We have to manually make all GSI changes after the first terraform apply and try to manually reconcile with terraform config. Almost makes it not usable for our GSI tables

ben-bourdin451 commented 4 years ago

I probably should have followed up on this a while ago but I still don't know how to migrate an attribute into its own resource without breaking changes.

I did find this really helpful doc a while ago: https://www.terraform.io/docs/extend/best-practices/deprecations.html but it doesn't talk about this use case.
I would happily open a proposal to add something to the site docs on how to approach this type of change if anyone thinks that would be useful. AWS security group and security group rules are the only other example of this type of change that can think of at the moment.

At this point in time I'm not sure what else to do apart from maybe:

If anyone has any other suggestions or thoughts please share! :smile:

ben-bourdin451 commented 4 years ago

@ewbankkit @maryelizbeth would it be possible to include this in the v3.0.0 breaking changes?

To provide more context: a nice way of solving this issue would be to split the GSIs into a new resource but that would introduce a breaking change, hence the question above 😄

maryelizbeth commented 4 years ago

@ben-bourdin451 Sorry for the delay! This got buried in my notifications. Unfortunately this did not make the cut for 3.0.0. However, we are tracking this issue internally and will consider how to address it in the near term future. We'll update this issue as they become available!

ben-bourdin451 commented 4 years ago

@maryelizbeth no problem thanks for the reply. This might not need to be a breaking change; I'm simply not sure how to split the state so that extracting the GSI to a new resource does not cause diffs on the dynamodb_table resource. This does seem possible given that a few resources work this way (e.g. security_group_rules)

I would be happy to spend more time on this and update my PR if I can get a push in the right direction :)

anGie44 commented 3 years ago

The fix (#9988) has been merged and will release with v3.5.0 of the Terraform AWS Provider, likely out this Thursday. Thanks again @shalka for your contribution!

oscarkaypee commented 3 years ago

hi @anGie44 could you give more details on how the linked pull request helps with this issue please?

I can understand how this helps #3807 but sadly I can't seem to understand how the ordering of the non_key_attributes attribute is going to help us to "ignore read/write capacity for the GSI" from this issue's description.

Could you please update the note at the top of the aws_dynamodb_table docs with the recommendation when an autoscaling policy is attached to a GSI? We're currently employing a not so nice workaround from https://github.com/terraform-providers/terraform-provider-aws/issues/2637#issuecomment-398401286 which is the only way to do "DynamoDB Autoscaling for GSIs" currently. The aforementioned ignore_changes = ["global_secondary_index"] is a great pain for us at the moment as we are not able to add new indexes or update existing indexes in our Terraform CI pipelines without manually updating ignore_changes each time if autoscaling is enabled.

It would be great to see a solution that enables ignore_changes to be set on read/write capacity for any defined GSI's, without having to change it after the aws_dynamodb_table resource is defined initially. Again, if the linked PR can be used to do so, please provide details on that, thanks!

ghost commented 3 years ago

This has been released in version 3.5.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

bflad commented 3 years ago

Hi folks 👋 Sorry for any confusion here. The non_key_attributes handling update in version 3.5.0 of the Terraform AWS Provider is simply to reduce some unexpected plan differences for GSIs. As mentioned above, this is a little different than the ask of this issue, which is generally due to the usage of Application Auto Scaling on the GSIs. I'm going to reopen this, but in full transparency I'm not aware of any further effort on this topic at this particular moment.

david-kirby commented 3 years ago

Would love to have this. Without it, we're constantly getting diffs for our DynamoDB table indexes when the capacity is in a scaled state

serhatcetinkaya commented 3 years ago

do we have any update on this ?

Luminoth commented 3 years ago

Would it be possible, if traction on this issue has stalled out, to allow something like this?

lifecycle {
  ignore_changes = [ global_secondary_index.*.read_capacity ]
}

Something like that seems a lot safer than moving indexes outside of the resource. However, that might be a change that would need to be done at the Terraform level rather than the provider level?

bflad commented 3 years ago

@Luminoth correct, changes to lifecycle and ignore_change handling must occur upstream in the Terraform CLI and/or Terraform Plugin SDK projects. That particular configuration is never shared with the provider itself.

sharebear commented 3 years ago

I'm just learning the provider-sdk apis now but couldn't this also be solved by implementing a CustomizeDiffFunc for the resource? If you add an argument ignore_gsi_capacity_on_update to the resource (default to false for backwards compatability), then in the diff func remove the capacity fields from the diff, with logic to make sure you don't break anything if the gsi was added/removed completely in the diff.

jufemaiz commented 3 years ago

Damn. I really love coming across 3.5yo bugs that are still unresolved.

jcollum commented 3 years ago

Our team is running into this too. It's causing significant slowdown in API calls in our production database. Adding an index shouldn't require us to (essentially) take the db offline for many hours while the index is recreated.

bacoboy-doordash commented 3 years ago

Still a problem on provider 3.42.00 and counting. :(

anGie44 commented 2 years ago

Upstream Terraform Issues related to plan-output showing changes for unchanged configuration blocks: https://github.com/hashicorp/terraform/issues/21901 https://github.com/hashicorp/terraform/issues/28281

danquack commented 2 years ago

A solution was initially put forward with the most upvotes to create aws_dynamodb_table_gsi. It appears that PR was closed over time and wasn't maintained. I've updated that PR but could use some guidance on how the cycle dependency of describing resources attributes and global secondary indexes would work. See the linked PR for more insight.

kopatsy commented 2 years ago

As a workaround, we, at Verkada, built a custom provider (https://github.com/verkada/terraform-provider-gsi) that takes ownership of global secondary indexes in a separate resource. As described in the documentation, it requires adding a lifecycle ignore_change on both attribute and global_secondary_index on the aws_dynamodb_table resource. Local secondary indexes will not work properly with this provider at the moment (https://github.com/verkada/terraform-provider-gsi/issues/11).

To facilitate the migration, we also introduced an auto_import flag on the provider that will make the first apply act as an import if an index with the same name already exists. Extra attention must be given on the subsequent plan as you've probably been ignoring all changes for a while now due to this issue and the next plan might require a replace if any drift was introduced.

I would recommend trying it on a test account/ table to get acquainted with the provider's behavior as it is obviously a workaround with the caveats I just described. Contributions and comments welcome. Hopefully it will save you as much time as it is saving us.

cmawhorter commented 2 years ago

every time i get a new computer i hit this issue because i forget it exists and stupidly install the latest tf.

terraform v0.14 doesn't have this issue.

if you're not paying attention this can be a huge problem because your tables will essentially be unavailable while the index rebuilds -- which can take minutes to hours.

justinretzolk commented 2 years ago

Hey everyone :wave: Thank you very much for your interest and continued feedback on this. This is something we’re monitoring and discussing in order to find the best solution for. At the moment, we’re currently limited by the functionality of the underlying API (something we discussed in more detail over on the pull request that was recently opened in order to try to fix this (#22513, thanks again to @danquack for his work on that!). That discussion includes some of the ideas presented here around a CustomizedDiff or separate resource, which we unfortunately feel won’t fix this in a way that we could guarantee wouldn’t have unintentional side effects.

With that said, we’ve started the conversation with the folks at AWS so that we can try to make progress towards a resolution. When we have more information, we’ll make sure to update this issue again.

ewbankkit commented 1 year ago

Relates https://github.com/hashicorp/terraform/issues/26359.

yasntrk commented 1 year ago

Damn. I really love coming across 3.5yo bugs that are still unresolved.

Double it and give it to next person.

Still experiencing this bug.

tarfeef101 commented 1 year ago

this is a known problem, with known solutions. see: https://github.com/mongodb/terraform-provider-mongodbatlas/issues/888

maybe steal inspiration from how this was fixed?

schmittjoaopedro commented 5 months ago

Come on, Starship development started in 2019 and they launched in 2023 and this bug is still here?

tl-alex-nicot commented 3 weeks ago

humanity will land on mars first