Skip to content

Conversation

@barbacbd
Copy link

CORS-4264: Update the GCP provider to allow users to skip firewall actions

cluster:
Update the scripts to include the new variables

providers/gce:

Update the config to include the new `ManageFirewallRules` boolean setting. This
variable will allow users to skip the creation, deletion, and updates to firewall
rules when set to false. Users may not want or have the ability to add the permissions
to perform these actions on their service account. When this is the case the firewall rules
should be pre created and managed by someone with permissions to achieve the same goal.

Fedosin and others added 30 commits September 22, 2021 14:42
We need to compile the whole module instead of a single file to
make GCP CCM work.
# Conflicts:
#	pkg/controller/nodeipam/OWNERS
….10-ose-gcp-cloud-controller-manager

Updating ose-gcp-cloud-controller-manager images to be consistent with ART
This commit changes owners of the project to people from OpenShift.
We need to compile the whole module instead of a single file to
make GCP CCM work.
…m rebase

# Conflicts:
#	vendor/github.com/google/go-tpm/tpmutil/BUILD
This commit changes owners of the project to people from OpenShift.
We need to compile the whole module instead of a single file to
make GCP CCM work.
…m rebase

# Conflicts:
#	vendor/github.com/google/go-tpm/tpmutil/BUILD

# Conflicts:
#	vendor/github.com/googleapis/gax-go/v2/BUILD
#	vendor/golang.org/x/oauth2/google/BUILD
#	vendor/golang.org/x/oauth2/google/internal/externalaccount/BUILD
#	vendor/golang.org/x/sys/unix/BUILD
#	vendor/golang.org/x/sys/windows/BUILD
#	vendor/google.golang.org/api/internal/gensupport/BUILD
#	vendor/google.golang.org/api/option/internaloption/BUILD
#	vendor/google.golang.org/protobuf/types/descriptorpb/BUILD
# Conflicts:
#	crd/client/gcpfirewall/informers/externalversions/BUILD
#	crd/client/gcpfirewall/informers/externalversions/gcpfirewall/v1alpha1/BUILD
#	crd/client/gcpfirewall/informers/externalversions/internalinterfaces/BUILD
#	crd/client/gcpfirewall/listers/gcpfirewall/v1alpha1/BUILD
#	vendor/github.com/googleapis/gax-go/v2/apierror/internal/proto/BUILD
#	vendor/golang.org/x/oauth2/authhandler/BUILD
#	vendor/google.golang.org/genproto/googleapis/rpc/code/BUILD
#	vendor/google.golang.org/genproto/googleapis/rpc/errdetails/BUILD
Ran:
go mod tidy && ./tools/update_vendor.sh && ./tools/update_bazel.sh
Bug 2041509: Rebase CCM onto latest changes with K8s 1.23 updates
Based on docs for internal loadbalancer here [1], backend services [2]
and instances in instance-groups [3], following restrictions apply:

- Internal LB can load balance to VMs in same region, but different
  subnets
- Instance groups for the backend service must contain instance of
  the same subnet
- An instance can only belong to one load balanced instance group
- It is probably useful use-case to have nodes for the cluster belong
  to more than one subnet. And the current setup fails to create an
  internal load balancer with nodes in multiple subnets.

This change finds pre-existing instance-groups that ONLY contain
instances that belong to the cluster, uses them for the backend
service. And only ensures instance-groups for remaining ones.

[1] https://cloud.google.com/load-balancing/docs/internal
[2] https://cloud.google.com/load-balancing/docs/backend-service#restrictions_and_guidance
[3] https://cloud.google.com/compute/docs/instance-groups/creating-groups-of-unmanaged-instances#addinstances

Co-authored-by: Abhinav Dahiya <[email protected]>
theobarberbany and others added 14 commits August 12, 2025 15:02
This commit rewrites 49f5389.

Work around GCP internal load balancer restrictions for multi-subnet clusters.

GCP internal load balancers have specific restrictions that prevent
straightforward load balancing across multiple subnets:

1. "Don't put a VM in more than one load-balanced instance group"
2. Instance groups can "only select VMs that are in the same zone, VPC network, and subnet"
3. "All VMs in an instance group must have their primary network interface in the same VPC network"
4. Internal LBs can load balance to VMs in same region but different subnets

For clusters with nodes across multiple subnets, the previous implementation
would fail to create internal load balancers. This change implements a
two-pass approach:

1. Find existing external instance groups (matching externalInstanceGroupsPrefix)
   that contain ONLY cluster nodes and reuse them for the backend service
2. Create internal instance groups only for remaining nodes not covered by
   external groups

This ensures compliance with GCP restrictions while enabling multi-subnet
load balancing for Kubernetes clusters.

References:
- Internal LB docs: https://cloud.google.com/load-balancing/docs/internal
- Backend service restrictions: https://cloud.google.com/load-balancing/docs/backend-service#restrictions_and_guidance
- Instance group constraints: https://cloud.google.com/compute/docs/instance-groups/creating-groups-of-unmanaged-instances#addinstances

🤖 Commit message & comments Generated with [Claude Code](https://claude.ai/code)
NO-JIRA: UPSTREAM: 894: Adding SyncMutex to nodeipam unit tests
 OCPBUGS-60772: Reuse instance groups
OCPBUGS-61006: Adjust vendoring to use go.work to get rid of the symlink
….21-ose-gcp-cloud-controller-manager

OCPBUGS-62572: Updating ose-gcp-cloud-controller-manager-container image to be consistent with ART for 4.21
…tions

cluster:
Update the scripts to include the new variables

providers/gce:

Update the config to include the new `ManageFirewallRules` boolean setting. This
variable will allow users to skip the creation, deletion, and updates to firewall
rules when set to false. Users may not want or have the ability to add the permissions
to perform these actions on their service account. When this is the case the firewall rules
should be pre created and managed by someone with permissions to achieve the same goal.
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Oct 20, 2025
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If the repository mantainers determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Details

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 needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2025
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

Details

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
Copy link
Contributor

Welcome @barbacbd!

It looks like this is your first PR to kubernetes/cloud-provider-gcp 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/cloud-provider-gcp has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

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

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

Details

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.

@linux-foundation-easycla
Copy link

CLA Missing ID CLA Not Signed

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. sig/network Categorizes an issue or PR as relevant to SIG Network. labels Oct 20, 2025
@k8s-ci-robot k8s-ci-robot requested review from aojea and bowei October 20, 2025 20:07
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details 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

@theobarberbany
Copy link
Contributor

This needs to branch from the upstream (kubernetes/cloud-provider-gcp) branch, not our OpenShift fork :)

@barbacbd barbacbd closed this by deleting the head repository Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/network Categorizes an issue or PR as relevant to SIG Network. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.