[Cloud Bigtable App Profile Beta Migration] Add direct controller #4345

djyau · 2025-04-11T15:50:01Z

Change description

Fixes #

Tests you have done

Run make ready-pr to ensure this PR is ready for review.
Perform necessary E2E testing for changed resources.

jingyih

We need to test the behavior of the new controller using the existing tests. Let's remove the annotation "cnrm.cloud.google.com/tf2crd=true";"cnrm.cloud.google.com/stability-level=stable" and regenerate the CRD. Then the resource should be reconciled by the new controller.

https://github.com/GoogleCloudPlatform/k8s-config-connector/blob/3bfa3c8c489c280e1d321e6b4f5d5b8c083f21fe/apis/bigtable/v1beta1/appprofile_types.go#L88C113-L88C195

djyau · 2025-04-23T00:07:13Z

We need to test the behavior of the new controller using the existing tests. Let's remove the annotation "cnrm.cloud.google.com/tf2crd=true";"cnrm.cloud.google.com/stability-level=stable" and regenerate the CRD. Then the resource should be reconciled by the new controller.

https://github.com/GoogleCloudPlatform/k8s-config-connector/blob/3bfa3c8c489c280e1d321e6b4f5d5b8c083f21fe/apis/bigtable/v1beta1/appprofile_types.go#L88C113-L88C195

Done - removed the annotations and pushed the resulting generated CRD to the PR

pkg/controller/direct/bigtable/bigtableappprofile_controller.go

mockgcp/mockbigtable/appprofile.go

...a/basic/bigtable/v1beta1/bigtableappprofile/_generated_object_bigtableappprofile.golden.yaml

jingyih · 2025-04-29T20:43:45Z

...a/basic/bigtable/v1beta1/bigtableappprofile/_generated_object_bigtableappprofile.golden.yaml

@@ -22,7 +21,6 @@ spec:
  - cluster1-${uniqueId}
  - cluster2-${uniqueId}
  multiClusterRoutingUseAny: true
-  resourceID: bigtableappprofile-${uniqueId}


@maqiuyujoyce For TF-based controller, we are writing "resourceID" back to Spec even though we set "state-into-spec: absent"?

Yes, we treat spec.resourceID differently in TF-based/DCL-based controllers, and we always write them back. E.g. https://github.com/GoogleCloudPlatform/k8s-config-connector/blob/master/pkg/krmtotf/tftokrm.go#L229

...a/basic/bigtable/v1beta1/bigtableappprofile/_generated_object_bigtableappprofile.golden.yaml

jingyih · 2025-04-29T22:46:52Z

Can you run "make manifests" to fix the validation error in presubmit?

Remove file to regenerate it Regenerate controller, make changes to it Fix Delete and Get Add Create/Update for existing fields Dont set Name in Create, since its unused Make helper function Tweak comment Pull latest bigtable goclient version Make use of updated bigtable goclient API in controller Clean up controller Run make ensure Remove annotations and regenerate CRD Remove HTTP client options from client creation call

Fix controller update function

…og with GRPC client options Record real gcp log after cleaning up instances

pkg/controller/direct/bigtable/appprofile_mappings.go

log: mock gcp with controller logic Address PR comments Put name back to golden file Fix to update mask Set Name Make manifests

Clean up log: mock gcp

jingyih · 2025-05-02T08:58:08Z

pkg/test/resourcefixture/testdata/basic/bigtable/v1beta1/bigtableappprofile/_http.log

@@ -309,6 +309,68 @@ OK

 GRPC /google.bigtable.admin.v2.BigtableInstanceAdmin/UpdateAppProfile

+{
+  "appProfile": {


Not sure why there an extra "UpdateAppProfile" call with this change? The RPC body still contains the old field values. Is this from the mock or the real GCP?

Oh yeah, that is weird. When I do either "record-gcp" or "compare-mock", neither one removes this

Upon closer look, there are 2 updates here for some reason...the first update (line 310) doesn't update the description but does update multiclusterRouting and standardIsolation

Then the second update (line 371) updates the description along with both of those fields that were already updated in the first call.

jingyih

/lgtm
/approve

google-oss-prow · 2025-05-05T21:45:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingyih

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jingyih]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jingyih · 2025-05-05T22:39:22Z

For reference, I have also verified that following sample tests passed.

$ GOLDEN_REQUEST_CHECKS=1 GOLDEN_OBJECT_CHECKS=1 E2E_KUBE_TARGET=envtest RUN_E2E=1 E2E_GCP_TARGET=real go test -test.count=1 -timeout 3600s -v ./tests/e2e -run TestAllInSeries/samples/multicluster-bigtable-app-profile

$ GOLDEN_REQUEST_CHECKS=1 GOLDEN_OBJECT_CHECKS=1 E2E_KUBE_TARGET=envtest RUN_E2E=1 E2E_GCP_TARGET=real go test -test.count=1 -timeout 3600s -v ./tests/e2e -run TestAllInSeries/samples/single-cluster-bigtable-app-profile

$ GOLDEN_REQUEST_CHECKS=1 GOLDEN_OBJECT_CHECKS=1 E2E_KUBE_TARGET=envtest RUN_E2E=1 E2E_GCP_TARGET=real go test -test.count=1 -timeout 3600s -v ./tests/e2e -run TestAllInSeries/samples/priority-bigtable-app-profile

doc: update release note from #4345

djyau force-pushed the controller branch 4 times, most recently from 8facc4b to fb138a6 Compare April 18, 2025 15:08

djyau force-pushed the controller branch 2 times, most recently from 9e4084f to 34b7ff4 Compare April 22, 2025 20:12

jingyih reviewed Apr 22, 2025

View reviewed changes

djyau force-pushed the controller branch from 34b7ff4 to 486a8c1 Compare April 23, 2025 00:04

jingyih reviewed Apr 23, 2025

View reviewed changes

pkg/controller/direct/bigtable/bigtableappprofile_controller.go Outdated Show resolved Hide resolved

djyau mentioned this pull request Apr 29, 2025

[Cloud Bigtable App Profile Beta Migration] Record real GCP log after cleaning up project instance #4450

Open

2 tasks

jingyih reviewed Apr 29, 2025

View reviewed changes

...a/basic/bigtable/v1beta1/bigtableappprofile/_generated_object_bigtableappprofile.golden.yaml Show resolved Hide resolved

djyau force-pushed the controller branch from 79b13f4 to 443f399 Compare April 29, 2025 23:42

djyau added 4 commits April 30, 2025 00:01

Use grpc client options

d3e8d74

Fix controller update function

Fix update paths in mockgcp for bigtable app profile

29623b9

Add IgnoreWarnings when switching to multi cluster. Record real gcp l…

210b371

…og with GRPC client options Record real gcp log after cleaning up instances

djyau force-pushed the controller branch from c0931e7 to 8c7b266 Compare April 30, 2025 00:01

jingyih reviewed Apr 30, 2025

View reviewed changes

pkg/controller/direct/bigtable/appprofile_mappings.go Outdated Show resolved Hide resolved

djyau added 2 commits April 30, 2025 15:19

Record real gcp log with controller

82abd3f

log: mock gcp with controller logic Address PR comments Put name back to golden file Fix to update mask Set Name Make manifests

Write resourceID back into Spec, re-record real gcp log

babeaab

Clean up log: mock gcp

djyau force-pushed the controller branch from 7b486f5 to babeaab Compare April 30, 2025 15:19

jingyih reviewed May 2, 2025

View reviewed changes

djyau and others added 2 commits May 5, 2025 14:28

Clear http log and re record real gcp from scratch

b06ab32

BigtableAppProfile: properly handle update in controller

8f5aa9d

jingyih reviewed May 5, 2025

View reviewed changes

google-oss-prow bot assigned jingyih May 5, 2025

google-oss-prow bot added the lgtm label May 5, 2025

google-oss-prow bot added the approved label May 5, 2025

google-oss-prow bot merged commit 9033f2e into GoogleCloudPlatform:master May 5, 2025
33 checks passed

jingyih added a commit to jingyih/k8s-config-connector that referenced this pull request May 5, 2025

doc: update release note from GoogleCloudPlatform#4345

d968626

jingyih added a commit to jingyih/k8s-config-connector that referenced this pull request May 6, 2025

doc: update release note from GoogleCloudPlatform#4345

ea8af44

google-oss-prow bot added a commit that referenced this pull request May 6, 2025

Merge pull request #4476 from jingyih/releasenote

51be73c

doc: update release note from #4345

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Cloud Bigtable App Profile Beta Migration] Add direct controller #4345

[Cloud Bigtable App Profile Beta Migration] Add direct controller #4345

Uh oh!

djyau commented Apr 11, 2025

Uh oh!

jingyih left a comment

Uh oh!

djyau commented Apr 23, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jingyih Apr 29, 2025

Uh oh!

maqiuyujoyce Apr 29, 2025

Uh oh!

Uh oh!

jingyih commented Apr 29, 2025

Uh oh!

Uh oh!

jingyih May 2, 2025

Uh oh!

djyau May 2, 2025

Uh oh!

djyau May 5, 2025

Uh oh!

jingyih left a comment

Uh oh!

google-oss-prow bot commented May 5, 2025

Uh oh!

Uh oh!

jingyih commented May 5, 2025

Uh oh!

Uh oh!

[Cloud Bigtable App Profile Beta Migration] Add direct controller #4345

[Cloud Bigtable App Profile Beta Migration] Add direct controller #4345

Uh oh!

Conversation

djyau commented Apr 11, 2025

Change description

Tests you have done

Uh oh!

jingyih left a comment

Choose a reason for hiding this comment

Uh oh!

djyau commented Apr 23, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jingyih Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

maqiuyujoyce Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jingyih commented Apr 29, 2025

Uh oh!

Uh oh!

jingyih May 2, 2025

Choose a reason for hiding this comment

Uh oh!

djyau May 2, 2025

Choose a reason for hiding this comment

Uh oh!

djyau May 5, 2025

Choose a reason for hiding this comment

Uh oh!

jingyih left a comment

Choose a reason for hiding this comment

Uh oh!

google-oss-prow bot commented May 5, 2025

Uh oh!

Uh oh!

jingyih commented May 5, 2025

Uh oh!

Uh oh!