Merge pull request #17849 from Camila-B/fix-instancegroup

gce: Require zones on instancegroup creation

Author: k8s-ci-robot

cloudmock/gce/mockcompute/zone.go

@@ -38,6 +38,10 @@ func newZoneClient(project string) *zoneClient {
 					Name:   "us-test1-a",
 					Region: "https://www.googleapis.com/compute/v1/projects/testproject/regions/us-test1",
 				},
+				"us-test1-b": {
+					Name:   "us-test1-b",
+					Region: "https://www.googleapis.com/compute/v1/projects/testproject/regions/us-test1",
+				},
 			},
 		},
 	}

cmd/kops/create_instancegroup.go

@@ -48,6 +48,7 @@ type CreateInstanceGroupOptions struct {
 	InstanceGroupName string
 	Role              string
 	Subnets           []string
+	Zones             []string
 	// DryRun mode output an ig manifest of Output type.
 	DryRun bool
 	// Output type during a DryRun
@@ -137,6 +138,9 @@ func NewCmdCreateInstanceGroup(f *util.Factory, out io.Writer) *cobra.Command {
 	})
 	cmd.Flags().StringSliceVar(&options.Subnets, "subnet", options.Subnets, "Subnet in which to create instance group. One of Availability Zone like eu-west-1a or a comma-separated list of multiple Availability Zones.")
 	cmd.RegisterFlagCompletionFunc("subnet", completeClusterSubnet(f, &options.Subnets))
+
+	cmd.Flags().StringSliceVar(&options.Zones, "zone", options.Zones, "Zones in which to create instance group. One of Availability Zone like eu-west-1a or a comma-separated list of multiple Availability Zones.")
+
 	// DryRun mode that will print YAML or JSON
 	cmd.Flags().BoolVar(&options.DryRun, "dry-run", options.DryRun, "Only print the object that would be created, without created it. This flag can be used to create an instance group YAML or JSON manifest.")
 	cmd.Flags().StringVarP(&options.Output, "output", "o", options.Output, "Output format. One of json or yaml")
@@ -188,6 +192,8 @@ func RunCreateInstanceGroup(ctx context.Context, f *util.Factory, out io.Writer,
 
 	ig.Spec.Subnets = options.Subnets
 
+	ig.Spec.Zones = options.Zones
+
 	cloud, err := cloudup.BuildCloud(cluster)
 	if err != nil {
 		return err
@@ -200,6 +206,9 @@ func RunCreateInstanceGroup(ctx context.Context, f *util.Factory, out io.Writer,
 
 	ig.AddInstanceGroupNodeLabel()
 	if cluster.GetCloudProvider() == kopsapi.CloudProviderGCE {
+		if len(ig.Spec.Zones) == 0 {
+			return fmt.Errorf("zone flag is required for GCE clusters")
+		}
 		fmt.Println("detected a GCE cluster; labeling nodes to receive metadata-proxy.")
 		ig.Spec.NodeLabels["cloud.google.com/metadata-proxy-ready"] = "true"
 	}

cmd/kops/create_instancegroup_test.go

@@ -0,0 +1,68 @@
+/*
+Copyright 2026 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"bytes"
+	"context"
+	"testing"
+
+	"k8s.io/kops/cmd/kops/util"
+	"k8s.io/kops/pkg/testutils"
+)
+
+func TestRunCreateInstanceGroupGCE(t *testing.T) {
+	h := testutils.NewIntegrationTestHarness(t)
+	defer h.Close()
+
+	clusterName := "test.k8s.io"
+
+	cloud := h.SetupMockGCE()
+
+	ctx := context.Background()
+	f := util.NewFactory(&util.FactoryOptions{
+		RegistryPath: "memfs://tests",
+	})
+
+	cluster := testutils.BuildMinimalClusterGCE(clusterName, cloud.Project())
+
+	clientset, err := f.KopsClient()
+	if err != nil {
+		t.Fatalf("error getting clientset: %v", err)
+	}
+	if _, err := clientset.CreateCluster(ctx, cluster); err != nil {
+		t.Fatalf("error creating cluster: %v", err)
+	}
+
+	var stdout bytes.Buffer
+	options := &CreateInstanceGroupOptions{
+		ClusterName:       clusterName,
+		InstanceGroupName: "nodes",
+		Role:              "Node",
+		Subnets:           []string{"us-test-1a"},
+	}
+
+	if err := RunCreateInstanceGroup(ctx, f, &stdout, options); err == nil {
+		t.Fatalf("Expected error when creating instancegroup, got nil")
+	}
+
+	options.Zones = []string{"us-test-1a"}
+
+	if err := RunCreateInstanceGroup(ctx, f, &stdout, options); err != nil {
+		t.Fatalf("could not create instance group: %v", err)
+	}
+}

cmd/kops/edit_instancegroup_test.go

@@ -36,7 +36,7 @@ func TestEditInstanceGroup(t *testing.T) {
 
 	clusterName := "test.k8s.io"
 
-	cluster := testutils.BuildMinimalCluster(clusterName)
+	cluster := testutils.BuildMinimalClusterAWS(clusterName)
 	nodes := testutils.BuildMinimalNodeInstanceGroup("nodes", "subnet-us-test-1a")
 	nodes.Spec.Taints = []string{"e2etest:NoSchedule"}
 

docs/cli/kops_create_instancegroup.md

@@ -44,5 +44,9 @@ func gceValidateInstanceGroup(ig *kops.InstanceGroup, cloud gce.GCECloud) field.
 		fieldSpec := field.NewPath("spec")
 		allErrs = append(allErrs, IsValidValue(fieldSpec.Child("gcpProvisioningModel"), ig.Spec.GCPProvisioningModel, []string{"STANDARD", "SPOT"})...)
 	}
+
+	if len(ig.Spec.Zones) == 0 {
+		allErrs = append(allErrs, field.Required(field.NewPath("spec", "zones"), "zones should be specified"))
+	}
 	return allErrs
 }

pkg/apis/kops/validation/gce.go

@@ -45,7 +45,7 @@ func getTestSetupOS(t *testing.T, ctx context.Context) (*RollingUpdateCluster, *
 
 	mockcloud := testutils.SetupMockOpenstack()
 
-	inCluster := testutils.BuildMinimalCluster("test.k8s.local")
+	inCluster := testutils.BuildMinimalClusterAWS("test.k8s.local")
 
 	inCluster.Spec.CloudProvider.Openstack = &kopsapi.OpenstackSpec{}
 	inCluster.Name = "test.k8s.local"

pkg/instancegroups/rollingupdate_os_test.go

@@ -115,7 +115,7 @@ func (f fakeKeyStore) MirrorTo(ctx context.Context, basedir vfs.Path) error {
 
 // build a generic minimal cluster
 func buildMinimalCluster(clusterName string, masterPublicName string, lbCert bool, nlb bool) *kops.Cluster {
-	cluster := testutils.BuildMinimalCluster(clusterName)
+	cluster := testutils.BuildMinimalClusterAWS(clusterName)
 	cluster.Spec.API.PublicName = masterPublicName
 	cluster.Spec.KubernetesVersion = "1.30.0"
 	if lbCert || nlb {

pkg/kubeconfig/create_kubecfg_test.go

@@ -35,7 +35,7 @@ const (
 )
 
 func buildMinimalCluster() *kops.Cluster {
-	return testutils.BuildMinimalCluster("testcluster.test.com")
+	return testutils.BuildMinimalClusterAWS("testcluster.test.com")
 }
 
 func buildNodeInstanceGroup(subnets ...string) *kops.InstanceGroup {

pkg/model/awsmodel/autoscalinggroup_test.go

@@ -266,6 +266,10 @@ func (b *AutoscalingGroupModelBuilder) splitToZones(ig *kops.InstanceGroup) (map
 			return nil, err
 		}
 
+		if len(zones) == 0 {
+			return nil, fmt.Errorf("no zones found for instance group %q", ig.ObjectMeta.Name)
+		}
+
 		// TODO: Duplicated from aws - move to defaults?
 		minSize := 1
 		if ig.Spec.MinSize != nil {
@@ -292,7 +296,7 @@ func (b *AutoscalingGroupModelBuilder) splitToZones(ig *kops.InstanceGroup) (map
 			totalSize++
 
 			i++
-			if i > len(targetSizes) {
+			if i >= len(targetSizes) {
 				i = 0
 			}
 		}