test: Add test case for reference regional target tcp proxy in CFR #3148

gemmahou · 2024-11-11T21:02:07Z

Change description

based on #3047, #3112

Add additional test case for compute forwarding rule, verify that it can now reference to a regional tcp proxy

Tests you have done

[N/A] Run make ready-pr to ensure this PR is ready for review.
Perform necessary E2E testing for changed resources.

gemmahou · 2024-11-20T20:35:19Z

No diffs in commit are related to forwarding rule or tcp proxy

yuwenma · 2024-12-12T21:43:10Z

apis/compute/v1beta1/targettcpproxy_types.go

@@ -87,7 +87,7 @@ type ComputeTargetTCPProxyObservedState struct {
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:resource:categories=gcp,shortName=gcpcomputetargettcpproxy;gcpcomputetargettcpproxies
 // +kubebuilder:subresource:status
-// +kubebuilder:metadata:labels="cnrm.cloud.google.com/tf2crd=true";"cnrm.cloud.google.com/stability-level=stable";"cnrm.cloud.google.com/managed-by-kcc=true";"cnrm.cloud.google.com/system=true"
+// +kubebuilder:metadata:labels="cnrm.cloud.google.com/stability-level=stable";"cnrm.cloud.google.com/managed-by-kcc=true";"cnrm.cloud.google.com/system=true"


Let's not turn off the TF-based controller in this PR

Done. Let's enable regional TCP Proxy first #3323 before merging this PR.

yuwenma · 2024-12-16T13:28:54Z

...ata/basic/compute/v1beta1/computeforwardingrule/regionalcomputeforwardingruletcp/create.yaml

+# limitations under the License.
+
+apiVersion: compute.cnrm.cloud.google.com/v1beta1
+kind: ComputeForwardingRule


I think we haven't turned on this resource to direct controller yet. Shall we add the test to make sure it works on direct?

We have it turned on in test:

k8s-config-connector/dev/tasks/run-e2e

Line 29 in 811204f

KCC_USE_DIRECT_RECONCILERS=ComputeForwardingRule,GKEHubFeatureMembership

Let me remove this and add direct annotation in test, to avoid confusions.

yuwenma

/lgtm
/approve
/hold

one suggestions

gemmahou · 2024-12-17T19:13:17Z

I removed the KCC_USE_DIRECT_RECONCILERS flag for CFR and used direct annotation in test yaml. Also added both TF and direct test case for CFR with regional target tcp proxy. For other existing direct tests, we can duplicate them in another PR, if needed.

gemmahou · 2024-12-17T19:17:45Z

...tdata/basic/compute/v1beta1/computeforwardingrule/regionalcomputeforwardingruletcp/_http.log

@@ -1052,18 +1052,13 @@ X-Xss-Protection: 0

 ---

-POST https://compute.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/forwardingRules/${forwardingRuleID}/setLabels
+POST https://compute.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/forwardingRules/${forwardingRuleID}/setTarget


The diffs in this file is because the order of setTarget and setLabels are different in direct controller and TF controller. This would not affect the functionality, and I think the order in TF controller is random... I can double check and if needed we can swap the order of the API calls in direct controller.

yuwenma · 2024-12-18T17:38:15Z

.../basic/compute/v1beta1/computeforwardingrule/globalcomputeforwardingruletcp-direct/_http.log

 Content-Type: application/json
-User-Agent: kcc/controller-manager
-x-goog-request-params: project=${projectId}&target_tcp_proxy=${targetTcpProxyID}
+User-Agent: Terraform/ (+https://www.terraform.io) Terraform-Plugin-SDK/2.10.1 terraform-provider-google-beta/kcc/controller-manager


This means the TCPProxy resource is changed from direct to TF-based. Could you investigate what is missing?

That's expected, in this test, the CFR is direct, TCP proxy is TF based. In another test globalcomputeforwardingruletcp-direct-directtcp, both are direct.

ok, I think I'm confused because this test name is "-direct".

yuwenma · 2024-12-18T17:39:31Z

...asic/compute/v1beta1/computeforwardingrule/regionalcomputeforwardingruletcp-direct/_http.log

Could you upload the real GCP log for this test suite?

This test is for direct CFR with direct regional TCP proxy, diff: https://paste.googleplex.com/6307517609803776

No changes to golden object log, some changes to http log but nothing related to CFR

yuwenma · 2024-12-18T17:39:54Z

...ionalcomputeforwardingruletcp/_generated_object_regionalcomputeforwardingruletcp.golden.yaml

ditto: Could you upload the real GCP Log for this test suite?

This test is for TF-based CFR with direct regional TCP proxy, here's the diff: https://paste.googleplex.com/5134954728783872

direct regional TCP proxy is also covered in its own tests.

I see. the past seems to only contain partial differences. Do you know which resource has those diff?

yuwenma

/lgtm
/approve

yuwenma · 2024-12-19T05:44:23Z

.../basic/compute/v1beta1/computeforwardingrule/globalcomputeforwardingruletcp-direct/_http.log

 Content-Type: application/json
-User-Agent: kcc/controller-manager
-x-goog-request-params: project=${projectId}&target_tcp_proxy=${targetTcpProxyID}
+User-Agent: Terraform/ (+https://www.terraform.io) Terraform-Plugin-SDK/2.10.1 terraform-provider-google-beta/kcc/controller-manager


ok, I think I'm confused because this test name is "-direct".

yuwenma · 2024-12-19T05:45:30Z

...ionalcomputeforwardingruletcp/_generated_object_regionalcomputeforwardingruletcp.golden.yaml

I see. the past seems to only contain partial differences. Do you know which resource has those diff?

yuwenma · 2024-12-19T05:45:42Z

...ata/basic/compute/v1beta1/computeforwardingrule/regionalcomputeforwardingruletcp/create.yaml

+# limitations under the License.
+
+apiVersion: compute.cnrm.cloud.google.com/v1beta1
+kind: ComputeForwardingRule


google-oss-prow · 2024-12-19T05:46:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yuwenma

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~~ [yuwenma]

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

…tests

yuwenma · 2024-12-30T21:33:25Z

/lgtm

gemmahou added do-not-merge/hold service:compute labels Nov 11, 2024

gemmahou force-pushed the reference-regional-tcp branch 2 times, most recently from 956987a to 852d1e5 Compare November 11, 2024 21:44

gemmahou force-pushed the reference-regional-tcp branch 6 times, most recently from 0e09f51 to 8c02d43 Compare November 20, 2024 20:30

gemmahou removed the do-not-merge/hold label Nov 20, 2024

gemmahou assigned yuwenma Nov 20, 2024

gemmahou force-pushed the reference-regional-tcp branch 2 times, most recently from dbae7f1 to 0a82f6a Compare November 20, 2024 23:26

gemmahou modified the milestones: 1.126, 1.127 Dec 4, 2024

yuwenma reviewed Dec 12, 2024

View reviewed changes

gemmahou force-pushed the reference-regional-tcp branch 3 times, most recently from 2361ea9 to 44f85c1 Compare December 13, 2024 22:41

yuwenma reviewed Dec 16, 2024

View reviewed changes

google-oss-prow bot added do-not-merge/hold lgtm approved labels Dec 16, 2024

gemmahou force-pushed the reference-regional-tcp branch from 44f85c1 to 85239aa Compare December 17, 2024 17:36

google-oss-prow bot removed the lgtm label Dec 17, 2024

gemmahou removed the do-not-merge/hold label Dec 17, 2024

gemmahou commented Dec 17, 2024

View reviewed changes

gemmahou force-pushed the reference-regional-tcp branch 3 times, most recently from c57dea4 to e4f26fb Compare December 17, 2024 22:09

yuwenma reviewed Dec 18, 2024

View reviewed changes

yuwenma reviewed Dec 19, 2024

View reviewed changes

google-oss-prow bot added the lgtm label Dec 19, 2024

gemmahou added 2 commits December 19, 2024 17:41

test case for regional TCP proxy in ComputeForwardingRule

ee88294

record mockGCP

fd86bf1

gemmahou force-pushed the reference-regional-tcp branch from e4f26fb to 8a25f08 Compare December 19, 2024 17:42

google-oss-prow bot removed the lgtm label Dec 19, 2024

Remove KCC_USE_DIRECT_RECONCILERS flag for CFR and use annotation in …

dd994d1

…tests

gemmahou force-pushed the reference-regional-tcp branch from 8a25f08 to dd994d1 Compare December 19, 2024 23:00

google-oss-prow bot added the lgtm label Dec 30, 2024

google-oss-prow bot merged commit 974a6ea into GoogleCloudPlatform:master Dec 30, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Add test case for reference regional target tcp proxy in CFR #3148

test: Add test case for reference regional target tcp proxy in CFR #3148

gemmahou commented Nov 11, 2024 •

edited

Loading

gemmahou commented Nov 20, 2024 •

edited

Loading

yuwenma Dec 12, 2024

gemmahou Dec 12, 2024

yuwenma Dec 16, 2024

gemmahou Dec 16, 2024

yuwenma Dec 19, 2024

yuwenma left a comment

gemmahou commented Dec 17, 2024 •

edited

Loading

gemmahou Dec 17, 2024 •

edited

Loading

yuwenma Dec 18, 2024

gemmahou Dec 18, 2024 •

edited

Loading

yuwenma Dec 19, 2024

yuwenma Dec 18, 2024

gemmahou Dec 18, 2024 •

edited

Loading

yuwenma Dec 18, 2024

gemmahou Dec 18, 2024 •

edited

Loading

yuwenma Dec 19, 2024

yuwenma left a comment

yuwenma Dec 19, 2024

yuwenma Dec 19, 2024

yuwenma Dec 19, 2024

google-oss-prow bot commented Dec 19, 2024

yuwenma commented Dec 30, 2024

test: Add test case for reference regional target tcp proxy in CFR #3148

test: Add test case for reference regional target tcp proxy in CFR #3148

Conversation

gemmahou commented Nov 11, 2024 • edited Loading

Change description

Tests you have done

gemmahou commented Nov 20, 2024 • edited Loading

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

yuwenma left a comment

Choose a reason for hiding this comment

gemmahou commented Dec 17, 2024 • edited Loading

gemmahou Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gemmahou Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gemmahou Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gemmahou Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuwenma 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

google-oss-prow bot commented Dec 19, 2024

yuwenma commented Dec 30, 2024

gemmahou commented Nov 11, 2024 •

edited

Loading

gemmahou commented Nov 20, 2024 •

edited

Loading

gemmahou commented Dec 17, 2024 •

edited

Loading

gemmahou Dec 17, 2024 •

edited

Loading

gemmahou Dec 18, 2024 •

edited

Loading

gemmahou Dec 18, 2024 •

edited

Loading

gemmahou Dec 18, 2024 •

edited

Loading