gce: Support cloudLabels for InstanceGroup by rramkumar1 · Pull Request #17821 · kubernetes/kops

rramkumar1 · 2025-12-16T21:39:25Z

Moved existing default label application logic for GCE IG to central CloudTagsForInstanceGroup. We then call this function when generating the IG template which also gives us the user supplied cloudLabels from both the Cluster and InstanceGroup resource. I have explicitly not plumbed through support for node labels and have left it for a TODO. There was an issue with GCE label length limits that I ran into in the e2e test.

Note that a simple refactor would be to move the shared CloudTagsForInstanceGroup function to respective context.go files in each cloud provider directory. I think this is better served in a follow up PR to ensure consistency at this very moment in case we don't get to the refactor.

In terms of testing, I have added a single integration test which verifies that a cloudLabel added to the InstanceGroup is reflected properly in the output Terraform. This doesn't necessarily cover all cases (e.g what happens when cluster.cloudLabel and instanceGroup.cloudLabel) are specified. I think these would be better served as unit tests which should be added when the aformentioned refactor is complete.

Ref: #17638

k8s-ci-robot · 2025-12-16T21:39:34Z

Welcome @rramkumar1!

It looks like this is your first PR to kubernetes/kops 🎉. 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/kops 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 · 2025-12-16T21:39:35Z

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

ameukam · 2025-12-17T10:20:59Z

/ok-to-test

justinsb · 2025-12-17T15:37:44Z

 		case kops.CloudProviderHetzner:
 			labels[hetzner.TagKubernetesNodeLabelPrefix+k] = v
+		case kops.CloudProviderGCE:
+			// TODO: Do nothing for now while we figure out how to address GCE label length limit of 63


I'm guessing the label is too long for GCE, and the cluster-autoscaler doesn't look at this label anyway on GCE?

I see, I was not aware that the labels might be a no-op on GCE. Does that need to be looked into more or are you sure on that fact? If so, I can just change the comment.

justinsb · 2025-12-17T15:38:56Z


 			t.ServiceAccounts = append(t.ServiceAccounts, b.LinkToServiceAccount(ig))

-			// labels, err := b.CloudTagsForInstanceGroup(ig)


lol, thanks for fixing ... I'll have to go splunking to figure out why we commented this out and special-cased it :-)

Yeah I tried but it's hard to follow the change history because a few of the recent changes were just formatting.

AFAICT this was commented out in the original code in 2017, so ... thanks for fixing :-)

rramkumar1 · 2025-12-17T20:01:41Z

/assign @justinsb

hakman · 2025-12-17T20:12:14Z

 	fs := bindFlags(d)

+	// Ensure https://github.com/Azure/rg-cleanup deletes removes resources
+	if d.CloudProvider == "azure" {


I don't think at this stage d.CloudProvider is available.

I think it becomes available after bindFlags() no? If not, any suggestions on where to place this conditional?

The issue I ran into is that the "creationTimestamp" label is not a valid label format for GCE.

The tags issue should be fixed via #17826. Let's see how things work afterwards.

rramkumar1 · 2025-12-18T16:01:56Z

/retest

rramkumar1 · 2025-12-18T19:47:18Z

/retest

hakman · 2025-12-18T20:39:18Z

Failed tests are unrelated. Thanks @rramkumar1!
/lgtm
/approve

k8s-ci-robot · 2025-12-18T20:39:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [hakman]

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

hakman · 2025-12-18T20:40:58Z

/override pull-kops-e2e-k8s-gce-ipalias
/override pull-kops-e2e-k8s-aws-calico

k8s-ci-robot · 2025-12-18T20:41:01Z

@hakman: Overrode contexts on behalf of hakman: pull-kops-e2e-k8s-aws-calico, pull-kops-e2e-k8s-gce-ipalias

Details

In response to this:

/override pull-kops-e2e-k8s-gce-ipalias
/override pull-kops-e2e-k8s-aws-calico

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-12-18T20:41:17Z

@rramkumar1: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kops-kubernetes-e2e-ubuntu-gce-build	`5d6094b`	link	false	`/test pull-kops-kubernetes-e2e-ubuntu-gce-build`
pull-kops-e2e-k8s-aws-calico	`cfafeee`	link	true	`/test pull-kops-e2e-k8s-aws-calico`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 16, 2025

k8s-ci-robot requested a review from hakman December 16, 2025 21:39

k8s-ci-robot added the area/provider/gcp Issues or PRs related to gcp provider label Dec 16, 2025

k8s-ci-robot requested a review from olemarkus December 16, 2025 21:39

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 16, 2025

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 16, 2025

rramkumar1 force-pushed the master branch 3 times, most recently from 2dd10bd to ebb3276 Compare December 17, 2025 01:37

rramkumar1 changed the title ~~[WIP] Support cloudLabels for GCE InstanceGroup~~ Support cloudLabels for GCE InstanceGroup Dec 17, 2025

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Dec 17, 2025

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 Dec 17, 2025

rramkumar1 force-pushed the master branch from ebb3276 to 7da14b0 Compare December 17, 2025 14:14

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 17, 2025

rramkumar1 force-pushed the master branch 2 times, most recently from bade583 to 9a58b0d Compare December 17, 2025 15:21

justinsb reviewed Dec 17, 2025

View reviewed changes

k8s-ci-robot assigned justinsb Dec 17, 2025

hakman reviewed Dec 17, 2025

View reviewed changes

hakman mentioned this pull request Dec 18, 2025

kubetest2: Fix cloud-labels for non-Azure providers #17826

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2025

rramkumar1 force-pushed the master branch 2 times, most recently from 859dbff to 5d6094b Compare December 18, 2025 15:00

Support cloudLabels for GCE InstanceGroup

cfafeee

rramkumar1 force-pushed the master branch from 5d6094b to cfafeee Compare December 18, 2025 15:08

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2025

hakman changed the title ~~Support cloudLabels for GCE InstanceGroup~~ gce: Support cloudLabels for InstanceGroup Dec 18, 2025

k8s-ci-robot assigned hakman Dec 18, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 18, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 18, 2025

k8s-ci-robot merged commit 05dba13 into kubernetes:master Dec 18, 2025
26 of 27 checks passed

devodev mentioned this pull request Mar 20, 2026

gcemodel: Add support for forwarding cloud labels to instance group #17759

Closed


		t.ServiceAccounts = append(t.ServiceAccounts, b.LinkToServiceAccount(ig))

		// labels, err := b.CloudTagsForInstanceGroup(ig)

Conversation

rramkumar1 commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Dec 16, 2025

Uh oh!

k8s-ci-robot commented Dec 16, 2025

Uh oh!

ameukam commented Dec 17, 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!

rramkumar1 commented Dec 17, 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!

rramkumar1 commented Dec 18, 2025

Uh oh!

rramkumar1 commented Dec 18, 2025

Uh oh!

hakman commented Dec 18, 2025

Uh oh!

k8s-ci-robot commented Dec 18, 2025

Uh oh!

hakman commented Dec 18, 2025

Uh oh!

k8s-ci-robot commented Dec 18, 2025

Uh oh!

k8s-ci-robot commented Dec 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rramkumar1 commented Dec 16, 2025 •

edited

Loading