Skip to content

[WIP] Initial change to add more KCP roles. #18495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cheftako wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP] Initial change to add more KCP roles. #18495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/kops-controller/pkg/server/node_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (s *Server) buildInstanceGroupFromCAPI(ctx context.Context, capiMachine *cl
// "machineType": "", // Should not matter
// "subnets": // Should not matter
ig.Spec.Zones = []string{failureDomain}
ig.Spec.Role = "Node" // TODO: Support other roles?
ig.Spec.Role = kops.InstanceGroupSubRoleNode.Role() // TODO: Support other roles?

@cheftako cheftako Jun 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Would it be better to make Role() more of a constructor which took 1 or more subroles?

@justinsb justinsb Jun 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like the type safety we have here. I wish we didn't have to pass .Role(), but I like this (so far, but I'm only one line in!)


log.Info("built InstanceGroup from CAPI Machine", "instanceGroup", ig)
return ig, nil
Expand Down
6 changes: 3 additions & 3 deletions cmd/kops/create_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,10 +585,10 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
var controlPlanes []*api.InstanceGroup
var nodes []*api.InstanceGroup
for _, ig := range instanceGroups {
switch ig.Spec.Role {
case api.InstanceGroupRoleControlPlane:
switch {
case ig.Spec.Role.HasControlPlane():

@cheftako cheftako Jun 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Would ContainsControlPlane() be a better name method name than HasControlPlane()?

@justinsb justinsb Jun 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We might end up getting more specific e.g. RunsKubeApiserver, RunsKubeScheduler, RunsEtcd. For now, I think it's fine, because it's internal (i.e. not part of our API)

controlPlanes = append(controlPlanes, ig)
case api.InstanceGroupRoleNode:
case ig.Spec.Role.HasNode():
nodes = append(nodes, ig)
}
}
Expand Down
13 changes: 8 additions & 5 deletions cmd/kops/create_instancegroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ var (
// NewCmdCreateInstanceGroup create a new cobra command object for creating a instancegroup.
func NewCmdCreateInstanceGroup(f *util.Factory, out io.Writer) *cobra.Command {
options := &CreateInstanceGroupOptions{
Role: kopsapi.InstanceGroupRoleNode.ToLowerString(),
Role: kopsapi.InstanceGroupSubRoleNode.Role().ToLowerString(),
Edit: true,
}

Expand Down Expand Up @@ -124,12 +124,15 @@ func NewCmdCreateInstanceGroup(f *util.Factory, out io.Writer) *cobra.Command {
},
}

allRoles := make([]string, 0, len(kopsapi.AllInstanceGroupRoles))
for _, r := range kopsapi.AllInstanceGroupRoles {
if r == kopsapi.InstanceGroupRoleAPIServer && !featureflag.APIServerNodes.Enabled() {

@cheftako cheftako Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we need a similar feature flag for multiple/new control plane roles.

allRoles := make([]string, 0, len(kopsapi.AllInstanceGroupSubRoles))
for _, subrole := range kopsapi.AllInstanceGroupSubRoles {
role := subrole.Role()
if role.HasAPIServer() && !featureflag.APIServerNodes.Enabled() {
continue
}
allRoles = append(allRoles, r.ToLowerString())
// TODO: Can we GA the APIServerNodes feature flag?
// TODO: Do we need feature flag for the new roles and multi role support?
allRoles = append(allRoles, role.ToLowerString())

@cheftako cheftako Jun 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

With canonicalization else where, do we still need ToLowerString() ?

}

cmd.Flags().StringVar(&options.Role, "role", options.Role, "Type of instance group to create ("+strings.Join(allRoles, ",")+")")
Expand Down
6 changes: 3 additions & 3 deletions cmd/kops/delete_instancegroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func NewCmdDeleteInstanceGroup(f *util.Factory, out io.Writer) *cobra.Command {

return nil
},
ValidArgsFunction: completeInstanceGroup(f, nil, &[]string{kops.InstanceGroupRoleControlPlane.ToLowerString()}),
ValidArgsFunction: completeInstanceGroup(f, nil, &[]string{kops.InstanceGroupSubRoleControlPlane.Role().ToLowerString()}),
RunE: func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()

Expand Down Expand Up @@ -147,15 +147,15 @@ func RunDeleteInstanceGroup(ctx context.Context, f *util.Factory, out io.Writer,

fmt.Fprintf(out, "InstanceGroup %q found for deletion\n", groupName)

if group.Spec.Role == kops.InstanceGroupRoleControlPlane {
if group.Spec.Role.HasControlPlane() {
groups, err := clientset.InstanceGroupsFor(cluster).List(ctx, metav1.ListOptions{})
if err != nil {
return fmt.Errorf("listing InstanceGroups: %v", err)
}

onlyMaster := true
for _, ig := range groups.Items {
if ig.Name != groupName && ig.Spec.Role == kops.InstanceGroupRoleControlPlane {
if ig.Name != groupName && ig.Spec.Role.HasControlPlane() {
onlyMaster = false
break
}
Expand Down
18 changes: 13 additions & 5 deletions cmd/kops/reconcile_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,12 @@ func RunReconcileCluster(ctx context.Context, f *util.Factory, out io.Writer, op
{
opt := *c
opt.InstanceGroupRoles = []string{
string(kops.InstanceGroupRoleAPIServer),
string(kops.InstanceGroupRoleControlPlane),
string(kops.InstanceGroupSubRoleAPIServer.Role()),
string(kops.InstanceGroupSubRoleControlPlane.Role()),
string(kops.InstanceGroupSubRoleEtcd.Role()),
string(kops.InstanceGroupSubRoleScheduler.Role()),
string(kops.InstanceGroupSubRoleCloudControllerManager.Role()),
string(kops.InstanceGroupSubRoleKubeControllerManager.Role()),
}
opt.Prune = false // Do not prune until after the last rolling update
if _, err := RunCoreUpdateCluster(ctx, f, out, &opt); err != nil {
Expand All @@ -160,7 +164,7 @@ func RunReconcileCluster(ctx context.Context, f *util.Factory, out io.Writer, op

// filter the instance group to only include the control plane
opt.filterInstanceGroups = func(ig *kops.InstanceGroup) bool {
return ig.Spec.Role == kops.InstanceGroupRoleAPIServer || ig.Spec.Role == kops.InstanceGroupRoleControlPlane
return ig.Spec.Role.IsControlPlaneType()
}

// Ignore all pods, we just want to check the control plane is responding
Expand All @@ -180,8 +184,12 @@ func RunReconcileCluster(ctx context.Context, f *util.Factory, out io.Writer, op
opt.ClusterName = c.ClusterName
opt.CreateKubecfgOptions = options.CreateKubecfgOptions
opt.InstanceGroupRoles = []string{
string(kops.InstanceGroupRoleAPIServer),
string(kops.InstanceGroupRoleControlPlane),
string(kops.InstanceGroupSubRoleAPIServer.Role()),
string(kops.InstanceGroupSubRoleControlPlane.Role()),
string(kops.InstanceGroupSubRoleEtcd.Role()),
string(kops.InstanceGroupSubRoleScheduler.Role()),
string(kops.InstanceGroupSubRoleCloudControllerManager.Role()),
string(kops.InstanceGroupSubRoleKubeControllerManager.Role()),
}
opt.Yes = c.Yes
if err := RunRollingUpdateCluster(ctx, f, out, opt); err != nil {
Expand Down
14 changes: 8 additions & 6 deletions cmd/kops/rolling-update_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,9 @@ func NewCmdRollingUpdateCluster(f *util.Factory, out io.Writer) *cobra.Command {
},
}

allRoles := make([]string, 0, len(kopsapi.AllInstanceGroupRoles))
for _, r := range kopsapi.AllInstanceGroupRoles {
allRoles = append(allRoles, r.ToLowerString())
allRoles := make([]string, 0, len(kopsapi.AllInstanceGroupSubRoles))
for _, subrole := range kopsapi.AllInstanceGroupSubRoles {
allRoles = append(allRoles, subrole.ToLowerString())
}

cmd.Flags().BoolVarP(&options.Yes, "yes", "y", options.Yes, "Perform rolling update immediately; without --yes rolling-update executes a dry-run")
Expand Down Expand Up @@ -280,7 +280,7 @@ func RunRollingUpdateCluster(ctx context.Context, f *util.Factory, out io.Writer
return err
}

countByRole := make(map[kopsapi.InstanceGroupRole]int32)
countByRole := make(map[kopsapi.InstanceGroupSubRole]int32)
var instanceGroups []*kopsapi.InstanceGroup
for i := range list.Items {
instanceGroup := &list.Items[i]
Expand All @@ -290,9 +290,11 @@ func RunRollingUpdateCluster(ctx context.Context, f *util.Factory, out io.Writer
if instanceGroup.Spec.MinSize != nil {
minSize = *instanceGroup.Spec.MinSize
}
countByRole[instanceGroup.Spec.Role] = countByRole[instanceGroup.Spec.Role] + minSize
for _, subrole := range instanceGroup.Spec.Role.SubRoles() {
countByRole[subrole] = countByRole[subrole] + minSize
}
}
if countByRole[kopsapi.InstanceGroupRoleAPIServer]+countByRole[kopsapi.InstanceGroupRoleControlPlane] <= 1 {
if countByRole[kopsapi.InstanceGroupSubRoleAPIServer]+countByRole[kopsapi.InstanceGroupSubRoleControlPlane] <= 1 {
options.DeregisterControlPlaneNodes = false
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/toolbox_instance-selector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ func validateAllPrivateOrPublicSubnets(userSubnets []string) error {
func createInstanceGroup(groupName, clusterName string, subnets []string) *kops.InstanceGroup {
ig := &kops.InstanceGroup{}
ig.ObjectMeta.Name = groupName
ig.Spec.Role = kops.InstanceGroupRoleNode
ig.Spec.Role = kops.InstanceGroupSubRoleNode.Role()
ig.Spec.Subnets = subnets
ig.ObjectMeta.Labels = make(map[string]string)
ig.ObjectMeta.Labels[kops.LabelClusterName] = clusterName
Expand Down
4 changes: 2 additions & 2 deletions cmd/kops/toolbox_instance-selector_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ func TestValidateUserSubnetsWithClusterSubnets(t *testing.T) {
func TestCreateInstanceGroup(t *testing.T) {
zones := []string{"us-east-2a", "us-east-2b", "us-east-2c"}
actualIG := createInstanceGroup("testGroup", "clusterTest", zones)
if actualIG.Spec.Role != kops.InstanceGroupRoleNode {
t.Fatalf("instance group should have the \"%s\" role but got %s", kops.InstanceGroupRoleNode, actualIG.Spec.Role)
if !actualIG.Spec.Role.HasNode() {
t.Fatalf("instance group should have the \"%s\" sub-role but got %s", kops.InstanceGroupSubRoleNode, actualIG.Spec.Role)
}
if !reflect.DeepEqual(actualIG.Spec.Subnets, zones) {
t.Fatalf("instance group should have all the zones passed in but got %s", actualIG.Spec.Subnets)
Expand Down
8 changes: 4 additions & 4 deletions cmd/kops/update_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ func NewCmdUpdateCluster(f *util.Factory, out io.Writer) *cobra.Command {
options := &UpdateClusterOptions{}
options.InitDefaults()

allRoles := make([]string, 0, len(kops.AllInstanceGroupRoles))
for _, r := range kops.AllInstanceGroupRoles {
allRoles = append(allRoles, r.ToLowerString())
allRoles := make([]string, 0, len(kops.AllInstanceGroupSubRoles))
for _, subrole := range kops.AllInstanceGroupSubRoles {
allRoles = append(allRoles, subrole.ToLowerString())
}

cmd := &cobra.Command{
Expand Down Expand Up @@ -510,7 +510,7 @@ func parseLifecycle(lifecycle string) (fi.Lifecycle, error) {

func usesBastion(instanceGroups []*kops.InstanceGroup) bool {
for _, ig := range instanceGroups {
if ig.Spec.Role == kops.InstanceGroupRoleBastion {
if ig.Spec.Role.HasBastion() {
return true
}
}
Expand Down
4 changes: 2 additions & 2 deletions examples/kops-api-example/up.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func up(vfsContext *vfs.VFSContext, ctx context.Context) error {
ig := &api.InstanceGroup{}
ig.ObjectMeta.Name = "master"
ig.Spec = api.InstanceGroupSpec{
Role: api.InstanceGroupRoleControlPlane,
Role: api.InstanceGroupSubRoleControlPlane.Role(),
Subnets: masterZones,
}
_, err := clientset.InstanceGroupsFor(cluster).Create(ctx, ig, metav1.CreateOptions{})
Expand All @@ -100,7 +100,7 @@ func up(vfsContext *vfs.VFSContext, ctx context.Context) error {
ig := &api.InstanceGroup{}
ig.ObjectMeta.Name = "nodes"
ig.Spec = api.InstanceGroupSpec{
Role: api.InstanceGroupRoleNode,
Role: api.InstanceGroupSubRoleNode.Role(),
Subnets: nodeZones,
}

Expand Down
6 changes: 3 additions & 3 deletions nodeup/pkg/model/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ func (c *NodeupModelContext) Init() error {

role := c.BootConfig.InstanceGroupRole

if role == kops.InstanceGroupRoleControlPlane {
if role.HasControlPlane() {
c.IsMaster = true

@justinsb justinsb Jun 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We probably need to deprecate c.IsMaster, but agree that we should not do it in this PR

}

if role == kops.InstanceGroupRoleControlPlane || role == kops.InstanceGroupRoleAPIServer {
if role.HasControlPlane() || role.HasAPIServer() {

@justinsb justinsb Jun 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is where it would be nice if this was just role.RunsKubeAPIServer, and that function returned true based if it has the ControlPlane or the APIServer role (and potentially other roles also).

c.HasAPIServer = true
}

Expand Down Expand Up @@ -566,7 +566,7 @@ func (c *NodeupModelContext) InstallNvidiaRuntime() bool {
// InstallGVisorRuntime returns true if the gVisor (runsc) runtime should be installed.
func (c *NodeupModelContext) InstallGVisorRuntime() bool {
return c.BootConfig != nil &&
c.BootConfig.InstanceGroupRole == kops.InstanceGroupRoleNode &&
c.BootConfig.InstanceGroupRole.HasNode() &&

@justinsb justinsb Jun 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we should start to transition away from HasNode to RunsWorkerPods or some similar concept

c.NodeupConfig.GVisor != nil &&
fi.ValueOf(c.NodeupConfig.GVisor.Enabled)
}
Expand Down
10 changes: 5 additions & 5 deletions nodeup/pkg/model/gvisor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,32 +36,32 @@ func TestGVisorBuilderBuild(t *testing.T) {
{
name: "disabled",
distribution: distributions.DistributionDebian13,
role: kops.InstanceGroupRoleNode,
role: kops.InstanceGroupSubRoleNode.Role(),
gvisor: &kops.GVisorConfig{Enabled: fi.PtrTo(false)},
},
{
name: "enabled debian",
distribution: distributions.DistributionDebian13,
role: kops.InstanceGroupRoleNode,
role: kops.InstanceGroupSubRoleNode.Role(),
gvisor: &kops.GVisorConfig{Enabled: fi.PtrTo(true)},
wantTasks: []string{"AptSource/gvisor", "Package/runsc"},
},
{
name: "enabled non debian",
distribution: distributions.DistributionRhel9,
role: kops.InstanceGroupRoleNode,
role: kops.InstanceGroupSubRoleNode.Role(),
gvisor: &kops.GVisorConfig{Enabled: fi.PtrTo(true)},
},
{
name: "enabled control plane",
distribution: distributions.DistributionDebian13,
role: kops.InstanceGroupRoleControlPlane,
role: kops.InstanceGroupSubRoleControlPlane.Role(),
gvisor: &kops.GVisorConfig{Enabled: fi.PtrTo(true)},
},
{
name: "unset",
distribution: distributions.DistributionDebian13,
role: kops.InstanceGroupRoleNode,
role: kops.InstanceGroupSubRoleNode.Role(),
},
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/kops/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,7 @@ func (in *WarmPoolSpec) IsEnabled() bool {
func (in *WarmPoolSpec) ResolveDefaults(ig *InstanceGroup) *WarmPoolSpec {
igWarmPool := ig.Spec.WarmPool
if igWarmPool == nil {
if in == nil || (ig.Spec.Role == InstanceGroupRoleControlPlane || ig.Spec.Role == InstanceGroupRoleBastion) {
if in == nil || (ig.Spec.Role.HasControlPlane() || ig.Spec.Role.HasBastion()) {
var zero int64
return &WarmPoolSpec{
MaxSize: &zero,
Expand All @@ -1188,7 +1188,7 @@ func (in *WarmPoolSpec) ResolveDefaults(ig *InstanceGroup) *WarmPoolSpec {
return in
}

if in == nil || (ig.Spec.Role == InstanceGroupRoleControlPlane || ig.Spec.Role == InstanceGroupRoleBastion) {
if in == nil || (ig.Spec.Role.HasControlPlane() || ig.Spec.Role.HasBastion()) {
return igWarmPool
}

Expand Down
Loading
Loading