Merge pull request #29272 from mksalawa/pending

pkg/controller/daemon/daemoncontroller.go

@@ -693,7 +693,7 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *api.Node, ds *exte
 		if pod.Status.Phase == api.PodSucceeded || pod.Status.Phase == api.PodFailed {
 			continue
 		}
-		// ignore pods that belong to the daemonset when taking into account wheter
+		// ignore pods that belong to the daemonset when taking into account whether
 		// a daemonset should bind to a node.
 		if pds := dsc.getPodDaemonSet(pod); pds != nil && ds.Name == pds.Name {
 			continue
@@ -703,18 +703,12 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *api.Node, ds *exte
 
 	nodeInfo := schedulercache.NewNodeInfo(pods...)
 	nodeInfo.SetNode(node)
-	fit, err := predicates.GeneralPredicates(newPod, nil, nodeInfo)
+	fit, reasons, err := predicates.GeneralPredicates(newPod, nil, nodeInfo)
 	if err != nil {
-		if re, ok := err.(*predicates.PredicateFailureError); ok {
-			message := re.Error()
-			glog.V(2).Infof("Predicate failed on Pod: %s, for reason: %v", newPod.Name, message)
-		}
-		if re, ok := err.(*predicates.InsufficientResourceError); ok {
-			message := re.Error()
-			glog.V(2).Infof("Predicate failed on Pod: %s, for reason: %v", newPod.Name, message)
-		}
-		message := fmt.Sprintf("GeneralPredicates failed due to %v.", err)
-		glog.Warningf("Predicate failed on Pod %s - %s", newPod.Name, message)
+		glog.Warningf("GeneralPredicates failed on pod %s due to unexpected error: %v", newPod.Name, err)
+	}
+	for _, r := range reasons {
+		glog.V(2).Infof("GeneralPredicates failed on pod %s for reason: %v", newPod.Name, r.GetReason())
 	}
 	return fit
 }

pkg/kubelet/kubelet.go

@@ -2059,23 +2059,40 @@ func (kl *Kubelet) canAdmitPod(pods []*api.Pod, pod *api.Pod) (bool, string, str
 	}
 	nodeInfo := schedulercache.NewNodeInfo(pods...)
 	nodeInfo.SetNode(node)
-	fit, err := predicates.GeneralPredicates(pod, nil, nodeInfo)
+	fit, reasons, err := predicates.GeneralPredicates(pod, nil, nodeInfo)
+	if err != nil {
+		message := fmt.Sprintf("GeneralPredicates failed due to %v, which is unexpected.", err)
+		glog.Warningf("Failed to admit pod %v - %s", format.Pod(pod), message)
+		return fit, "UnexpectedError", message
+	}
 	if !fit {
-		if re, ok := err.(*predicates.PredicateFailureError); ok {
-			reason := re.PredicateName
-			message := re.Error()
+		var reason string
+		var message string
+		if len(reasons) == 0 {
+			message = fmt.Sprint("GeneralPredicates failed due to unknown reason, which is unexpected.")
+			glog.Warningf("Failed to admit pod %v - %s", format.Pod(pod), message)
+			return fit, "UnknownReason", message
+		}
+		// If there are failed predicates, we only return the first one as a reason.
+		r := reasons[0]
+		switch re := r.(type) {
+		case *predicates.PredicateFailureError:
+			reason = re.PredicateName
+			message = re.Error()
 			glog.V(2).Infof("Predicate failed on Pod: %v, for reason: %v", format.Pod(pod), message)
-			return fit, reason, message
-		}
-		if re, ok := err.(*predicates.InsufficientResourceError); ok {
-			reason := fmt.Sprintf("OutOf%s", re.ResourceName)
+		case *predicates.InsufficientResourceError:
+			reason = fmt.Sprintf("OutOf%s", re.ResourceName)
 			message := re.Error()
 			glog.V(2).Infof("Predicate failed on Pod: %v, for reason: %v", format.Pod(pod), message)
-			return fit, reason, message
+		case *predicates.FailureReason:
+			reason = re.GetReason()
+			message = fmt.Sprintf("Failure: %s", re.GetReason())
+			glog.V(2).Infof("Predicate failed on Pod: %v, for reason: %v", format.Pod(pod), message)
+		default:
+			reason = "UnexpectedPredicateFailureType"
+			message := fmt.Sprintf("GeneralPredicates failed due to %v, which is unexpected.", r)
+			glog.Warningf("Failed to admit pod %v - %s", format.Pod(pod), message)
 		}
-		reason := "UnexpectedPredicateFailureType"
-		message := fmt.Sprintf("GeneralPredicates failed due to %v, which is unexpected.", err)
-		glog.Warningf("Failed to admit pod %v - %s", format.Pod(pod), message)
 		return fit, reason, message
 	}
 	// TODO: When disk space scheduling is implemented (#11976), remove the out-of-disk check here and

plugin/pkg/scheduler/algorithm/predicates/error.go

@@ -16,13 +16,9 @@ limitations under the License.
 
 package predicates
 
-import "fmt"
-
-const (
-	podCountResourceName  string = "PodCount"
-	cpuResourceName       string = "CPU"
-	memoryResourceName    string = "Memory"
-	nvidiaGpuResourceName string = "NvidiaGpu"
+import (
+	"fmt"
+	"k8s.io/kubernetes/pkg/api"
 )
 
 var (
@@ -49,13 +45,13 @@ var (
 // hit and caused the unfitting failure.
 type InsufficientResourceError struct {
 	// resourceName is the name of the resource that is insufficient
-	ResourceName string
+	ResourceName api.ResourceName
 	requested    int64
 	used         int64
 	capacity     int64
 }
 
-func newInsufficientResourceError(resourceName string, requested, used, capacity int64) *InsufficientResourceError {
+func NewInsufficientResourceError(resourceName api.ResourceName, requested, used, capacity int64) *InsufficientResourceError {
 	return &InsufficientResourceError{
 		ResourceName: resourceName,
 		requested:    requested,
@@ -69,14 +65,34 @@ func (e *InsufficientResourceError) Error() string {
 		e.ResourceName, e.requested, e.used, e.capacity)
 }
 
+func (e *InsufficientResourceError) GetReason() string {
+	return fmt.Sprintf("Insufficient %v", e.ResourceName)
+}
+
 type PredicateFailureError struct {
 	PredicateName string
 }
 
 func newPredicateFailureError(predicateName string) *PredicateFailureError {
-	return &PredicateFailureError{predicateName}
+	return &PredicateFailureError{PredicateName: predicateName}
 }
 
 func (e *PredicateFailureError) Error() string {
 	return fmt.Sprintf("Predicate %s failed", e.PredicateName)
 }
+
+func (e *PredicateFailureError) GetReason() string {
+	return e.PredicateName
+}
+
+type FailureReason struct {
+	reason string
+}
+
+func NewFailureReason(msg string) *FailureReason {
+	return &FailureReason{reason: msg}
+}
+
+func (e *FailureReason) GetReason() string {
+	return e.reason
+}

plugin/pkg/scheduler/algorithm/types.go

@@ -24,7 +24,7 @@ import (
 
 // FitPredicate is a function that indicates if a pod fits into an existing node.
 // The failure information is given by the error.
-type FitPredicate func(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, error)
+type FitPredicate func(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []PredicateFailureReason, error)
 
 type PriorityFunction func(pod *api.Pod, nodeNameToInfo map[string]*schedulercache.NodeInfo, nodes []*api.Node) (schedulerapi.HostPriorityList, error)
 
@@ -32,3 +32,7 @@ type PriorityConfig struct {
 	Function PriorityFunction
 	Weight   int
 }
+
+type PredicateFailureReason interface {
+	GetReason() string
+}

plugin/pkg/scheduler/factory/factory_test.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/types"
 	utiltesting "k8s.io/kubernetes/pkg/util/testing"
+	"k8s.io/kubernetes/plugin/pkg/scheduler/algorithm"
 	schedulerapi "k8s.io/kubernetes/plugin/pkg/scheduler/api"
 	latestschedulerapi "k8s.io/kubernetes/plugin/pkg/scheduler/api/latest"
 	"k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache"
@@ -115,12 +116,12 @@ func TestCreateFromEmptyConfig(t *testing.T) {
 	factory.CreateFromConfig(policy)
 }
 
-func PredicateOne(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, error) {
-	return true, nil
+func PredicateOne(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
+	return true, nil, nil
 }
 
-func PredicateTwo(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, error) {
-	return true, nil
+func PredicateTwo(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
+	return true, nil, nil
 }
 
 func PriorityOne(pod *api.Pod, nodeNameToInfo map[string]*schedulercache.NodeInfo, nodes []*api.Node) (schedulerapi.HostPriorityList, error) {

plugin/pkg/scheduler/generic_scheduler.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"fmt"
 	"sort"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -35,7 +36,7 @@ import (
 	"k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache"
 )
 
-type FailedPredicateMap map[string]string
+type FailedPredicateMap map[string][]algorithm.PredicateFailureReason
 
 type FitError struct {
 	Pod              *api.Pod
@@ -48,9 +49,13 @@ var ErrNoNodesAvailable = fmt.Errorf("no nodes available to schedule pods")
 func (f *FitError) Error() string {
 	var buf bytes.Buffer
 	buf.WriteString(fmt.Sprintf("pod (%s) failed to fit in any node\n", f.Pod.Name))
-	for node, predicate := range f.FailedPredicates {
-		reason := fmt.Sprintf("fit failure on node (%s): %s\n", node, predicate)
-		buf.WriteString(reason)
+	for node, predicates := range f.FailedPredicates {
+		reasons := make([]string, 0)
+		for _, pred := range predicates {
+			reasons = append(reasons, pred.GetReason())
+		}
+		reasonMsg := fmt.Sprintf("fit failure on node (%s): %s\n", node, strings.Join(reasons, ", "))
+		buf.WriteString(reasonMsg)
 	}
 	return buf.String()
 }
@@ -159,7 +164,7 @@ func findNodesThatFit(
 		var filteredLen int32
 		checkNode := func(i int) {
 			nodeName := nodes[i].Name
-			fits, failedPredicate, err := podFitsOnNode(pod, meta, nodeNameToInfo[nodeName], predicateFuncs)
+			fits, failedPredicates, err := podFitsOnNode(pod, meta, nodeNameToInfo[nodeName], predicateFuncs)
 			if err != nil {
 				predicateResultLock.Lock()
 				errs = append(errs, err)
@@ -170,7 +175,7 @@ func findNodesThatFit(
 				filtered[atomic.AddInt32(&filteredLen, 1)-1] = nodes[i]
 			} else {
 				predicateResultLock.Lock()
-				failedPredicateMap[nodeName] = failedPredicate
+				failedPredicateMap[nodeName] = failedPredicates
 				predicateResultLock.Unlock()
 			}
 		}
@@ -189,7 +194,10 @@ func findNodesThatFit(
 			}
 
 			for failedNodeName, failedMsg := range failedMap {
-				failedPredicateMap[failedNodeName] = failedMsg
+				if _, found := failedPredicateMap[failedNodeName]; !found {
+					failedPredicateMap[failedNodeName] = []algorithm.PredicateFailureReason{}
+				}
+				failedPredicateMap[failedNodeName] = append(failedPredicateMap[failedNodeName], predicates.NewFailureReason(failedMsg))
 			}
 			filtered = filteredList
 			if len(filtered) == 0 {
@@ -201,38 +209,19 @@ func findNodesThatFit(
 }
 
 // Checks whether node with a given name and NodeInfo satisfies all predicateFuncs.
-func podFitsOnNode(pod *api.Pod, meta interface{}, info *schedulercache.NodeInfo, predicateFuncs map[string]algorithm.FitPredicate) (bool, string, error) {
+func podFitsOnNode(pod *api.Pod, meta interface{}, info *schedulercache.NodeInfo, predicateFuncs map[string]algorithm.FitPredicate) (bool, []algorithm.PredicateFailureReason, error) {
+	var failedPredicates []algorithm.PredicateFailureReason
 	for _, predicate := range predicateFuncs {
-		fit, err := predicate(pod, meta, info)
+		fit, reasons, err := predicate(pod, meta, info)
 		if err != nil {
-			switch e := err.(type) {
-			case *predicates.InsufficientResourceError:
-				if fit {
-					err := fmt.Errorf("got InsufficientResourceError: %v, but also fit='true' which is unexpected", e)
-					return false, "", err
-				}
-			case *predicates.PredicateFailureError:
-				if fit {
-					err := fmt.Errorf("got PredicateFailureError: %v, but also fit='true' which is unexpected", e)
-					return false, "", err
-				}
-			default:
-				return false, "", err
-			}
+			err := fmt.Errorf("SchedulerPredicates failed due to %v, which is unexpected.", err)
+			return false, []algorithm.PredicateFailureReason{}, err
 		}
 		if !fit {
-			if re, ok := err.(*predicates.InsufficientResourceError); ok {
-				return false, fmt.Sprintf("Insufficient %s", re.ResourceName), nil
-			}
-			if re, ok := err.(*predicates.PredicateFailureError); ok {
-				return false, re.PredicateName, nil
-			} else {
-				err := fmt.Errorf("SchedulerPredicates failed due to %v, which is unexpected.", err)
-				return false, "", err
-			}
+			failedPredicates = append(failedPredicates, reasons...)
 		}
 	}
-	return true, "", nil
+	return len(failedPredicates) == 0, failedPredicates, nil
 }
 
 // Prioritizes the nodes by running the individual priority functions in parallel.

plugin/pkg/scheduler/generic_scheduler_test.go

@@ -33,30 +33,30 @@ import (
 	"k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache"
 )
 
-func falsePredicate(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, error) {
-	return false, algorithmpredicates.ErrFakePredicate
+func falsePredicate(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
+	return false, []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate}, nil
 }
 
-func truePredicate(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, error) {
-	return true, nil
+func truePredicate(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
+	return true, nil, nil
 }
 
-func matchesPredicate(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, error) {
+func matchesPredicate(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
 	node := nodeInfo.Node()
 	if node == nil {
-		return false, fmt.Errorf("node not found")
+		return false, nil, fmt.Errorf("node not found")
 	}
 	if pod.Name == node.Name {
-		return true, nil
+		return true, nil, nil
 	}
-	return false, algorithmpredicates.ErrFakePredicate
+	return false, []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate}, nil
 }
 
-func hasNoPodsPredicate(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, error) {
+func hasNoPodsPredicate(pod *api.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
 	if len(nodeInfo.Pods()) == 0 {
-		return true, nil
+		return true, nil, nil
 	}
-	return false, algorithmpredicates.ErrFakePredicate
+	return false, []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate}, nil
 }
 
 func numericPriority(pod *api.Pod, nodeNameToInfo map[string]*schedulercache.NodeInfo, nodes []*api.Node) (schedulerapi.HostPriorityList, error) {
@@ -193,8 +193,8 @@ func TestGenericScheduler(t *testing.T) {
 			wErr: &FitError{
 				Pod: &api.Pod{ObjectMeta: api.ObjectMeta{Name: "2"}},
 				FailedPredicates: FailedPredicateMap{
-					"machine1": algorithmpredicates.ErrFakePredicate.PredicateName,
-					"machine2": algorithmpredicates.ErrFakePredicate.PredicateName,
+					"machine1": []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate},
+					"machine2": []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate},
 				}},
 		},
 		{
@@ -251,9 +251,9 @@ func TestGenericScheduler(t *testing.T) {
 			wErr: &FitError{
 				Pod: &api.Pod{ObjectMeta: api.ObjectMeta{Name: "2"}},
 				FailedPredicates: FailedPredicateMap{
-					"3": algorithmpredicates.ErrFakePredicate.PredicateName,
-					"2": algorithmpredicates.ErrFakePredicate.PredicateName,
-					"1": algorithmpredicates.ErrFakePredicate.PredicateName,
+					"3": []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate},
+					"2": []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate},
+					"1": []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate},
 				},
 			},
 		},
@@ -282,8 +282,8 @@ func TestGenericScheduler(t *testing.T) {
 			wErr: &FitError{
 				Pod: &api.Pod{ObjectMeta: api.ObjectMeta{Name: "2"}},
 				FailedPredicates: FailedPredicateMap{
-					"1": algorithmpredicates.ErrFakePredicate.PredicateName,
-					"2": algorithmpredicates.ErrFakePredicate.PredicateName,
+					"1": []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate},
+					"2": []algorithm.PredicateFailureReason{algorithmpredicates.ErrFakePredicate},
 				},
 			},
 		},
@@ -327,12 +327,12 @@ func TestFindFitAllError(t *testing.T) {
 	}
 
 	for _, node := range nodes {
-		failure, found := predicateMap[node]
+		failures, found := predicateMap[node]
 		if !found {
 			t.Errorf("failed to find node: %s in %v", node, predicateMap)
 		}
-		if failure != "FakePredicateError" {
-			t.Errorf("unexpected failures: %v", failure)
+		if len(failures) != 1 || failures[0] != algorithmpredicates.ErrFakePredicate {
+			t.Errorf("unexpected failures: %v", failures)
 		}
 	}
 }
@@ -351,7 +351,7 @@ func TestFindFitSomeError(t *testing.T) {
 	}
 
 	_, predicateMap, err := findNodesThatFit(pod, nodeNameToInfo, makeNodeList(nodes), predicates, nil)
-	if err != nil && !reflect.DeepEqual(err, algorithmpredicates.ErrFakePredicate) {
+	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
 
@@ -363,12 +363,12 @@ func TestFindFitSomeError(t *testing.T) {
 		if node == pod.Name {
 			continue
 		}
-		failure, found := predicateMap[node]
+		failures, found := predicateMap[node]
 		if !found {
 			t.Errorf("failed to find node: %s in %v", node, predicateMap)
 		}
-		if failure != "FakePredicateError" {
-			t.Errorf("unexpected failures: %v", failure)
+		if len(failures) != 1 || failures[0] != algorithmpredicates.ErrFakePredicate {
+			t.Errorf("unexpected failures: %v", failures)
 		}
 	}
 }