Skip to content

Conversation

@Qqkyu
Copy link
Contributor

@Qqkyu Qqkyu commented Oct 21, 2025

When migrating tests from kubetest(1) to kubetest2, the --gke-subnet-mode=custom is not migratable for a single-project profile. With this PR we can keep the existing logic when the --subnet-mode is not specified, and allow to overwrite it when needed

@k8s-ci-robot k8s-ci-robot requested review from dims and upodroid October 21, 2025 10:58
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 21, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @Qqkyu. 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.

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 21, 2025
@Qqkyu
Copy link
Contributor Author

Qqkyu commented Oct 21, 2025

cc: @BenTheElder @aojea

@aojea
Copy link
Contributor

aojea commented Oct 21, 2025

/ok-to-test

@k8s-ci-robot 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 21, 2025
@k8s-triage-robot
Copy link

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-contributor-experience at kubernetes/community.

/check-cla
/easycla

1 similar comment
@k8s-triage-robot
Copy link

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-contributor-experience at kubernetes/community.

/check-cla
/easycla

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 21, 2025
@Qqkyu Qqkyu changed the title Add '--subnet-mode' gke-deployer flag Add --subnet-mode gke-deployer flag Oct 22, 2025
PrivateClusterAccessLevel string `flag:"~private-cluster-access-level" desc:"Private cluster access level, if not empty, must be one of 'no', 'limited' or 'unrestricted'. See the details in https://cloud.google.com/kubernetes-engine/docs/how-to/private-clusters."`
PrivateClusterMasterIPRanges []string `flag:"~private-cluster-master-ip-range" desc:"Private cluster master IP ranges. It should be IPv4 CIDR(s), and its length must be the same as the number of clusters if private cluster is requested."`
SubnetworkRanges []string `flag:"~subnetwork-ranges" desc:"Subnetwork ranges as required for shared VPC setup as described in https://cloud.google.com/kubernetes-engine/docs/how-to/cluster-shared-vpc#creating_a_network_and_two_subnets. For multi-project profile, it is required and should be in the format of 10.0.4.0/22 10.0.32.0/20 10.4.0.0/14,172.16.4.0/22 172.16.16.0/20 172.16.4.0/22, where the subnetworks configuration for different project are separated by comma, and the ranges of each subnetwork configuration is separated by space."`
SubnetMode string `flag:"~subnet-mode" desc:"Subnet creation mode of the GKE cluster network (default 'auto' for single-project, and 'custom' for multi-project use cases)."`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems that the only thing we can choose is to set the default subnet to be auto or custom, and even that . it can only be changed if there is one project ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, correct. --subnet-mode flag can only effectively change the behavior in a single-project scenario.

Do you think the flag description should make this more clear or do you have any other suggestions?

I can update the flag description to something like: Set the subnet creation mode ('auto' or 'custom'). This flag can only override the default mode for single-project deployments. Multi-project deployments always use 'custom'. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I was wondering, if this is final and you choose a bool, then is less error prone, you always have a working setup or I think there is only a wrong combination
If you set the enum, then the user will have to figure out the combination of working solutions, taht seems to be multiple

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I changed it - ptal

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 28, 2025
@Qqkyu Qqkyu force-pushed the gke-deployer-add-subnet-mode-option branch from bcf894e to 5065895 Compare October 29, 2025 11:32
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 29, 2025
@Qqkyu
Copy link
Contributor Author

Qqkyu commented Nov 7, 2025

@aojea can I get an lgtm here or do you have any other suggestions?

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 13, 2025
@Qqkyu Qqkyu force-pushed the gke-deployer-add-subnet-mode-option branch from 3c805ed to 87111f1 Compare November 13, 2025 08:19
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Qqkyu

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:

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

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 13, 2025
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 13, 2025
@aojea
Copy link
Contributor

aojea commented Nov 17, 2025

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2025
@k8s-ci-robot k8s-ci-robot merged commit 568faa0 into kubernetes-sigs:master Nov 17, 2025
12 checks passed
@Qqkyu Qqkyu deleted the gke-deployer-add-subnet-mode-option branch November 17, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants