diff --git a/pkg/api/resource/quantity.go b/pkg/api/resource/quantity.go index 19d497df829..aeb43c89767 100644 --- a/pkg/api/resource/quantity.go +++ b/pkg/api/resource/quantity.go @@ -349,6 +349,11 @@ func (q *Quantity) Add(y Quantity) error { q.Amount = &inf.Dec{} return q.Add(y) default: + // we want to preserve the format of the non-zero value + zero := &inf.Dec{} + if q.Amount.Cmp(zero) == 0 && y.Amount.Cmp(zero) != 0 { + q.Format = y.Format + } q.Amount.Add(q.Amount, y.Amount) } return nil @@ -362,11 +367,32 @@ func (q *Quantity) Sub(y Quantity) error { q.Amount = &inf.Dec{} return q.Sub(y) default: + // we want to preserve the format of the non-zero value + zero := &inf.Dec{} + if q.Amount.Cmp(zero) == 0 && y.Amount.Cmp(zero) != 0 { + q.Format = y.Format + } q.Amount.Sub(q.Amount, y.Amount) } return nil } +// Neg sets q to the negative value of y. +// It updates the format of q to match y. +func (q *Quantity) Neg(y Quantity) error { + switch { + case y.Amount == nil: + *q = y + case q.Amount == nil: + q.Amount = &inf.Dec{} + fallthrough + default: + q.Amount.Neg(y.Amount) + q.Format = y.Format + } + return nil +} + // MarshalJSON implements the json.Marshaller interface. func (q Quantity) MarshalJSON() ([]byte, error) { return []byte(`"` + q.String() + `"`), nil diff --git a/pkg/api/resource/quantity_test.go b/pkg/api/resource/quantity_test.go index a9f4d6aaf4d..bb62251e237 100644 --- a/pkg/api/resource/quantity_test.go +++ b/pkg/api/resource/quantity_test.go @@ -65,6 +65,70 @@ func TestQuantityParseZero(t *testing.T) { } } +// TestQuantityAddZeroPreservesSuffix verifies that a suffix is preserved +// independent of the order of operations when adding a zero and non-zero val +func TestQuantityAddZeroPreservesSuffix(t *testing.T) { + testValues := []string{"100m", "1Gi"} + zero := MustParse("0") + for _, testValue := range testValues { + value := MustParse(testValue) + v1 := *value.Copy() + // ensure non-zero + zero = non-zero (suffix preserved) + err := v1.Add(zero) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + // ensure zero + non-zero = non-zero (suffix preserved) + v2 := *zero.Copy() + err = v2.Add(value) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + // ensure we preserved the input value + if v1.String() != testValue { + t.Errorf("Expected %v, actual %v", testValue, v1.String()) + } + if v2.String() != testValue { + t.Errorf("Expected %v, actual %v", testValue, v2.String()) + } + } +} + +// TestQuantitySubZeroPreservesSuffix verifies that a suffix is preserved +// independent of the order of operations when subtracting a zero and non-zero val +func TestQuantitySubZeroPreservesSuffix(t *testing.T) { + testValues := []string{"100m", "1Gi"} + zero := MustParse("0") + for _, testValue := range testValues { + value := MustParse(testValue) + v1 := *value.Copy() + // ensure non-zero - zero = non-zero (suffix preserved) + err := v1.Sub(zero) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + // ensure we preserved the input value + if v1.String() != testValue { + t.Errorf("Expected %v, actual %v", testValue, v1.String()) + } + + // ensure zero - non-zero = -non-zero (suffix preserved) + v2 := *zero.Copy() + err = v2.Sub(value) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + negVal := *value.Copy() + err = negVal.Neg(negVal) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + if v2.String() != negVal.String() { + t.Errorf("Expected %v, actual %v", negVal.String(), v2.String()) + } + } +} + // Verifies that you get 0 as canonical value if internal value is 0, and not 0 func TestQuantityCanocicalizeZero(t *testing.T) { val := MustParse("1000m") @@ -667,6 +731,62 @@ func TestSub(t *testing.T) { } } +func TestNeg(t *testing.T) { + tests := []struct { + a Quantity + b Quantity + expected Quantity + }{ + { + a: Quantity{dec(0, 0), DecimalSI}, + b: Quantity{dec(10, 0), DecimalSI}, + expected: Quantity{dec(-10, 0), DecimalSI}, + }, + { + a: Quantity{dec(0, 0), DecimalSI}, + b: Quantity{dec(-10, 0), DecimalSI}, + expected: Quantity{dec(10, 0), DecimalSI}, + }, + { + a: Quantity{dec(0, 0), DecimalSI}, + b: Quantity{dec(10, 0), BinarySI}, + expected: Quantity{dec(-10, 0), BinarySI}, + }, + { + a: Quantity{dec(0, 0), DecimalSI}, + b: Quantity{dec(0, 0), BinarySI}, + expected: Quantity{dec(0, 0), BinarySI}, + }, + { + a: Quantity{}, + b: Quantity{dec(10, 0), BinarySI}, + expected: Quantity{dec(-10, 0), BinarySI}, + }, + { + a: Quantity{dec(10, 0), BinarySI}, + b: Quantity{}, + expected: Quantity{}, + }, + { + a: Quantity{dec(10, 0), BinarySI}, + b: Quantity{Format: DecimalSI}, + expected: Quantity{dec(0, 0), DecimalSI}, + }, + } + + for i, test := range tests { + test.a.Neg(test.b) + // ensure value is same + if test.a.Cmp(test.expected) != 0 { + t.Errorf("[%d] Expected %q, got %q", i, test.expected.String(), test.a.String()) + } + // ensure format is updated + if test.a.Format != test.expected.Format { + t.Errorf("[%d] Expected format %v, got format %v", i, test.expected.Format, test.a.Format) + } + } +} + func TestAdd(t *testing.T) { tests := []struct { a Quantity