From e50059de07b560a48e6a990fa5f5cf63d461058e Mon Sep 17 00:00:00 2001 From: "Tim St. Clair" Date: Thu, 7 Jan 2016 14:55:19 -0800 Subject: [PATCH] Enforce minimum resource granularity Enforce minimum resource granularity of milli-{core, bytes} for Storage, Memory, and CPU resource types. For Storage and Memory, milli-bytes are allowed for backwards compatability, but the behavior is undefinied (depends on docker implementation). --- pkg/api/testing/fuzzer.go | 13 ++- pkg/api/v1/conversion.go | 26 +++++ pkg/api/v1/conversion_generated.go | 182 +++++------------------------ pkg/api/v1/conversion_test.go | 55 +++++++++ 4 files changed, 117 insertions(+), 159 deletions(-) diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index 56ddc004b2a..c34f07effad 100644 --- a/pkg/api/testing/fuzzer.go +++ b/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) diff --git a/pkg/api/v1/conversion.go b/pkg/api/v1/conversion.go index fed3ab6820b..c235f4b9b01 100644 --- a/pkg/api/v1/conversion.go +++ b/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 +} diff --git a/pkg/api/v1/conversion_generated.go b/pkg/api/v1/conversion_generated.go index e32ae2d05b0..25aaddf88ca 100644 --- a/pkg/api/v1/conversion_generated.go +++ b/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 } diff --git a/pkg/api/v1/conversion_test.go b/pkg/api/v1/conversion_test.go index ad911599485..8cc2f047429 100644 --- a/pkg/api/v1/conversion_test.go +++ b/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) + } + } +}