Migrate BQ reservation from TF-based to direct controller v1alpha1

[600lyy]

Change description

1. Generated API and CRD
2. Added resource mappings between KRM and API
3. Generated direct controller

Tests you have done

KCC_USE_DIRECT_RECONCILERS=BigqueryReservationReservation hack/compare-mock fixture/bigqueryreservationenterprise

KCC_USE_DIRECT_RECONCILERS=BigqueryReservationReservation hack/compare-mock fixture/bigqueryreservationstandard

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

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign justinsb for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[jingyih]

Can we rerun the test to update the golden object?

https://github.com/GoogleCloudPlatform/k8s-config-connector/actions/runs/13117527154/job/36595380463#step:4:17534

The diff looks fine to me. The default value of annotation "cnrm.cloud.google.com/management-conflict-prevention-policy" is "none".

    utils.go:235: FAIL: unexpected diff in /home/runner/work/k8s-config-connector/k8s-config-connector/pkg/test/resourcefixture/testdata/basic/bigqueryreservation/v1alpha1/bigqueryreservationreservation/bigqueryreservationenterprise/_generated_object_bigqueryreservationenterprise.golden.yaml:   map[string]any{
          	"apiVersion": string("bigqueryreservation.cnrm.cloud.google.com/v1alpha1"),
          	"kind":       string("BigQueryReservationReservation"),
          	"metadata": map[string]any{
        - 		"annotations": map[string]any{"cnrm.cloud.google.com/management-conflict-prevention-policy": string("none")},
          		"finalizers":  []any{string("cnrm.cloud.google.com/finalizer"), string("cnrm.cloud.google.com/deletion-defender")},
          		"generation":  float64(2),
          		... // 3 identical entries
          	},
          	"spec":   map[string]any{"autoscale": map[string]any{"maxSlots": float64(100)}, "concurrency": float64(1), "edition": string("ENTERPRISE"), "ignoreIdleSlots": bool(true), ...},
          	"status": map[string]any{"conditions": []any{map[string]any{"lastTransitionTime": string("1970-01-01T00:00:00Z"), "message": string("The resource is up to date"), "reason": string("UpToDate"), "status": string("True"), ...}}, "externalRef": string("projects/${projectId}/locations/us-west2/reservations/bigqueryre"...), "observedGeneration": float64(2), "observedState": map[string]any{}},
          }

[jingyih]

I'm not sure how primaryLocation, secondaryLocation, and originalPrimaryLocation are being used. Could we capture their usage by adding a test? Alternatively, we could comment them out for now and reintroduce them later.

[600lyy]

These fields are being used only for the BQ managed DR when the BQ edition is enterprise-plus.
I will be adding more tests to capture different scenarios

[600lyy]

primaryLocation and originalPrimaryLocation are fine, they're both output-only. So when the secondaryLocation is being specified, this means the managed DR is enabled, in this case the API will return primaryLocation, secondaryLocation and originalPrimaryLocation in the response.

see the http.log:

  "originalPrimaryLocation": "us-west2",
  "primaryLocation": "us-west2",
  "secondaryLocation": "us-east1",

The only tricky part is secondaryLocation. According to the API, users can set this in create reservation calls to create a failover reservation or in update reservation calls to convert a non-failover reservation to a failover reservation(or vice versa).

[jingyih]

According to the API, users can set this in create reservation calls to create a failover reservation or in update reservation calls to convert a non-failover reservation to a failover reservation(or vice versa).

Can we add one test case to convert a failover reservation to a non-failover; and one test case to convert a non failover to a failover?

[600lyy]

Uploaded two tests to capture the http logs from real gcp. Turns out that API doesn't change the output returned to the PATCH requests

commit 7

[jingyih]

Turns out that API doesn't change the output returned to the PATCH requests

I left a comment regarding updateMask in the http PATCH request. I think it is related to the behavior you observed here. Let's fix the update logic in controller first.

[600lyy]

Thanks!

I've fixed the updateMask and rerun tests against both mockgcp and real target.

• Fixed the update Mask
  [commit 9] (74a97ee)

• Fixed the morkgcp
  [commit 13] (c3ad7e0)

[jingyih]

Could you record the controller builder commands used for types and mappings in generate.sh? Also, when rerunning the script, it should remove the type Reservation struct since it already exists in the reservation_types file.

https://github.com/GoogleCloudPlatform/k8s-config-connector/blob/master/dev/tools/controllerbuilder/generate.sh

[600lyy]

I'm a bit confused, do you mean that I should rerun the command below to generate the type for reservation?

go run main.go generate-controller \
     --service google.cloud.bigquery.reservation.v1 \
     --api-version "bigqueryreservation.cnrm.cloud.google.com/v1alpha1" \
     --kind BigqueryReservationReservation \
     --proto-resource Reservation

[jingyih]

Yeah rerunning the command should help cleanup the unnecessary code in types.generated.go.

[600lyy]

Nothing has changed after rerunning the command, this is the output:

file /usr/local/google/home/luyul/go/src/github.com/600lyy/k8s-config-connector/pkg/controller/direct/bigqueryreservation/reservation_controller.go already exists, skipping
New controller Reservation has been registered.

[jingyih]

Turns out that API doesn't change the output returned to the PATCH requests

I left a comment regarding updateMask in the http PATCH request. I think it is related to the behavior you observed here. Let's fix the update logic in controller first.