From 59ae92bce2c6ef36a3c6e2994f99ce8a77224e1b Mon Sep 17 00:00:00 2001 From: Rodrigo Okamoto Date: Tue, 14 Jun 2022 16:06:49 -0500 Subject: [PATCH 1/9] fixed issue with OD pricing for european regions --- pkg/ec2pricing/odpricing.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/ec2pricing/odpricing.go b/pkg/ec2pricing/odpricing.go index 3052191..6f97131 100644 --- a/pkg/ec2pricing/odpricing.go +++ b/pkg/ec2pricing/odpricing.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "time" "github.com/aws/aws-sdk-go/aws" @@ -214,6 +215,12 @@ func (c *OnDemandPricing) getRegionForPricingAPI() string { regionDescription = region.Description() } } + + // endpoints package returns European regions with the word "Europe," but the pricing API expects the word "EU." + // This formatting mismatch is only present with European regions. + // So replace "Europe" with "EU" if it exists in the regionDescription string. + regionDescription = strings.Replace(regionDescription, "Europe", "EU", 2) + return regionDescription } From f704d3810d053f9110c1b9b11126e2b0e169cb57 Mon Sep 17 00:00:00 2001 From: Rodrigo Okamoto Date: Wed, 15 Jun 2022 16:20:59 -0500 Subject: [PATCH 2/9] made string replacement more readable in getRegionForPricingAPI --- pkg/ec2pricing/odpricing.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ec2pricing/odpricing.go b/pkg/ec2pricing/odpricing.go index 6f97131..f4ea64b 100644 --- a/pkg/ec2pricing/odpricing.go +++ b/pkg/ec2pricing/odpricing.go @@ -219,7 +219,7 @@ func (c *OnDemandPricing) getRegionForPricingAPI() string { // endpoints package returns European regions with the word "Europe," but the pricing API expects the word "EU." // This formatting mismatch is only present with European regions. // So replace "Europe" with "EU" if it exists in the regionDescription string. - regionDescription = strings.Replace(regionDescription, "Europe", "EU", 2) + regionDescription = strings.ReplaceAll(regionDescription, "Europe", "EU") return regionDescription } From b19f08559b4a0217e147a3170bc43a038721af4e Mon Sep 17 00:00:00 2001 From: Rodrigo Okamoto Date: Thu, 16 Jun 2022 16:30:05 -0500 Subject: [PATCH 3/9] implemented sorting of instance types --- cmd/main.go | 146 +++++++++++++++++++++++++++++++++++---- pkg/selector/selector.go | 11 +-- 2 files changed, 140 insertions(+), 17 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index f57e56d..e97ee23 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -18,6 +18,7 @@ import ( "log" "os" "os/signal" + "sort" "strings" "sync" "syscall" @@ -25,6 +26,7 @@ import ( commandline "github.com/aws/amazon-ec2-instance-selector/v2/pkg/cli" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/env" + "github.com/aws/amazon-ec2-instance-selector/v2/pkg/instancetypes" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector/outputs" "github.com/aws/aws-sdk-go/aws/session" @@ -45,6 +47,14 @@ const ( tableOutput = "table" tableWideOutput = "table-wide" oneLine = "one-line" + + ODPriceSort = "on-demand-price" + spotPriceSort = "spot-price" + vcpuSort = "vcpu" + memorySort = "memory" + + sortAscending = "ascending" + sortDescending = "descending" ) // Filter Flag Constants @@ -103,15 +113,17 @@ const ( // Configuration Flag Constants const ( - maxResults = "max-results" - profile = "profile" - help = "help" - verbose = "verbose" - version = "version" - region = "region" - output = "output" - cacheTTL = "cache-ttl" - cacheDir = "cache-dir" + maxResults = "max-results" + profile = "profile" + help = "help" + verbose = "verbose" + version = "version" + region = "region" + output = "output" + cacheTTL = "cache-ttl" + cacheDir = "cache-dir" + sortDirection = "sort-direction" + sortFilter = "sort-filter" ) var ( @@ -142,6 +154,18 @@ Full docs can be found at github.com/aws/amazon-` + binName } resultsOutputFn := outputs.SimpleInstanceTypeOutput + cliSortCriteria := []string{ + ODPriceSort, + spotPriceSort, + vcpuSort, + memorySort, + } + + cliSortDirections := []string{ + sortAscending, + sortDescending, + } + // Registers flags with specific input types from the cli pkg // Filter Flags - These will be grouped at the top of the help flags @@ -206,6 +230,8 @@ Full docs can be found at github.com/aws/amazon-` + binName cli.ConfigBoolFlag(verbose, cli.StringMe("v"), nil, "Verbose - will print out full instance specs") cli.ConfigBoolFlag(help, cli.StringMe("h"), nil, "Help") cli.ConfigBoolFlag(version, nil, nil, "Prints CLI version") + cli.ConfigStringFlag(sortFilter, nil, nil, fmt.Sprintf("Specify the field to sort by (%s) (NOTE: if not passed in, will be sorted by instance type name)", strings.Join(cliSortCriteria, ", ")), nil) + cli.ConfigStringFlag(sortDirection, nil, nil, fmt.Sprintf("Specify the direction to sort in (%s) (NOTE: if not passed in, will be ascending by default)", strings.Join(cliSortDirections, ", ")), nil) // Parses the user input with the registered flags and runs type specific validation on the user input flags, err := cli.ParseAndValidateFlags() @@ -340,20 +366,38 @@ Full docs can be found at github.com/aws/amazon-` + binName outputFn := getOutputFn(outputFlag, selector.InstanceTypesOutputFn(resultsOutputFn)) - instanceTypes, itemsTruncated, err := instanceSelector.FilterWithOutput(filters, outputFn) + // get instance types based on filters + // TODO: make filterVerbose return number of truncated values + instanceTypesDetails, itemsTruncated, err := instanceSelector.FilterVerbose(filters) if err != nil { - fmt.Printf("An error occurred when filtering instance types: %v", err) + fmt.Printf("An error occured when filtering instance types: %v", err) os.Exit(1) } - if len(instanceTypes) == 0 { + if len(instanceTypesDetails) == 0 { log.Println("The criteria was too narrow and returned no valid instance types. Consider broadening your criteria so that more instance types are returned.") os.Exit(1) } + // sort instance types + sortFilterFlag := cli.StringMe(flags[sortFilter]) + sortDirectionFlag := cli.StringMe(flags[sortDirection]) + instanceTypesDetails = sortInstanceTypes(sortFilterFlag, sortDirectionFlag, instanceTypesDetails) + + // format output + instanceTypes := outputFn(instanceTypesDetails) + + // print output for _, instanceType := range instanceTypes { fmt.Println(instanceType) } + if !validateSortFilter(sortFilterFlag) { + log.Printf("\"%s\" is not a valid sorting filter (Valid filters: %s). Results were sorted by instance type name", *sortFilterFlag, strings.Join(cliSortCriteria, ", ")) + } + if !validateSortDirection(sortDirectionFlag) { + log.Printf("\"%s\" is not a valid sorting direction (Valid directions: %s). Results were sorted in ascending order", *sortDirectionFlag, strings.Join(cliSortDirections, ", ")) + } + if itemsTruncated > 0 { log.Printf("%d entries were truncated, increase --%s to see more", itemsTruncated, maxResults) } @@ -479,6 +523,84 @@ func getProfileRegion(profileName string) (string, error) { return regionConfig.String(), nil } +// determines whether the given sort direction flag is valid or not +func validateSortDirection(sortDirectionFlag *string) bool { + return sortDirectionFlag == nil || *sortDirectionFlag == sortAscending || *sortDirectionFlag == sortDescending +} + +// determines whether the sort filter flag is valid +func validateSortFilter(sortFilterFlag *string) bool { + if sortFilterFlag == nil { + return true + } + + valid := false + + switch *sortFilterFlag { + case ODPriceSort: + valid = true + case spotPriceSort: + valid = true + case vcpuSort: + valid = true + case memorySort: + valid = true + } + + return valid +} + +// sorts the given list of instance type details based on the sorting filter flag and the sort direction flag +func sortInstanceTypes(sortFilterFlag *string, sortDirectionFlag *string, instanceTypes []*instancetypes.Details) []*instancetypes.Details { + // by default, sorted in ascending order. + shouldReverse := false + if sortDirectionFlag != nil && *sortDirectionFlag == sortDescending { + shouldReverse = true + } + + // sort instance types based on filter flag and reverse list if the order should be descending + if sortFilterFlag != nil { + sort.Slice(instanceTypes, func(i, j int) bool { + firstType := instanceTypes[i] + secondType := instanceTypes[j] + switch *sortFilterFlag { + case ODPriceSort: + if shouldReverse { + return *firstType.OndemandPricePerHour > *secondType.OndemandPricePerHour + } else { + return *firstType.OndemandPricePerHour < *secondType.OndemandPricePerHour + } + case spotPriceSort: + if shouldReverse { + return *firstType.SpotPrice > *secondType.SpotPrice + } else { + return *firstType.SpotPrice < *secondType.SpotPrice + } + case vcpuSort: + if shouldReverse { + return *firstType.VCpuInfo.DefaultVCpus > *secondType.VCpuInfo.DefaultVCpus + } else { + return *firstType.VCpuInfo.DefaultVCpus < *secondType.VCpuInfo.DefaultVCpus + } + case memorySort: + if shouldReverse { + return *firstType.MemoryInfo.SizeInMiB > *secondType.MemoryInfo.SizeInMiB + } else { + return *firstType.MemoryInfo.SizeInMiB < *secondType.MemoryInfo.SizeInMiB + } + default: + if shouldReverse { + return i > j + } else { + return i < j + } + } + }) + } + + return instanceTypes +} + func registerShutdown(shutdown func()) { sigs := make(chan os.Signal, 1) signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) diff --git a/pkg/selector/selector.go b/pkg/selector/selector.go index ebb10a0..88551ee 100644 --- a/pkg/selector/selector.go +++ b/pkg/selector/selector.go @@ -139,14 +139,15 @@ func (itf Selector) Filter(filters Filters) ([]string, error) { } // FilterVerbose accepts a Filters struct which is used to select the available instance types -// matching the criteria within Filters and returns a list instanceTypeInfo -func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, error) { +// matching the criteria within Filters and returns a list instanceTypeInfo along with the number +// of truncated items. +func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, int, error) { instanceTypeInfoSlice, err := itf.rawFilter(filters) if err != nil { - return nil, err + return nil, 0, err } - instanceTypeInfoSlice, _ = itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) - return instanceTypeInfoSlice, nil + instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) + return instanceTypeInfoSlice, numOfItemsTruncated, nil } // FilterWithOutput accepts a Filters struct which is used to select the available instance types From eb53d76147ef03ebc27871bb9d3dd42d146b56a0 Mon Sep 17 00:00:00 2001 From: Rodrigo Okamoto Date: Thu, 16 Jun 2022 16:43:54 -0500 Subject: [PATCH 4/9] fixed typo in filtering error message --- cmd/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main.go b/cmd/main.go index e97ee23..c8fa206 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -370,7 +370,7 @@ Full docs can be found at github.com/aws/amazon-` + binName // TODO: make filterVerbose return number of truncated values instanceTypesDetails, itemsTruncated, err := instanceSelector.FilterVerbose(filters) if err != nil { - fmt.Printf("An error occured when filtering instance types: %v", err) + fmt.Printf("An error occurred when filtering instance types: %v", err) os.Exit(1) } if len(instanceTypesDetails) == 0 { From 94749fdd400f81eaf341bd41a788f87d2ae53345 Mon Sep 17 00:00:00 2001 From: Rodrigo Okamoto Date: Mon, 20 Jun 2022 12:48:53 -0500 Subject: [PATCH 5/9] moved sort to selector.go and refactored FilterVerbose tests --- cmd/main.go | 124 ++++------------------------- pkg/selector/selector.go | 142 +++++++++++++++++++++++++++++----- pkg/selector/selector_test.go | 21 +++-- 3 files changed, 148 insertions(+), 139 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index c8fa206..fb3a10c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -18,7 +18,6 @@ import ( "log" "os" "os/signal" - "sort" "strings" "sync" "syscall" @@ -26,7 +25,6 @@ import ( commandline "github.com/aws/amazon-ec2-instance-selector/v2/pkg/cli" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/env" - "github.com/aws/amazon-ec2-instance-selector/v2/pkg/instancetypes" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector/outputs" "github.com/aws/aws-sdk-go/aws/session" @@ -47,14 +45,6 @@ const ( tableOutput = "table" tableWideOutput = "table-wide" oneLine = "one-line" - - ODPriceSort = "on-demand-price" - spotPriceSort = "spot-price" - vcpuSort = "vcpu" - memorySort = "memory" - - sortAscending = "ascending" - sortDescending = "descending" ) // Filter Flag Constants @@ -155,15 +145,16 @@ Full docs can be found at github.com/aws/amazon-` + binName resultsOutputFn := outputs.SimpleInstanceTypeOutput cliSortCriteria := []string{ - ODPriceSort, - spotPriceSort, - vcpuSort, - memorySort, + selector.ODPriceSortFlag, + selector.SpotPriceSortFlag, + selector.VcpuSortFlag, + selector.MemorySortFlag, + selector.NameSortFlag, } cliSortDirections := []string{ - sortAscending, - sortDescending, + selector.SortAscendingFlag, + selector.SortDescendingFlag, } // Registers flags with specific input types from the cli pkg @@ -230,8 +221,8 @@ Full docs can be found at github.com/aws/amazon-` + binName cli.ConfigBoolFlag(verbose, cli.StringMe("v"), nil, "Verbose - will print out full instance specs") cli.ConfigBoolFlag(help, cli.StringMe("h"), nil, "Help") cli.ConfigBoolFlag(version, nil, nil, "Prints CLI version") - cli.ConfigStringFlag(sortFilter, nil, nil, fmt.Sprintf("Specify the field to sort by (%s) (NOTE: if not passed in, will be sorted by instance type name)", strings.Join(cliSortCriteria, ", ")), nil) - cli.ConfigStringFlag(sortDirection, nil, nil, fmt.Sprintf("Specify the direction to sort in (%s) (NOTE: if not passed in, will be ascending by default)", strings.Join(cliSortDirections, ", ")), nil) + cli.ConfigStringOptionsFlag(sortDirection, nil, cli.StringMe(selector.SortAscendingFlag), fmt.Sprintf("Specify the direction to sort in (%s)", strings.Join(cliSortDirections, ", ")), cliSortDirections) + cli.ConfigStringOptionsFlag(sortFilter, nil, cli.StringMe(selector.NameSortFlag), fmt.Sprintf("Specify the field to sort by (%s)", strings.Join(cliSortCriteria, ", ")), cliSortCriteria) // Parses the user input with the registered flags and runs type specific validation on the user input flags, err := cli.ParseAndValidateFlags() @@ -366,9 +357,10 @@ Full docs can be found at github.com/aws/amazon-` + binName outputFn := getOutputFn(outputFlag, selector.InstanceTypesOutputFn(resultsOutputFn)) - // get instance types based on filters - // TODO: make filterVerbose return number of truncated values - instanceTypesDetails, itemsTruncated, err := instanceSelector.FilterVerbose(filters) + // get instance types based on filters and sort them based on sorting flags + sortFilterFlag := cli.StringMe(flags[sortFilter]) + sortDirectionFlag := cli.StringMe(flags[sortDirection]) + instanceTypesDetails, itemsTruncated, err := instanceSelector.FilterAndSort(filters, sortFilterFlag, sortDirectionFlag) if err != nil { fmt.Printf("An error occurred when filtering instance types: %v", err) os.Exit(1) @@ -378,11 +370,6 @@ Full docs can be found at github.com/aws/amazon-` + binName os.Exit(1) } - // sort instance types - sortFilterFlag := cli.StringMe(flags[sortFilter]) - sortDirectionFlag := cli.StringMe(flags[sortDirection]) - instanceTypesDetails = sortInstanceTypes(sortFilterFlag, sortDirectionFlag, instanceTypesDetails) - // format output instanceTypes := outputFn(instanceTypesDetails) @@ -391,13 +378,6 @@ Full docs can be found at github.com/aws/amazon-` + binName fmt.Println(instanceType) } - if !validateSortFilter(sortFilterFlag) { - log.Printf("\"%s\" is not a valid sorting filter (Valid filters: %s). Results were sorted by instance type name", *sortFilterFlag, strings.Join(cliSortCriteria, ", ")) - } - if !validateSortDirection(sortDirectionFlag) { - log.Printf("\"%s\" is not a valid sorting direction (Valid directions: %s). Results were sorted in ascending order", *sortDirectionFlag, strings.Join(cliSortDirections, ", ")) - } - if itemsTruncated > 0 { log.Printf("%d entries were truncated, increase --%s to see more", itemsTruncated, maxResults) } @@ -523,84 +503,6 @@ func getProfileRegion(profileName string) (string, error) { return regionConfig.String(), nil } -// determines whether the given sort direction flag is valid or not -func validateSortDirection(sortDirectionFlag *string) bool { - return sortDirectionFlag == nil || *sortDirectionFlag == sortAscending || *sortDirectionFlag == sortDescending -} - -// determines whether the sort filter flag is valid -func validateSortFilter(sortFilterFlag *string) bool { - if sortFilterFlag == nil { - return true - } - - valid := false - - switch *sortFilterFlag { - case ODPriceSort: - valid = true - case spotPriceSort: - valid = true - case vcpuSort: - valid = true - case memorySort: - valid = true - } - - return valid -} - -// sorts the given list of instance type details based on the sorting filter flag and the sort direction flag -func sortInstanceTypes(sortFilterFlag *string, sortDirectionFlag *string, instanceTypes []*instancetypes.Details) []*instancetypes.Details { - // by default, sorted in ascending order. - shouldReverse := false - if sortDirectionFlag != nil && *sortDirectionFlag == sortDescending { - shouldReverse = true - } - - // sort instance types based on filter flag and reverse list if the order should be descending - if sortFilterFlag != nil { - sort.Slice(instanceTypes, func(i, j int) bool { - firstType := instanceTypes[i] - secondType := instanceTypes[j] - switch *sortFilterFlag { - case ODPriceSort: - if shouldReverse { - return *firstType.OndemandPricePerHour > *secondType.OndemandPricePerHour - } else { - return *firstType.OndemandPricePerHour < *secondType.OndemandPricePerHour - } - case spotPriceSort: - if shouldReverse { - return *firstType.SpotPrice > *secondType.SpotPrice - } else { - return *firstType.SpotPrice < *secondType.SpotPrice - } - case vcpuSort: - if shouldReverse { - return *firstType.VCpuInfo.DefaultVCpus > *secondType.VCpuInfo.DefaultVCpus - } else { - return *firstType.VCpuInfo.DefaultVCpus < *secondType.VCpuInfo.DefaultVCpus - } - case memorySort: - if shouldReverse { - return *firstType.MemoryInfo.SizeInMiB > *secondType.MemoryInfo.SizeInMiB - } else { - return *firstType.MemoryInfo.SizeInMiB < *secondType.MemoryInfo.SizeInMiB - } - default: - if shouldReverse { - return i > j - } else { - return i < j - } - } - }) - } - - return instanceTypes -} - func registerShutdown(shutdown func()) { sigs := make(chan os.Signal, 1) signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) diff --git a/pkg/selector/selector.go b/pkg/selector/selector.go index 88551ee..4b55535 100644 --- a/pkg/selector/selector.go +++ b/pkg/selector/selector.go @@ -99,6 +99,17 @@ const ( virtualizationTypePV = "pv" pricePerHour = "pricePerHour" + + // Sorting costants + + ODPriceSortFlag = "on-demand-price" + SpotPriceSortFlag = "spot-price" + VcpuSortFlag = "vcpu" + MemorySortFlag = "memory" + NameSortFlag = "instance-type-name" + + SortAscendingFlag = "ascending" + SortDescendingFlag = "descending" ) // New creates an instance of Selector provided an aws session @@ -134,8 +145,12 @@ func (itf Selector) Save() error { // matching the criteria within Filters and returns a simple list of instance type strings func (itf Selector) Filter(filters Filters) ([]string, error) { outputFn := InstanceTypesOutputFn(outputs.SimpleInstanceTypeOutput) - output, _, err := itf.FilterWithOutput(filters, outputFn) - return output, err + instaceTypeDetails, _, err := itf.FilterVerbose(filters) + var formatedOutput []string = nil + if err == nil { + formatedOutput = outputFn(instaceTypeDetails) + } + return formatedOutput, err } // FilterVerbose accepts a Filters struct which is used to select the available instance types @@ -150,16 +165,113 @@ func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, in return instanceTypeInfoSlice, numOfItemsTruncated, nil } -// FilterWithOutput accepts a Filters struct which is used to select the available instance types -// matching the criteria within Filters and returns a list of strings based on the custom outputFn -func (itf Selector) FilterWithOutput(filters Filters, outputFn InstanceTypesOutput) ([]string, int, error) { +// FilterAndSort accepts a Filters struct, which is used to select the available instance types +// matching the criteria within Filters, as well as a sortFilterFlag and a sortDirectionsFlag, which are used +// to sort the instances, and returns a list of instanceTypeInfo along with the number of truncated items. +// Acceptable sorting filters: on-demand-price, spot-price, vcpu, memory, instance-type-name. +// Acceptable sorting directions: ascending, descending. +func (itf Selector) FilterAndSort(filters Filters, sortFilterFlag *string, sortDirectionFlag *string) ([]*instancetypes.Details, int, error) { + // get instance types based on filter instanceTypeInfoSlice, err := itf.rawFilter(filters) if err != nil { return nil, 0, err } + + // sort results + instanceTypeInfoSlice = sortInstanceTypes(sortFilterFlag, sortDirectionFlag, instanceTypeInfoSlice) + + // truncate results instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) - output := outputFn.Output(instanceTypeInfoSlice) - return output, numOfItemsTruncated, nil + + return instanceTypeInfoSlice, numOfItemsTruncated, nil +} + +// sorts the given list of instance type details based on the sorting filter flag and the sort direction flag +func sortInstanceTypes(sortFilterFlag *string, sortDirectionFlag *string, instanceTypes []*instancetypes.Details) []*instancetypes.Details { + if len(instanceTypes) <= 1 { + return instanceTypes + } + + // by default, sorted in ascending order. + shouldReverse := false + if sortDirectionFlag != nil && *sortDirectionFlag == SortDescendingFlag { + shouldReverse = true + } + + // sort instance types based on filter flag and reverse list if the order should be descending + sort.Slice(instanceTypes, func(i, j int) bool { + firstType := instanceTypes[i] + secondType := instanceTypes[j] + + // Determine which value to sort by. + // Handle nil values by making non nil values always less than the nil values. That way the + // nil values can be bubbled up to the end of the list. + switch *sortFilterFlag { + case ODPriceSortFlag: + if firstType.OndemandPricePerHour == nil { + return false + } else if secondType.OndemandPricePerHour == nil { + return true + } + + if shouldReverse { + return *firstType.OndemandPricePerHour > *secondType.OndemandPricePerHour + } else { + return *firstType.OndemandPricePerHour <= *secondType.OndemandPricePerHour + } + case SpotPriceSortFlag: + if firstType.SpotPrice == nil { + return false + } else if secondType.SpotPrice == nil { + return true + } + + if shouldReverse { + return *firstType.SpotPrice > *secondType.SpotPrice + } else { + return *firstType.SpotPrice <= *secondType.SpotPrice + } + case VcpuSortFlag: + if firstType.VCpuInfo == nil || firstType.VCpuInfo.DefaultVCpus == nil { + return false + } else if secondType.VCpuInfo == nil || secondType.VCpuInfo.DefaultVCpus == nil { + return true + } + + if shouldReverse { + return *firstType.VCpuInfo.DefaultVCpus > *secondType.VCpuInfo.DefaultVCpus + } else { + return *firstType.VCpuInfo.DefaultVCpus <= *secondType.VCpuInfo.DefaultVCpus + } + case MemorySortFlag: + if firstType.MemoryInfo == nil || firstType.MemoryInfo.SizeInMiB == nil { + return false + } else if secondType.MemoryInfo == nil || secondType.MemoryInfo.SizeInMiB == nil { + return true + } + + if shouldReverse { + return *firstType.MemoryInfo.SizeInMiB > *secondType.MemoryInfo.SizeInMiB + } else { + return *firstType.MemoryInfo.SizeInMiB <= *secondType.MemoryInfo.SizeInMiB + } + default: + if firstType.InstanceType == nil { + return false + } else if secondType.InstanceType == nil { + return true + } + + // sort alphanumerically by the instace-type name + if shouldReverse { + return strings.Compare(aws.StringValue(secondType.InstanceType), aws.StringValue(firstType.InstanceType)) <= 0 + } else { + return strings.Compare(aws.StringValue(firstType.InstanceType), aws.StringValue(secondType.InstanceType)) <= 0 + } + } + }) + + return instanceTypes } func (itf Selector) truncateResults(maxResults *int, instanceTypeInfoSlice []*instancetypes.Details) ([]*instancetypes.Details, int) { @@ -241,7 +353,8 @@ func (itf Selector) rawFilter(filters Filters) ([]*instancetypes.Details, error) for it := range instanceTypes { filteredInstanceTypes = append(filteredInstanceTypes, it) } - return sortInstanceTypeInfo(filteredInstanceTypes), nil + + return filteredInstanceTypes, nil } func (itf Selector) prepareFilter(filters Filters, instanceTypeInfo instancetypes.Details, availabilityZones []string, locationInstanceOfferings map[string]string) (*instancetypes.Details, error) { @@ -343,19 +456,6 @@ func (itf Selector) prepareFilter(filters Filters, instanceTypeInfo instancetype return &instanceTypeInfo, nil } -// sortInstanceTypeInfo will sort based on instance type info alpha-numerically -func sortInstanceTypeInfo(instanceTypeInfoSlice []*instancetypes.Details) []*instancetypes.Details { - if len(instanceTypeInfoSlice) < 2 { - return instanceTypeInfoSlice - } - sort.Slice(instanceTypeInfoSlice, func(i, j int) bool { - iInstanceInfo := instanceTypeInfoSlice[i] - jInstanceInfo := instanceTypeInfoSlice[j] - return strings.Compare(aws.StringValue(iInstanceInfo.InstanceType), aws.StringValue(jInstanceInfo.InstanceType)) <= 0 - }) - return instanceTypeInfoSlice -} - // executeFilters accepts a mapping of filter name to filter pairs which are iterated through // to determine if the instance type matches the filter values. func (itf Selector) executeFilters(filterToInstanceSpecMapping map[string]filterPair, instanceType string) (bool, error) { diff --git a/pkg/selector/selector_test.go b/pkg/selector/selector_test.go index a270938..0bd1d66 100644 --- a/pkg/selector/selector_test.go +++ b/pkg/selector/selector_test.go @@ -159,10 +159,11 @@ func TestFilterVerbose(t *testing.T) { filters := selector.Filters{ VCpusRange: &selector.IntRangeFilter{LowerBound: 2, UpperBound: 2}, } - results, err := itf.FilterVerbose(filters) + results, truncatedItems, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 1, "Should only return 1 instance type with 2 vcpus but actually returned "+strconv.Itoa(len(results))) h.Assert(t, *results[0].InstanceType == "t3.micro", "Should return t3.micro, got %s instead", results[0].InstanceType) + h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_NoResults(t *testing.T) { @@ -170,9 +171,10 @@ func TestFilterVerbose_NoResults(t *testing.T) { filters := selector.Filters{ VCpusRange: &selector.IntRangeFilter{LowerBound: 4, UpperBound: 4}, } - results, err := itf.FilterVerbose(filters) + results, truncatedItems, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 0, "Should return 0 instance type with 4 vcpus") + h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_Failure(t *testing.T) { @@ -180,9 +182,10 @@ func TestFilterVerbose_Failure(t *testing.T) { filters := selector.Filters{ VCpusRange: &selector.IntRangeFilter{LowerBound: 4, UpperBound: 4}, } - results, err := itf.FilterVerbose(filters) + results, truncatedItems, err := itf.FilterVerbose(filters) h.Assert(t, results == nil, "Results should be nil") h.Assert(t, err != nil, "An error should be returned") + h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_AZFilteredIn(t *testing.T) { @@ -196,10 +199,11 @@ func TestFilterVerbose_AZFilteredIn(t *testing.T) { VCpusRange: &selector.IntRangeFilter{LowerBound: 2, UpperBound: 2}, AvailabilityZones: &[]string{"us-east-2a"}, } - results, err := itf.FilterVerbose(filters) + results, truncatedItems, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 1, "Should only return 1 instance type with 2 vcpus but actually returned "+strconv.Itoa(len(results))) h.Assert(t, *results[0].InstanceType == "t3.micro", "Should return t3.micro, got %s instead", results[0].InstanceType) + h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_AZFilteredOut(t *testing.T) { @@ -212,9 +216,10 @@ func TestFilterVerbose_AZFilteredOut(t *testing.T) { filters := selector.Filters{ AvailabilityZones: &[]string{"us-east-2a"}, } - results, err := itf.FilterVerbose(filters) + results, truncatedItems, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 0, "Should return 0 instance types in us-east-2a but actually returned "+strconv.Itoa(len(results))) + h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerboseAZ_FilteredErr(t *testing.T) { @@ -223,8 +228,9 @@ func TestFilterVerboseAZ_FilteredErr(t *testing.T) { VCpusRange: &selector.IntRangeFilter{LowerBound: 2, UpperBound: 2}, AvailabilityZones: &[]string{"blah"}, } - _, err := itf.FilterVerbose(filters) + _, truncatedItems, err := itf.FilterVerbose(filters) h.Assert(t, err != nil, "Should error since bad zone was passed in") + h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_Gpus(t *testing.T) { @@ -238,10 +244,11 @@ func TestFilterVerbose_Gpus(t *testing.T) { UpperBound: gpuMemory, }, } - results, err := itf.FilterVerbose(filters) + results, truncatedItems, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 1, "Should only return 1 instance type with 2 vcpus but actually returned "+strconv.Itoa(len(results))) h.Assert(t, *results[0].InstanceType == "p3.16xlarge", "Should return p3.16xlarge, got %s instead", *results[0].InstanceType) + h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilter(t *testing.T) { From 0edb169f962b198cddb159f7dac78345b4493fee Mon Sep 17 00:00:00 2001 From: Rodrigo Okamoto Date: Mon, 20 Jun 2022 13:28:42 -0500 Subject: [PATCH 6/9] brought back FilterWithOutput() in selector.go --- pkg/selector/selector.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/pkg/selector/selector.go b/pkg/selector/selector.go index 4b55535..9f9d431 100644 --- a/pkg/selector/selector.go +++ b/pkg/selector/selector.go @@ -145,12 +145,8 @@ func (itf Selector) Save() error { // matching the criteria within Filters and returns a simple list of instance type strings func (itf Selector) Filter(filters Filters) ([]string, error) { outputFn := InstanceTypesOutputFn(outputs.SimpleInstanceTypeOutput) - instaceTypeDetails, _, err := itf.FilterVerbose(filters) - var formatedOutput []string = nil - if err == nil { - formatedOutput = outputFn(instaceTypeDetails) - } - return formatedOutput, err + output, _, err := itf.FilterWithOutput(filters, outputFn) + return output, err } // FilterVerbose accepts a Filters struct which is used to select the available instance types @@ -165,6 +161,18 @@ func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, in return instanceTypeInfoSlice, numOfItemsTruncated, nil } +// FilterWithOutput accepts a Filters struct which is used to select the available instance types +// matching the criteria within Filters and returns a list of strings based on the custom outputFn +func (itf Selector) FilterWithOutput(filters Filters, outputFn InstanceTypesOutput) ([]string, int, error) { + instanceTypeInfoSlice, err := itf.rawFilter(filters) + if err != nil { + return nil, 0, err + } + instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) + output := outputFn.Output(instanceTypeInfoSlice) + return output, numOfItemsTruncated, nil +} + // FilterAndSort accepts a Filters struct, which is used to select the available instance types // matching the criteria within Filters, as well as a sortFilterFlag and a sortDirectionsFlag, which are used // to sort the instances, and returns a list of instanceTypeInfo along with the number of truncated items. From d0a6e4c6a90b2adc445129899d33c646f79b53c6 Mon Sep 17 00:00:00 2001 From: Rodrigo Okamoto Date: Thu, 23 Jun 2022 13:12:20 -0500 Subject: [PATCH 7/9] working implementation of refactoring solution --- cmd/examples/example1.go | 38 ++++++- cmd/main.go | 60 ++++------ pkg/selector/selector.go | 238 ++++++++++++++++++++++++--------------- pkg/selector/types.go | 3 - 4 files changed, 203 insertions(+), 136 deletions(-) diff --git a/cmd/examples/example1.go b/cmd/examples/example1.go index 31b44b0..94eb714 100644 --- a/cmd/examples/example1.go +++ b/cmd/examples/example1.go @@ -46,12 +46,42 @@ func main() { CPUArchitecture: &cpuArch, } - // Pass the Filter struct to the Filter function of your selector instance - instanceTypesSlice, err := instanceSelector.Filter(filters) + // TODO: ask Austin if all of the 3 interfaces should be used in this example + // // Pass the Filter struct to the Filter function of your selector instance + // instanceTypesSlice, err := instanceSelector.Filter(filters) + // if err != nil { + // fmt.Printf("Oh no, there was an error :( %v", err) + // return + // } + + // Pass the Filter struct to the GetFilteredInstanceTypes function of your + // selector instance to get a list of filtered instance types and their details + instanceTypesSlice, err := instanceSelector.GetFilteredInstanceTypes(filters) + if err != nil { + fmt.Printf("Oh no, there was an error getting instance types: %v", err) + return + } + + // Pass in the list of instance type details to the SortInstanceTypes if you + // wish to sort the instances based on set filters. + sortFilter := selector.SpotPriceSortFlag + sortDirection := selector.SortAscendingFlag + instanceTypesSlice, err = instanceSelector.SortInstanceTypes(instanceTypesSlice, &sortFilter, &sortDirection) if err != nil { - fmt.Printf("Oh no, there was an error :( %v", err) + fmt.Printf("Oh no, there was an error filtering instance types: %v", err) return } + + // Pass in your list of instance type details to the OuputInstanceTypes function + // in order to format the instance types as a printable string. + outputFormat := selector.SimpleOutput + maxResults := 100 + instanceTypesStrings, _, err := instanceSelector.OutputInstanceTypes(instanceTypesSlice, maxResults, &outputFormat) + if err != nil { + fmt.Printf("Oh no, there was an error outputting instance types: %v", err) + return + } + // Print the returned instance types slice - fmt.Println(instanceTypesSlice) + fmt.Println(instanceTypesStrings) } diff --git a/cmd/main.go b/cmd/main.go index fb3a10c..10c4d1d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -26,7 +26,6 @@ import ( commandline "github.com/aws/amazon-ec2-instance-selector/v2/pkg/cli" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/env" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector" - "github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector/outputs" "github.com/aws/aws-sdk-go/aws/session" homedir "github.com/mitchellh/go-homedir" "github.com/spf13/cobra" @@ -41,10 +40,6 @@ const ( defaultProfile = "default" awsConfigFile = "~/.aws/config" spotPricingDaysBack = 30 - - tableOutput = "table" - tableWideOutput = "table-wide" - oneLine = "one-line" ) // Filter Flag Constants @@ -138,11 +133,10 @@ Full docs can be found at github.com/aws/amazon-` + binName cli := commandline.New(binName, shortUsage, longUsage, examples, runFunc) cliOutputTypes := []string{ - tableOutput, - tableWideOutput, - oneLine, + selector.TableOutput, + selector.TableWideOutput, + selector.OneLineOutput, } - resultsOutputFn := outputs.SimpleInstanceTypeOutput cliSortCriteria := []string{ selector.ODPriceSortFlag, @@ -255,7 +249,7 @@ Full docs can be found at github.com/aws/amazon-` + binName } registerShutdown(shutdown) outputFlag := cli.StringMe(flags[output]) - if outputFlag != nil && *outputFlag == tableWideOutput { + if outputFlag != nil && *outputFlag == selector.TableWideOutput { // If output type is `table-wide`, simply print both prices for better comparison, // even if the actual filter is applied on any one of those based on usage class // Save time by hydrating all caches in parallel @@ -305,7 +299,6 @@ Full docs can be found at github.com/aws/amazon-` + binName Region: cli.StringMe(flags[region]), AvailabilityZones: cli.StringSliceMe(flags[availabilityZones]), CurrentGeneration: cli.BoolMe(flags[currentGeneration]), - MaxResults: cli.IntMe(flags[maxResults]), NetworkInterfaces: cli.IntRangeMe(flags[networkInterfaces]), NetworkPerformance: cli.IntRangeMe(flags[networkPerformance]), NetworkEncryption: cli.BoolMe(flags[networkEncryption]), @@ -331,7 +324,7 @@ Full docs can be found at github.com/aws/amazon-` + binName } if flags[verbose] != nil { - resultsOutputFn = outputs.VerboseInstanceTypeOutput + outputFlag = cli.StringMe(selector.VerboseOutput) transformedFilters, err := instanceSelector.AggregateFilterTransform(filters) if err != nil { fmt.Printf("An error occurred while transforming the aggregate filters") @@ -355,26 +348,36 @@ Full docs can be found at github.com/aws/amazon-` + binName } } - outputFn := getOutputFn(outputFlag, selector.InstanceTypesOutputFn(resultsOutputFn)) + // get filtered instance types + instanceTypeDetails, err := instanceSelector.GetFilteredInstanceTypes(filters) + if err != nil { + fmt.Printf("An error occurred when filtering instance types: %v", err) + os.Exit(1) + } - // get instance types based on filters and sort them based on sorting flags + // sort instance types sortFilterFlag := cli.StringMe(flags[sortFilter]) sortDirectionFlag := cli.StringMe(flags[sortDirection]) - instanceTypesDetails, itemsTruncated, err := instanceSelector.FilterAndSort(filters, sortFilterFlag, sortDirectionFlag) + instanceTypeDetails, err = instanceSelector.SortInstanceTypes(instanceTypeDetails, sortFilterFlag, sortDirectionFlag) if err != nil { - fmt.Printf("An error occurred when filtering instance types: %v", err) + fmt.Printf("An error occurred when sorting instance types: %v", err) os.Exit(1) } - if len(instanceTypesDetails) == 0 { + + // format instance types as a string + maxOutputResults := cli.IntMe(flags[maxResults]) + instanceTypesString, itemsTruncated, err := instanceSelector.OutputInstanceTypes(instanceTypeDetails, *maxOutputResults, outputFlag) + if err != nil { + fmt.Printf("An error occured ouputting instance types: %v", err) + os.Exit(1) + } + if len(instanceTypesString) == 0 { log.Println("The criteria was too narrow and returned no valid instance types. Consider broadening your criteria so that more instance types are returned.") os.Exit(1) } - // format output - instanceTypes := outputFn(instanceTypesDetails) - // print output - for _, instanceType := range instanceTypes { + for _, instanceType := range instanceTypesString { fmt.Println(instanceType) } @@ -423,21 +426,6 @@ func hydrateCaches(instanceSelector selector.Selector) (errs error) { return errs } -func getOutputFn(outputFlag *string, currentFn selector.InstanceTypesOutputFn) selector.InstanceTypesOutputFn { - outputFn := selector.InstanceTypesOutputFn(currentFn) - if outputFlag != nil { - switch *outputFlag { - case tableWideOutput: - return selector.InstanceTypesOutputFn(outputs.TableOutputWide) - case tableOutput: - return selector.InstanceTypesOutputFn(outputs.TableOutputShort) - case oneLine: - return selector.InstanceTypesOutputFn(outputs.OneLineOutput) - } - } - return outputFn -} - func getRegionAndProfileAWSSession(regionName *string, profileName *string) (*session.Session, error) { sessOpts := session.Options{SharedConfigState: session.SharedConfigEnable} if regionName != nil { diff --git a/pkg/selector/selector.go b/pkg/selector/selector.go index 9f9d431..3f53abe 100644 --- a/pkg/selector/selector.go +++ b/pkg/selector/selector.go @@ -100,7 +100,7 @@ const ( pricePerHour = "pricePerHour" - // Sorting costants + // Sorting constants ODPriceSortFlag = "on-demand-price" SpotPriceSortFlag = "spot-price" @@ -110,6 +110,14 @@ const ( SortAscendingFlag = "ascending" SortDescendingFlag = "descending" + + // Output constants + + TableOutput = "table" + TableWideOutput = "table-wide" + OneLineOutput = "one-line" + SimpleOutput = "simple" + VerboseOutput = "verbose" ) // New creates an instance of Selector provided an aws session @@ -141,71 +149,134 @@ func (itf Selector) Save() error { return multierr.Append(itf.EC2Pricing.Save(), itf.InstanceTypesProvider.Save()) } +// TODO: remove these commented out methods // Filter accepts a Filters struct which is used to select the available instance types // matching the criteria within Filters and returns a simple list of instance type strings -func (itf Selector) Filter(filters Filters) ([]string, error) { - outputFn := InstanceTypesOutputFn(outputs.SimpleInstanceTypeOutput) - output, _, err := itf.FilterWithOutput(filters, outputFn) - return output, err -} +// func (itf Selector) Filter(filters Filters) ([]string, error) { +// outputFn := InstanceTypesOutputFn(outputs.SimpleInstanceTypeOutput) +// output, _, err := itf.FilterWithOutput(filters, outputFn) +// return output, err +// } // FilterVerbose accepts a Filters struct which is used to select the available instance types // matching the criteria within Filters and returns a list instanceTypeInfo along with the number // of truncated items. -func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, int, error) { - instanceTypeInfoSlice, err := itf.rawFilter(filters) - if err != nil { - return nil, 0, err - } - instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) - return instanceTypeInfoSlice, numOfItemsTruncated, nil -} +// func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, int, error) { +// instanceTypeInfoSlice, err := itf.rawFilter(filters) +// if err != nil { +// return nil, 0, err +// } +// instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) +// return instanceTypeInfoSlice, numOfItemsTruncated, nil +// } // FilterWithOutput accepts a Filters struct which is used to select the available instance types // matching the criteria within Filters and returns a list of strings based on the custom outputFn -func (itf Selector) FilterWithOutput(filters Filters, outputFn InstanceTypesOutput) ([]string, int, error) { - instanceTypeInfoSlice, err := itf.rawFilter(filters) - if err != nil { - return nil, 0, err - } - instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) - output := outputFn.Output(instanceTypeInfoSlice) - return output, numOfItemsTruncated, nil -} +// func (itf Selector) FilterWithOutput(filters Filters, outputFn InstanceTypesOutput) ([]string, int, error) { +// instanceTypeInfoSlice, err := itf.rawFilter(filters) +// if err != nil { +// return nil, 0, err +// } +// instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) +// output := outputFn.Output(instanceTypeInfoSlice) +// return output, numOfItemsTruncated, nil +// } // FilterAndSort accepts a Filters struct, which is used to select the available instance types // matching the criteria within Filters, as well as a sortFilterFlag and a sortDirectionsFlag, which are used // to sort the instances, and returns a list of instanceTypeInfo along with the number of truncated items. // Acceptable sorting filters: on-demand-price, spot-price, vcpu, memory, instance-type-name. // Acceptable sorting directions: ascending, descending. -func (itf Selector) FilterAndSort(filters Filters, sortFilterFlag *string, sortDirectionFlag *string) ([]*instancetypes.Details, int, error) { - // get instance types based on filter - instanceTypeInfoSlice, err := itf.rawFilter(filters) +// func (itf Selector) FilterAndSort(filters Filters, sortFilterFlag *string, sortDirectionFlag *string) ([]*instancetypes.Details, int, error) { +// // get instance types based on filter +// instanceTypeInfoSlice, err := itf.rawFilter(filters) +// if err != nil { +// return nil, 0, err +// } + +// // sort results +// instanceTypeInfoSlice = sortInstanceTypes(sortFilterFlag, sortDirectionFlag, instanceTypeInfoSlice) + +// // truncate results +// instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) + +// return instanceTypeInfoSlice, numOfItemsTruncated, nil +// } + +// GetFilteredInstanceTypes accepts a Filters struct which is used to select the available instance types +// matching the criteria within Filters and returns the detailed specs of matching instance types +func (itf Selector) GetFilteredInstanceTypes(filters Filters) ([]*instancetypes.Details, error) { + filters, err := itf.AggregateFilterTransform(filters) if err != nil { - return nil, 0, err + return nil, err } + var locations, availabilityZones []string - // sort results - instanceTypeInfoSlice = sortInstanceTypes(sortFilterFlag, sortDirectionFlag, instanceTypeInfoSlice) + if filters.CPUArchitecture != nil && *filters.CPUArchitecture == cpuArchitectureAMD64 { + *filters.CPUArchitecture = cpuArchitectureX8664 + } + if filters.VirtualizationType != nil && *filters.VirtualizationType == virtualizationTypePV { + *filters.VirtualizationType = virtualizationTypeParaVirtual + } + if filters.AvailabilityZones != nil { + availabilityZones = *filters.AvailabilityZones + locations = *filters.AvailabilityZones + } else if filters.Region != nil { + locations = []string{*filters.Region} + } + locationInstanceOfferings, err := itf.RetrieveInstanceTypesSupportedInLocations(locations) + if err != nil { + return nil, err + } - // truncate results - instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) + instanceTypeDetails, err := itf.InstanceTypesProvider.Get(nil) + if err != nil { + return nil, err + } + filteredInstanceTypes := []*instancetypes.Details{} + var wg sync.WaitGroup + instanceTypes := make(chan *instancetypes.Details, len(instanceTypeDetails)) + for _, instanceTypeInfo := range instanceTypeDetails { + wg.Add(1) + go func(instanceTypeInfo instancetypes.Details) { + defer wg.Done() + it, err := itf.prepareFilter(filters, instanceTypeInfo, availabilityZones, locationInstanceOfferings) + if err != nil { + log.Println(err) + } + if it != nil { + instanceTypes <- it + } + }(*instanceTypeInfo) + } + wg.Wait() + close(instanceTypes) + for it := range instanceTypes { + filteredInstanceTypes = append(filteredInstanceTypes, it) + } - return instanceTypeInfoSlice, numOfItemsTruncated, nil + return filteredInstanceTypes, nil } // sorts the given list of instance type details based on the sorting filter flag and the sort direction flag -func sortInstanceTypes(sortFilterFlag *string, sortDirectionFlag *string, instanceTypes []*instancetypes.Details) []*instancetypes.Details { +// TODO: better method comment and regular comments +func (itf Selector) SortInstanceTypes(instanceTypes []*instancetypes.Details, sortFilterFlag *string, sortDirectionFlag *string) ([]*instancetypes.Details, error) { if len(instanceTypes) <= 1 { - return instanceTypes + return instanceTypes, nil } // by default, sorted in ascending order. - shouldReverse := false + var shouldReverse bool if sortDirectionFlag != nil && *sortDirectionFlag == SortDescendingFlag { shouldReverse = true + } else if sortDirectionFlag != nil && *sortDirectionFlag == SortAscendingFlag { + shouldReverse = false + } else { + return nil, fmt.Errorf("invalid sort direction flag: %s", *sortDirectionFlag) } + isSortingFlagValid := true + // sort instance types based on filter flag and reverse list if the order should be descending sort.Slice(instanceTypes, func(i, j int) bool { firstType := instanceTypes[i] @@ -263,23 +334,59 @@ func sortInstanceTypes(sortFilterFlag *string, sortDirectionFlag *string, instan } else { return *firstType.MemoryInfo.SizeInMiB <= *secondType.MemoryInfo.SizeInMiB } - default: + case NameSortFlag: if firstType.InstanceType == nil { return false } else if secondType.InstanceType == nil { return true } - // sort alphanumerically by the instace-type name if shouldReverse { return strings.Compare(aws.StringValue(secondType.InstanceType), aws.StringValue(firstType.InstanceType)) <= 0 } else { return strings.Compare(aws.StringValue(firstType.InstanceType), aws.StringValue(secondType.InstanceType)) <= 0 } + default: + isSortingFlagValid = false + return true } }) - return instanceTypes + if !isSortingFlagValid { + return nil, fmt.Errorf("invalid sort filter flag: %s", *sortFilterFlag) + } + return instanceTypes, nil +} + +// TODO: add method comment and comment inside method +func (itf Selector) OutputInstanceTypes(instanceTypes []*instancetypes.Details, maxResults int, outputFlag *string) ([]string, int, error) { + if outputFlag == nil { + // TODO: check to see how string errors should be formatted + return nil, 0, fmt.Errorf("output flag is nil") + } + + instanceTypes, numOfItemsTruncated := itf.truncateResults(&maxResults, instanceTypes) + _ = numOfItemsTruncated + + // TODO: switch statement to see which output to use + var outputString []string + switch *outputFlag { + case SimpleOutput: + outputString = outputs.SimpleInstanceTypeOutput(instanceTypes) + case OneLineOutput: + outputString = outputs.OneLineOutput(instanceTypes) + case TableOutput: + outputString = outputs.TableOutputShort(instanceTypes) + case TableWideOutput: + outputString = outputs.TableOutputWide(instanceTypes) + case VerboseOutput: + outputString = outputs.VerboseInstanceTypeOutput(instanceTypes) + default: + // TODO: check to see how string errors should be formatted + return nil, 0, fmt.Errorf("invalid output flag") + } + + return outputString, numOfItemsTruncated, nil } func (itf Selector) truncateResults(maxResults *int, instanceTypeInfoSlice []*instancetypes.Details) ([]*instancetypes.Details, int) { @@ -310,61 +417,6 @@ func (itf Selector) AggregateFilterTransform(filters Filters) (Filters, error) { return filters, nil } -// rawFilter accepts a Filters struct which is used to select the available instance types -// matching the criteria within Filters and returns the detailed specs of matching instance types -func (itf Selector) rawFilter(filters Filters) ([]*instancetypes.Details, error) { - filters, err := itf.AggregateFilterTransform(filters) - if err != nil { - return nil, err - } - var locations, availabilityZones []string - - if filters.CPUArchitecture != nil && *filters.CPUArchitecture == cpuArchitectureAMD64 { - *filters.CPUArchitecture = cpuArchitectureX8664 - } - if filters.VirtualizationType != nil && *filters.VirtualizationType == virtualizationTypePV { - *filters.VirtualizationType = virtualizationTypeParaVirtual - } - if filters.AvailabilityZones != nil { - availabilityZones = *filters.AvailabilityZones - locations = *filters.AvailabilityZones - } else if filters.Region != nil { - locations = []string{*filters.Region} - } - locationInstanceOfferings, err := itf.RetrieveInstanceTypesSupportedInLocations(locations) - if err != nil { - return nil, err - } - - instanceTypeDetails, err := itf.InstanceTypesProvider.Get(nil) - if err != nil { - return nil, err - } - filteredInstanceTypes := []*instancetypes.Details{} - var wg sync.WaitGroup - instanceTypes := make(chan *instancetypes.Details, len(instanceTypeDetails)) - for _, instanceTypeInfo := range instanceTypeDetails { - wg.Add(1) - go func(instanceTypeInfo instancetypes.Details) { - defer wg.Done() - it, err := itf.prepareFilter(filters, instanceTypeInfo, availabilityZones, locationInstanceOfferings) - if err != nil { - log.Println(err) - } - if it != nil { - instanceTypes <- it - } - }(*instanceTypeInfo) - } - wg.Wait() - close(instanceTypes) - for it := range instanceTypes { - filteredInstanceTypes = append(filteredInstanceTypes, it) - } - - return filteredInstanceTypes, nil -} - func (itf Selector) prepareFilter(filters Filters, instanceTypeInfo instancetypes.Details, availabilityZones []string, locationInstanceOfferings map[string]string) (*instancetypes.Details, error) { instanceTypeName := *instanceTypeInfo.InstanceType isFpga := instanceTypeInfo.FpgaInfo != nil diff --git a/pkg/selector/types.go b/pkg/selector/types.go index 95a634d..b5773fb 100644 --- a/pkg/selector/types.go +++ b/pkg/selector/types.go @@ -169,9 +169,6 @@ type Filters struct { // Possibly values are: xen or nitro Hypervisor *string - // MaxResults is the maximum number of instance types to return that match the filter criteria - MaxResults *int - // MemoryRange filter is a range of acceptable DRAM memory in Gibibytes (GiB) for the instance type MemoryRange *ByteQuantityRangeFilter From d0cbd21416633012f5f40ad5541e169a8aff8389 Mon Sep 17 00:00:00 2001 From: Rodrigo Okamoto Date: Fri, 24 Jun 2022 12:44:12 -0500 Subject: [PATCH 8/9] Deprecated the filter methods in selector.go --- cmd/examples/example1.go | 38 +---- cmd/main.go | 94 +++++------ pkg/selector/selector.go | 303 +++++++++------------------------- pkg/selector/selector_test.go | 21 +-- pkg/selector/types.go | 3 + 5 files changed, 129 insertions(+), 330 deletions(-) diff --git a/cmd/examples/example1.go b/cmd/examples/example1.go index 94eb714..31b44b0 100644 --- a/cmd/examples/example1.go +++ b/cmd/examples/example1.go @@ -46,42 +46,12 @@ func main() { CPUArchitecture: &cpuArch, } - // TODO: ask Austin if all of the 3 interfaces should be used in this example - // // Pass the Filter struct to the Filter function of your selector instance - // instanceTypesSlice, err := instanceSelector.Filter(filters) - // if err != nil { - // fmt.Printf("Oh no, there was an error :( %v", err) - // return - // } - - // Pass the Filter struct to the GetFilteredInstanceTypes function of your - // selector instance to get a list of filtered instance types and their details - instanceTypesSlice, err := instanceSelector.GetFilteredInstanceTypes(filters) - if err != nil { - fmt.Printf("Oh no, there was an error getting instance types: %v", err) - return - } - - // Pass in the list of instance type details to the SortInstanceTypes if you - // wish to sort the instances based on set filters. - sortFilter := selector.SpotPriceSortFlag - sortDirection := selector.SortAscendingFlag - instanceTypesSlice, err = instanceSelector.SortInstanceTypes(instanceTypesSlice, &sortFilter, &sortDirection) + // Pass the Filter struct to the Filter function of your selector instance + instanceTypesSlice, err := instanceSelector.Filter(filters) if err != nil { - fmt.Printf("Oh no, there was an error filtering instance types: %v", err) + fmt.Printf("Oh no, there was an error :( %v", err) return } - - // Pass in your list of instance type details to the OuputInstanceTypes function - // in order to format the instance types as a printable string. - outputFormat := selector.SimpleOutput - maxResults := 100 - instanceTypesStrings, _, err := instanceSelector.OutputInstanceTypes(instanceTypesSlice, maxResults, &outputFormat) - if err != nil { - fmt.Printf("Oh no, there was an error outputting instance types: %v", err) - return - } - // Print the returned instance types slice - fmt.Println(instanceTypesStrings) + fmt.Println(instanceTypesSlice) } diff --git a/cmd/main.go b/cmd/main.go index 10c4d1d..f57e56d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -26,6 +26,7 @@ import ( commandline "github.com/aws/amazon-ec2-instance-selector/v2/pkg/cli" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/env" "github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector" + "github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector/outputs" "github.com/aws/aws-sdk-go/aws/session" homedir "github.com/mitchellh/go-homedir" "github.com/spf13/cobra" @@ -40,6 +41,10 @@ const ( defaultProfile = "default" awsConfigFile = "~/.aws/config" spotPricingDaysBack = 30 + + tableOutput = "table" + tableWideOutput = "table-wide" + oneLine = "one-line" ) // Filter Flag Constants @@ -98,17 +103,15 @@ const ( // Configuration Flag Constants const ( - maxResults = "max-results" - profile = "profile" - help = "help" - verbose = "verbose" - version = "version" - region = "region" - output = "output" - cacheTTL = "cache-ttl" - cacheDir = "cache-dir" - sortDirection = "sort-direction" - sortFilter = "sort-filter" + maxResults = "max-results" + profile = "profile" + help = "help" + verbose = "verbose" + version = "version" + region = "region" + output = "output" + cacheTTL = "cache-ttl" + cacheDir = "cache-dir" ) var ( @@ -133,23 +136,11 @@ Full docs can be found at github.com/aws/amazon-` + binName cli := commandline.New(binName, shortUsage, longUsage, examples, runFunc) cliOutputTypes := []string{ - selector.TableOutput, - selector.TableWideOutput, - selector.OneLineOutput, - } - - cliSortCriteria := []string{ - selector.ODPriceSortFlag, - selector.SpotPriceSortFlag, - selector.VcpuSortFlag, - selector.MemorySortFlag, - selector.NameSortFlag, - } - - cliSortDirections := []string{ - selector.SortAscendingFlag, - selector.SortDescendingFlag, + tableOutput, + tableWideOutput, + oneLine, } + resultsOutputFn := outputs.SimpleInstanceTypeOutput // Registers flags with specific input types from the cli pkg // Filter Flags - These will be grouped at the top of the help flags @@ -215,8 +206,6 @@ Full docs can be found at github.com/aws/amazon-` + binName cli.ConfigBoolFlag(verbose, cli.StringMe("v"), nil, "Verbose - will print out full instance specs") cli.ConfigBoolFlag(help, cli.StringMe("h"), nil, "Help") cli.ConfigBoolFlag(version, nil, nil, "Prints CLI version") - cli.ConfigStringOptionsFlag(sortDirection, nil, cli.StringMe(selector.SortAscendingFlag), fmt.Sprintf("Specify the direction to sort in (%s)", strings.Join(cliSortDirections, ", ")), cliSortDirections) - cli.ConfigStringOptionsFlag(sortFilter, nil, cli.StringMe(selector.NameSortFlag), fmt.Sprintf("Specify the field to sort by (%s)", strings.Join(cliSortCriteria, ", ")), cliSortCriteria) // Parses the user input with the registered flags and runs type specific validation on the user input flags, err := cli.ParseAndValidateFlags() @@ -249,7 +238,7 @@ Full docs can be found at github.com/aws/amazon-` + binName } registerShutdown(shutdown) outputFlag := cli.StringMe(flags[output]) - if outputFlag != nil && *outputFlag == selector.TableWideOutput { + if outputFlag != nil && *outputFlag == tableWideOutput { // If output type is `table-wide`, simply print both prices for better comparison, // even if the actual filter is applied on any one of those based on usage class // Save time by hydrating all caches in parallel @@ -299,6 +288,7 @@ Full docs can be found at github.com/aws/amazon-` + binName Region: cli.StringMe(flags[region]), AvailabilityZones: cli.StringSliceMe(flags[availabilityZones]), CurrentGeneration: cli.BoolMe(flags[currentGeneration]), + MaxResults: cli.IntMe(flags[maxResults]), NetworkInterfaces: cli.IntRangeMe(flags[networkInterfaces]), NetworkPerformance: cli.IntRangeMe(flags[networkPerformance]), NetworkEncryption: cli.BoolMe(flags[networkEncryption]), @@ -324,7 +314,7 @@ Full docs can be found at github.com/aws/amazon-` + binName } if flags[verbose] != nil { - outputFlag = cli.StringMe(selector.VerboseOutput) + resultsOutputFn = outputs.VerboseInstanceTypeOutput transformedFilters, err := instanceSelector.AggregateFilterTransform(filters) if err != nil { fmt.Printf("An error occurred while transforming the aggregate filters") @@ -348,36 +338,19 @@ Full docs can be found at github.com/aws/amazon-` + binName } } - // get filtered instance types - instanceTypeDetails, err := instanceSelector.GetFilteredInstanceTypes(filters) - if err != nil { - fmt.Printf("An error occurred when filtering instance types: %v", err) - os.Exit(1) - } - - // sort instance types - sortFilterFlag := cli.StringMe(flags[sortFilter]) - sortDirectionFlag := cli.StringMe(flags[sortDirection]) - instanceTypeDetails, err = instanceSelector.SortInstanceTypes(instanceTypeDetails, sortFilterFlag, sortDirectionFlag) - if err != nil { - fmt.Printf("An error occurred when sorting instance types: %v", err) - os.Exit(1) - } + outputFn := getOutputFn(outputFlag, selector.InstanceTypesOutputFn(resultsOutputFn)) - // format instance types as a string - maxOutputResults := cli.IntMe(flags[maxResults]) - instanceTypesString, itemsTruncated, err := instanceSelector.OutputInstanceTypes(instanceTypeDetails, *maxOutputResults, outputFlag) + instanceTypes, itemsTruncated, err := instanceSelector.FilterWithOutput(filters, outputFn) if err != nil { - fmt.Printf("An error occured ouputting instance types: %v", err) + fmt.Printf("An error occurred when filtering instance types: %v", err) os.Exit(1) } - if len(instanceTypesString) == 0 { + if len(instanceTypes) == 0 { log.Println("The criteria was too narrow and returned no valid instance types. Consider broadening your criteria so that more instance types are returned.") os.Exit(1) } - // print output - for _, instanceType := range instanceTypesString { + for _, instanceType := range instanceTypes { fmt.Println(instanceType) } @@ -426,6 +399,21 @@ func hydrateCaches(instanceSelector selector.Selector) (errs error) { return errs } +func getOutputFn(outputFlag *string, currentFn selector.InstanceTypesOutputFn) selector.InstanceTypesOutputFn { + outputFn := selector.InstanceTypesOutputFn(currentFn) + if outputFlag != nil { + switch *outputFlag { + case tableWideOutput: + return selector.InstanceTypesOutputFn(outputs.TableOutputWide) + case tableOutput: + return selector.InstanceTypesOutputFn(outputs.TableOutputShort) + case oneLine: + return selector.InstanceTypesOutputFn(outputs.OneLineOutput) + } + } + return outputFn +} + func getRegionAndProfileAWSSession(regionName *string, profileName *string) (*session.Session, error) { sessOpts := session.Options{SharedConfigState: session.SharedConfigEnable} if regionName != nil { diff --git a/pkg/selector/selector.go b/pkg/selector/selector.go index 3f53abe..e289093 100644 --- a/pkg/selector/selector.go +++ b/pkg/selector/selector.go @@ -99,25 +99,6 @@ const ( virtualizationTypePV = "pv" pricePerHour = "pricePerHour" - - // Sorting constants - - ODPriceSortFlag = "on-demand-price" - SpotPriceSortFlag = "spot-price" - VcpuSortFlag = "vcpu" - MemorySortFlag = "memory" - NameSortFlag = "instance-type-name" - - SortAscendingFlag = "ascending" - SortDescendingFlag = "descending" - - // Output constants - - TableOutput = "table" - TableWideOutput = "table-wide" - OneLineOutput = "one-line" - SimpleOutput = "simple" - VerboseOutput = "verbose" ) // New creates an instance of Selector provided an aws session @@ -149,63 +130,74 @@ func (itf Selector) Save() error { return multierr.Append(itf.EC2Pricing.Save(), itf.InstanceTypesProvider.Save()) } -// TODO: remove these commented out methods // Filter accepts a Filters struct which is used to select the available instance types // matching the criteria within Filters and returns a simple list of instance type strings -// func (itf Selector) Filter(filters Filters) ([]string, error) { -// outputFn := InstanceTypesOutputFn(outputs.SimpleInstanceTypeOutput) -// output, _, err := itf.FilterWithOutput(filters, outputFn) -// return output, err -// } +// +// Deprecated: This function will no longer exist in the next major version. +func (itf Selector) Filter(filters Filters) ([]string, error) { + outputFn := InstanceTypesOutputFn(outputs.SimpleInstanceTypeOutput) + output, _, err := itf.FilterWithOutput(filters, outputFn) + return output, err +} // FilterVerbose accepts a Filters struct which is used to select the available instance types -// matching the criteria within Filters and returns a list instanceTypeInfo along with the number -// of truncated items. -// func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, int, error) { -// instanceTypeInfoSlice, err := itf.rawFilter(filters) -// if err != nil { -// return nil, 0, err -// } -// instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) -// return instanceTypeInfoSlice, numOfItemsTruncated, nil -// } +// matching the criteria within Filters and returns a list instanceTypeInfo +// +// Deprecated: This function will no longer exist in the next major version. +func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, error) { + instanceTypeInfoSlice, err := itf.rawFilter(filters) + if err != nil { + return nil, err + } + instanceTypeInfoSlice, _ = itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) + return instanceTypeInfoSlice, nil +} // FilterWithOutput accepts a Filters struct which is used to select the available instance types // matching the criteria within Filters and returns a list of strings based on the custom outputFn -// func (itf Selector) FilterWithOutput(filters Filters, outputFn InstanceTypesOutput) ([]string, int, error) { -// instanceTypeInfoSlice, err := itf.rawFilter(filters) -// if err != nil { -// return nil, 0, err -// } -// instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) -// output := outputFn.Output(instanceTypeInfoSlice) -// return output, numOfItemsTruncated, nil -// } - -// FilterAndSort accepts a Filters struct, which is used to select the available instance types -// matching the criteria within Filters, as well as a sortFilterFlag and a sortDirectionsFlag, which are used -// to sort the instances, and returns a list of instanceTypeInfo along with the number of truncated items. -// Acceptable sorting filters: on-demand-price, spot-price, vcpu, memory, instance-type-name. -// Acceptable sorting directions: ascending, descending. -// func (itf Selector) FilterAndSort(filters Filters, sortFilterFlag *string, sortDirectionFlag *string) ([]*instancetypes.Details, int, error) { -// // get instance types based on filter -// instanceTypeInfoSlice, err := itf.rawFilter(filters) -// if err != nil { -// return nil, 0, err -// } - -// // sort results -// instanceTypeInfoSlice = sortInstanceTypes(sortFilterFlag, sortDirectionFlag, instanceTypeInfoSlice) - -// // truncate results -// instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) - -// return instanceTypeInfoSlice, numOfItemsTruncated, nil -// } - -// GetFilteredInstanceTypes accepts a Filters struct which is used to select the available instance types +// +// Deprecated: This function will no longer exist in the next major version. +func (itf Selector) FilterWithOutput(filters Filters, outputFn InstanceTypesOutput) ([]string, int, error) { + instanceTypeInfoSlice, err := itf.rawFilter(filters) + if err != nil { + return nil, 0, err + } + instanceTypeInfoSlice, numOfItemsTruncated := itf.truncateResults(filters.MaxResults, instanceTypeInfoSlice) + output := outputFn.Output(instanceTypeInfoSlice) + return output, numOfItemsTruncated, nil +} + +func (itf Selector) truncateResults(maxResults *int, instanceTypeInfoSlice []*instancetypes.Details) ([]*instancetypes.Details, int) { + if maxResults == nil { + return instanceTypeInfoSlice, 0 + } + upperIndex := *maxResults + if *maxResults > len(instanceTypeInfoSlice) { + upperIndex = len(instanceTypeInfoSlice) + } + return instanceTypeInfoSlice[0:upperIndex], len(instanceTypeInfoSlice) - upperIndex +} + +// AggregateFilterTransform takes higher level filters which are used to affect multiple raw filters in an opinionated way. +func (itf Selector) AggregateFilterTransform(filters Filters) (Filters, error) { + transforms := []FiltersTransform{ + TransformFn(itf.TransformBaseInstanceType), + TransformFn(itf.TransformFlexible), + TransformFn(itf.TransformForService), + } + var err error + for _, transform := range transforms { + filters, err = transform.Transform(filters) + if err != nil { + return filters, err + } + } + return filters, nil +} + +// rawFilter accepts a Filters struct which is used to select the available instance types // matching the criteria within Filters and returns the detailed specs of matching instance types -func (itf Selector) GetFilteredInstanceTypes(filters Filters) ([]*instancetypes.Details, error) { +func (itf Selector) rawFilter(filters Filters) ([]*instancetypes.Details, error) { filters, err := itf.AggregateFilterTransform(filters) if err != nil { return nil, err @@ -254,167 +246,7 @@ func (itf Selector) GetFilteredInstanceTypes(filters Filters) ([]*instancetypes. for it := range instanceTypes { filteredInstanceTypes = append(filteredInstanceTypes, it) } - - return filteredInstanceTypes, nil -} - -// sorts the given list of instance type details based on the sorting filter flag and the sort direction flag -// TODO: better method comment and regular comments -func (itf Selector) SortInstanceTypes(instanceTypes []*instancetypes.Details, sortFilterFlag *string, sortDirectionFlag *string) ([]*instancetypes.Details, error) { - if len(instanceTypes) <= 1 { - return instanceTypes, nil - } - - // by default, sorted in ascending order. - var shouldReverse bool - if sortDirectionFlag != nil && *sortDirectionFlag == SortDescendingFlag { - shouldReverse = true - } else if sortDirectionFlag != nil && *sortDirectionFlag == SortAscendingFlag { - shouldReverse = false - } else { - return nil, fmt.Errorf("invalid sort direction flag: %s", *sortDirectionFlag) - } - - isSortingFlagValid := true - - // sort instance types based on filter flag and reverse list if the order should be descending - sort.Slice(instanceTypes, func(i, j int) bool { - firstType := instanceTypes[i] - secondType := instanceTypes[j] - - // Determine which value to sort by. - // Handle nil values by making non nil values always less than the nil values. That way the - // nil values can be bubbled up to the end of the list. - switch *sortFilterFlag { - case ODPriceSortFlag: - if firstType.OndemandPricePerHour == nil { - return false - } else if secondType.OndemandPricePerHour == nil { - return true - } - - if shouldReverse { - return *firstType.OndemandPricePerHour > *secondType.OndemandPricePerHour - } else { - return *firstType.OndemandPricePerHour <= *secondType.OndemandPricePerHour - } - case SpotPriceSortFlag: - if firstType.SpotPrice == nil { - return false - } else if secondType.SpotPrice == nil { - return true - } - - if shouldReverse { - return *firstType.SpotPrice > *secondType.SpotPrice - } else { - return *firstType.SpotPrice <= *secondType.SpotPrice - } - case VcpuSortFlag: - if firstType.VCpuInfo == nil || firstType.VCpuInfo.DefaultVCpus == nil { - return false - } else if secondType.VCpuInfo == nil || secondType.VCpuInfo.DefaultVCpus == nil { - return true - } - - if shouldReverse { - return *firstType.VCpuInfo.DefaultVCpus > *secondType.VCpuInfo.DefaultVCpus - } else { - return *firstType.VCpuInfo.DefaultVCpus <= *secondType.VCpuInfo.DefaultVCpus - } - case MemorySortFlag: - if firstType.MemoryInfo == nil || firstType.MemoryInfo.SizeInMiB == nil { - return false - } else if secondType.MemoryInfo == nil || secondType.MemoryInfo.SizeInMiB == nil { - return true - } - - if shouldReverse { - return *firstType.MemoryInfo.SizeInMiB > *secondType.MemoryInfo.SizeInMiB - } else { - return *firstType.MemoryInfo.SizeInMiB <= *secondType.MemoryInfo.SizeInMiB - } - case NameSortFlag: - if firstType.InstanceType == nil { - return false - } else if secondType.InstanceType == nil { - return true - } - - if shouldReverse { - return strings.Compare(aws.StringValue(secondType.InstanceType), aws.StringValue(firstType.InstanceType)) <= 0 - } else { - return strings.Compare(aws.StringValue(firstType.InstanceType), aws.StringValue(secondType.InstanceType)) <= 0 - } - default: - isSortingFlagValid = false - return true - } - }) - - if !isSortingFlagValid { - return nil, fmt.Errorf("invalid sort filter flag: %s", *sortFilterFlag) - } - return instanceTypes, nil -} - -// TODO: add method comment and comment inside method -func (itf Selector) OutputInstanceTypes(instanceTypes []*instancetypes.Details, maxResults int, outputFlag *string) ([]string, int, error) { - if outputFlag == nil { - // TODO: check to see how string errors should be formatted - return nil, 0, fmt.Errorf("output flag is nil") - } - - instanceTypes, numOfItemsTruncated := itf.truncateResults(&maxResults, instanceTypes) - _ = numOfItemsTruncated - - // TODO: switch statement to see which output to use - var outputString []string - switch *outputFlag { - case SimpleOutput: - outputString = outputs.SimpleInstanceTypeOutput(instanceTypes) - case OneLineOutput: - outputString = outputs.OneLineOutput(instanceTypes) - case TableOutput: - outputString = outputs.TableOutputShort(instanceTypes) - case TableWideOutput: - outputString = outputs.TableOutputWide(instanceTypes) - case VerboseOutput: - outputString = outputs.VerboseInstanceTypeOutput(instanceTypes) - default: - // TODO: check to see how string errors should be formatted - return nil, 0, fmt.Errorf("invalid output flag") - } - - return outputString, numOfItemsTruncated, nil -} - -func (itf Selector) truncateResults(maxResults *int, instanceTypeInfoSlice []*instancetypes.Details) ([]*instancetypes.Details, int) { - if maxResults == nil { - return instanceTypeInfoSlice, 0 - } - upperIndex := *maxResults - if *maxResults > len(instanceTypeInfoSlice) { - upperIndex = len(instanceTypeInfoSlice) - } - return instanceTypeInfoSlice[0:upperIndex], len(instanceTypeInfoSlice) - upperIndex -} - -// AggregateFilterTransform takes higher level filters which are used to affect multiple raw filters in an opinionated way. -func (itf Selector) AggregateFilterTransform(filters Filters) (Filters, error) { - transforms := []FiltersTransform{ - TransformFn(itf.TransformBaseInstanceType), - TransformFn(itf.TransformFlexible), - TransformFn(itf.TransformForService), - } - var err error - for _, transform := range transforms { - filters, err = transform.Transform(filters) - if err != nil { - return filters, err - } - } - return filters, nil + return sortInstanceTypeInfo(filteredInstanceTypes), nil } func (itf Selector) prepareFilter(filters Filters, instanceTypeInfo instancetypes.Details, availabilityZones []string, locationInstanceOfferings map[string]string) (*instancetypes.Details, error) { @@ -516,6 +348,19 @@ func (itf Selector) prepareFilter(filters Filters, instanceTypeInfo instancetype return &instanceTypeInfo, nil } +// sortInstanceTypeInfo will sort based on instance type info alpha-numerically +func sortInstanceTypeInfo(instanceTypeInfoSlice []*instancetypes.Details) []*instancetypes.Details { + if len(instanceTypeInfoSlice) < 2 { + return instanceTypeInfoSlice + } + sort.Slice(instanceTypeInfoSlice, func(i, j int) bool { + iInstanceInfo := instanceTypeInfoSlice[i] + jInstanceInfo := instanceTypeInfoSlice[j] + return strings.Compare(aws.StringValue(iInstanceInfo.InstanceType), aws.StringValue(jInstanceInfo.InstanceType)) <= 0 + }) + return instanceTypeInfoSlice +} + // executeFilters accepts a mapping of filter name to filter pairs which are iterated through // to determine if the instance type matches the filter values. func (itf Selector) executeFilters(filterToInstanceSpecMapping map[string]filterPair, instanceType string) (bool, error) { diff --git a/pkg/selector/selector_test.go b/pkg/selector/selector_test.go index 0bd1d66..a270938 100644 --- a/pkg/selector/selector_test.go +++ b/pkg/selector/selector_test.go @@ -159,11 +159,10 @@ func TestFilterVerbose(t *testing.T) { filters := selector.Filters{ VCpusRange: &selector.IntRangeFilter{LowerBound: 2, UpperBound: 2}, } - results, truncatedItems, err := itf.FilterVerbose(filters) + results, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 1, "Should only return 1 instance type with 2 vcpus but actually returned "+strconv.Itoa(len(results))) h.Assert(t, *results[0].InstanceType == "t3.micro", "Should return t3.micro, got %s instead", results[0].InstanceType) - h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_NoResults(t *testing.T) { @@ -171,10 +170,9 @@ func TestFilterVerbose_NoResults(t *testing.T) { filters := selector.Filters{ VCpusRange: &selector.IntRangeFilter{LowerBound: 4, UpperBound: 4}, } - results, truncatedItems, err := itf.FilterVerbose(filters) + results, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 0, "Should return 0 instance type with 4 vcpus") - h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_Failure(t *testing.T) { @@ -182,10 +180,9 @@ func TestFilterVerbose_Failure(t *testing.T) { filters := selector.Filters{ VCpusRange: &selector.IntRangeFilter{LowerBound: 4, UpperBound: 4}, } - results, truncatedItems, err := itf.FilterVerbose(filters) + results, err := itf.FilterVerbose(filters) h.Assert(t, results == nil, "Results should be nil") h.Assert(t, err != nil, "An error should be returned") - h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_AZFilteredIn(t *testing.T) { @@ -199,11 +196,10 @@ func TestFilterVerbose_AZFilteredIn(t *testing.T) { VCpusRange: &selector.IntRangeFilter{LowerBound: 2, UpperBound: 2}, AvailabilityZones: &[]string{"us-east-2a"}, } - results, truncatedItems, err := itf.FilterVerbose(filters) + results, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 1, "Should only return 1 instance type with 2 vcpus but actually returned "+strconv.Itoa(len(results))) h.Assert(t, *results[0].InstanceType == "t3.micro", "Should return t3.micro, got %s instead", results[0].InstanceType) - h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_AZFilteredOut(t *testing.T) { @@ -216,10 +212,9 @@ func TestFilterVerbose_AZFilteredOut(t *testing.T) { filters := selector.Filters{ AvailabilityZones: &[]string{"us-east-2a"}, } - results, truncatedItems, err := itf.FilterVerbose(filters) + results, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 0, "Should return 0 instance types in us-east-2a but actually returned "+strconv.Itoa(len(results))) - h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerboseAZ_FilteredErr(t *testing.T) { @@ -228,9 +223,8 @@ func TestFilterVerboseAZ_FilteredErr(t *testing.T) { VCpusRange: &selector.IntRangeFilter{LowerBound: 2, UpperBound: 2}, AvailabilityZones: &[]string{"blah"}, } - _, truncatedItems, err := itf.FilterVerbose(filters) + _, err := itf.FilterVerbose(filters) h.Assert(t, err != nil, "Should error since bad zone was passed in") - h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilterVerbose_Gpus(t *testing.T) { @@ -244,11 +238,10 @@ func TestFilterVerbose_Gpus(t *testing.T) { UpperBound: gpuMemory, }, } - results, truncatedItems, err := itf.FilterVerbose(filters) + results, err := itf.FilterVerbose(filters) h.Ok(t, err) h.Assert(t, len(results) == 1, "Should only return 1 instance type with 2 vcpus but actually returned "+strconv.Itoa(len(results))) h.Assert(t, *results[0].InstanceType == "p3.16xlarge", "Should return p3.16xlarge, got %s instead", *results[0].InstanceType) - h.Assert(t, truncatedItems == 0, "No items should be truncated, but "+strconv.Itoa(truncatedItems)+" items were truncated") } func TestFilter(t *testing.T) { diff --git a/pkg/selector/types.go b/pkg/selector/types.go index b5773fb..95a634d 100644 --- a/pkg/selector/types.go +++ b/pkg/selector/types.go @@ -169,6 +169,9 @@ type Filters struct { // Possibly values are: xen or nitro Hypervisor *string + // MaxResults is the maximum number of instance types to return that match the filter criteria + MaxResults *int + // MemoryRange filter is a range of acceptable DRAM memory in Gibibytes (GiB) for the instance type MemoryRange *ByteQuantityRangeFilter From ac511dddd1b21482726c22e116142bac181d2d79 Mon Sep 17 00:00:00 2001 From: Rodrigo Okamoto Date: Fri, 24 Jun 2022 15:36:12 -0500 Subject: [PATCH 9/9] added the names of replacement functions to deprecated comments --- pkg/selector/selector.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/selector/selector.go b/pkg/selector/selector.go index e289093..4088d2d 100644 --- a/pkg/selector/selector.go +++ b/pkg/selector/selector.go @@ -133,7 +133,8 @@ func (itf Selector) Save() error { // Filter accepts a Filters struct which is used to select the available instance types // matching the criteria within Filters and returns a simple list of instance type strings // -// Deprecated: This function will no longer exist in the next major version. +// Deprecated: This function will be replaced with GetFilteredInstanceTypes() and +// OutputInstanceTypes() in the next major version. func (itf Selector) Filter(filters Filters) ([]string, error) { outputFn := InstanceTypesOutputFn(outputs.SimpleInstanceTypeOutput) output, _, err := itf.FilterWithOutput(filters, outputFn) @@ -143,7 +144,8 @@ func (itf Selector) Filter(filters Filters) ([]string, error) { // FilterVerbose accepts a Filters struct which is used to select the available instance types // matching the criteria within Filters and returns a list instanceTypeInfo // -// Deprecated: This function will no longer exist in the next major version. +// Deprecated: This function will be replaced with GetFilteredInstanceTypes() in the next +// major version. func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, error) { instanceTypeInfoSlice, err := itf.rawFilter(filters) if err != nil { @@ -156,7 +158,8 @@ func (itf Selector) FilterVerbose(filters Filters) ([]*instancetypes.Details, er // FilterWithOutput accepts a Filters struct which is used to select the available instance types // matching the criteria within Filters and returns a list of strings based on the custom outputFn // -// Deprecated: This function will no longer exist in the next major version. +// Deprecated: This function will be replaced with GetFilteredInstanceTypes() and +// OutputInstanceTypes() in the next major version. func (itf Selector) FilterWithOutput(filters Filters, outputFn InstanceTypesOutput) ([]string, int, error) { instanceTypeInfoSlice, err := itf.rawFilter(filters) if err != nil {