From c0d0f5421dd199e54c63f280f39286077de14d79 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 5 Aug 2020 11:35:36 -0400 Subject: [PATCH] Do not evict pods which tolerate all NoExecute taints --- pkg/controller/nodelifecycle/scheduler/BUILD | 1 + .../nodelifecycle/scheduler/taint_manager.go | 3 +- .../scheduler/taint_manager_test.go | 87 ++++++++++++++++++- 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/pkg/controller/nodelifecycle/scheduler/BUILD b/pkg/controller/nodelifecycle/scheduler/BUILD index ed3be9c03f7f..18d312a8ae95 100644 --- a/pkg/controller/nodelifecycle/scheduler/BUILD +++ b/pkg/controller/nodelifecycle/scheduler/BUILD @@ -42,6 +42,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/testing:go_default_library", diff --git a/pkg/controller/nodelifecycle/scheduler/taint_manager.go b/pkg/controller/nodelifecycle/scheduler/taint_manager.go index d3eaab630499..c9bbfb79fb64 100644 --- a/pkg/controller/nodelifecycle/scheduler/taint_manager.go +++ b/pkg/controller/nodelifecycle/scheduler/taint_manager.go @@ -357,7 +357,8 @@ func (tc *NoExecuteTaintManager) processPodOnNode( minTolerationTime := getMinTolerationTime(usedTolerations) // getMinTolerationTime returns negative value to denote infinite toleration. if minTolerationTime < 0 { - klog.V(4).Infof("New tolerations for %v tolerate forever. Scheduled deletion won't be cancelled if already scheduled.", podNamespacedName.String()) + klog.V(4).Infof("Current tolerations for %v tolerate forever, cancelling any scheduled deletion.", podNamespacedName.String()) + tc.cancelWorkWithEvent(podNamespacedName) return } diff --git a/pkg/controller/nodelifecycle/scheduler/taint_manager_test.go b/pkg/controller/nodelifecycle/scheduler/taint_manager_test.go index 6bff92e803a2..e6de4ea11d35 100644 --- a/pkg/controller/nodelifecycle/scheduler/taint_manager_test.go +++ b/pkg/controller/nodelifecycle/scheduler/taint_manager_test.go @@ -26,6 +26,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" "k8s.io/kubernetes/pkg/controller/testutil" @@ -82,10 +83,20 @@ func (p *podHolder) setPod(pod *v1.Pod) { } type nodeHolder struct { + lock sync.Mutex + node *v1.Node } +func (n *nodeHolder) setNode(node *v1.Node) { + n.lock.Lock() + defer n.lock.Unlock() + n.node = node +} + func (n *nodeHolder) getNode(name string) (*v1.Node, error) { + n.lock.Lock() + defer n.lock.Unlock() return n.node, nil } @@ -361,7 +372,7 @@ func TestCreateNode(t *testing.T) { for _, item := range testCases { stopCh := make(chan struct{}) fakeClientset := fake.NewSimpleClientset(&v1.PodList{Items: item.pods}) - controller := NewNoExecuteTaintManager(fakeClientset, getPodFromClientset(fakeClientset), (&nodeHolder{item.node}).getNode, getPodsAssignedToNode(fakeClientset)) + controller := NewNoExecuteTaintManager(fakeClientset, getPodFromClientset(fakeClientset), (&nodeHolder{node: item.node}).getNode, getPodsAssignedToNode(fakeClientset)) controller.recorder = testutil.NewFakeRecorder() go controller.Run(stopCh) controller.NodeUpdated(nil, item.node) @@ -482,7 +493,7 @@ func TestUpdateNode(t *testing.T) { for _, item := range testCases { stopCh := make(chan struct{}) fakeClientset := fake.NewSimpleClientset(&v1.PodList{Items: item.pods}) - controller := NewNoExecuteTaintManager(fakeClientset, getPodFromClientset(fakeClientset), (&nodeHolder{item.newNode}).getNode, getPodsAssignedToNode(fakeClientset)) + controller := NewNoExecuteTaintManager(fakeClientset, getPodFromClientset(fakeClientset), (&nodeHolder{node: item.newNode}).getNode, getPodsAssignedToNode(fakeClientset)) controller.recorder = testutil.NewFakeRecorder() go controller.Run(stopCh) controller.NodeUpdated(item.oldNode, item.newNode) @@ -505,6 +516,74 @@ func TestUpdateNode(t *testing.T) { } } +func TestUpdateNodeWithMultipleTaints(t *testing.T) { + taint1 := createNoExecuteTaint(1) + taint2 := createNoExecuteTaint(2) + + minute := int64(60) + pod := testutil.NewPod("pod1", "node1") + pod.Spec.Tolerations = []v1.Toleration{ + {Key: taint1.Key, Operator: v1.TolerationOpExists, Effect: v1.TaintEffectNoExecute}, + {Key: taint2.Key, Operator: v1.TolerationOpExists, Effect: v1.TaintEffectNoExecute, TolerationSeconds: &minute}, + } + podNamespacedName := types.NamespacedName{Namespace: pod.Namespace, Name: pod.Name} + + untaintedNode := testutil.NewNode("node1") + + doubleTaintedNode := testutil.NewNode("node1") + doubleTaintedNode.Spec.Taints = []v1.Taint{taint1, taint2} + + singleTaintedNode := testutil.NewNode("node1") + singleTaintedNode.Spec.Taints = []v1.Taint{taint1} + + stopCh := make(chan struct{}) + fakeClientset := fake.NewSimpleClientset(pod) + holder := &nodeHolder{node: untaintedNode} + controller := NewNoExecuteTaintManager(fakeClientset, getPodFromClientset(fakeClientset), (holder).getNode, getPodsAssignedToNode(fakeClientset)) + controller.recorder = testutil.NewFakeRecorder() + go controller.Run(stopCh) + + // no taint + holder.setNode(untaintedNode) + controller.handleNodeUpdate(nodeUpdateItem{"node1"}) + // verify pod is not queued for deletion + if controller.taintEvictionQueue.GetWorkerUnsafe(podNamespacedName.String()) != nil { + t.Fatalf("pod queued for deletion with no taints") + } + + // no taint -> infinitely tolerated taint + holder.setNode(singleTaintedNode) + controller.handleNodeUpdate(nodeUpdateItem{"node1"}) + // verify pod is not queued for deletion + if controller.taintEvictionQueue.GetWorkerUnsafe(podNamespacedName.String()) != nil { + t.Fatalf("pod queued for deletion with permanently tolerated taint") + } + + // infinitely tolerated taint -> temporarily tolerated taint + holder.setNode(doubleTaintedNode) + controller.handleNodeUpdate(nodeUpdateItem{"node1"}) + // verify pod is queued for deletion + if controller.taintEvictionQueue.GetWorkerUnsafe(podNamespacedName.String()) == nil { + t.Fatalf("pod not queued for deletion after addition of temporarily tolerated taint") + } + + // temporarily tolerated taint -> infinitely tolerated taint + holder.setNode(singleTaintedNode) + controller.handleNodeUpdate(nodeUpdateItem{"node1"}) + // verify pod is not queued for deletion + if controller.taintEvictionQueue.GetWorkerUnsafe(podNamespacedName.String()) != nil { + t.Fatalf("pod queued for deletion after removal of temporarily tolerated taint") + } + + // verify pod is not deleted + for _, action := range fakeClientset.Actions() { + if action.GetVerb() == "delete" && action.GetResource().Resource == "pods" { + t.Error("Unexpected deletion") + } + } + close(stopCh) +} + func TestUpdateNodeWithMultiplePods(t *testing.T) { testCases := []struct { description string @@ -548,7 +627,7 @@ func TestUpdateNodeWithMultiplePods(t *testing.T) { stopCh := make(chan struct{}) fakeClientset := fake.NewSimpleClientset(&v1.PodList{Items: item.pods}) sort.Sort(item.expectedDeleteTimes) - controller := NewNoExecuteTaintManager(fakeClientset, getPodFromClientset(fakeClientset), (&nodeHolder{item.newNode}).getNode, getPodsAssignedToNode(fakeClientset)) + controller := NewNoExecuteTaintManager(fakeClientset, getPodFromClientset(fakeClientset), (&nodeHolder{node: item.newNode}).getNode, getPodsAssignedToNode(fakeClientset)) controller.recorder = testutil.NewFakeRecorder() go controller.Run(stopCh) controller.NodeUpdated(item.oldNode, item.newNode) @@ -748,7 +827,7 @@ func TestEventualConsistency(t *testing.T) { stopCh := make(chan struct{}) fakeClientset := fake.NewSimpleClientset(&v1.PodList{Items: item.pods}) holder := &podHolder{} - controller := NewNoExecuteTaintManager(fakeClientset, holder.getPod, (&nodeHolder{item.newNode}).getNode, getPodsAssignedToNode(fakeClientset)) + controller := NewNoExecuteTaintManager(fakeClientset, holder.getPod, (&nodeHolder{node: item.newNode}).getNode, getPodsAssignedToNode(fakeClientset)) controller.recorder = testutil.NewFakeRecorder() go controller.Run(stopCh)