Skip to content

Commit

Permalink
Improve stability if the taint_manager tests
Browse files Browse the repository at this point in the history
  • Loading branch information
mimowo committed Nov 13, 2022
1 parent 8e48df1 commit 3b5c3ac
Showing 1 changed file with 32 additions and 34 deletions.
66 changes: 32 additions & 34 deletions pkg/controller/nodelifecycle/scheduler/taint_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
Expand Down Expand Up @@ -206,8 +207,6 @@ func TestCreatePod(t *testing.T) {

podIndexer.Add(item.pod)
controller.PodUpdated(nil, item.pod)
// wait a bit
time.Sleep(timeForControllerToProgress)

verifyPodActions(t, item.description, fakeClientset, item.expectPatch, item.expectDelete)

Expand Down Expand Up @@ -240,7 +239,6 @@ func TestUpdatePod(t *testing.T) {
taintedNodes map[string][]v1.Taint
expectPatch bool
expectDelete bool
additionalSleep time.Duration
enablePodDisruptionConditions bool
}{
{
Expand Down Expand Up @@ -288,8 +286,7 @@ func TestUpdatePod(t *testing.T) {
taintedNodes: map[string][]v1.Taint{
"node1": {createNoExecuteTaint(1)},
},
expectDelete: true,
additionalSleep: 1500 * time.Millisecond,
expectDelete: true,
},
}

Expand All @@ -300,21 +297,16 @@ func TestUpdatePod(t *testing.T) {
fakeClientset := fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*item.prevPod}})
controller, podIndexer, _ := setupNewNoExecuteTaintManager(context.TODO(), fakeClientset)
controller.recorder = testutil.NewFakeRecorder()
go controller.Run(ctx)
controller.taintedNodes = item.taintedNodes
go controller.Run(ctx)

podIndexer.Add(item.prevPod)
controller.PodUpdated(nil, item.prevPod)

fakeClientset.ClearActions()
time.Sleep(timeForControllerToProgress)

podIndexer.Update(item.newPod)
controller.PodUpdated(item.prevPod, item.newPod)
// wait a bit
time.Sleep(timeForControllerToProgress)
if item.additionalSleep > 0 {
time.Sleep(item.additionalSleep)
}

verifyPodActions(t, item.description, fakeClientset, item.expectPatch, item.expectDelete)
cancel()
Expand Down Expand Up @@ -367,8 +359,6 @@ func TestCreateNode(t *testing.T) {
controller.recorder = testutil.NewFakeRecorder()
go controller.Run(ctx)
controller.NodeUpdated(nil, item.node)
// wait a bit
time.Sleep(timeForControllerToProgress)

verifyPodActions(t, item.description, fakeClientset, item.expectPatch, item.expectDelete)

Expand All @@ -386,13 +376,17 @@ func TestDeleteNode(t *testing.T) {
}
go controller.Run(ctx)
controller.NodeUpdated(testutil.NewNode("node1"), nil)
// wait a bit to see if nothing will panic
time.Sleep(timeForControllerToProgress)
controller.taintedNodesLock.Lock()
if _, ok := controller.taintedNodes["node1"]; ok {
t.Error("Node should have been deleted from taintedNodes list")

// await until controller.taintedNodes is empty
err := wait.PollImmediate(10*time.Millisecond, time.Second, func() (bool, error) {
controller.taintedNodesLock.Lock()
defer controller.taintedNodesLock.Unlock()
_, ok := controller.taintedNodes["node1"]
return !ok, nil
})
if err != nil {
t.Errorf("Failed to await for processing node deleted: %q", err)
}
controller.taintedNodesLock.Unlock()
cancel()
}

Expand Down Expand Up @@ -480,10 +474,9 @@ func TestUpdateNode(t *testing.T) {
},
},
},
oldNode: testutil.NewNode("node1"),
newNode: addTaintsToNode(testutil.NewNode("node1"), "testTaint1", "taint1", []int{1, 2}),
expectDelete: true,
additionalSleep: 1500 * time.Millisecond,
oldNode: testutil.NewNode("node1"),
newNode: addTaintsToNode(testutil.NewNode("node1"), "testTaint1", "taint1", []int{1, 2}),
expectDelete: true,
},
}

Expand All @@ -499,8 +492,7 @@ func TestUpdateNode(t *testing.T) {
controller.recorder = testutil.NewFakeRecorder()
go controller.Run(ctx)
controller.NodeUpdated(item.oldNode, item.newNode)
// wait a bit
time.Sleep(timeForControllerToProgress)

if item.additionalSleep > 0 {
time.Sleep(item.additionalSleep)
}
Expand Down Expand Up @@ -841,8 +833,6 @@ func TestEventualConsistency(t *testing.T) {

// First we simulate NodeUpdate that should delete 'pod1'. It doesn't know about 'pod2' yet.
controller.NodeUpdated(item.oldNode, item.newNode)
// TODO(mborsz): Remove this sleep and other sleeps in this file.
time.Sleep(timeForControllerToProgress)

verifyPodActions(t, item.description, fakeClientset, item.expectPatch, item.expectDelete)
fakeClientset.ClearActions()
Expand All @@ -860,13 +850,21 @@ func verifyPodActions(t *testing.T, description string, fakeClientset *fake.Clie
t.Helper()
podPatched := false
podDeleted := false
for _, action := range fakeClientset.Actions() {
if action.GetVerb() == "patch" && action.GetResource().Resource == "pods" {
podPatched = true
}
if action.GetVerb() == "delete" && action.GetResource().Resource == "pods" {
podDeleted = true
// use Poll instead of PollImmediate to give some processing time to the controller that the expected
// actions are likely to be already sent
err := wait.Poll(10*time.Millisecond, 5*time.Second, func() (bool, error) {
for _, action := range fakeClientset.Actions() {
if action.GetVerb() == "patch" && action.GetResource().Resource == "pods" {
podPatched = true
}
if action.GetVerb() == "delete" && action.GetResource().Resource == "pods" {
podDeleted = true
}
}
return podPatched == expectPatch && podDeleted == expectDelete, nil
})
if err != nil {
t.Errorf("Failed waiting for the expected actions: %q", err)
}
if podPatched != expectPatch {
t.Errorf("[%v]Unexpected test result. Expected patch %v, got %v", description, expectPatch, podPatched)
Expand Down

0 comments on commit 3b5c3ac

Please sign in to comment.