diff --git a/vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator.go b/vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator.go index cbc184f8796f..9b60ff9e7cbf 100644 --- a/vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator.go +++ b/vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator.go @@ -19,7 +19,8 @@ package limitrange import ( "fmt" - "k8s.io/api/core/v1" + core "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/informers" listers "k8s.io/client-go/listers/core/v1" @@ -28,12 +29,12 @@ import ( // LimitRangeCalculator calculates limit range items that has the same effect as all limit range items present in the cluster. type LimitRangeCalculator interface { // GetContainerLimitRangeItem returns LimitRangeItem that describes limitation on container limits in the given namespace. - GetContainerLimitRangeItem(namespace string) (*v1.LimitRangeItem, error) + GetContainerLimitRangeItem(namespace string) (*core.LimitRangeItem, error) } type noopLimitsRangeCalculator struct{} -func (lc *noopLimitsRangeCalculator) GetContainerLimitRangeItem(namespace string) (*v1.LimitRangeItem, error) { +func (lc *noopLimitsRangeCalculator) GetContainerLimitRangeItem(namespace string) (*core.LimitRangeItem, error) { return nil, nil } @@ -64,19 +65,47 @@ func NewNoopLimitsCalculator() *noopLimitsRangeCalculator { return &noopLimitsRangeCalculator{} } -func (lc *limitsChecker) GetContainerLimitRangeItem(namespace string) (*v1.LimitRangeItem, error) { +func (lc *limitsChecker) GetContainerLimitRangeItem(namespace string) (*core.LimitRangeItem, error) { limitRanges, err := lc.limitRangeLister.LimitRanges(namespace).List(labels.Everything()) if err != nil { return nil, fmt.Errorf("error loading limit ranges: %s", err) } + updatedResult := func(result core.ResourceList, lrItem core.ResourceList, + resourceName core.ResourceName, comparator func(q1, q2 resource.Quantity) bool) core.ResourceList { + if lrItem == nil { + return result + } + if result == nil { + return lrItem.DeepCopy() + } + if lrResource, lrHas := lrItem[resourceName]; lrHas { + resultResource, resultHas := result[resourceName] + if !resultHas || comparator(resultResource, lrResource) { + result[resourceName] = lrResource + } + } + return result + } + maxComparator := func(q1, q2 resource.Quantity) bool { return q1.Cmp(q2) < 0 } + minComparator := func(q1, q2 resource.Quantity) bool { return q1.Cmp(q2) > 0 } + + result := &core.LimitRangeItem{Type: core.LimitTypeContainer} for _, lr := range limitRanges { for _, lri := range lr.Spec.Limits { - if lri.Type == v1.LimitTypeContainer && (lri.Max != nil || lri.Default != nil) { - // TODO: handle multiple limit ranges matching a pod. - return &lri, nil + if lri.Type == core.LimitTypeContainer && (lri.Max != nil || lri.Default != nil || lri.Min != nil) { + if lri.Default != nil { + result.Default = lri.Default + } + result.Max = updatedResult(result.Max, lri.Max, core.ResourceCPU, maxComparator) + result.Max = updatedResult(result.Max, lri.Max, core.ResourceMemory, maxComparator) + result.Min = updatedResult(result.Min, lri.Min, core.ResourceCPU, minComparator) + result.Min = updatedResult(result.Min, lri.Min, core.ResourceMemory, minComparator) } } } + if result.Min != nil || result.Max != nil || result.Default != nil { + return result, nil + } return nil, nil } diff --git a/vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator_test.go b/vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator_test.go index d7e65e4d0f6c..e9b1b05d4182 100644 --- a/vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator_test.go +++ b/vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator_test.go @@ -20,6 +20,7 @@ import ( "testing" apiv1 "k8s.io/api/core/v1" + core "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test" "k8s.io/client-go/informers" @@ -28,11 +29,11 @@ import ( "github.com/stretchr/testify/assert" ) -const defaultNamespace = "default" +const testNamespace = "test-namespace" func TestNewNoopLimitsChecker(t *testing.T) { nlc := NewNoopLimitsCalculator() - limitRange, err := nlc.GetContainerLimitRangeItem(defaultNamespace) + limitRange, err := nlc.GetContainerLimitRangeItem(testNamespace) assert.NoError(t, err) assert.Nil(t, limitRange) } @@ -43,15 +44,17 @@ func TestNoLimitRange(t *testing.T) { lc, err := NewLimitsRangeCalculator(factory) if assert.NoError(t, err) { - limitRange, err := lc.GetContainerLimitRangeItem(defaultNamespace) + limitRange, err := lc.GetContainerLimitRangeItem(testNamespace) assert.NoError(t, err) assert.Nil(t, limitRange) } } func TestGetContainerLimitRangeItem(t *testing.T) { - containerLimitRangeWithMax := test.LimitRange().WithName("default").WithNamespace(defaultNamespace).WithType(apiv1.LimitTypeContainer).WithMax(test.Resources("2", "2")).Get() - containerLimitRangeWithDefault := test.LimitRange().WithName("default").WithNamespace(defaultNamespace).WithType(apiv1.LimitTypeContainer).WithDefault(test.Resources("2", "2")).Get() + baseContainerLimitRange := test.LimitRange().WithName("test-lr").WithNamespace(testNamespace).WithType(apiv1.LimitTypeContainer) + containerLimitRangeWithMax := baseContainerLimitRange.WithMax(test.Resources("2", "2")).Get() + containerLimitRangeWithDefault := baseContainerLimitRange.WithDefault(test.Resources("2", "2")).Get() + containerLimitRangeWithMin := baseContainerLimitRange.WithMax(test.Resources("2", "2")).Get() testCases := []struct { name string limitRanges []runtime.Object @@ -62,7 +65,7 @@ func TestGetContainerLimitRangeItem(t *testing.T) { name: "no matching limit ranges", limitRanges: []runtime.Object{ test.LimitRange().WithName("different-namespace").WithNamespace("different").WithType(apiv1.LimitTypeContainer).WithMax(test.Resources("2", "2")).Get(), - test.LimitRange().WithName("differen-type").WithNamespace(defaultNamespace).WithType(apiv1.LimitTypePersistentVolumeClaim).WithMax(test.Resources("2", "2")).Get(), + test.LimitRange().WithName("different-type").WithNamespace(testNamespace).WithType(apiv1.LimitTypePersistentVolumeClaim).WithMax(test.Resources("2", "2")).Get(), }, expectedErr: nil, expectedLimits: nil, @@ -83,6 +86,28 @@ func TestGetContainerLimitRangeItem(t *testing.T) { expectedErr: nil, expectedLimits: &containerLimitRangeWithDefault.Spec.Limits[0], }, + { + name: "respects min", + limitRanges: []runtime.Object{ + containerLimitRangeWithMin, + }, + expectedErr: nil, + expectedLimits: &containerLimitRangeWithMin.Spec.Limits[0], + }, + { + name: "multiple items", + limitRanges: []runtime.Object{ + baseContainerLimitRange.WithMax(test.Resources("2", "2")).WithDefault(test.Resources("1.5", "1.5")). + WithMin(test.Resources("1", "1")).Get(), + }, + expectedErr: nil, + expectedLimits: &core.LimitRangeItem{ + Type: core.LimitTypeContainer, + Min: test.Resources("1", "1"), + Max: test.Resources("2", "2"), + Default: test.Resources("1.5", "1.5"), + }, + }, } for _, tc := range testCases { @@ -91,7 +116,7 @@ func TestGetContainerLimitRangeItem(t *testing.T) { factory := informers.NewSharedInformerFactory(cs, 0) lc, err := NewLimitsRangeCalculator(factory) if assert.NoError(t, err) { - limitRange, err := lc.GetContainerLimitRangeItem(defaultNamespace) + limitRange, err := lc.GetContainerLimitRangeItem(testNamespace) if tc.expectedErr == nil { assert.NoError(t, err) } else { diff --git a/vertical-pod-autoscaler/pkg/utils/test/test_limit_range.go b/vertical-pod-autoscaler/pkg/utils/test/test_limit_range.go index e62efe059bfe..af42481f4697 100644 --- a/vertical-pod-autoscaler/pkg/utils/test/test_limit_range.go +++ b/vertical-pod-autoscaler/pkg/utils/test/test_limit_range.go @@ -17,8 +17,8 @@ limitations under the License. package test import ( - "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + core "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) // LimitRange returns an object that helps build a LimitRangeItem object for tests. @@ -29,9 +29,10 @@ func LimitRange() *limitRangeBuilder { type limitRangeBuilder struct { namespace string name string - rangeType v1.LimitType - defaultValues *v1.ResourceList - max *v1.ResourceList + rangeType core.LimitType + defaultValues *core.ResourceList + max *core.ResourceList + min *core.ResourceList } func (lrb *limitRangeBuilder) WithName(name string) *limitRangeBuilder { @@ -46,47 +47,59 @@ func (lrb *limitRangeBuilder) WithNamespace(namespace string) *limitRangeBuilder return &result } -func (lrb *limitRangeBuilder) WithType(rangeType v1.LimitType) *limitRangeBuilder { +func (lrb *limitRangeBuilder) WithType(rangeType core.LimitType) *limitRangeBuilder { result := *lrb result.rangeType = rangeType return &result } -func (lrb *limitRangeBuilder) WithDefault(defaultValues v1.ResourceList) *limitRangeBuilder { +func (lrb *limitRangeBuilder) WithDefault(defaultValues core.ResourceList) *limitRangeBuilder { result := *lrb result.defaultValues = &defaultValues return &result } -func (lrb *limitRangeBuilder) WithMax(max v1.ResourceList) *limitRangeBuilder { +func (lrb *limitRangeBuilder) WithMax(max core.ResourceList) *limitRangeBuilder { result := *lrb result.max = &max return &result } -func (lrb *limitRangeBuilder) Get() *v1.LimitRange { - result := v1.LimitRange{ - ObjectMeta: metav1.ObjectMeta{ +func (lrb *limitRangeBuilder) WithMin(min core.ResourceList) *limitRangeBuilder { + result := *lrb + result.min = &min + return &result +} + +func (lrb *limitRangeBuilder) Get() *core.LimitRange { + result := core.LimitRange{ + ObjectMeta: meta.ObjectMeta{ Namespace: lrb.namespace, Name: lrb.name, }, } if lrb.defaultValues != nil || lrb.max != nil { - result.Spec = v1.LimitRangeSpec{ - Limits: []v1.LimitRangeItem{}, + result.Spec = core.LimitRangeSpec{ + Limits: []core.LimitRangeItem{}, } } if lrb.defaultValues != nil { - result.Spec.Limits = append(result.Spec.Limits, v1.LimitRangeItem{ + result.Spec.Limits = append(result.Spec.Limits, core.LimitRangeItem{ Type: lrb.rangeType, Default: *lrb.defaultValues, }) } if lrb.max != nil { - result.Spec.Limits = append(result.Spec.Limits, v1.LimitRangeItem{ + result.Spec.Limits = append(result.Spec.Limits, core.LimitRangeItem{ Type: lrb.rangeType, Max: *lrb.max, }) } + if lrb.min != nil { + result.Spec.Limits = append(result.Spec.Limits, core.LimitRangeItem{ + Type: lrb.rangeType, + Min: *lrb.min, + }) + } return &result }