fix: correct the computation of the percentage differences and stats

Fix the computation of the CPU/Memory difference columns to express
the result as the increase/decrease of the request in terms of the
recommendation.

Print the memory recommendation quantity using the same scale as the
request in a human-friendly format.

Scale the statistics values to the number of replicas requested in
the spec of each controller.

Author: wI2L

README.md

@@ -43,6 +43,10 @@ It creates a single binary file for the host machine platform/architecture in th
 
 ## Usage
 
+### How to interpret the output?
+
+The columns `% CPU DIFF` and `% MEMORY DIFF` represents the percentage of increase/decrease for the request in terms of the recommendation. For example, if the request is 4 CPU (4000m), and the recommendation is only 1 CPU (1000m), the difference is +300%. As a rule of thumb, you can think of positive values as "overcommitment"  and negative values as "under commitment".
+
 ### Demo
 
 The following examples were produced from a brand-new Kubernetes cluster created with [`k3d`](https://k3d.io/v5.2.2/). The `VerticalPodAutoscaler` resources were automatically created by the [`goldilocks`](https://github.com/FairwindsOps/goldilocks) operator.

cli/command.go

@@ -272,6 +272,7 @@ func newTableRow(v *vpav1.VerticalPodAutoscaler, tc *vpa.TargetController, name
 		Namespace:        v.Namespace,
 		GVK:              v.GroupVersionKind(),
 		Mode:             updateModeFromSpec(v.Spec.UpdatePolicy),
+		Target:           tc,
 		TargetName:       tc.Name,
 		TargetGVK:        tc.GroupVersionKind,
 		Requests:         rqs,

cli/table.go

@@ -8,13 +8,13 @@ import (
 	"sort"
 	"strings"
 
-	"github.com/dustin/go-humanize"
 	"github.com/muesli/termenv"
 	"github.com/olekukonko/tablewriter"
 	"gopkg.in/inf.v0"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
+	"github.com/wI2L/kubectl-vpa-recommendation/internal/humanize"
 	"github.com/wI2L/kubectl-vpa-recommendation/vpa"
 )
 
@@ -54,8 +54,10 @@ type tableRow struct {
 	Namespace        string
 	GVK              schema.GroupVersionKind
 	Mode             string
+	Target           *vpa.TargetController
 	TargetName       string
 	TargetGVK        schema.GroupVersionKind
+	TargetReplicas   int32
 	Requests         vpa.ResourceQuantities
 	Recommendations  vpa.ResourceQuantities
 	CPUDifference    *float64
@@ -88,18 +90,30 @@ func (tr tableRow) toTableData(flags *Flags, isChild bool) []string {
 
 	if flags.wide {
 		rowData = append(rowData,
-			formatQuantity(tr.Requests.CPU),
-			formatQuantity(tr.Recommendations.CPU),
+			formatQuantity(tr.Requests.CPU), formatQuantity(tr.Recommendations.CPU),
 		)
 	}
 	rowData = append(rowData, formatPercentage(tr.CPUDifference, flags.NoColors))
 	if flags.wide {
+		var str string
+		d := inf.Dec{}
+		d.Round(tr.Recommendations.Memory.AsDec(), 0, inf.RoundUp)
+		b := d.UnscaledBig()
+		switch tr.Requests.Memory.Format {
+		case resource.DecimalSI:
+			str = humanize.BigBytes(b, 2)
+		case resource.BinarySI:
+			str = humanize.BigIBytes(b, 2)
+		default:
+			str = tr.Recommendations.Memory.String()
+		}
 		rowData = append(rowData,
 			formatQuantity(tr.Requests.Memory),
-			formatQuantity(tr.Recommendations.Memory),
+			str,
 		)
 	}
 	rowData = append(rowData, formatPercentage(tr.MemoryDifference, flags.NoColors))
+
 	return rowData
 }
 
@@ -191,35 +205,48 @@ func (t table) printStats(w io.Writer) error {
 		t.medianQuantities,
 	}
 	rows := []struct {
-		name    string
-		getter  func(i int) *resource.Quantity
-		asBytes bool
+		name           string
+		quantityGetter func(i int) *resource.Quantity
+		asBytes        bool
 	}{
-		{"CPU Recommendations (# cores)", func(i int) *resource.Quantity { return t[i].Recommendations.CPU }, false},
 		{"CPU Requests (# cores)", func(i int) *resource.Quantity { return t[i].Requests.CPU }, false},
-		{"MEM Recommendations (IEC/SI)", func(i int) *resource.Quantity { return t[i].Recommendations.Memory }, true},
+		{"CPU Recommendations (# cores)", func(i int) *resource.Quantity { return t[i].Recommendations.CPU }, false},
 		{"MEM Requests (IEC/SI)", func(i int) *resource.Quantity { return t[i].Requests.Memory }, true},
+		{"MEM Recommendations (IEC/SI)", func(i int) *resource.Quantity { return t[i].Recommendations.Memory }, true},
 	}
 	for _, row := range rows {
 		values := make([]string, 0, len(statFuncs))
+
 		for _, fn := range statFuncs {
-			q := fn(row.getter)
+			scaledQuantity := func(i int) *resource.Quantity {
+				q := row.quantityGetter(i)
+
+				// Scale the quantity according to the number
+				// of replicas declared in the controller's spec.
+				var replicas int64
+				n, err := t[i].Target.ReplicasCount()
+				if err != nil {
+					replicas = 1
+				} else {
+					replicas = n
+				}
+				return multiplyQuantity(q, replicas)
+			}
+			q := fn(scaledQuantity)
 
-			var str string
-			if q == nil {
-				str = tableUnsetCell
-			} else {
+			s := tableUnsetCell
+			if q != nil {
 				if row.asBytes {
-					tmp := inf.Dec{}
-					tmp.Round(q.AsDec(), 0, inf.RoundUp)
-					big := tmp.UnscaledBig()
-					str = humanize.BigIBytes(big) + "/" + humanize.BigBytes(big)
-					str = strings.ReplaceAll(str, " ", "")
+					d := inf.Dec{}
+					d.Round(q.AsDec(), 0, inf.RoundUp)
+					b := d.UnscaledBig()
+					s = humanize.BigIBytes(b, 2) + "/" + humanize.BigBytes(b, 2)
+					s = strings.ReplaceAll(s, " ", "")
 				} else {
-					str = q.AsDec().String()
+					s = q.AsDec().Round(q.AsDec(), 2, inf.RoundCeil).String()
 				}
 			}
-			values = append(values, str)
+			values = append(values, s)
 		}
 		tw.Append(append([]string{row.name}, values...))
 	}
@@ -229,6 +256,23 @@ func (t table) printStats(w io.Writer) error {
 	return nil
 }
 
+func multiplyQuantity(q *resource.Quantity, n int64) *resource.Quantity {
+	if q == nil || n == 0 {
+		return nil
+	}
+	if n == 1 {
+		return q
+	}
+	// The resource.Quantity type does not define a
+	// multiplication method, so instead we add the
+	// same amount n-1 times.
+	ret := q.DeepCopy()
+	for i := 0; int64(i) < n-1; i++ {
+		ret.Add(*q)
+	}
+	return &ret
+}
+
 func (t table) sumQuantities(column func(i int) *resource.Quantity) *resource.Quantity {
 	var sum resource.Quantity
 	for i := range t {
@@ -279,7 +323,7 @@ func (t table) medianQuantities(column func(i int) *resource.Quantity) *resource
 		q := values[l/2-1]
 		q.Add(*(values[l/2]))
 		tmp := inf.Dec{}
-		tmp.QuoRound(q.AsDec(), inf.NewDec(2, 0), 3, inf.RoundDown)
+		tmp.QuoRound(q.AsDec(), inf.NewDec(2, 0), 2, inf.RoundUp)
 
 		return resource.NewDecimalQuantity(tmp, resource.DecimalSI)
 	}

internal/humanize/bigbytes.go

@@ -0,0 +1,52 @@
+package humanize
+
+import (
+	"fmt"
+	"math/big"
+)
+
+var (
+	bigSIExp  = big.NewInt(1000)
+	bigIECExp = big.NewInt(1024)
+)
+
+// BigBytes produces a human-readable representation of an SI size.
+func BigBytes(s *big.Int, precision int) string {
+	sizes := []string{"B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"}
+	return humanizeBigBytes(s, bigSIExp, sizes, precision)
+}
+
+// BigIBytes produces a human-readable representation of an IEC size.
+func BigIBytes(s *big.Int, precision int) string {
+	sizes := []string{"B", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi"}
+	return humanizeBigBytes(s, bigIECExp, sizes, precision)
+}
+
+var ten = big.NewInt(10)
+
+func humanizeBigBytes(s, base *big.Int, sizes []string, precision int) string {
+	if s.Cmp(ten) < 0 {
+		return fmt.Sprintf("%d B", s)
+	}
+	c := (&big.Int{}).Set(s)
+	val, mag := orderOfMagnitude(c, base, len(sizes)-1)
+	suffix := sizes[mag]
+	f := "%.0f %s"
+	if val < 10 {
+		f = fmt.Sprintf("%%.%df %%s", precision)
+	}
+	return fmt.Sprintf(f, val, suffix)
+}
+
+func orderOfMagnitude(n, b *big.Int, max int) (float64, int) {
+	mag := 0
+	m := &big.Int{}
+	for n.Cmp(b) >= 0 {
+		n.DivMod(n, b, m)
+		mag++
+		if mag == max && max >= 0 {
+			break
+		}
+	}
+	return float64(n.Int64()) + (float64(m.Int64()) / float64(b.Int64())), mag
+}

vpa/resource.go

@@ -2,14 +2,10 @@ package vpa
 
 import (
 	"math"
-	"strconv"
 
-	"gopkg.in/inf.v0"
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
-const precision = 2
-
 // ResourceQuantities is a pair of resource quantities
 // that can represent the recommendations of a VerticalPodAutoscaler
 // of the requests of a pod's container.
@@ -18,39 +14,18 @@ type ResourceQuantities struct {
 	Memory *resource.Quantity
 }
 
-// DiffQuantitiesAsPercent return the difference between
-// two quantities expressed as a percentage of each other,
-// where x is the request and y the recommendation.
-func DiffQuantitiesAsPercent(x, y *resource.Quantity) *float64 {
-	if x == nil || y == nil || x.IsZero() || y.IsZero() {
+// DiffQuantitiesAsPercent return the difference between two
+// quantities. The return value is expressed as the increase/decrease
+// of the request in terms of the recommendation.
+func DiffQuantitiesAsPercent(req, rec *resource.Quantity) *float64 {
+	if req == nil || rec == nil || req.IsZero() || rec.IsZero() {
 		return nil
 	}
-	ai, oka := x.AsInt64()
-	bi, okb := y.AsInt64()
-
-	if oka && okb {
-		f := (float64(ai) - float64(bi)) / float64(ai) * 100
-		exp := powFloat64(10, precision)
-		// Round down to 'precision' decimal places.
-		f = math.Floor(f*exp) / exp
-		return &f
-	}
-	ad := x.AsDec()
-	bd := y.AsDec()
-
-	p := &inf.Dec{}
-	p.Sub(ad, bd)
-	p.QuoRound(p, ad, precision+precision, inf.RoundDown)
-	p.Mul(p, inf.NewDec(100, 0))
-	p.Round(p, precision, inf.RoundDown)
+	xf := req.AsApproximateFloat64()
+	yf := rec.AsApproximateFloat64()
 
-	f, err := strconv.ParseFloat(p.String(), 64)
-	if err != nil {
-		return nil
-	}
-	return &f
-}
+	p := (xf - yf) / yf * 100.0
+	p = math.Round(p*100) / 100 // round nearest
 
-func powFloat64(x, y int) float64 {
-	return math.Pow(float64(x), float64(y))
+	return &p
 }

vpa/resource_test.go

@@ -9,60 +9,60 @@ import (
 
 func TestDiffQuantitiesAsPercent(t *testing.T) {
 	testCases := []struct {
-		X *resource.Quantity
-		Y *resource.Quantity
-		P *float64
+		Req *resource.Quantity
+		Rec *resource.Quantity
+		P   *float64
 	}{
 		{
-			X: resource.NewQuantity(0, resource.DecimalSI),
-			Y: resource.NewQuantity(25, resource.DecimalSI),
-			P: nil,
+			Req: resource.NewQuantity(4, resource.DecimalSI),
+			Rec: resource.NewQuantity(1, resource.DecimalSI),
+			P:   pointer.Float64(300.00),
 		},
 		{
-			X: resource.NewQuantity(25, resource.DecimalSI),
-			Y: resource.NewQuantity(0, resource.DecimalSI),
-			P: nil,
+			Req: resource.NewQuantity(100, resource.DecimalSI),
+			Rec: resource.NewQuantity(25, resource.DecimalSI),
+			P:   pointer.Float64(300.00),
 		},
 		{
-			X: resource.NewQuantity(100, resource.DecimalSI),
-			Y: resource.NewQuantity(25, resource.DecimalSI),
-			P: pointer.Float64(75.00),
+			Req: resource.NewQuantity(444, resource.BinarySI),
+			Rec: resource.NewQuantity(1000, resource.BinarySI),
+			P:   pointer.Float64(-55.60),
 		},
 		{
-			X: resource.NewQuantity(444, resource.BinarySI),
-			Y: resource.NewQuantity(1000, resource.BinarySI),
-			P: pointer.Float64(-125.23),
+			Req: resource.NewQuantity(1000, resource.BinarySI),
+			Rec: resource.NewQuantity(444, resource.BinarySI),
+			P:   pointer.Float64(125.23),
 		},
 		{
-			X: resource.NewQuantity(1000, resource.BinarySI),
-			Y: resource.NewQuantity(444, resource.BinarySI),
-			P: pointer.Float64(55.60),
+			Req: resource.NewScaledQuantity(8, resource.Peta),
+			Rec: resource.NewScaledQuantity(24, resource.Peta),
+			P:   pointer.Float64(-66.67),
 		},
 		{
-			X: resource.NewScaledQuantity(8, resource.Peta),
-			Y: resource.NewScaledQuantity(24, resource.Peta),
-			P: pointer.Float64(-200.00),
+			Req: resource.NewScaledQuantity(128, resource.Exa),
+			Rec: resource.NewScaledQuantity(512, resource.Exa),
+			P:   pointer.Float64(-75.00),
 		},
 		{
-			X: resource.NewScaledQuantity(128, resource.Exa),
-			Y: resource.NewScaledQuantity(512, resource.Exa),
-			P: pointer.Float64(-300.00),
+			Req: resource.NewScaledQuantity(666, resource.Giga),
+			Rec: resource.NewScaledQuantity(32, resource.Exa),
+			P:   pointer.Float64(-100.00),
 		},
 		{
-			X: resource.NewScaledQuantity(666, resource.Giga),
-			Y: resource.NewScaledQuantity(32, resource.Exa),
-			P: pointer.Float64(-4804804704.80),
+			Req: resource.NewScaledQuantity(32, resource.Exa),
+			Rec: resource.NewScaledQuantity(666, resource.Giga),
+			P:   pointer.Float64(4804804704.80),
 		},
 		{
-			X: resource.NewScaledQuantity(32, resource.Exa),
-			Y: resource.NewScaledQuantity(666, resource.Giga),
-			P: pointer.Float64(99.99),
+			Req: resource.NewMilliQuantity(100, resource.DecimalSI),
+			Rec: resource.NewMilliQuantity(25, resource.DecimalSI),
+			P:   pointer.Float64(300.00),
 		},
 	}
 	for _, tc := range testCases {
-		t.Logf("X: %s, Y: %s", tc.X, tc.Y)
+		t.Logf("Req: %s, Rec: %s", tc.Req, tc.Rec)
 
-		p := DiffQuantitiesAsPercent(tc.X, tc.Y)
+		p := DiffQuantitiesAsPercent(tc.Req, tc.Rec)
 		if p == nil {
 			if tc.P != nil {
 				t.Errorf("got nil result, want %.2f", *tc.P)