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

google_bigtable_gc_policy max_age.days is deprecated but "doesn't support update" #8416

Closed
daisy1754 opened this issue Feb 8, 2021 · 14 comments · Fixed by GoogleCloudPlatform/magic-modules#4556, #8639 or hashicorp/terraform-provider-google-beta#3037
Assignees
Labels

Comments

@daisy1754
Copy link

daisy1754 commented Feb 8, 2021

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
  • If an issue is assigned to the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

Terraform v0.12.29
+ provider.google v3.54.0
+ provider.google-beta v3.54.0
+ provider.local v2.0.0
+ provider.random v2.3.0

Affected Resource(s)

  • google_bigtable_gc_policy

Terraform Configuration Files

resource "google_bigtable_gc_policy" "my_gc_policy" {
  instance_name = var.my_instance_name
  table         = "mytable"
  column_family = "mycolumn"

  mode = "UNION"
  max_age {
    days = 365
  }
  max_version {
    number = 1
  }
}

Expected Behavior

The latest version of terraform-provider-google shows warning about max_age.days being deprecated so i should be able to update it to duration.

Actual Behavior

i already have a gc policy created with above terraform config. After i updated the provider, i got a deprecated notice:

Warning: Deprecated Attribute

  on ../my.tf line 48, in resource "google_bigtable_gc_policy" "my_gc_policy":
  48:     days = 180

Deprecated in favor of duration

so i tried to change days=365 to duration = "4344h". terraform plan doesn't complain but when i try to run apply, i got:

Error: doesn't support update

@edwardmedia edwardmedia added the bug label Feb 8, 2021
@edwardmedia edwardmedia self-assigned this Feb 8, 2021
@ScottSuarez
Copy link
Collaborator

given that this is fairly small and there is a workaround (destroying and recreating manually) I think its safe to label this as an enhancement. @rileykarson do you agree?

@rileykarson
Copy link
Collaborator

Changing a deprecated field to the new representation should generally work without a diff imo- I'd prefer we treat it as a bug. We should be able to handle this transition using a customizediff, I think?

@wyardley
Copy link

Thanks for deduping this @rileykarson - sorry for not finding this one first.

@wyardley
Copy link

As a possible workaround, do we know if just destroying state for the old gc policies and letting tf create / recreate them is safe, or if it will result in duplicate policies? I already discovered that removing the state and importing won't work since the resource isn't importable.

@ScottSuarez
Copy link
Collaborator

ScottSuarez commented Feb 25, 2021

Ahh okay cool, so it looks like this is definitely a bug on our part! This resource actually doesn't support updating at all but creating/destruction is roughly equivalent. We didn't have forceNew on the field so ya! Fix coming in soon :)

@wyardley
Copy link

One thing I'm seeing is:
Cloud Bigtable will not allow you to delete an age-based garbage-collection policy for a column family in a replicated table.
Since the resource can't be deleted, how would force new work in this case? wouldn't either the deletion or the creation fail?

We are seeing this when I removed the state for a policy and tried to recreate it

@rileykarson
Copy link
Collaborator

Got it! That's a... surprisingly specific rule from the API. Looks like we'll want to at least investigate handling this with more finesse. We'd decided it was OK to just set ForceNew and attempt to recreate the resource because the resource doesn't support update, and there isn't much impact from deleting+recreating the resource (the field is temporarily set to have no policy).

@ScottSuarez can you look into this again when you have the chance? We'd want users to be able to transition from the old -> new representation with no diff.

@rileykarson rileykarson reopened this Feb 25, 2021
@wyardley
Copy link

The specific error I got was:

Error: rpc error: code = FailedPrecondition desc = Cannot relax pure age-based GC for a replicated family (event_data). If you must relax the age constraint, unreplicate the instance and try again.

For most, removing the state and re-creating seemed to work. However, for one resource, the existing state seemed to have been consistent, but applying a change after updating to the new language gave the error above. Since import isn't supported, it's not possible really to do anything else. 😞

We suspect that disabling replication, updating the gc policy, and then re-enabling replication might work, but haven't tested yet.

Also, https://cloud.google.com/bigtable/docs/replication-overview has some more details and is the source for what I posted above.

@ScottSuarez
Copy link
Collaborator

ScottSuarez commented Feb 26, 2021

I'm not too familiar with replicated tables. I'll play around with it to see if updating this gc_policy is something feasible.

@wyardley
Copy link

@ScottSuarez @rileykarson if the values got normalized under the hood to a common unit, would that help? i.e., if the provider can tell that the value is equivalent, it wouldn't need to update it at all.

@rileykarson
Copy link
Collaborator

The provider ends up being a little pickier than that (it can't recognise that two fields are related), but that's what should be possible per #8416 (comment)

@ScottSuarez
Copy link
Collaborator

I've got a fix in the works @wyardley utilizing your suggestion! :)
GoogleCloudPlatform/magic-modules#4556

@wyardley
Copy link

wyardley commented Mar 3, 2021

Awesome - thanks!

@ghost
Copy link

ghost commented Apr 9, 2021

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. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Apr 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.