Skip to content

Commit

Permalink
Add tests to autoscaler.runOnce for unknown pods
Browse files Browse the repository at this point in the history
  • Loading branch information
@atwamahmoud
atwamahmoud committed Oct 18, 2023
1 parent fd84689 commit d67ad22
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 22 deletions.
50 changes: 29 additions & 21 deletions cluster-autoscaler/core/static_autoscaler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,14 @@ func TestStaticAutoscalerRunOnce(t *testing.T) {
n2 := BuildTestNode("n2", 1000, 1000)
SetNodeReadyState(n2, true, time.Now())
n3 := BuildTestNode("n3", 1000, 1000)
SetNodeReadyState(n3, true, time.Now())
n4 := BuildTestNode("n4", 1000, 1000)
n5 := BuildTestNode("n5", 1000, 1000)

p1 := BuildTestPod("p1", 600, 100)
p1.Spec.NodeName = "n1"
p2 := BuildTestPod("p2", 600, 100, MarkUnschedulable())
p3 := BuildTestPod("p3", 600, 100) // Not yet processed by scheduler

tn := BuildTestNode("tn", 1000, 1000)
tni := schedulerframework.NewNodeInfo()
Expand Down Expand Up @@ -248,7 +251,7 @@ func TestStaticAutoscalerRunOnce(t *testing.T) {
// MaxNodesTotal reached.
readyNodeLister.SetNodes([]*apiv1.Node{n1})
allNodeLister.SetNodes([]*apiv1.Node{n1})
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2}, nil).Twice()
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2, p3}, nil).Twice()
daemonSetListerMock.On("List", labels.Everything()).Return([]*appsv1.DaemonSet{}, nil).Once()
podDisruptionBudgetListerMock.On("List").Return([]*policyv1.PodDisruptionBudget{}, nil).Once()

Expand All @@ -259,10 +262,10 @@ func TestStaticAutoscalerRunOnce(t *testing.T) {
// Scale up.
readyNodeLister.SetNodes([]*apiv1.Node{n1})
allNodeLister.SetNodes([]*apiv1.Node{n1})
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2}, nil).Twice()
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2, p3}, nil).Twice()
daemonSetListerMock.On("List", labels.Everything()).Return([]*appsv1.DaemonSet{}, nil).Once()
podDisruptionBudgetListerMock.On("List").Return([]*policyv1.PodDisruptionBudget{}, nil).Once()
onScaleUpMock.On("ScaleUp", "ng1", 1).Return(nil).Once()
onScaleUpMock.On("ScaleUp", "ng1", 2).Return(nil).Once()

context.MaxNodesTotal = 10
err = autoscaler.RunOnce(time.Now().Add(time.Hour))
Expand Down Expand Up @@ -302,24 +305,24 @@ func TestStaticAutoscalerRunOnce(t *testing.T) {
// Mark unregistered nodes.
readyNodeLister.SetNodes([]*apiv1.Node{n1, n2})
allNodeLister.SetNodes([]*apiv1.Node{n1, n2})
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2}, nil).Twice()
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2, p3}, nil).Twice()
daemonSetListerMock.On("List", labels.Everything()).Return([]*appsv1.DaemonSet{}, nil).Once()
podDisruptionBudgetListerMock.On("List").Return([]*policyv1.PodDisruptionBudget{}, nil).Once()

provider.AddNodeGroup("ng2", 0, 10, 1)
provider.AddNode("ng2", n3)
provider.AddNode("ng2", n4)

err = autoscaler.RunOnce(time.Now().Add(4 * time.Hour))
assert.NoError(t, err)
mock.AssertExpectationsForObjects(t, allPodListerMock,
podDisruptionBudgetListerMock, daemonSetListerMock, onScaleUpMock, onScaleDownMock)

// Remove unregistered nodes.
readyNodeLister.SetNodes([]*apiv1.Node{n1, n2})
allNodeLister.SetNodes([]*apiv1.Node{n1, n2})
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2}, nil).Twice()
readyNodeLister.SetNodes([]*apiv1.Node{n1, n2, n3})
allNodeLister.SetNodes([]*apiv1.Node{n1, n2, n3})
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2, p3}, nil).Twice()
daemonSetListerMock.On("List", labels.Everything()).Return([]*appsv1.DaemonSet{}, nil).Once()
onScaleDownMock.On("ScaleDown", "ng2", "n3").Return(nil).Once()
onScaleDownMock.On("ScaleDown", "ng2", "n4").Return(nil).Once()
podDisruptionBudgetListerMock.On("List").Return([]*policyv1.PodDisruptionBudget{}, nil).Once()

err = autoscaler.RunOnce(time.Now().Add(5 * time.Hour))
Expand All @@ -329,15 +332,15 @@ func TestStaticAutoscalerRunOnce(t *testing.T) {
podDisruptionBudgetListerMock, daemonSetListerMock, onScaleUpMock, onScaleDownMock)

// Scale up to node gorup min size.
readyNodeLister.SetNodes([]*apiv1.Node{n4})
allNodeLister.SetNodes([]*apiv1.Node{n4})
readyNodeLister.SetNodes([]*apiv1.Node{n5})
allNodeLister.SetNodes([]*apiv1.Node{n5})
allPodListerMock.On("List").Return([]*apiv1.Pod{}, nil).Twice()
daemonSetListerMock.On("List", labels.Everything()).Return([]*appsv1.DaemonSet{}, nil)
podDisruptionBudgetListerMock.On("List").Return([]*policyv1.PodDisruptionBudget{}, nil)
onScaleUpMock.On("ScaleUp", "ng3", 2).Return(nil).Once() // 2 new nodes are supposed to be scaled up.

provider.AddNodeGroup("ng3", 3, 10, 1)
provider.AddNode("ng3", n4)
provider.AddNode("ng3", n5)

err = autoscaler.RunOnce(time.Now().Add(5 * time.Hour))
assert.NoError(t, err)
Expand Down Expand Up @@ -366,6 +369,7 @@ func TestStaticAutoscalerRunOnceWithAutoprovisionedEnabled(t *testing.T) {
p1 := BuildTestPod("p1", 100, 100)
p1.Spec.NodeName = "n1"
p2 := BuildTestPod("p2", 600, 100, MarkUnschedulable())
p3 := BuildTestPod("p3", 600, 100) // Not yet processed by scheduler

tn1 := BuildTestNode("tn1", 100, 1000)
SetNodeReadyState(tn1, true, time.Now())
Expand Down Expand Up @@ -457,11 +461,11 @@ func TestStaticAutoscalerRunOnceWithAutoprovisionedEnabled(t *testing.T) {
// Scale up.
readyNodeLister.SetNodes([]*apiv1.Node{n1})
allNodeLister.SetNodes([]*apiv1.Node{n1})
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2}, nil).Twice()
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2, p3}, nil).Twice()
podDisruptionBudgetListerMock.On("List").Return([]*policyv1.PodDisruptionBudget{}, nil).Once()
daemonSetListerMock.On("List", labels.Everything()).Return([]*appsv1.DaemonSet{}, nil).Once()
onNodeGroupCreateMock.On("Create", "autoprovisioned-TN2").Return(nil).Once()
onScaleUpMock.On("ScaleUp", "autoprovisioned-TN2", 1).Return(nil).Once()
onScaleUpMock.On("ScaleUp", "autoprovisioned-TN2", 2).Return(nil).Once()

err = autoscaler.RunOnce(time.Now().Add(time.Hour))
assert.NoError(t, err)
Expand Down Expand Up @@ -521,10 +525,13 @@ func TestStaticAutoscalerRunOnceWithALongUnregisteredNode(t *testing.T) {
SetNodeReadyState(n1, true, time.Now())
n2 := BuildTestNode("n2", 1000, 1000)
SetNodeReadyState(n2, true, time.Now())
n3 := BuildTestNode("n3", 1000, 1000)
SetNodeReadyState(n3, true, time.Now())

p1 := BuildTestPod("p1", 600, 100)
p1.Spec.NodeName = "n1"
p2 := BuildTestPod("p2", 600, 100, MarkUnschedulable())
p3 := BuildTestPod("p3", 600, 100)

provider := testprovider.NewTestCloudProvider(
func(id string, delta int) error {
Expand All @@ -534,7 +541,7 @@ func TestStaticAutoscalerRunOnceWithALongUnregisteredNode(t *testing.T) {
deleteFinished <- true
return ret
})
provider.AddNodeGroup("ng1", 2, 10, 2)
provider.AddNodeGroup("ng1", 3, 10, 2)
provider.AddNode("ng1", n1)

// broken node, that will be just hanging out there during
Expand Down Expand Up @@ -606,10 +613,10 @@ func TestStaticAutoscalerRunOnceWithALongUnregisteredNode(t *testing.T) {
// Scale up.
readyNodeLister.SetNodes([]*apiv1.Node{n1})
allNodeLister.SetNodes([]*apiv1.Node{n1})
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2}, nil).Twice()
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2, p3}, nil).Twice()
daemonSetListerMock.On("List", labels.Everything()).Return([]*appsv1.DaemonSet{}, nil).Once()
podDisruptionBudgetListerMock.On("List").Return([]*policyv1.PodDisruptionBudget{}, nil).Once()
onScaleUpMock.On("ScaleUp", "ng1", 1).Return(nil).Once()
onScaleUpMock.On("ScaleUp", "ng1", 2).Return(nil).Once()

err = autoscaler.RunOnce(later.Add(time.Hour))
assert.NoError(t, err)
Expand All @@ -618,11 +625,12 @@ func TestStaticAutoscalerRunOnceWithALongUnregisteredNode(t *testing.T) {

// Remove broken node after going over min size
provider.AddNode("ng1", n2)
ng1.SetTargetSize(3)
provider.AddNode("ng1", n3)
ng1.SetTargetSize(4)

readyNodeLister.SetNodes([]*apiv1.Node{n1, n2})
allNodeLister.SetNodes([]*apiv1.Node{n1, n2})
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2}, nil).Twice()
readyNodeLister.SetNodes([]*apiv1.Node{n1, n2, n3})
allNodeLister.SetNodes([]*apiv1.Node{n1, n2, n3})
allPodListerMock.On("List").Return([]*apiv1.Pod{p1, p2, p3}, nil).Twice()
onScaleDownMock.On("ScaleDown", "ng1", "broken").Return(nil).Once()
daemonSetListerMock.On("List", labels.Everything()).Return([]*appsv1.DaemonSet{}, nil).Once()
podDisruptionBudgetListerMock.On("List").Return([]*policyv1.PodDisruptionBudget{}, nil).Once()
Expand Down
2 changes: 1 addition & 1 deletion cluster-autoscaler/utils/kubernetes/listers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func UnknownPods(allPods []*apiv1.Pod) []*apiv1.Pod {
}
// Make sure it's not unschedulable
_, condition := podv1.GetPodCondition(&pod.Status, apiv1.PodScheduled)
if condition == nil || (condition.Status == apiv1.ConditionFalse && condition.Reason != apiv1.PodReasonUnschedulable) {
if condition == nil || (condition.Status == apiv1.ConditionFalse && condition.Reason == "") {
unknownPods = append(unknownPods, pod)
}
}
Expand Down

0 comments on commit d67ad22

Please sign in to comment.