move from zones input to subnets input

cmd/kops/toolbox_instance_selector.go

@@ -48,7 +48,7 @@ const (
 	usageClass             = "usage-class"
 	enaSupport             = "ena-support"
 	burstSupport           = "burst-support"
-	availabilityZones      = "zones"
+	subnets                = "subnets"
 	networkInterfaces      = "network-interfaces"
 	networkPerformance     = "network-performance"
 	allowList              = "allow-list"
@@ -184,7 +184,7 @@ func NewCmdToolboxInstanceSelector(f *util.Factory, out io.Writer) *cobra.Comman
 	commandline.StringOptionsFlag(usageClass, nil, &usageClassDefault, fmt.Sprintf("Usage class: [%s]", strings.Join(usageClasses, ", ")), usageClasses)
 	commandline.BoolFlag(enaSupport, nil, nil, "Instance types where ENA is supported or required")
 	commandline.BoolFlag(burstSupport, nil, nil, "Burstable instance types")
-	commandline.StringSliceFlag(availabilityZones, nil, nil, "Availability zones or zone ids to check only EC2 capacity offered in those specific AZs")
+	commandline.StringSliceFlag(subnets, nil, nil, "Subnet(s) in which to create the instance group. One of Availability Zone like eu-west-1a or utility-eu-west-1a,")
 	commandline.IntMinMaxRangeFlags(networkInterfaces, nil, nil, "Number of network interfaces (ENIs) that can be attached to the instance")
 	commandline.RegexFlag(allowList, nil, nil, "List of allowed instance types to select from w/ regex syntax (Example: m[3-5]\\.*)")
 	commandline.RegexFlag(denyList, nil, nil, "List of instance types which should be excluded w/ regex syntax (Example: m[1-2]\\.*)")
@@ -216,20 +216,29 @@ func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, out io.Wri
 		return fmt.Errorf("cannot select instance types from non-aws cluster")
 	}
 
-	clusterZones, err := getClusterZones(cluster.Spec.Subnets)
+	if len(cluster.Spec.Subnets) == 0 {
+		return fmt.Errorf("error the cluster must have at least 1 subnet")
+	}
+	firstClusterSubnet := strings.ReplaceAll(cluster.Spec.Subnets[0].Name, "utility-", "")
+	region := firstClusterSubnet[:len(firstClusterSubnet)-1]
+
+	igSubnets := []string{}
+	for _, clusterSubnet := range cluster.Spec.Subnets {
+		igSubnets = append(igSubnets, clusterSubnet.Name)
+	}
+	
+	if commandline.Flags[subnets] != nil {
+		userSubnets := *commandline.StringSliceMe(commandline.Flags[subnets])
+		err := validateUserSubnets(userSubnets, cluster.Spec.Subnets)
 		if err != nil {
 			return err
 		}
-	region := clusterZones[0][:len(clusterZones[0])-1]
+		igSubnets = userSubnets
-
-	zones := clusterZones
-	if commandline.Flags[availabilityZones] != nil {
-		userZones := *commandline.StringSliceMe(commandline.Flags[availabilityZones])
-		zonesValid := validateZonesForCluster(userZones, clusterZones)
-		if !zonesValid {
-			fmt.Errorf("cannot pass in zones that the cluster does not have a subnet in: %v", err)
 	}
-		zones = userZones
+	
+	zones := []string{}
+	for _, igSubnet := range igSubnets {
+		zones = append(zones, strings.ReplaceAll(igSubnet, "utility-", ""))
 	}
 
 	tags := map[string]string{"KubernetesCluster": clusterName}
@@ -246,7 +255,7 @@ func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, out io.Wri
 	if flags[instanceGroupCount] == nil {
 		igCount = 1
 	}
-	filters := getFilters(commandline, region)
+	filters := getFilters(commandline, region, zones)
 	mutatedFilters := filters
 	if flags[instanceGroupCount] != nil || filters.Flexible != nil {
 		if filters.VCpusToMemoryRatio == nil {
@@ -272,16 +281,7 @@ func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, out io.Wri
 		}
 		usageClass := *filters.UsageClass
 
-		subnets := []string{}
+		ig := createInstanceGroup(igNameForRun, clusterName, igSubnets)
-		for _, zone := range zones {
-			subnetName, err := getClusterSubnetNameFromZone(cluster.Spec.Subnets, zone)
-			if err != nil {
-				return err
-			}
-			subnets = append(subnets, subnetName)
-		}
-
-		ig := createInstanceGroup(igNameForRun, clusterName, subnets)
 		ig = decorateWithInstanceGroupSpecs(ig, instanceSelectorOpts)
 		ig, err = decorateWithMixedInstancesPolicy(ig, usageClass, selectedInstanceTypes)
 		if err != nil {
@@ -382,7 +382,7 @@ func retrieveClusterRefs(ctx context.Context, f *util.Factory, clusterName strin
 	return clientset, cluster, channel, nil
 }
 
-func getFilters(commandline *cli.CommandLineInterface, region string) selector.Filters {
+func getFilters(commandline *cli.CommandLineInterface, region string, zones []string) selector.Filters {
 	flags := commandline.Flags
 	return selector.Filters{
 		VCpusRange:             commandline.IntRangeMe(flags[vcpus]),
@@ -396,7 +396,7 @@ func getFilters(commandline *cli.CommandLineInterface, region string) selector.F
 		EnaSupport:             commandline.BoolMe(flags[enaSupport]),
 		Burstable:              commandline.BoolMe(flags[burstSupport]),
 		Region:                 commandline.StringMe(region),
-		AvailabilityZones:      commandline.StringSliceMe(flags[availabilityZones]),
+		AvailabilityZones:      commandline.StringSliceMe(zones),
 		MaxResults:             commandline.IntMe(flags[maxResults]),
 		NetworkInterfaces:      commandline.IntRangeMe(flags[networkInterfaces]),
 		NetworkPerformance:     commandline.IntRangeMe(flags[networkPerformance]),
@@ -429,40 +429,47 @@ func getInstanceSelectorOpts(commandline *cli.CommandLineInterface) InstanceSele
 	return opts
 }
 
-func getClusterZones(subnets []kops.ClusterSubnetSpec) ([]string, error) {
+func validateUserSubnets(userSubnets []string, clusterSubnets []kops.ClusterSubnetSpec) error {
-	region := ""
+	err := validateUserSubnetsWithClusterSubnets(userSubnets, clusterSubnets)
-	zones := []string{}
+	if err != nil {
-	for _, subnet := range subnets {
+		return err
-		zoneRegion := subnet.Zone[:len(subnet.Zone)-1]
-		zones = append(zones, subnet.Zone)
-		if region != "" && zoneRegion != region {
-			return nil, fmt.Errorf("clusters cannot span multiple regions")
 	}
-		region = zoneRegion
+	err = validateAllPrivateOrPublicSubnets(userSubnets)
+	if err != nil {
+		return err
 	}
-	if len(zones) == 0 {
+	return nil
-		return nil, fmt.Errorf("the cluster must include at least 1 subnet")
-	}
-	return zones, nil
 }
 
-func validateZonesForCluster(userZones []string, clusterZones []string) bool {
+// validateUserSubnetsWithClusterSubnets makes sure the userSubnets are part of the cluster subnets
-	allClusterZones := strings.Join(clusterZones, ", ") 
+func validateUserSubnetsWithClusterSubnets(userSubnets []string, clusterSubnets []kops.ClusterSubnetSpec) error {
-	for _, userZone := range userZones {
+	for _, clusterSubnet := range clusterSubnets {
-		if !strings.Contains(allClusterZones, userZone) {
+		userSubnetValid := false
-			return false
+		for _, userSubnet := range userSubnets {
+			if clusterSubnet.Name == userSubnet {
+				userSubnetValid = true
+				break
 			}
 		}
-	return true
+		if !userSubnetValid {
+			return fmt.Errorf("error subnets must exist in the cluster")
+		}
+	}
+	return nil
 } 
 
-func getClusterSubnetNameFromZone(subnets []kops.ClusterSubnetSpec, zone string) (string, error) {
+// validateAllPrivateOrPublicSubnets makes sure the passed in subnets are all utility (public) subnets or private subnets
-	for _, subnet := range subnets {
+func validateAllPrivateOrPublicSubnets(userSubnets []string) error {
-		if subnet.Zone == zone {
+	utilitySubnets := 0
-			return subnet.Name, nil
+	for _, userSubnet := range userSubnets {
+		if strings.HasPrefix("utility-", userSubnet) {
+			utilitySubnets++
 		}
 	}
-	return "", fmt.Errorf("error no subnets found for zone %s", zone)
+	if utilitySubnets != 0 && len(userSubnets) == utilitySubnets {
+		return fmt.Errorf("error instance group cannot span public and private subnets")
+	}
+	return nil
 }
 
 func createInstanceGroup(groupName, clusterName string, subnets []string) *kops.InstanceGroup {
@@ -486,6 +493,7 @@ func decorateWithInstanceGroupSpecs(instanceGroup *kops.InstanceGroup, instanceG
 	return ig
 }
 
+// decorateWithMixedInstancesPolicy adds a mixed instance policy based on usageClass to the instance-group
 func decorateWithMixedInstancesPolicy(instanceGroup *kops.InstanceGroup, usageClass string, instanceSelections []string) (*kops.InstanceGroup, error) {
 	ig := instanceGroup
 	ig.Spec.MachineType = instanceSelections[0]
@@ -517,6 +525,7 @@ func decorateWithMixedInstancesPolicy(instanceGroup *kops.InstanceGroup, usageCl
 	return ig, nil
 }
 
+// decorateWithClusterAutoscalerLabels adds cluster-autoscaler discovery tags to the cloudlabels slice
 func decorateWithClusterAutoscalerLabels(instanceGroup *kops.InstanceGroup) *kops.InstanceGroup {
 	ig := instanceGroup
 	clusterName := instanceGroup.ObjectMeta.Name

cmd/kops/toolbox_instance_selector_internal_test.go

@@ -33,7 +33,7 @@ func TestGetFilters(t *testing.T) {
 		denyList:  regexp.MustCompile("t3.nano"),
 	}
 
-	filters := getFilters(&commandline, "us-east-2")
+	filters := getFilters(&commandline, "us-east-2", []string{"us-east-2a"})
 	if !*filters.Flexible {
 		t.Fatalf("Flexible should be true")
 	}
@@ -75,32 +75,72 @@ func TestGetInstanceSelectorOpts(t *testing.T) {
 	}
 }
 
-func TestGetClusterZones(t *testing.T) {
+func TestValidateAllPrivateOrPublicSubnets(t *testing.T) {
-	subnets := []kops.ClusterSubnetSpec{
+	userSubnets := []string{"utility-us-east-2a", "utility-us-east-2b", "utility-us-east-2c"}
-		{
+	err := validateAllPrivateOrPublicSubnets(userSubnets)
-			Name: "subnet-1234",
-			Zone: "us-east-2a",
-		},
-		{
-			Name: "subnet-987",
-			Zone: "us-east-2b",
-		},
-	}
-	zones, err := getClusterZones(subnets)
 	if err != nil {
-		t.Fatalf("an error occurred getting cluster zones")
+		t.Fatalf("should have passed validation since all user subnets were utility- subnets")
-	}
-	if len(zones) != 2 {
-		t.Fatalf("should have received two zones from the cluster subnets but only received %d", len(zones))
 	}
 
-	// Cross-Region Failure Case
+	userSubnets = []string{"us-east-2a", "us-east-2b", "us-east-2c"}
-	subnets[0].Zone = "us-west-2a"
+	err = validateAllPrivateOrPublicSubnets(userSubnets)
-	_, err = getClusterZones(subnets)
+	if err != nil {
+		t.Fatalf("should have passed validation since all user subnets were private subnets")
+	}
+
+	userSubnets = []string{"utility-us-east-2a", "utility-us-east-2b", "us-east-2c"}
+	err = validateAllPrivateOrPublicSubnets(userSubnets)
 	if err == nil {
-		t.Fatalf("should receive an error when passing in subnets that span multiple regions")
+		t.Fatalf("should have failed validation since one zone is not a utility- subnet")
+	}
 }
 
+func TestValidateUserSubnetsWithClusterSubnets(t *testing.T) {
+	clusterSubnets := []kops.ClusterSubnetSpec{
+		{ 
+			Name: "us-east-2a", 
+		}, 
+		{ 
+			Name: "us-east-2b", 
+		},
+	}
+	userSubnets := []string{"us-east-2a", "us-east-2b"}
+
+	err := validateUserSubnetsWithClusterSubnets(userSubnets, clusterSubnets)
+	if err != nil {
+		t.Fatalf("should have passed since userSubnets and clusterSubnets match")
+	}
+
+	clusterSubnets = []kops.ClusterSubnetSpec{
+		{
+			Name: "us-east-2a",
+		},
+		{
+			Name: "us-east-2b",
+		},
+	}
+	userSubnets = []string{"us-east-2a"}
+
+	err = validateUserSubnetsWithClusterSubnets(userSubnets, clusterSubnets)
+	if err != nil {
+		t.Fatalf("should have passed since userSubnets are a subset of clusterSubnets")
+	}
+
+
+	clusterSubnets = []kops.ClusterSubnetSpec{
+		{
+			Name: "us-east-2a",
+		},
+		{
+			Name: "us-east-2b",
+		},
+	}
+	userSubnets = []string{"us-east-2c"}
+
+	err = validateUserSubnetsWithClusterSubnets(userSubnets, clusterSubnets)
+	if err == nil {
+		t.Fatalf("should have failed since userSubnets are not a subset of clusterSubnets")
+	}
 }
 
 func TestCreateInstanceGroup(t *testing.T) {