Enforce minimum resource granularity

pkg/api/testing/fuzzer.go

@@ -56,6 +56,9 @@ func FuzzerFor(t *testing.T, version unversioned.GroupVersion, src rand.Source)
 				*j = nil
 			}
 		},
+		func(q *resource.Quantity, c fuzz.Continue) {
+			*q = *resource.NewQuantity(c.Int63n(1000), resource.DecimalExponent)
+		},
 		func(j *runtime.PluginBase, c fuzz.Continue) {
 			// Do nothing; this struct has only a Kind field and it must stay blank in memory.
 		},
@@ -203,7 +206,9 @@ func FuzzerFor(t *testing.T, version unversioned.GroupVersion, src rand.Source)
 		},
 		func(q *api.ResourceRequirements, c fuzz.Continue) {
 			randomQuantity := func() resource.Quantity {
-				return *resource.NewQuantity(c.Int63n(1000), resource.DecimalExponent)
+				var q resource.Quantity
+				c.Fuzz(&q)
+				return q
 			}
 			q.Limits = make(api.ResourceList)
 			q.Requests = make(api.ResourceList)
@@ -218,10 +223,8 @@ func FuzzerFor(t *testing.T, version unversioned.GroupVersion, src rand.Source)
 			q.Requests[api.ResourceStorage] = *storageLimit.Copy()
 		},
 		func(q *api.LimitRangeItem, c fuzz.Continue) {
-			randomQuantity := func() resource.Quantity {
-				return *resource.NewQuantity(c.Int63n(1000), resource.DecimalExponent)
-			}
-			cpuLimit := randomQuantity()
+			var cpuLimit resource.Quantity
+			c.Fuzz(&cpuLimit)
 
 			q.Type = api.LimitTypeContainer
 			q.Default = make(api.ResourceList)

pkg/api/v1/conversion.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/conversion"
 	"k8s.io/kubernetes/pkg/runtime"
+	"speter.net/go/exp/math/dec/inf"
 )
 
 const (
@@ -44,6 +45,7 @@ func addConversionFuncs(scheme *runtime.Scheme) {
 		Convert_v1_PodSpec_To_api_PodSpec,
 		Convert_v1_ReplicationControllerSpec_To_api_ReplicationControllerSpec,
 		Convert_v1_ServiceSpec_To_api_ServiceSpec,
+		Convert_v1_ResourceList_To_api_ResourceList,
 	)
 	if err != nil {
 		// If one of the conversion functions is malformed, detect it immediately.
@@ -540,3 +542,27 @@ func Convert_v1_PodSecurityContext_To_api_PodSecurityContext(in *PodSecurityCont
 	}
 	return nil
 }
+
+func Convert_v1_ResourceList_To_api_ResourceList(in *ResourceList, out *api.ResourceList, s conversion.Scope) error {
+	if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
+		defaulting.(func(*ResourceList))(in)
+	}
+	if *in == nil {
+		return nil
+	}
+
+	converted := make(api.ResourceList)
+	for key, val := range *in {
+		value := val.Copy()
+
+		// TODO(#18538): We round up resource values to milli scale to maintain API compatibility.
+		// In the future, we should instead reject values that need rounding.
+		const milliScale = 3
+		value.Amount.Round(value.Amount, milliScale, inf.RoundUp)
+
+		converted[api.ResourceName(key)] = *value
+	}
+
+	*out = converted
+	return nil
+}

pkg/api/v1/conversion_generated.go

@@ -4203,65 +4203,20 @@ func autoConvert_v1_LimitRangeItem_To_api_LimitRangeItem(in *LimitRangeItem, out
 		defaulting.(func(*LimitRangeItem))(in)
 	}
 	out.Type = api.LimitType(in.Type)
-	if in.Max != nil {
-		out.Max = make(api.ResourceList)
-		for key, val := range in.Max {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Max[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Max = nil
+	if err := s.Convert(&in.Max, &out.Max, 0); err != nil {
+		return err
 	}
-	if in.Min != nil {
-		out.Min = make(api.ResourceList)
-		for key, val := range in.Min {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Min[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Min = nil
+	if err := s.Convert(&in.Min, &out.Min, 0); err != nil {
+		return err
 	}
-	if in.Default != nil {
-		out.Default = make(api.ResourceList)
-		for key, val := range in.Default {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Default[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Default = nil
+	if err := s.Convert(&in.Default, &out.Default, 0); err != nil {
+		return err
 	}
-	if in.DefaultRequest != nil {
-		out.DefaultRequest = make(api.ResourceList)
-		for key, val := range in.DefaultRequest {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.DefaultRequest[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.DefaultRequest = nil
+	if err := s.Convert(&in.DefaultRequest, &out.DefaultRequest, 0); err != nil {
+		return err
 	}
-	if in.MaxLimitRequestRatio != nil {
-		out.MaxLimitRequestRatio = make(api.ResourceList)
-		for key, val := range in.MaxLimitRequestRatio {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.MaxLimitRequestRatio[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.MaxLimitRequestRatio = nil
+	if err := s.Convert(&in.MaxLimitRequestRatio, &out.MaxLimitRequestRatio, 0); err != nil {
+		return err
 	}
 	return nil
 }
@@ -4624,29 +4579,11 @@ func autoConvert_v1_NodeStatus_To_api_NodeStatus(in *NodeStatus, out *api.NodeSt
 	if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
 		defaulting.(func(*NodeStatus))(in)
 	}
-	if in.Capacity != nil {
-		out.Capacity = make(api.ResourceList)
-		for key, val := range in.Capacity {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Capacity[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Capacity = nil
+	if err := s.Convert(&in.Capacity, &out.Capacity, 0); err != nil {
+		return err
 	}
-	if in.Allocatable != nil {
-		out.Allocatable = make(api.ResourceList)
-		for key, val := range in.Allocatable {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Allocatable[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Allocatable = nil
+	if err := s.Convert(&in.Allocatable, &out.Allocatable, 0); err != nil {
+		return err
 	}
 	out.Phase = api.NodePhase(in.Phase)
 	if in.Conditions != nil {
@@ -4893,17 +4830,8 @@ func autoConvert_v1_PersistentVolumeClaimStatus_To_api_PersistentVolumeClaimStat
 	} else {
 		out.AccessModes = nil
 	}
-	if in.Capacity != nil {
-		out.Capacity = make(api.ResourceList)
-		for key, val := range in.Capacity {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Capacity[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Capacity = nil
+	if err := s.Convert(&in.Capacity, &out.Capacity, 0); err != nil {
+		return err
 	}
 	return nil
 }
@@ -5075,17 +5003,8 @@ func autoConvert_v1_PersistentVolumeSpec_To_api_PersistentVolumeSpec(in *Persist
 	if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
 		defaulting.(func(*PersistentVolumeSpec))(in)
 	}
-	if in.Capacity != nil {
-		out.Capacity = make(api.ResourceList)
-		for key, val := range in.Capacity {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Capacity[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Capacity = nil
+	if err := s.Convert(&in.Capacity, &out.Capacity, 0); err != nil {
+		return err
 	}
 	if err := Convert_v1_PersistentVolumeSource_To_api_PersistentVolumeSource(&in.PersistentVolumeSource, &out.PersistentVolumeSource, s); err != nil {
 		return err
@@ -5720,17 +5639,8 @@ func autoConvert_v1_ResourceQuotaSpec_To_api_ResourceQuotaSpec(in *ResourceQuota
 	if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
 		defaulting.(func(*ResourceQuotaSpec))(in)
 	}
-	if in.Hard != nil {
-		out.Hard = make(api.ResourceList)
-		for key, val := range in.Hard {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Hard[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Hard = nil
+	if err := s.Convert(&in.Hard, &out.Hard, 0); err != nil {
+		return err
 	}
 	return nil
 }
@@ -5743,29 +5653,11 @@ func autoConvert_v1_ResourceQuotaStatus_To_api_ResourceQuotaStatus(in *ResourceQ
 	if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
 		defaulting.(func(*ResourceQuotaStatus))(in)
 	}
-	if in.Hard != nil {
-		out.Hard = make(api.ResourceList)
-		for key, val := range in.Hard {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Hard[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Hard = nil
+	if err := s.Convert(&in.Hard, &out.Hard, 0); err != nil {
+		return err
 	}
-	if in.Used != nil {
-		out.Used = make(api.ResourceList)
-		for key, val := range in.Used {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Used[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Used = nil
+	if err := s.Convert(&in.Used, &out.Used, 0); err != nil {
+		return err
 	}
 	return nil
 }
@@ -5778,29 +5670,11 @@ func autoConvert_v1_ResourceRequirements_To_api_ResourceRequirements(in *Resourc
 	if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
 		defaulting.(func(*ResourceRequirements))(in)
 	}
-	if in.Limits != nil {
-		out.Limits = make(api.ResourceList)
-		for key, val := range in.Limits {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Limits[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Limits = nil
+	if err := s.Convert(&in.Limits, &out.Limits, 0); err != nil {
+		return err
 	}
-	if in.Requests != nil {
-		out.Requests = make(api.ResourceList)
-		for key, val := range in.Requests {
-			newVal := resource.Quantity{}
-			if err := api.Convert_resource_Quantity_To_resource_Quantity(&val, &newVal, s); err != nil {
-				return err
-			}
-			out.Requests[api.ResourceName(key)] = newVal
-		}
-	} else {
-		out.Requests = nil
+	if err := s.Convert(&in.Requests, &out.Requests, 0); err != nil {
+		return err
 	}
 	return nil
 }

pkg/api/v1/conversion_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/resource"
 	versioned "k8s.io/kubernetes/pkg/api/v1"
 )
 
@@ -68,3 +69,57 @@ func TestPodSpecConversion(t *testing.T) {
 		}
 	}
 }
+
+func TestResourceListConversion(t *testing.T) {
+	bigMilliQuantity := resource.NewQuantity(resource.MaxMilliValue, resource.DecimalSI)
+	bigMilliQuantity.Add(resource.MustParse("12345m"))
+
+	tests := []struct {
+		input    versioned.ResourceList
+		expected api.ResourceList
+	}{
+		{ // No changes necessary.
+			input: versioned.ResourceList{
+				versioned.ResourceMemory:  resource.MustParse("30M"),
+				versioned.ResourceCPU:     resource.MustParse("100m"),
+				versioned.ResourceStorage: resource.MustParse("1G"),
+			},
+			expected: api.ResourceList{
+				api.ResourceMemory:  resource.MustParse("30M"),
+				api.ResourceCPU:     resource.MustParse("100m"),
+				api.ResourceStorage: resource.MustParse("1G"),
+			},
+		},
+		{ // Nano-scale values should be rounded up to milli-scale.
+			input: versioned.ResourceList{
+				versioned.ResourceCPU:    resource.MustParse("3.000023m"),
+				versioned.ResourceMemory: resource.MustParse("500.000050m"),
+			},
+			expected: api.ResourceList{
+				api.ResourceCPU:    resource.MustParse("4m"),
+				api.ResourceMemory: resource.MustParse("501m"),
+			},
+		},
+		{ // Large values should still be accurate.
+			input: versioned.ResourceList{
+				versioned.ResourceCPU:     *bigMilliQuantity.Copy(),
+				versioned.ResourceStorage: *bigMilliQuantity.Copy(),
+			},
+			expected: api.ResourceList{
+				api.ResourceCPU:     *bigMilliQuantity.Copy(),
+				api.ResourceStorage: *bigMilliQuantity.Copy(),
+			},
+		},
+	}
+
+	output := api.ResourceList{}
+	for i, test := range tests {
+		err := api.Scheme.Convert(&test.input, &output)
+		if err != nil {
+			t.Fatalf("unexpected error for case %d: %v", i, err)
+		}
+		if !api.Semantic.DeepEqual(test.expected, output) {
+			t.Errorf("unexpected conversion for case %d: Expected %+v; Got %+v", i, test.expected, output)
+		}
+	}
+}