Skip to content

Commit

Permalink
Support multiple limitrange items
Browse files Browse the repository at this point in the history
  • Loading branch information
@bskiba
bskiba committed May 30, 2019
1 parent 63039fb commit 4e815ff
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 29 deletions.
43 changes: 36 additions & 7 deletions vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
39 changes: 32 additions & 7 deletions vertical-pod-autoscaler/pkg/utils/limitrange/limit_range_calculator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
43 changes: 28 additions & 15 deletions vertical-pod-autoscaler/pkg/utils/test/test_limit_range.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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
}

0 comments on commit 4e815ff

Please sign in to comment.