Add test case for 3 control plane nodes and internal load balancer #1550

bochengchu · 2025-10-23T21:23:13Z

What type of PR is this?
/kind other

What this PR does / why we need it:
Follow-up for #1536. This should fail now, but should be passing after #1533 is merged.
Local test log: log.txt
With #1533, local test passed:
log.txt

Release note:

NONE

k8s-ci-robot · 2025-10-23T21:23:16Z

@bochengchu: The label(s) kind/other cannot be applied, because the repository doesn't have them.

In response to this:

What type of PR is this?
/kind other

What this PR does / why we need it:
Follow-up for #1536. This should fail now, but should be passing after #1533 is merged.

Release note:
NONE

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot · 2025-10-23T21:23:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bochengchu
Once this PR has been reviewed and has the lgtm label, please assign cpanato for approval. For more information see the 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

netlify · 2025-10-23T21:23:20Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-gcp ready!

Name	Link
🔨 Latest commit	`b733860`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-cluster-api-gcp/deploys/6904f450430e230007077092
😎 Deploy Preview	https://deploy-preview-1550--kubernetes-sigs-cluster-api-gcp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2025-10-23T21:23:23Z

Hi @bochengchu. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

justinsb · 2025-10-24T12:25:48Z

config/default/manager_image_patch.yaml

      containers:
      # Change the value of image field below to your controller image URL
-      - image: gcr.io/k8s-staging-cluster-api-gcp/cluster-api-gcp-controller:e2e
+      - image: ${CONTROLLER_IMAGE}


Do we need to to do this? The tests are running I believe?

It's not a blocker for ok-to-test, but it will likely be a blocker for merging. If we have to do this I suggest splitting it out into its own commit (or PR?), and a comment would help out future selves understand why

This is needed for the GKE cluster, since we have to push the image to GCR. Unlike the existing kind clusters where we can load the image into the cluster, for GKE we have to upload it.

justinsb · 2025-10-24T12:27:52Z

scripts/ci-e2e.sh


+# create a GKE cluster to be used as a bootstrap cluster
+create_gke_bootstrap_cluster() {
+  gcloud container clusters create "${TEST_NAME}-gke-bootstrap" --project "$GCP_PROJECT" \


OK this is nice, because we can we can develop Workload Identity support, and then hopefully add the same thing to kind. (Workload Identity is Google's name for automatic authentication to GCP without needing a ServiceAccount Key in a secret, historically it used a proxy but now it can use identity federation via IAM workloadPools).

But ... probably could be a separate commit or PR. This one is less of a blocker though IMO, because it's additive

justinsb · 2025-10-24T12:30:21Z

scripts/ci-e2e.sh

+  if [[ -n "${SKIP_INIT_GKE_BOOTSTRAP:-}" ]]; then
+    echo "Skipping GKE bootstrap cluster initialization..."
+  else
+    create_gke_bootstrap_cluster


I think we don't have permission to create GKE clusters in prow (and we probably don't want to take a hard dependency on GKE in OSS testing). I suggest this should be a different script, or that the default behavior should not change (e.g. only create a GKE cluster if TEST_MANAGEMENT_CLUSTER=gke or something like that)

Sounds good, if we end up using GKE cluster I can separate that.

justinsb · 2025-10-24T12:31:32Z

test/e2e/data/infrastructure-gcp/cluster-template-ci-with-external-and-internal-lb.yaml

@@ -0,0 +1,173 @@
+---
+apiVersion: cluster.x-k8s.io/v1beta1


Not something to be addressed in this PR (i.e. this problem already exists), but we should consider using kustomize or patching a base config in code, so we can understand more easily what is different in each scenario.

justinsb · 2025-10-24T12:52:54Z

test/e2e/e2e_test.go

 		Expect(e2eConfig).ToNot(BeNil(), "Invalid argument. e2eConfig can't be nil when calling %s spec", specName)
 		Expect(clusterctlConfigPath).To(BeAnExistingFile(), "Invalid argument. clusterctlConfigPath must be an existing file when calling %s spec", specName)
 		Expect(bootstrapClusterProxy).ToNot(BeNil(), "Invalid argument. bootstrapClusterProxy can't be nil when calling %s spec", specName)
+		Expect(bootstrapGKEClusterProxy).ToNot(BeNil(), "Invalid argument. bootstrapGKEClusterProxy can't be nil when calling %s spec", specName)


I'm not sure what this is (yet). Is this required if we use kind for the bootstrap cluster (instead of GKE)?

justinsb · 2025-10-24T12:53:45Z

test/e2e/e2e_test.go


+	Context("Creating a control-plane cluster with an internal load balancer", func() {
+		It("Should create a cluster with 1 control-plane and 1 worker node with an internal load balancer", func() {
+			// This test requires a GKE bootstrap cluster.


Does it have to require a GKE cluster? I don't think we allow GKE in prow (though we'll find out in a minute I guess!)

It doesn't required to be GKE, but using a GKE cluster is the simplest way I can think of. Because of the ILB, we have to let the controllers stay within the same private network. GKE is one way to do that, and we can also have a kind cluster like the existing tests, but it'd be much more complicated to configure that. One possibility I found is to use Cloud VPN and create a tunnel or something, and we have to configure the network (and router?) stuff on where the kind cluster is hosted.

justinsb · 2025-10-24T12:55:39Z

I'm not sure we can take a dependency on GKE in our prow tests, and I don't know if we have to (maybe it's because otherwise the internal LB is not reachable?)

But ... that's a potential blocker for merging, not for testing...

/ok-to-test

bochengchu · 2025-10-24T20:55:17Z

In addition to the permission to create a GKE cluster, the permission to push image to GCR is also needed in this PR. I'm not sure how can we add that though.

justinsb · 2025-10-28T19:19:17Z

So IIUC:

We want to test with internal load balancers because that is where the bug shows up
To test with internal load balancers, we need to be on the same network
So you added testing with GKE (without removing kind support)

That seems sensible to me. We could add a VPN connection or an SSH port-forward or something similar, but that seems pretty complicated. It would also only be valuable for testing, whereas I suspect support for GKE as management for cluster (and testing it) would be generally useful. We could also verify that we support workload identity on GKE, then we could add that support to kind-management-clusters. So lots to like here.

But: we are missing some prow permissions, in particular creating a GKE cluster. For images, there should be places we can push to, possibly even the staging locations.

I don't know if we would consider the lack of automated testing a blocker for the bug fix to merge, or even for this test to merge. I could see us running it manually and then calling that good enough while we figure out how/whether we can get automated testing going with prow.

bochengchu · 2025-11-03T18:31:46Z

I skip the tests for now and remove the code for uploading images and creating GKE clusters while we are still figuring them out.

Also attached the logs from local testing.

bochengchu · 2025-11-04T21:54:46Z

@justinsb Could you take a look at this again? Thanks!

k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Oct 23, 2025

k8s-ci-robot requested review from cpanato and salasberryfin October 23, 2025 21:23

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 23, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 23, 2025

justinsb mentioned this pull request Oct 24, 2025

Minimal MachinePool support #1506

Open

justinsb reviewed Oct 24, 2025

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 24, 2025

bochengchu force-pushed the e2e-3cp branch from 5e170ad to 428c23d Compare October 30, 2025 23:24

bochengchu added 2 commits October 31, 2025 10:36

add a e2e test for internal load balancer only

bee30e1

add test case for 3 control plane nodes and ilb

b733860

bochengchu force-pushed the e2e-3cp branch from 428c23d to b733860 Compare October 31, 2025 17:39

Add test case for 3 control plane nodes and internal load balancer #1550

Are you sure you want to change the base?

Add test case for 3 control plane nodes and internal load balancer #1550

Conversation

bochengchu commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Oct 23, 2025

Uh oh!

k8s-ci-robot commented Oct 23, 2025

Uh oh!

netlify bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-cluster-api-gcp ready!

Uh oh!

k8s-ci-robot commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinsb commented Oct 24, 2025

Uh oh!

bochengchu commented Oct 24, 2025

Uh oh!

justinsb commented Oct 28, 2025

Uh oh!

bochengchu commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bochengchu commented Nov 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bochengchu commented Oct 23, 2025 •

edited

Loading

netlify bot commented Oct 23, 2025 •

edited

Loading

bochengchu commented Nov 3, 2025 •

edited

Loading