Adding dynamic channel labels by cheftako · Pull Request #18640 · kubernetes/kops

cheftako · 2026-07-30T00:45:19Z

Prepare for split control plane IGs. #18597 18597

Removed concept of CCM node as it is dynamic.
Fixed KubController -> KubeController.
Added setting kops role labels on nodes via kops-channel.

make gen-cli-docs
Fixed direct role equality check.
Fixed APIServer label test.

justinsb · 2026-07-30T00:58:04Z

+}
+
+// IsKubeControllerManagerOnly checks if instanceGroup runs only KubeControllerManager
+func (g *InstanceGroup) IsKubeControllerManagerOnly() bool {


I think I commented on the other PR (or it might be pending). It feels like calling IsKubeControllerManagerOnly would be an antipattern, we should be calling HasKubeControllerManager

Not sure I entirely agree. Maybe it should be private, assuming the packaging works. However I suspect we internally want to run checks on exactly which components are local. For instance we need to know IsKubeControllerManagerOnly && !IsAPIServerOnly to configure load balancing.

justinsb · 2026-07-30T00:58:25Z

+	return g.Spec.Role.HasKubeControllerManager()
+}
+
+// HasKubeControllerManager checks if instanceGroup runs KubeControllerManager


A maybe-better name might be RunsKubeControllerManager. Not a blocker though, easy to tweak when we're done.

justinsb · 2026-07-30T00:59:14Z

+// HasEtcdOnlyInstanceGroups returns true if the cluster has any Etcd-only instance groups.
+func (c *GCEModelContext) HasEtcdOnlyInstanceGroups() bool {
+	for _, ig := range c.InstanceGroups {
+		if ig.Spec.Role.HasEtcd() {


Should this be IsEtcdOnly ? (This one looks like a blocker, depending on how HasEtcdOnlyInstanceGroups is used)

https://github.com/kubernetes/kops/pull/18597/changes#diff-18feeb66368bce3894a4538f9cda34a931f0fb76e6e5833f56e8cb71e753cf0dR207 it is used to determine if Etcd is running on a separate IG from the APIServer and there for needs a LB in between. HasEtcd is determining an exact match on the role. So as long as a IG has only 1 role this is correct. When we go to supporting multiple roles for an IG this will need to be fixed.

IsEtcdOnly on the IG is a wrapper around HasEtcd on the Role. Probably is safer to use that.

justinsb · 2026-07-30T01:00:28Z

+	// New Experimental control plane roles associated with static manifests
+	RoleLabelEtcd                  = "node-role.kubernetes.io/etcd"
+	RoleLabelScheduler             = "node-role.kubernetes.io/scheduler"
+	RoleLabelKubeControllerManager = "node-role.kubernetes.io/kube-controller-manager"


Is there any way to avoiding putting these into node-role.kubernetes.io? That feels like a broader conversation than kops.k8s.io ...

Point. I was starting to think we should have IG/role labels. Would you be ok with a kops based IG label? I think thats basically what these really are. Tempted to suggest that we use a well know label name and maybe the IG name as the value?

I think something like kops.k8s.io/instance-group: foo sounds good to me - is that what you were thinking. @hakman @rifelpet any objections?

none from me 👍

I had been thinking more ig-role.kops.k8s.io/api-server: external where external might be the name of that particular IG. kops.k8s.io/instance-group: external allows the role to be looked up by the IG name but I thought having it in the label might be convenient.

Not sure we have use cases for multiple IGs for the same role beyond api-server and node. However we do have those two. Also need to think about what happens if 1 IG satisfies multiple roles. Your suggestion doesn't care. Mine would leave you having multiple labels for that case. Eg ig-role.kops.k8s.io/api-server: internal and ig-role.kops.k8s.io/etcd: internal.

Chatted with @justinsb, I believe we agreed to the following.
For statically deployed control-plane elements we would use: node-role.kubernetes.io/<type>
For dynamically deployed control-plane elements we would use: node-role.kops.k8s.io/<type>

Justin also pointed out that we already have a flag indicating the instance-group.
kops.k8s.io/instancegroup=<IG name>

The reason was that these give a nice presentation in kubectl get nodes, and this is what they are for.

Removed concept of CCM node as it is dynamic. Fixed KubController -> KubeController. Added setting kops role labels on nodes via kops-channel.

Fixed APIServer label test.

Fix IG HasAPIServer to RunsAPIServer

cheftako · 2026-07-31T16:37:56Z

/test pull-kops-verify-terraform

cheftako · 2026-07-31T20:54:46Z

/test pull-kops-scenario-splitkcp-gcp

kubernetes-prow · 2026-07-31T21:18:57Z

@cheftako: The following test 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-scenario-splitkcp-gcp	`8852de9`	link	false	`/test pull-kops-scenario-splitkcp-gcp`

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.

justinsb · 2026-08-01T14:57:18Z

    - --v=4
    - --yes
    - --interval=1m0s
+    - --node-labels=node-role.kubernetes.io/control-plane


It does look like this label was not set previously (by kops-channels)...

OK, I think I understand. We used to add this in BuildMandatoryControlPlaneLabels, unconditionally. We still do, so technically this flag is not needed, but we're working towards making it a little less magical and a little more explicit, which I think is a good thing.

justinsb · 2026-08-01T15:12:16Z

Thanks @cheftako - and thanks for putting up with my node-role flip/flop

/approve
/lgtm

kubernetes-prow · 2026-08-01T15:15:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb

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

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

kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 30, 2026

kubernetes-prow Bot requested a review from hakman July 30, 2026 00:45

kubernetes-prow Bot added the area/api label Jul 30, 2026

kubernetes-prow Bot requested a review from olemarkus July 30, 2026 00:45

kubernetes-prow Bot added area/channels area/documentation area/provider/gcp Issues or PRs related to gcp provider labels Jul 30, 2026

justinsb reviewed Jul 30, 2026

View reviewed changes

Comment thread cmd/kops/create_cluster.go

justinsb reviewed Jul 30, 2026

View reviewed changes

kubernetes-prow Bot added area/provider/aws Issues or PRs related to aws provider needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 30, 2026

cheftako added 4 commits July 30, 2026 21:33

Prepare for split control plane IGs.

1b1e61e

Removed concept of CCM node as it is dynamic. Fixed KubController -> KubeController. Added setting kops role labels on nodes via kops-channel.

make gen-cli-docs

d1bd839

Fixed direct role equality check.

f24f248

Fixed APIServer label test.

Factored in feedback.

64dcbbd

Fix IG HasAPIServer to RunsAPIServer

cheftako force-pushed the channelLabels branch from 8806cdd to 64dcbbd Compare July 30, 2026 21:42

kubernetes-prow Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 30, 2026

hack/update-expected.sh

8852de9

kubernetes-prow Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 30, 2026

Fixing labels based on latest agreement.

2b515b4

justinsb reviewed Aug 1, 2026

View reviewed changes

kubernetes-prow Bot assigned justinsb Aug 1, 2026

kubernetes-prow Bot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2026

kubernetes-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2026

kubernetes-prow Bot merged commit a3da178 into kubernetes:master Aug 1, 2026
27 checks passed

Conversation

cheftako commented Jul 30, 2026

Uh oh!

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!

cheftako Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

cheftako Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cheftako commented Jul 31, 2026

Uh oh!

cheftako commented Jul 31, 2026

Uh oh!

kubernetes-prow Bot commented Jul 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinsb commented Aug 1, 2026

Uh oh!

kubernetes-prow Bot commented Aug 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cheftako Jul 30, 2026 •

edited

Loading

cheftako Jul 31, 2026 •

edited

Loading