From 2b311fefba9cc106ed1a8cebc6c61e32814fd9e5 Mon Sep 17 00:00:00 2001 From: Guangya Liu Date: Sun, 28 May 2017 11:26:57 +0800 Subject: [PATCH] Do not fire InsufficientResourceError when there are intentional reasons. --- pkg/controller/daemon/daemoncontroller.go | 44 +++++++++++++------ .../daemon/daemoncontroller_test.go | 43 ++++++++++++++++-- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/pkg/controller/daemon/daemoncontroller.go b/pkg/controller/daemon/daemoncontroller.go index 71e35e31bb6..7b52b6210b2 100644 --- a/pkg/controller/daemon/daemoncontroller.go +++ b/pkg/controller/daemon/daemoncontroller.go @@ -1016,6 +1016,30 @@ func (dsc *DaemonSetsController) syncDaemonSet(key string) error { return dsc.updateDaemonSetStatus(ds) } +// hasIntentionalPredicatesReasons checks if any of the given predicate failure reasons +// is intentional. +func hasIntentionalPredicatesReasons(reasons []algorithm.PredicateFailureReason) bool { + for _, r := range reasons { + switch reason := r.(type) { + case *predicates.PredicateFailureError: + switch reason { + // intentional + case + predicates.ErrNodeSelectorNotMatch, + predicates.ErrPodNotMatchHostName, + predicates.ErrNodeLabelPresenceViolated, + // this one is probably intentional since it's a workaround for not having + // pod hard anti affinity. + predicates.ErrPodNotFitsHostPorts, + // DaemonSet is expected to respect taints and tolerations + predicates.ErrTaintsTolerationsNotMatch: + return true + } + } + } + return false +} + // nodeShouldRunDaemonPod checks a set of preconditions against a (node,daemonset) and returns a // summary. Returned booleans are: // * wantToRun: @@ -1105,6 +1129,12 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *exten return false, false, false, err } + // Return directly if there is any intentional predicate failure reason, so that daemonset controller skips + // checking other predicate failures, such as InsufficientResourceError and unintentional errors. + if hasIntentionalPredicatesReasons(reasons) { + return false, false, false, nil + } + for _, r := range reasons { glog.V(4).Infof("DaemonSet Predicates failed on node %s for ds '%s/%s' for reason: %v", node.Name, ds.ObjectMeta.Namespace, ds.ObjectMeta.Name, r.GetReason()) switch reason := r.(type) { @@ -1113,20 +1143,8 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *exten shouldSchedule = false case *predicates.PredicateFailureError: var emitEvent bool - // we try to partition predicates into two partitions here: intentional on the part of the operator and not. switch reason { - // intentional - case - predicates.ErrNodeSelectorNotMatch, - predicates.ErrPodNotMatchHostName, - predicates.ErrNodeLabelPresenceViolated, - // this one is probably intentional since it's a workaround for not having - // pod hard anti affinity. - predicates.ErrPodNotFitsHostPorts, - // DaemonSet is expected to respect taints and tolerations - predicates.ErrTaintsTolerationsNotMatch: - wantToRun, shouldSchedule, shouldContinueRunning = false, false, false - // unintentional + // unintentional predicates reasons need to be fired out to event. case predicates.ErrDiskConflict, predicates.ErrVolumeZoneConflict, diff --git a/pkg/controller/daemon/daemoncontroller_test.go b/pkg/controller/daemon/daemoncontroller_test.go index a0ead557e1f..0e7fabc07bc 100644 --- a/pkg/controller/daemon/daemoncontroller_test.go +++ b/pkg/controller/daemon/daemoncontroller_test.go @@ -273,9 +273,10 @@ func (f *fakePodControl) DeletePod(namespace string, podID string, object runtim type daemonSetsController struct { *DaemonSetsController - dsStore cache.Store - podStore cache.Store - nodeStore cache.Store + dsStore cache.Store + podStore cache.Store + nodeStore cache.Store + fakeRecorder *record.FakeRecorder } func newTestController(initialObjects ...runtime.Object) (*daemonSetsController, *fakePodControl, *fake.Clientset) { @@ -289,7 +290,9 @@ func newTestController(initialObjects ...runtime.Object) (*daemonSetsController, informerFactory.Core().V1().Nodes(), clientset, ) - manager.eventRecorder = record.NewFakeRecorder(100) + + fakeRecorder := record.NewFakeRecorder(100) + manager.eventRecorder = fakeRecorder manager.podStoreSynced = alwaysReady manager.nodeStoreSynced = alwaysReady @@ -303,6 +306,7 @@ func newTestController(initialObjects ...runtime.Object) (*daemonSetsController, informerFactory.Extensions().V1beta1().DaemonSets().Informer().GetStore(), informerFactory.Core().V1().Pods().Informer().GetStore(), informerFactory.Core().V1().Nodes().Informer().GetStore(), + fakeRecorder, }, podControl, clientset } @@ -486,6 +490,16 @@ func resourcePodSpec(nodeName, memory, cpu string) v1.PodSpec { } } +func resourcePodSpecWithoutNodeName(memory, cpu string) v1.PodSpec { + return v1.PodSpec{ + Containers: []v1.Container{{ + Resources: v1.ResourceRequirements{ + Requests: allocatableResources(memory, cpu), + }, + }}, + } +} + func allocatableResources(memory, cpu string) v1.ResourceList { return v1.ResourceList{ v1.ResourceMemory: resource.MustParse(memory), @@ -533,6 +547,27 @@ func TestInsufficientCapacityNodeDaemonDoesNotUnscheduleRunningPod(t *testing.T) } } +// DaemonSets should only place onto nodes with sufficient free resource and matched node selector +func TestInsufficientCapacityNodeSufficientCapacityWithNodeLabelDaemonLaunchPod(t *testing.T) { + podSpec := resourcePodSpecWithoutNodeName("50M", "75m") + ds := newDaemonSet("foo") + ds.Spec.Template.Spec = podSpec + ds.Spec.Template.Spec.NodeSelector = simpleNodeLabel + manager, podControl, _ := newTestController(ds) + node1 := newNode("not-enough-resource", nil) + node1.Status.Allocatable = allocatableResources("10M", "20m") + node2 := newNode("enough-resource", simpleNodeLabel) + node2.Status.Allocatable = allocatableResources("100M", "200m") + manager.nodeStore.Add(node1) + manager.nodeStore.Add(node2) + manager.dsStore.Add(ds) + syncAndValidateDaemonSets(t, manager, ds, podControl, 1, 0) + // we do not expect any event for insufficient free resource + if len(manager.fakeRecorder.Events) != 0 { + t.Fatalf("unexpected events, got %v, expected %v: %+v", len(manager.fakeRecorder.Events), 0, manager.fakeRecorder.Events) + } +} + func TestSufficientCapacityWithTerminatedPodsDaemonLaunchesPod(t *testing.T) { for _, strategy := range updateStrategies() { podSpec := resourcePodSpec("too-much-mem", "75M", "75m")