From e9ff391e11d8886458afb65b76bf195227d62b31 Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Fri, 3 Nov 2023 18:48:19 +0100 Subject: [PATCH] Support Mi units --- cmd/dashboard.go | 13 ++++++----- cmd/summary.go | 5 +++++ pkg/dashboard/dashboard.go | 2 ++ pkg/dashboard/options.go | 8 +++++++ pkg/summary/options.go | 8 +++++++ pkg/summary/summary.go | 12 +++++----- pkg/utils/utils.go | 45 +++++++++++++++++++++++++++++++++----- pkg/utils/utils_test.go | 27 ++++++++++++++++------- 8 files changed, 96 insertions(+), 24 deletions(-) diff --git a/cmd/dashboard.go b/cmd/dashboard.go index 72d4543f..16bb828d 100644 --- a/cmd/dashboard.go +++ b/cmd/dashboard.go @@ -27,11 +27,12 @@ import ( ) var ( - serverPort int - showAllVPAs bool - basePath string - insightsHost string - enableCost bool + serverPort int + showAllVPAs bool + basePath string + insightsHost string + enableCost bool + useMemoryBinarySI bool ) func init() { @@ -43,6 +44,7 @@ func init() { dashboardCmd.PersistentFlags().StringVar(&basePath, "base-path", "/", "Path on which the dashboard is served.") dashboardCmd.PersistentFlags().BoolVar(&enableCost, "enable-cost", true, "If set to false, the cost integration will be disabled on the dashboard.") dashboardCmd.PersistentFlags().StringVar(&insightsHost, "insights-host", "https://insights.fairwinds.com", "Insights host for retrieving optional cost data.") + dashboardCmd.PersistentFlags().BoolVar(&useMemoryBinarySI, "binary-si", false, "Use binary SI units for memory (base 2) instead of decimal SI units (base 10).") } var dashboardCmd = &cobra.Command{ @@ -59,6 +61,7 @@ var dashboardCmd = &cobra.Command{ dashboard.ShowAllVPAs(showAllVPAs), dashboard.InsightsHost(insightsHost), dashboard.EnableCost(enableCost), + dashboard.UseMemoryBinarySI(useMemoryBinarySI), ) http.Handle("/", router) klog.Infof("Starting goldilocks dashboard server on port %d and basePath %v", serverPort, validBasePath) diff --git a/cmd/summary.go b/cmd/summary.go index 5c2344a9..4a20434c 100644 --- a/cmd/summary.go +++ b/cmd/summary.go @@ -30,12 +30,14 @@ import ( var excludeContainers string var outputFile string var namespace string +var summaryUseMemoryBinarySI bool func init() { rootCmd.AddCommand(summaryCmd) summaryCmd.PersistentFlags().StringVarP(&excludeContainers, "exclude-containers", "e", "", "Comma delimited list of containers to exclude from recommendations.") summaryCmd.PersistentFlags().StringVarP(&outputFile, "output-file", "f", "", "File to write output from audit.") summaryCmd.PersistentFlags().StringVarP(&namespace, "namespace", "n", "", "Limit the summary to only a single Namespace.") + summaryCmd.PersistentFlags().BoolVarP(&summaryUseMemoryBinarySI, "binary-si", "b", false, "Use binary SI units for memory (base 2) instead of decimal SI units (base 10).") } var summaryCmd = &cobra.Command{ @@ -57,6 +59,9 @@ By default the summary will be about all VPAs in all namespaces.`, opts = append(opts, summary.ExcludeContainers(sets.New[string](strings.Split(excludeContainers, ",")...))) } + // use binary SI units for memory + summary.UseMemoryBinarySI(summaryUseMemoryBinarySI) + summarizer := summary.NewSummarizer(opts...) data, err := summarizer.GetSummary() if err != nil { diff --git a/pkg/dashboard/dashboard.go b/pkg/dashboard/dashboard.go index f98c2df0..a0417ee7 100644 --- a/pkg/dashboard/dashboard.go +++ b/pkg/dashboard/dashboard.go @@ -120,6 +120,7 @@ func getVPAData(opts Options, namespace, costPerCPU, costPerGB string) (summary. summary.ForNamespace(namespace), summary.ForVPAsWithLabels(filterLabels), summary.ExcludeContainers(opts.ExcludedContainers), + summary.UseMemoryBinarySI(opts.UseMemoryBinarySI), ) vpaData, err := summarizer.GetSummary() @@ -149,6 +150,7 @@ func getVPAData(opts Options, namespace, costPerCPU, costPerGB string) (summary. } } } + return vpaData, nil } diff --git a/pkg/dashboard/options.go b/pkg/dashboard/options.go index 3aa24dd7..c34146ef 100644 --- a/pkg/dashboard/options.go +++ b/pkg/dashboard/options.go @@ -18,6 +18,7 @@ type Options struct { ShowAllVPAs bool InsightsHost string EnableCost bool + UseMemoryBinarySI bool } // default options for the dashboard @@ -30,6 +31,7 @@ func defaultOptions() *Options { OnByDefault: false, ShowAllVPAs: false, EnableCost: true, + UseMemoryBinarySI: false, } } @@ -84,3 +86,9 @@ func EnableCost(enableCost bool) Option { opts.EnableCost = enableCost } } + +func UseMemoryBinarySI(useMemoryBinarySI bool) Option { + return func(opts *Options) { + opts.UseMemoryBinarySI = useMemoryBinarySI + } +} diff --git a/pkg/summary/options.go b/pkg/summary/options.go index 0792cead..c4950763 100644 --- a/pkg/summary/options.go +++ b/pkg/summary/options.go @@ -17,6 +17,7 @@ type options struct { namespace string vpaLabels map[string]string excludedContainers sets.Set[string] + useMemoryBinarySI bool } // defaultOptions for a Summarizer @@ -52,3 +53,10 @@ func ForVPAsWithLabels(vpaLabels map[string]string) Option { opts.vpaLabels = vpaLabels } } + +// UseMemoryBinarySI is an Option for using binary SI units for memory (base 2) instead of decimal SI units (base 10) +func UseMemoryBinarySI(useMemoryBinarySI bool) Option { + return func(opts *options) { + opts.useMemoryBinarySI = useMemoryBinarySI + } +} diff --git a/pkg/summary/summary.go b/pkg/summary/summary.go index a634a6d5..e2657237 100644 --- a/pkg/summary/summary.go +++ b/pkg/summary/summary.go @@ -212,12 +212,12 @@ func (s Summarizer) GetSummary() (Summary, error) { if c.Name == containerRecommendation.ContainerName { cSummary = ContainerSummary{ ContainerName: containerRecommendation.ContainerName, - UpperBound: utils.FormatResourceList(containerRecommendation.UpperBound), - LowerBound: utils.FormatResourceList(containerRecommendation.LowerBound), - Target: utils.FormatResourceList(containerRecommendation.Target), - UncappedTarget: utils.FormatResourceList(containerRecommendation.UncappedTarget), - Limits: utils.FormatResourceList(c.Resources.Limits), - Requests: utils.FormatResourceList(c.Resources.Requests), + UpperBound: utils.FormatResourceList(containerRecommendation.UpperBound, s.useMemoryBinarySI), + LowerBound: utils.FormatResourceList(containerRecommendation.LowerBound, s.useMemoryBinarySI), + Target: utils.FormatResourceList(containerRecommendation.Target, s.useMemoryBinarySI), + UncappedTarget: utils.FormatResourceList(containerRecommendation.UncappedTarget, s.useMemoryBinarySI), + Limits: utils.FormatResourceList(c.Resources.Limits, s.useMemoryBinarySI), + Requests: utils.FormatResourceList(c.Resources.Requests, s.useMemoryBinarySI), } klog.V(6).Infof("Resources for %s/%s/%s: Requests: %v Limits: %v", wSummary.ControllerType, wSummary.ControllerName, c.Name, cSummary.Requests, cSummary.Limits) wSummary.Containers[cSummary.ContainerName] = cSummary diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 96e3499d..8eb8ce11 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -77,9 +77,37 @@ func Difference(a, b []string) (diff []string) { return } +const ( + Kibibyte = 1024 + Mebibyte = Kibibyte * 1024 + Gibibyte = Mebibyte * 1024 + Tebibyte = Gibibyte * 1024 +) + +func FormatToBinarySI(actual resource.Quantity, scale resource.Scale) resource.Quantity { + var scaleValue int64 + switch scale { + case resource.Kilo: + scaleValue = Kibibyte + case resource.Mega: + scaleValue = Mebibyte + case resource.Giga: + scaleValue = Gibibyte + case resource.Tera: + scaleValue = Tebibyte + default: + scaleValue = Mebibyte + } + value := actual.Value() / scaleValue + if actual.Value()%scaleValue != 0 { + value++ + } + return *resource.NewQuantity(value*scaleValue, resource.BinarySI) +} + // FormatResourceList scales the units of a ResourceList so that they are // human readable -func FormatResourceList(rl v1.ResourceList) v1.ResourceList { +func FormatResourceList(rl v1.ResourceList, useBinarySI bool) v1.ResourceList { memoryScales := []resource.Scale{ resource.Kilo, resource.Mega, @@ -87,11 +115,18 @@ func FormatResourceList(rl v1.ResourceList) v1.ResourceList { resource.Tera, } if mem, exists := rl[v1.ResourceMemory]; exists { - i := 0 - maxAllowableStringLen := 5 - for len(mem.String()) > maxAllowableStringLen && i < len(memoryScales)-1 { + maxLength := 5 + if len(mem.String()) <= maxLength { + return rl + } + for i := 0; i < len(memoryScales); i++ { mem.RoundUp(memoryScales[i]) - i++ + if len(mem.String()) <= maxLength { + if useBinarySI { + mem = FormatToBinarySI(mem, memoryScales[i]) + } + break + } } rl[v1.ResourceMemory] = mem } diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 7e04c02f..0c26ce8c 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -18,7 +18,7 @@ import ( "testing" "github.com/stretchr/testify/assert" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" ) @@ -83,7 +83,7 @@ var testDifferenceCases = []struct { func TestFormatResourceList(t *testing.T) { for _, tc := range testFormatResourceCases { - res := FormatResourceList(tc.testData) + res := FormatResourceList(tc.testData, tc.useMemoryBinarySI) resource := res[tc.resourceType] got := resource.String() assert.Equal(t, tc.expected, got) @@ -91,10 +91,11 @@ func TestFormatResourceList(t *testing.T) { } var testFormatResourceCases = []struct { - description string - testData v1.ResourceList - resourceType v1.ResourceName - expected string + description string + testData v1.ResourceList + resourceType v1.ResourceName + useMemoryBinarySI bool + expected string }{ { description: "Unmodified cpu", @@ -117,7 +118,17 @@ var testFormatResourceCases = []struct { testData: v1.ResourceList{ "memory": resource.MustParse("123456k"), }, - resourceType: "memory", - expected: "124M", + resourceType: "memory", + useMemoryBinarySI: false, + expected: "124M", + }, + { + description: "Memory in too large of units", + testData: v1.ResourceList{ + "memory": resource.MustParse("123456k"), + }, + resourceType: "memory", + useMemoryBinarySI: true, + expected: "119Mi", }, }