From 443ed67b20c650186469431f5e7b25a4c7f48282 Mon Sep 17 00:00:00 2001 From: Brandon Wagner Date: Thu, 30 Jul 2020 18:13:01 -0500 Subject: [PATCH] use byte quantity flag instead of int MiBs for memory args --- cmd/kops/toolbox_instance_selector.go | 13 ++++++------- cmd/kops/toolbox_instance_selector_internal_test.go | 9 ++++++--- docs/cli/kops_toolbox_instance-selector.md | 12 ++++++------ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/cmd/kops/toolbox_instance_selector.go b/cmd/kops/toolbox_instance_selector.go index 4ecd40084ef33..7d4d465e79655 100644 --- a/cmd/kops/toolbox_instance_selector.go +++ b/cmd/kops/toolbox_instance_selector.go @@ -157,11 +157,11 @@ func NewCmdToolboxInstanceSelector(f *util.Factory, out io.Writer) *cobra.Comman // Raw Filters 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.ByteQuantityMinMaxRangeFlags(memory, nil, nil, "Amount of memory available (Example: 4 GiB)") 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, ", ")), 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.ByteQuantityMinMaxRangeFlags(gpuMemoryTotal, nil, nil, "Number of GPUs' total memory (Example: 4 GiB)") commandline.StringOptionsFlag(placementGroupStrategy, nil, nil, fmt.Sprintf("Placement group strategy: [%s]", strings.Join(placementGroupStrategies, ", ")), placementGroupStrategies) 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") @@ -275,7 +275,7 @@ func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, args []str return err } if instanceSelectorOpts.ClusterAutoscaler { - ig = decorateWithClusterAutoscalerLabels(ig) + ig = decorateWithClusterAutoscalerLabels(ig, cluster.ClusterName) } ig, err = cloudup.PopulateInstanceGroupSpec(cluster, ig, channel) if err != nil { @@ -369,11 +369,11 @@ func getFilters(commandline *cli.CommandLineInterface, region string, zones []st flags := commandline.Flags return selector.Filters{ VCpusRange: commandline.IntRangeMe(flags[vcpus]), - MemoryRange: commandline.IntRangeMe(flags[memory]), + MemoryRange: commandline.ByteQuantityRangeMe(flags[memory]), VCpusToMemoryRatio: commandline.Float64Me(flags[vcpusToMemoryRatio]), CPUArchitecture: commandline.StringMe(flags[cpuArchitecture]), GpusRange: commandline.IntRangeMe(flags[gpus]), - GpuMemoryRange: commandline.IntRangeMe(flags[gpuMemoryTotal]), + GpuMemoryRange: commandline.ByteQuantityRangeMe(flags[gpuMemoryTotal]), PlacementGroupStrategy: commandline.StringMe(flags[placementGroupStrategy]), UsageClass: commandline.StringMe(flags[usageClass]), EnaSupport: commandline.BoolMe(flags[enaSupport]), @@ -509,9 +509,8 @@ func decorateWithMixedInstancesPolicy(instanceGroup *kops.InstanceGroup, usageCl } // decorateWithClusterAutoscalerLabels adds cluster-autoscaler discovery tags to the cloudlabels slice -func decorateWithClusterAutoscalerLabels(instanceGroup *kops.InstanceGroup) *kops.InstanceGroup { +func decorateWithClusterAutoscalerLabels(instanceGroup *kops.InstanceGroup, clusterName string) *kops.InstanceGroup { ig := instanceGroup - clusterName := instanceGroup.ObjectMeta.Name if ig.Spec.CloudLabels == nil { ig.Spec.CloudLabels = make(map[string]string) } diff --git a/cmd/kops/toolbox_instance_selector_internal_test.go b/cmd/kops/toolbox_instance_selector_internal_test.go index 8b72e728d86c1..e5f7b485d2a56 100644 --- a/cmd/kops/toolbox_instance_selector_internal_test.go +++ b/cmd/kops/toolbox_instance_selector_internal_test.go @@ -194,10 +194,13 @@ func TestDecorateWithMixedInstancesPolicy(t *testing.T) { func TestDecorateWithClusterAutoscalerLabels(t *testing.T) { initialIG := kops.InstanceGroup{} - initialIG.ObjectMeta.Name = "testInstanceGroup" + clusterName := "testClusterName" - actualIG := decorateWithClusterAutoscalerLabels(&initialIG) + actualIG := decorateWithClusterAutoscalerLabels(&initialIG, clusterName) if _, ok := actualIG.Spec.CloudLabels["k8s.io/cluster-autoscaler/enabled"]; !ok { - t.Fatalf("cloudLabels for cluster autoscaler should have been added to the instance group spec") + t.Fatalf("enabled cloudLabel for cluster autoscaler should have been added to the instance group spec") + } + if _, ok := actualIG.Spec.CloudLabels["k8s.io/cluster-autoscaler/"+clusterName]; !ok { + t.Fatalf("cluster cloudLabel for cluster autoscaler should have been added to the instance group spec") } } diff --git a/docs/cli/kops_toolbox_instance-selector.md b/docs/cli/kops_toolbox_instance-selector.md index b889089a9e7e5..290f69ba76394 100644 --- a/docs/cli/kops_toolbox_instance-selector.md +++ b/docs/cli/kops_toolbox_instance-selector.md @@ -38,18 +38,18 @@ kops toolbox instance-selector [flags] --dry-run If true, only print the object that would be sent, without sending it. This flag can be used to create a cluster YAML or JSON manifest. --ena-support Instance types where ENA is supported or required --flexible Retrieves a group of instance types spanning multiple generations based on opinionated defaults and user overridden resource filters - --gpu-memory-total int Number of GPUs' total memory in MiB (Example: 4096) (sets --gpu-memory-total-min and -max to the same value) - --gpu-memory-total-max int Maximum Number of GPUs' total memory in MiB (Example: 4096) If --gpu-memory-total-min is not specified, the lower bound will be 0 - --gpu-memory-total-min int Minimum Number of GPUs' total memory in MiB (Example: 4096) If --gpu-memory-total-max is not specified, the upper bound will be infinity + --gpu-memory-total string Number of GPUs' total memory (Example: 4 GiB) (sets --gpu-memory-total-min and -max to the same value) + --gpu-memory-total-max string Maximum Number of GPUs' total memory (Example: 4 GiB) If --gpu-memory-total-min is not specified, the lower bound will be 0 + --gpu-memory-total-min string Minimum Number of GPUs' total memory (Example: 4 GiB) If --gpu-memory-total-max is not specified, the upper bound will be infinity --gpus int Total number of GPUs (Example: 4) (sets --gpus-min and -max to the same value) --gpus-max int Maximum Total number of GPUs (Example: 4) If --gpus-min is not specified, the lower bound will be 0 --gpus-min int Minimum Total number of GPUs (Example: 4) If --gpus-max is not specified, the upper bound will be infinity -h, --help help for instance-selector --ig-count int Number of instance groups to create w/ different vcpus-to-memory-ratios starting at 1:2 and doubling. --max-results int Maximum number of instance types to return back (default 20) - --memory int Amount of memory available in MiB (Example: 4096) (sets --memory-min and -max to the same value) - --memory-max int Maximum Amount of memory available in MiB (Example: 4096) If --memory-min is not specified, the lower bound will be 0 - --memory-min int Minimum Amount of memory available in MiB (Example: 4096) If --memory-max is not specified, the upper bound will be infinity + --memory string Amount of memory available (Example: 4 GiB) (sets --memory-min and -max to the same value) + --memory-max string Maximum Amount of memory available (Example: 4 GiB) If --memory-min is not specified, the lower bound will be 0 + --memory-min string Minimum Amount of memory available (Example: 4 GiB) If --memory-max is not specified, the upper bound will be infinity --network-interfaces int Number of network interfaces (ENIs) that can be attached to the instance (sets --network-interfaces-min and -max to the same value) --network-interfaces-max int Maximum Number of network interfaces (ENIs) that can be attached to the instance If --network-interfaces-min is not specified, the lower bound will be 0 --network-interfaces-min int Minimum Number of network interfaces (ENIs) that can be attached to the instance If --network-interfaces-max is not specified, the upper bound will be infinity