adding new resource google_bigtable_gc_policy #3293

eraac · 2019-03-21T08:25:29Z

Adding new resource for manage garbage collection into bigtable

make testacc TEST=./google TESTARGS='-run=TestAccBigtableGC'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccBigtableGC -timeout 240m -ldflags="-X=github.com/terraform-providers/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccBigtableGCPolicy_basic
=== PAUSE TestAccBigtableGCPolicy_basic
=== RUN   TestAccBigtableGCPolicy_union
=== PAUSE TestAccBigtableGCPolicy_union
=== CONT  TestAccBigtableGCPolicy_basic
=== CONT  TestAccBigtableGCPolicy_union
--- PASS: TestAccBigtableGCPolicy_union (21.21s)
--- PASS: TestAccBigtableGCPolicy_basic (23.85s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	23.902s

eraac · 2019-03-21T08:39:54Z

Actually if you re-create bigtable_table, gc_policy is not re-apply. Discussion for this kind of issue is still open hashicorp/terraform#8099 but maybe something is possible for "fixing" this ?

gnalsa · 2019-04-18T17:51:16Z

Hi @rileykarson I am not very familiar with feature process, but this would be a great addition to enable management of bigtable.

rileykarson · 2019-05-14T15:54:52Z

Sorry for the radio silence on my end here! At first glance, your contribution looks great, but I'm wondering if this is something we should add to this provider.

It's come up with a few other APIs- BigQuery, Spanner, etc. where we're not sure whether some features are GCP or "data" level. In general, we want to keep this provider focused on provisioning GCP infrastructure and not performing "data" operations on services- even those exclusive to GCP.

Since this isn't exposed in the Bigtable Admin API, I'm inclined to say this falls more on the data side, but I'll bring this up with the rest of my team in our sync later this week.

eraac · 2019-05-14T18:52:09Z

@rileykarson I understood but is like "lifecycle" ressources, I not agree with "data" level

Furthermore is actually documented here below modifyColumnFamilies

rileykarson · 2019-05-20T15:53:02Z

Understood! We're weighing some considerations to make a decision about whether it falls into the scope of the Google provider or not.

eraac · 2019-08-12T09:29:16Z

@rileykarson Any news?

rileykarson

Previously, I was slightly 👎 on adding this because it wasn't represented in the REST Admin API, but that wasn't accurate. https://cloud.google.com/bigtable/docs/reference/admin/rest/v2/projects.instances.tables/modifyColumnFamilies#GcRule. The interface to the admin API / this client lib are a little different, so I guess I didn't notice it before.

It looks like some validation added in Terraform 0.12 made the test fail; retyping the field to TypeList (TypeMap works in very limited circumstances, not including this one) will resolve that.

google/resource_bigtable_gc_policy.go

website/docs/r/bigtable_gc_policy.html.markdown

google/resource_bigtable_gc_policy.go

rileykarson · 2019-09-13T20:56:16Z

google/resource_bigtable_gc_policy.go

+	return resourceBigtableGCPolicyRead(d, meta)
+}
+
+func resourceBigtableGCPolicyRead(d *schema.ResourceData, meta interface{}) error {


Is it possible to recreate a GCPolicy from the string in FamilyInfo? https://github.com/terraform-providers/terraform-provider-google/blob/ab7c29f09dafc6d836888b3db57b53f148e37727///vendor/cloud.google.com/go/bigtable/admin.go#L231

Drift detection would be nice if it can, but it's not a must-have.

It's weird I forget about this, you mean about the SetId("...") to ensure nothing change between two plan/apply? If yes, should be good now

The id value won't actually change anything, so not setting it is fine. I'm curious whether we can extract values like max_age from the GCPolicy string, so we can set those in state.

I see, it's probably possible, but I thinking about all cases, what if Google change something? Or add another policy? I am not even sure order of the policy is guaranteed (can be max_age && max_version / max_version && max_age)

I wonder, what's happen in terraform if you have 2 resources, with the same ID (actually, that can happen if you set same gc policy on two table), should I prefix ID with the column name?

2 resources can share an id, Terraform only distinguishes between the id value being unset and set.

Signed-off-by: Kevin Labesse <[email protected]>

rileykarson

I ran the tests and they panicked. I think it's because of the type change, commented inline with a likely fix.

rileykarson · 2019-09-19T15:03:46Z

google/resource_bigtable_gc_policy.go

+	}
+
+	if aok {
+		d, err := strconv.Atoi(ma.(map[string]interface{})["days"].(string))


I think that ma needs to be converted to []interface{}, and the 0 element should be a map[string]interface{}.

I've change, not sure I should check if slice has at least one element, shouldn't go in this statement if value isn't set in .tf files

rileykarson · 2019-09-19T15:03:56Z

google/resource_bigtable_gc_policy.go

+	}
+
+	if vok {
+		n, err := strconv.Atoi(mv.(map[string]interface{})["number"].(string))


Same as above.

rileykarson

Sorry for taking so long to get back to this! We've updated our plugin SDK since the last pass I made, do you mind replacing the github.com/hashicorp/terraform/ imports with github.com/hashicorp/terraform-plugin-sdk/?

Signed-off-by: Kevin Labesse <[email protected]>

eraac · 2019-09-30T10:04:38Z

@rileykarson Sure, no problem. I've update the resource file (and test)

rileykarson

LGTM. Thanks for contributing, @eraac! Sorry about the delay in getting back to this.

I'm going to upstream this change to Magic Modules so that it's applied to the google-beta provider as well, and then merge this PR.

ghost · 2019-10-31T13:51:08Z

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 added size/xl documentation labels Mar 21, 2019

adding new resource google_bigtable_gc_policy

ab7c29f

rileykarson self-requested a review March 21, 2019 16:07

rileykarson reviewed Sep 13, 2019

View reviewed changes

eraac added 4 commits September 16, 2019 17:29

rename family to column_family

8c4e9e7

Signed-off-by: Kevin Labesse <[email protected]>

adding more example

461fc12

Signed-off-by: Kevin Labesse <[email protected]>

change from TypeMap to TypeList

926e6b9

Signed-off-by: Kevin Labesse <[email protected]>

adding drift detection

e84849a

Signed-off-by: Kevin Labesse <[email protected]>

rileykarson reviewed Sep 19, 2019

View reviewed changes

fix panic from bad cast

ebe02e2

rileykarson reviewed Sep 27, 2019

View reviewed changes

update import to new terraform sdk

eac037e

Signed-off-by: Kevin Labesse <[email protected]>

rileykarson approved these changes Sep 30, 2019

View reviewed changes

rileykarson mentioned this pull request Sep 30, 2019

Add bigtable GCPolicy resource GoogleCloudPlatform/magic-modules#2395

Merged

rileykarson merged commit 3d81db1 into hashicorp:master Sep 30, 2019

paddycarver added the changelog: no-release-note label Oct 8, 2019

ghost locked and limited conversation to collaborators Oct 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding new resource google_bigtable_gc_policy #3293

adding new resource google_bigtable_gc_policy #3293

eraac commented Mar 21, 2019

eraac commented Mar 21, 2019

gnalsa commented Apr 18, 2019

rileykarson commented May 14, 2019

eraac commented May 14, 2019

rileykarson commented May 20, 2019

eraac commented Aug 12, 2019

rileykarson left a comment

rileykarson Sep 13, 2019

eraac Sep 16, 2019

rileykarson Sep 18, 2019

eraac Sep 19, 2019

rileykarson Sep 19, 2019

rileykarson left a comment

rileykarson Sep 19, 2019

eraac Sep 19, 2019

rileykarson Sep 19, 2019

rileykarson left a comment

eraac commented Sep 30, 2019

rileykarson left a comment

ghost commented Oct 31, 2019

adding new resource google_bigtable_gc_policy #3293

adding new resource google_bigtable_gc_policy #3293

Conversation

eraac commented Mar 21, 2019

eraac commented Mar 21, 2019

gnalsa commented Apr 18, 2019

rileykarson commented May 14, 2019

eraac commented May 14, 2019

rileykarson commented May 20, 2019

eraac commented Aug 12, 2019

rileykarson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rileykarson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rileykarson left a comment

Choose a reason for hiding this comment

eraac commented Sep 30, 2019

rileykarson left a comment

Choose a reason for hiding this comment

ghost commented Oct 31, 2019