Skip to content

Commit

Permalink
consider initContainers when calculating a pod's requests (#203)
Browse files Browse the repository at this point in the history
This commit also:
 - Standardises uses of resource.Quantity
 - Adds expectedNodeDelta to scale up tests

fixes #202
  • Loading branch information
awprice committed Jun 24, 2021
1 parent b2d8ed8 commit 1227a32
Show file tree
Hide file tree
Showing 7 changed files with 306 additions and 107 deletions.
41 changes: 29 additions & 12 deletions pkg/controller/controller_scale_node_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import (
"testing"
duration "time"

"github.com/atlassian/escalator/pkg/k8s/resource"
"github.com/atlassian/escalator/pkg/test"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
time "github.com/stephanos/clock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
)

type ListerOptions struct {
Expand Down Expand Up @@ -215,9 +215,10 @@ func TestScaleNodeGroup(t *testing.T) {
}

tests := []struct {
name string
args args
err error
name string
args args
expectedNodeDelta int
err error
}{
{
"100% cpu, 50% threshold",
Expand All @@ -233,6 +234,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
10,
nil,
},
{
Expand All @@ -249,6 +251,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
10,
nil,
},
{
Expand All @@ -265,6 +268,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
5,
nil,
},
{
Expand All @@ -281,6 +285,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
12,
nil,
},
{
Expand All @@ -297,6 +302,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
0,
nil,
},
{
Expand All @@ -313,6 +319,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
1,
nil,
},
{
Expand All @@ -327,6 +334,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
0,
errors.New("node count less than the minimum"),
},
{
Expand All @@ -341,6 +349,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
0,
errors.New("node count larger than the maximum"),
},
{
Expand All @@ -356,6 +365,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
0,
errors.New("cannot divide by zero in percent calculation"),
},
{
Expand All @@ -371,6 +381,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
0,
errors.New("cannot divide by zero in percent calculation"),
},
{
Expand All @@ -386,6 +397,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
0,
errors.New("cannot divide by zero in percent calculation"),
},
{
Expand All @@ -406,6 +418,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
},
},
0,
errors.New("unable to list pods"),
},
{
Expand All @@ -426,6 +439,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
},
},
0,
errors.New("unable to list nodes"),
},
{
Expand All @@ -442,6 +456,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
0,
nil,
},
{
Expand All @@ -458,6 +473,7 @@ func TestScaleNodeGroup(t *testing.T) {
},
ListerOptions{},
},
38,
nil,
},
}
Expand Down Expand Up @@ -507,6 +523,7 @@ func TestScaleNodeGroup(t *testing.T) {
require.EqualError(t, tt.err, err.Error())
}

assert.Equal(t, tt.expectedNodeDelta, nodesDelta)
if nodesDelta <= 0 {
return
}
Expand Down Expand Up @@ -542,7 +559,7 @@ func TestScaleNodeGroup_MultipleRuns(t *testing.T) {
nodeGroupOptions NodeGroupOptions
listerOptions ListerOptions
}
var defaultNodeCPUCapaity int64 = 2000
var defaultNodeCPUCapacity int64 = 2000
var defaultNodeMemCapacity int64 = 8000

tests := []struct {
Expand All @@ -557,7 +574,7 @@ func TestScaleNodeGroup_MultipleRuns(t *testing.T) {
{
"10 nodes, 0 pods, min nodes 5, fast node removal",
args{
buildTestNodes(10, defaultNodeCPUCapaity, defaultNodeMemCapacity),
buildTestNodes(10, defaultNodeCPUCapacity, defaultNodeMemCapacity),
buildTestPods(0, 0, 0),
NodeGroupOptions{
Name: "default",
Expand All @@ -583,7 +600,7 @@ func TestScaleNodeGroup_MultipleRuns(t *testing.T) {
{
"10 nodes, 10 pods, slow node removal",
args{
buildTestNodes(10, defaultNodeCPUCapaity, defaultNodeMemCapacity),
buildTestNodes(10, defaultNodeCPUCapacity, defaultNodeMemCapacity),
buildTestPods(10, 1000, 1000),
NodeGroupOptions{
Name: "default",
Expand All @@ -609,7 +626,7 @@ func TestScaleNodeGroup_MultipleRuns(t *testing.T) {
{
"4 nodes, 0 pods, min nodes 0, fast node removal to scale down to 0",
args{
buildTestNodes(4, defaultNodeCPUCapaity, defaultNodeMemCapacity),
buildTestNodes(4, defaultNodeCPUCapacity, defaultNodeMemCapacity),
buildTestPods(0, 0, 0),
NodeGroupOptions{
Name: "default",
Expand All @@ -635,7 +652,7 @@ func TestScaleNodeGroup_MultipleRuns(t *testing.T) {
{
"0 nodes, 10 pods, min nodes 0, scale up from 0 without cache",
args{
buildTestNodes(0, defaultNodeCPUCapaity, defaultNodeMemCapacity),
buildTestNodes(0, defaultNodeCPUCapacity, defaultNodeMemCapacity),
buildTestPods(40, 200, 800),
NodeGroupOptions{
Name: "default",
Expand All @@ -662,7 +679,7 @@ func TestScaleNodeGroup_MultipleRuns(t *testing.T) {
{
"0 nodes, 10 pods, min nodes 0, scale up from 0 with cache",
args{
buildTestNodes(0, defaultNodeCPUCapaity, defaultNodeMemCapacity),
buildTestNodes(0, defaultNodeCPUCapacity, defaultNodeMemCapacity),
buildTestPods(40, 200, 800),
NodeGroupOptions{
Name: "default",
Expand Down Expand Up @@ -717,8 +734,8 @@ func TestScaleNodeGroup_MultipleRuns(t *testing.T) {
// add cached node allocatable capacity when configured
if tt.scaleUpWithCachedCapacity {
defaultNodeGroupState := nodeGroupsState[tt.args.nodeGroupOptions.Name]
defaultNodeGroupState.cpuCapacity = *resource.NewMilliQuantity(defaultNodeCPUCapaity, resource.DecimalSI)
defaultNodeGroupState.memCapacity = *resource.NewQuantity(defaultNodeMemCapacity, resource.DecimalSI)
defaultNodeGroupState.cpuCapacity = *resource.NewCPUQuantity(defaultNodeCPUCapacity)
defaultNodeGroupState.memCapacity = *resource.NewMemoryQuantity(defaultNodeMemCapacity)
nodeGroupsState[tt.args.nodeGroupOptions.Name] = defaultNodeGroupState
}

Expand Down
57 changes: 31 additions & 26 deletions pkg/controller/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import (
"testing"

"github.com/atlassian/escalator/pkg/k8s"
"github.com/atlassian/escalator/pkg/k8s/resource"
"github.com/atlassian/escalator/pkg/test"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
)

func TestCalcScaleUpDeltaBelowThreshold(t *testing.T) {
Expand Down Expand Up @@ -203,10 +203,10 @@ func calculatePercentageUsage(pods []*v1.Pod, nodes []*v1.Node) (float64, float6

func TestCalcPercentUsage(t *testing.T) {
type args struct {
cpuRequest resource.Quantity
memRequest resource.Quantity
cpuCapacity resource.Quantity
memCapacity resource.Quantity
cpuRequest int64
memRequest int64
cpuCapacity int64
memCapacity int64
numberOfUntaintedNodes int64
}
tests := []struct {
Expand All @@ -219,10 +219,10 @@ func TestCalcPercentUsage(t *testing.T) {
{
"basic test",
args{
*resource.NewMilliQuantity(50, resource.DecimalSI),
*resource.NewQuantity(50, resource.DecimalSI),
*resource.NewMilliQuantity(100, resource.DecimalSI),
*resource.NewQuantity(100, resource.DecimalSI),
50,
50,
100,
100,
1,
},
50,
Expand All @@ -232,10 +232,10 @@ func TestCalcPercentUsage(t *testing.T) {
{
"divide by zero test",
args{
*resource.NewMilliQuantity(50, resource.DecimalSI),
*resource.NewQuantity(50, resource.DecimalSI),
*resource.NewMilliQuantity(0, resource.DecimalSI),
*resource.NewQuantity(0, resource.DecimalSI),
50,
50,
0,
0,
10,
},
0,
Expand All @@ -245,10 +245,10 @@ func TestCalcPercentUsage(t *testing.T) {
{
"no pods request while number of nodes is not 0",
args{
*resource.NewMilliQuantity(0, resource.DecimalSI),
*resource.NewQuantity(0, resource.DecimalSI),
*resource.NewMilliQuantity(0, resource.DecimalSI),
*resource.NewQuantity(0, resource.DecimalSI),
0,
0,
0,
0,
1,
},
0,
Expand All @@ -258,10 +258,10 @@ func TestCalcPercentUsage(t *testing.T) {
{
"zero numerator test",
args{
*resource.NewMilliQuantity(0, resource.DecimalSI),
*resource.NewQuantity(0, resource.DecimalSI),
*resource.NewMilliQuantity(66, resource.DecimalSI),
*resource.NewQuantity(66, resource.DecimalSI),
0,
0,
66,
66,
1,
},
0,
Expand All @@ -271,10 +271,10 @@ func TestCalcPercentUsage(t *testing.T) {
{
"zero all test",
args{
*resource.NewMilliQuantity(0, resource.DecimalSI),
*resource.NewQuantity(0, resource.DecimalSI),
*resource.NewMilliQuantity(0, resource.DecimalSI),
*resource.NewQuantity(0, resource.DecimalSI),
0,
0,
0,
0,
0,
},
0,
Expand All @@ -284,7 +284,12 @@ func TestCalcPercentUsage(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cpu, mem, err := calcPercentUsage(tt.args.cpuRequest, tt.args.memRequest, tt.args.cpuCapacity, tt.args.memCapacity, tt.args.numberOfUntaintedNodes)
cpuRequest := *resource.NewCPUQuantity(tt.args.cpuRequest)
cpuCapacity := *resource.NewCPUQuantity(tt.args.cpuCapacity)
memRequest := *resource.NewMemoryQuantity(tt.args.memRequest)
memCapacity := *resource.NewMemoryQuantity(tt.args.memCapacity)

cpu, mem, err := calcPercentUsage(cpuRequest, memRequest, cpuCapacity, memCapacity, tt.args.numberOfUntaintedNodes)
if tt.err == nil {
require.NoError(t, err)
} else {
Expand Down
17 changes: 17 additions & 0 deletions pkg/k8s/resource/quantity.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package resource

import (
"k8s.io/apimachinery/pkg/api/resource"
)

func NewMemoryQuantity(value int64) *resource.Quantity {
return resource.NewQuantity(value, resource.BinarySI)
}

func NewCPUQuantity(value int64) *resource.Quantity {
return resource.NewMilliQuantity(value, resource.DecimalSI)
}

func NewPodQuantity(value int64) *resource.Quantity {
return resource.NewQuantity(value, resource.DecimalSI)
}
Loading

0 comments on commit 1227a32

Please sign in to comment.