From d462b4cbc83c71a0c2020d18023751ebc2dced31 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Wed, 28 Jun 2017 11:29:54 +0200 Subject: [PATCH] Partially revert "Do not fire InsufficientResourceError when there are intentional reasons." This partially reverts commit 2b311fefba9cc106ed1a8cebc6c61e32814fd9e5. We drop the changes to the DaemonSet controller but leave the test. By reverting the changes, we make it easier to return different values of shouldContinueRunning for intentional predicate failures, rather then lumping all intentional predicate failures together. The test should continue to pass after the fix. --- pkg/controller/daemon/daemoncontroller.go | 44 +++++++---------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/pkg/controller/daemon/daemoncontroller.go b/pkg/controller/daemon/daemoncontroller.go index c45e17ab30a..5f9cc6b2867 100644 --- a/pkg/controller/daemon/daemoncontroller.go +++ b/pkg/controller/daemon/daemoncontroller.go @@ -1022,30 +1022,6 @@ func (dsc *DaemonSetsController) syncDaemonSet(key string) error { return dsc.updateDaemonSetStatus(ds, hash) } -// 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: @@ -1135,12 +1111,6 @@ 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) { @@ -1149,8 +1119,20 @@ 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 { - // unintentional predicates reasons need to be fired out to event. + // 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 case predicates.ErrDiskConflict, predicates.ErrVolumeZoneConflict,