ove instance-group-name to arg like create ig

cmd/kops/toolbox_instance_selector.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"regexp"
 	"strings"
 
 	"github.com/aws/amazon-ec2-instance-selector/v2/pkg/cli"
@@ -64,21 +63,19 @@ const (
 
 // Control Flag Constants
 const (
-	instanceGroupName  = "ig-name"
+	instanceGroupCount   = "ig-count"
-	instanceGroupCount = "ig-count"
+	nodeCountMin         = "node-count-min"
-	nodeCountMin       = "node-count-min"
+	nodeCountMax         = "node-count-max"
-	nodeCountMax       = "node-count-max"
+	nodeVolumeSize       = "node-volume-size"
-	nodeVolumeSize     = "node-volume-size"
+	nodeSecurityGroups   = "node-security-groups"
-	nodeSecurityGroups = "node-security-groups"
+	clusterAutoscaler    = "cluster-autoscaler"
-	clusterAutoscaler  = "cluster-autoscaler"
+	usageClassSpot       = "spot"
-	usageClassSpot     = "spot"
+	usageClassOndemand   = "on-demand"
-	usageClassOndemand = "on-demand"
+	dryRun               = "dry-run"
-	dryRun             = "dry-run"
+	output               = "output"
-	output             = "output"
+	cpuArchitectureAMD64 = "amd64"
-)
+	cpuArchitectureX8664 = "x86_64"
-
+	cpuArchitectureARM64 = "arm64"
-const (
-	nameRegex = `^[a-zA-Z0-9\-_]{1,128}$`
 )
 
 // InstanceSelectorOptions is a struct representing non-filter flags passed into instance-selector
@@ -100,12 +97,14 @@ var (
 
 	toolboxInstanceSelectorExample = templates.Examples(i18n.T(`
 
+	kops toolbox instance-selector <ig name> [flags ...]
+
 	## Create a best-practices spot instance-group using a MixInstancesPolicy and Capacity-Optimized spot allocation strategy
 	## --flexible defaults to a 1:2 vcpus to memory ratio and 4 vcpus
-	kops toolbox instance-selector --usage-class spot --flexible --ig-name my-spot-mig
+	kops toolbox instance-selector my-spot-mig --usage-class spot --flexible
 
 	## Create a best-practices on-demand instance-group with custom vcpus and memory range filters
-	kops toolbox instance-selector --ig-name ondemand-ig --vcpus-min=2 --vcpus-max=4 --memory-min 2048 --memory-max 4096
+	kops toolbox instance-selector ondemand-ig --vcpus-min=2 --vcpus-max=4 --memory-min 2048 --memory-max 4096
 	`))
 
 	toolboxInstanceSelectorShort = i18n.T(`Generate on-demand or spot instance-group specs by providing resource specs like vcpus and memory.`)
@@ -123,17 +122,14 @@ func NewCmdToolboxInstanceSelector(f *util.Factory, out io.Writer) *cobra.Comman
 	commandline.Command.Run = func(cmd *cobra.Command, args []string) {
 		ctx := context.TODO()
 
-		if err := rootCommand.ProcessArgs(args); err != nil {
+		err := RunToolboxInstanceSelector(ctx, f, args, out, &commandline)
-			exitWithError(err)
-		}
-		err := RunToolboxInstanceSelector(ctx, f, out, rootCommand.ClusterName(), &commandline)
 		if err != nil {
 			exitWithError(err)
 		}
 	}
 
-	cpuArchs := []string{"x86_64", "arm64"}
+	cpuArchs := []string{cpuArchitectureAMD64, cpuArchitectureARM64}
-	cpuArchDefault := "x86_64"
+	cpuArchDefault := cpuArchitectureAMD64
 	placementGroupStrategies := []string{"cluster", "partition", "spread"}
 	usageClasses := []string{usageClassSpot, usageClassOndemand}
 	usageClassDefault := usageClassOndemand
@@ -144,20 +140,6 @@ func NewCmdToolboxInstanceSelector(f *util.Factory, out io.Writer) *cobra.Comman
 	nodeCountMaxDefault := 15
 	maxResultsDefault := 20
 
-	commandline.StringFlag(instanceGroupName, nil, nil, "Name of the Instance-Group", func(val interface{}) error {
-		if val == nil {
-			return fmt.Errorf("error you must supply --%s", instanceGroupName)
-		}
-		matched, err := regexp.MatchString(nameRegex, *val.(*string))
-		if err != nil {
-			return err
-		}
-		if matched {
-			return nil
-		}
-		return fmt.Errorf("error --%s must conform to the regex: \"%s\"", instanceGroupName, nameRegex)
-	})
-
 	// Instance Group Node Configurations
 
 	commandline.IntFlag(nodeCountMin, nil, &nodeCountMinDefault, "Set the minimum number of nodes")
@@ -177,7 +159,7 @@ func NewCmdToolboxInstanceSelector(f *util.Factory, out io.Writer) *cobra.Comman
 	commandline.IntMinMaxRangeFlags(vcpus, nil, nil, "Number of vcpus available to the instance type.")
 	commandline.IntMinMaxRangeFlags(memory, nil, nil, "Amount of memory available in MiB (Example: 4096)")
 	commandline.RatioFlag(vcpusToMemoryRatio, nil, nil, "The ratio of vcpus to memory in MiB. (Example: 1:2)")
-	commandline.StringOptionsFlag(cpuArchitecture, nil, &cpuArchDefault, fmt.Sprintf("CPU architecture [%s]", strings.Join(cpuArchs, ", ")), cpuArchs)
+	commandline.StringOptionsFlag(cpuArchitecture, nil, &cpuArchDefault, fmt.Sprintf("CPU architecture [%s]", strings.Join(cpuArchs, ", ")), append(cpuArchs, cpuArchitectureX8664))
 	commandline.IntMinMaxRangeFlags(gpus, nil, nil, "Total number of GPUs (Example: 4)")
 	commandline.IntMinMaxRangeFlags(gpuMemoryTotal, nil, nil, "Number of GPUs' total memory in MiB (Example: 4096)")
 	commandline.StringOptionsFlag(placementGroupStrategy, nil, nil, fmt.Sprintf("Placement group strategy: [%s]", strings.Join(placementGroupStrategies, ", ")), placementGroupStrategies)
@@ -199,26 +181,31 @@ func NewCmdToolboxInstanceSelector(f *util.Factory, out io.Writer) *cobra.Comman
 }
 
 // RunToolboxInstanceSelector executes the instance-selector tool to create instance groups with declarative resource specifications
-func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, out io.Writer, clusterName string, commandline *cli.CommandLineInterface) error {
+func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, args []string, out io.Writer, commandline *cli.CommandLineInterface) error {
+	if len(args) == 0 {
+		return fmt.Errorf("Specify name of instance group to create")
+	}
+	if len(args) != 1 {
+		return fmt.Errorf("Can only specify one instance group name at a time")
+	}
+	igName := args[0]
 
-	flags, err := processAndValidateFlags(commandline, clusterName)
+	flags, err := processAndValidateFlags(commandline)
 	if err != nil {
 		return err
 	}
 	instanceSelectorOpts := getInstanceSelectorOpts(commandline)
+	instanceSelectorOpts.InstanceGroupName = igName
 
-	clientset, cluster, channel, err := retrieveClusterRefs(ctx, f, clusterName)
+	clientset, cluster, channel, err := retrieveClusterRefs(ctx, f)
 	if err != nil {
 		return err
 	}
-	
+
 	if kops.CloudProviderID(cluster.Spec.CloudProvider) != kops.CloudProviderAWS {
 		return fmt.Errorf("cannot select instance types from non-aws cluster")
 	}
 
-	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]
 
@@ -226,22 +213,23 @@ func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, out io.Wri
 	for _, clusterSubnet := range cluster.Spec.Subnets {
 		igSubnets = append(igSubnets, clusterSubnet.Name)
 	}
-	
+
 	if commandline.Flags[subnets] != nil {
 		userSubnets := *commandline.StringSliceMe(commandline.Flags[subnets])
+		dryRun := *commandline.BoolMe(commandline.Flags[dryRun])
 		err := validateUserSubnets(userSubnets, cluster.Spec.Subnets)
-		if err != nil {
+		if err != nil && !dryRun {
 			return err
 		}
 		igSubnets = userSubnets
 	}
-	
+
 	zones := []string{}
 	for _, igSubnet := range igSubnets {
 		zones = append(zones, strings.ReplaceAll(igSubnet, "utility-", ""))
 	}
 
-	tags := map[string]string{"KubernetesCluster": clusterName}
+	tags := map[string]string{"KubernetesCluster": cluster.ClusterName}
 	cloud, err := awsup.NewAWSCloud(region, tags)
 	if err != nil {
 		return fmt.Errorf("error initializing AWS client: %v", err)
@@ -264,7 +252,6 @@ func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, out io.Wri
 		}
 	}
 
-	igName := instanceSelectorOpts.InstanceGroupName
 	newInstanceGroups := []*kops.InstanceGroup{}
 
 	for i := 0; i < igCount; i++ {
@@ -281,7 +268,7 @@ func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, out io.Wri
 		}
 		usageClass := *filters.UsageClass
 
-		ig := createInstanceGroup(igNameForRun, clusterName, igSubnets)
+		ig := createInstanceGroup(igNameForRun, cluster.ClusterName, igSubnets)
 		ig = decorateWithInstanceGroupSpecs(ig, instanceSelectorOpts)
 		ig, err = decorateWithMixedInstancesPolicy(ig, usageClass, selectedInstanceTypes)
 		if err != nil {
@@ -335,7 +322,7 @@ func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, out io.Wri
 	return nil
 }
 
-func processAndValidateFlags(commandline *cli.CommandLineInterface, clusterName string) (map[string]interface{}, error) {
+func processAndValidateFlags(commandline *cli.CommandLineInterface) (map[string]interface{}, error) {
 	if err := commandline.SetUntouchedFlagValuesToNil(); err != nil {
 		return nil, err
 	}
@@ -348,26 +335,22 @@ func processAndValidateFlags(commandline *cli.CommandLineInterface, clusterName
 		return nil, err
 	}
 
-	if clusterName == "" {
-		return nil, fmt.Errorf("ClusterName is required")
-	}
-
 	return commandline.Flags, nil
 }
 
-func retrieveClusterRefs(ctx context.Context, f *util.Factory, clusterName string) (simple.Clientset, *kops.Cluster, *kops.Channel, error) {
+func retrieveClusterRefs(ctx context.Context, f *util.Factory) (simple.Clientset, *kops.Cluster, *kops.Channel, error) {
 	clientset, err := f.Clientset()
 	if err != nil {
 		return nil, nil, nil, err
 	}
 
-	cluster, err := clientset.GetCluster(ctx, clusterName)
+	cluster, err := rootCommand.Cluster(ctx)
 	if err != nil {
 		return nil, nil, nil, err
 	}
 
 	if cluster == nil {
-		return nil, nil, nil, fmt.Errorf("cluster %q not found", clusterName)
+		return nil, nil, nil, fmt.Errorf("cluster not found")
 	}
 
 	channel, err := cloudup.ChannelForCluster(cluster)
@@ -384,7 +367,7 @@ func retrieveClusterRefs(ctx context.Context, f *util.Factory, clusterName strin
 
 func getFilters(commandline *cli.CommandLineInterface, region string, zones []string) selector.Filters {
 	flags := commandline.Flags
-	return selector.Filters{
+	filters := selector.Filters{
 		VCpusRange:             commandline.IntRangeMe(flags[vcpus]),
 		MemoryRange:            commandline.IntRangeMe(flags[memory]),
 		VCpusToMemoryRatio:     commandline.Float64Me(flags[vcpusToMemoryRatio]),
@@ -405,6 +388,11 @@ func getFilters(commandline *cli.CommandLineInterface, region string, zones []st
 		InstanceTypeBase:       commandline.StringMe(flags[instanceTypeBase]),
 		Flexible:               commandline.BoolMe(flags[flexible]),
 	}
+	// convert amd64 to x86_64 for the ec2 api
+	if filters.CPUArchitecture != nil && *filters.CPUArchitecture == cpuArchitectureAMD64 {
+		filters.CPUArchitecture = commandline.StringMe(cpuArchitectureX8664)
+	}
+	return filters
 }
 
 func getInstanceSelectorOpts(commandline *cli.CommandLineInterface) InstanceSelectorOptions {
@@ -412,7 +400,6 @@ func getInstanceSelectorOpts(commandline *cli.CommandLineInterface) InstanceSele
 	flags := commandline.Flags
 	opts.NodeCountMin = int32(*commandline.IntMe(flags[nodeCountMin]))
 	opts.NodeCountMax = int32(*commandline.IntMe(flags[nodeCountMax]))
-	opts.InstanceGroupName = *commandline.StringMe(flags[instanceGroupName])
 	opts.Output = *commandline.StringMe(flags[output])
 	opts.DryRun = *commandline.BoolMe(flags[dryRun])
 	opts.ClusterAutoscaler = *commandline.BoolMe(flags[clusterAutoscaler])
@@ -443,9 +430,9 @@ func validateUserSubnets(userSubnets []string, clusterSubnets []kops.ClusterSubn
 
 // validateUserSubnetsWithClusterSubnets makes sure the userSubnets are part of the cluster subnets
 func validateUserSubnetsWithClusterSubnets(userSubnets []string, clusterSubnets []kops.ClusterSubnetSpec) error {
-	for _, clusterSubnet := range clusterSubnets {
+	for _, userSubnet := range userSubnets {
 		userSubnetValid := false
-		for _, userSubnet := range userSubnets {
+		for _, clusterSubnet := range clusterSubnets {
 			if clusterSubnet.Name == userSubnet {
 				userSubnetValid = true
 				break
@@ -456,17 +443,18 @@ func validateUserSubnetsWithClusterSubnets(userSubnets []string, clusterSubnets
 		}
 	}
 	return nil
-} 
+}
 
 // validateAllPrivateOrPublicSubnets makes sure the passed in subnets are all utility (public) subnets or private subnets
 func validateAllPrivateOrPublicSubnets(userSubnets []string) error {
 	utilitySubnets := 0
 	for _, userSubnet := range userSubnets {
-		if strings.HasPrefix("utility-", userSubnet) {
+		if strings.HasPrefix(userSubnet, "utility-") {
 			utilitySubnets++
 		}
 	}
-	if utilitySubnets != 0 && len(userSubnets) == utilitySubnets {
+
+	if utilitySubnets != 0 && len(userSubnets) != utilitySubnets {
 		return fmt.Errorf("error instance group cannot span public and private subnets")
 	}
 	return nil

cmd/kops/toolbox_instance_selector_internal_test.go

@@ -47,7 +47,6 @@ func TestGetInstanceSelectorOpts(t *testing.T) {
 	outputStr := "json"
 	commandline := cli.New("test", "test", "test", "test", nil)
 	commandline.Flags = map[string]interface{}{
-		instanceGroupName:  "test",
 		instanceGroupCount: count,
 		nodeCountMax:       count,
 		nodeCountMin:       count,
@@ -70,9 +69,6 @@ func TestGetInstanceSelectorOpts(t *testing.T) {
 	if !instanceSelectorOpts.DryRun || !instanceSelectorOpts.ClusterAutoscaler {
 		t.Fatalf("dryRun and clusterAutoscaler should be true, got false instead")
 	}
-	if instanceSelectorOpts.InstanceGroupName != "test" {
-		t.Fatalf("instance group name should have been test")
-	}
 }
 
 func TestValidateAllPrivateOrPublicSubnets(t *testing.T) {
@@ -97,11 +93,11 @@ func TestValidateAllPrivateOrPublicSubnets(t *testing.T) {
 
 func TestValidateUserSubnetsWithClusterSubnets(t *testing.T) {
 	clusterSubnets := []kops.ClusterSubnetSpec{
-		{ 
+		{
-			Name: "us-east-2a", 
+			Name: "us-east-2a",
-		}, 
+		},
-		{ 
+		{
-			Name: "us-east-2b", 
+			Name: "us-east-2b",
 		},
 	}
 	userSubnets := []string{"us-east-2a", "us-east-2b"}
@@ -126,7 +122,6 @@ func TestValidateUserSubnetsWithClusterSubnets(t *testing.T) {
 		t.Fatalf("should have passed since userSubnets are a subset of clusterSubnets")
 	}
 
-
 	clusterSubnets = []kops.ClusterSubnetSpec{
 		{
 			Name: "us-east-2a",