Skip to content

Commit

Permalink
Merge pull request #119952 from sanposhiho/automated-cherry-pick-of-#…
Browse files Browse the repository at this point in the history 
…119778-upstream-release-1.27

Automated cherry pick of #119778: fix: when PreFilter returns UnschedulableAndUnresolvable, copy the state in all nodes in statusmap
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Sep 6, 2023
2 parents 23c2594 + e451735 commit ecb92c9
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 23 deletions.
49 changes: 31 additions & 18 deletions pkg/scheduler/framework/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,32 +244,45 @@ const (
// Error returns detailed information of why the pod failed to fit on each node.
// A message format is "0/X nodes are available: <PreFilterMsg>. <FilterMsg>. <PostFilterMsg>."
func (f *FitError) Error() string {
reasons := make(map[string]int)
for _, status := range f.Diagnosis.NodeToStatusMap {
for _, reason := range status.Reasons() {
reasons[reason]++
}
}

reasonMsg := fmt.Sprintf(NoNodeAvailableMsg+":", f.NumAllNodes)
// Add the messages from PreFilter plugins to reasonMsg.
preFilterMsg := f.Diagnosis.PreFilterMsg
if preFilterMsg != "" {
// PreFilter plugin returns unschedulable.
// Add the messages from PreFilter plugins to reasonMsg.
reasonMsg += fmt.Sprintf(SeparatorFormat, preFilterMsg)
}
sortReasonsHistogram := func() []string {
var reasonStrings []string
for k, v := range reasons {
reasonStrings = append(reasonStrings, fmt.Sprintf("%v %v", v, k))

if preFilterMsg == "" {
// the scheduling cycle went through PreFilter extension point successfully.
//
// When the prefilter plugin returns unschedulable,
// the scheduling framework inserts the same unschedulable status to all nodes in NodeToStatusMap.
// So, we shouldn't add the message from NodeToStatusMap when the PreFilter failed.
// Otherwise, we will have duplicated reasons in the error message.
reasons := make(map[string]int)
for _, status := range f.Diagnosis.NodeToStatusMap {
for _, reason := range status.Reasons() {
reasons[reason]++
}
}

sortReasonsHistogram := func() []string {
var reasonStrings []string
for k, v := range reasons {
reasonStrings = append(reasonStrings, fmt.Sprintf("%v %v", v, k))
}
sort.Strings(reasonStrings)
return reasonStrings
}
sortedFilterMsg := sortReasonsHistogram()
if len(sortedFilterMsg) != 0 {
reasonMsg += fmt.Sprintf(SeparatorFormat, strings.Join(sortedFilterMsg, ", "))
}
sort.Strings(reasonStrings)
return reasonStrings
}
sortedFilterMsg := sortReasonsHistogram()
if len(sortedFilterMsg) != 0 {
reasonMsg += fmt.Sprintf(SeparatorFormat, strings.Join(sortedFilterMsg, ", "))
}

// Add the messages from PostFilter plugins to reasonMsg.
// We can add this message regardless of whether the scheduling cycle fails at PreFilter or Filter
// since we may run PostFilter (if enabled) in both cases.
postFilterMsg := f.Diagnosis.PostFilterMsg
if postFilterMsg != "" {
reasonMsg += fmt.Sprintf(SeparatorFormat, postFilterMsg)
Expand Down
24 changes: 24 additions & 0 deletions pkg/scheduler/framework/types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1390,9 +1390,33 @@ func TestFitError_Error(t *testing.T) {
numAllNodes: 3,
diagnosis: Diagnosis{
PreFilterMsg: "Node(s) failed PreFilter plugin FalsePreFilter",
NodeToStatusMap: NodeToStatusMap{
// They're inserted by the framework.
// We don't include them in the reason message because they'd be just duplicates.
"node1": NewStatus(Unschedulable, "Node(s) failed PreFilter plugin FalsePreFilter"),
"node2": NewStatus(Unschedulable, "Node(s) failed PreFilter plugin FalsePreFilter"),
"node3": NewStatus(Unschedulable, "Node(s) failed PreFilter plugin FalsePreFilter"),
},
},
wantReasonMsg: "0/3 nodes are available: Node(s) failed PreFilter plugin FalsePreFilter.",
},
{
name: "nodes failed Prefilter plugin and the preemption also failed",
numAllNodes: 3,
diagnosis: Diagnosis{
PreFilterMsg: "Node(s) failed PreFilter plugin FalsePreFilter",
NodeToStatusMap: NodeToStatusMap{
// They're inserted by the framework.
// We don't include them in the reason message because they'd be just duplicates.
"node1": NewStatus(Unschedulable, "Node(s) failed PreFilter plugin FalsePreFilter"),
"node2": NewStatus(Unschedulable, "Node(s) failed PreFilter plugin FalsePreFilter"),
"node3": NewStatus(Unschedulable, "Node(s) failed PreFilter plugin FalsePreFilter"),
},
// PostFilterMsg will be included.
PostFilterMsg: "Error running PostFilter plugin FailedPostFilter",
},
wantReasonMsg: "0/3 nodes are available: Node(s) failed PreFilter plugin FalsePreFilter. Error running PostFilter plugin FailedPostFilter.",
},
{
name: "nodes failed one Filter plugin with an empty PostFilterMsg",
numAllNodes: 3,
Expand Down
6 changes: 6 additions & 0 deletions pkg/scheduler/schedule_one.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,12 @@ func (sched *Scheduler) findNodesThatFitPod(ctx context.Context, fwk framework.F
if !s.IsUnschedulable() {
return nil, diagnosis, s.AsError()
}
// All nodes in NodeToStatusMap will have the same status so that they can be handled in the preemption.
// Some non trivial refactoring is needed to avoid this copy.
for _, n := range allNodes {
diagnosis.NodeToStatusMap[n.Node().Name] = s
}

// Record the messages from PreFilter in Diagnosis.PreFilterMsg.
msg := s.Message()
diagnosis.PreFilterMsg = msg
Expand Down
25 changes: 20 additions & 5 deletions pkg/scheduler/schedule_one_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1700,7 +1700,10 @@ func TestSchedulerSchedulePod(t *testing.T) {
Pod: st.MakePod().Name("ignore").UID("ignore").PVC("unknownPVC").Obj(),
NumAllNodes: 2,
Diagnosis: framework.Diagnosis{
NodeToStatusMap: framework.NodeToStatusMap{},
NodeToStatusMap: framework.NodeToStatusMap{
"node1": framework.NewStatus(framework.UnschedulableAndUnresolvable, `persistentvolumeclaim "unknownPVC" not found`).WithFailedPlugin("VolumeBinding"),
"node2": framework.NewStatus(framework.UnschedulableAndUnresolvable, `persistentvolumeclaim "unknownPVC" not found`).WithFailedPlugin("VolumeBinding"),
},
PreFilterMsg: `persistentvolumeclaim "unknownPVC" not found`,
UnschedulablePlugins: sets.NewString(volumebinding.Name),
},
Expand All @@ -1722,7 +1725,10 @@ func TestSchedulerSchedulePod(t *testing.T) {
Pod: st.MakePod().Name("ignore").UID("ignore").Namespace(v1.NamespaceDefault).PVC("existingPVC").Obj(),
NumAllNodes: 2,
Diagnosis: framework.Diagnosis{
NodeToStatusMap: framework.NodeToStatusMap{},
NodeToStatusMap: framework.NodeToStatusMap{
"node1": framework.NewStatus(framework.UnschedulableAndUnresolvable, `persistentvolumeclaim "existingPVC" is being deleted`).WithFailedPlugin("VolumeBinding"),
"node2": framework.NewStatus(framework.UnschedulableAndUnresolvable, `persistentvolumeclaim "existingPVC" is being deleted`).WithFailedPlugin("VolumeBinding"),
},
PreFilterMsg: `persistentvolumeclaim "existingPVC" is being deleted`,
UnschedulablePlugins: sets.NewString(volumebinding.Name),
},
Expand Down Expand Up @@ -1880,7 +1886,10 @@ func TestSchedulerSchedulePod(t *testing.T) {
Pod: st.MakePod().Name("test-prefilter").UID("test-prefilter").Obj(),
NumAllNodes: 2,
Diagnosis: framework.Diagnosis{
NodeToStatusMap: framework.NodeToStatusMap{},
NodeToStatusMap: framework.NodeToStatusMap{
"1": framework.NewStatus(framework.UnschedulableAndUnresolvable, "injected unschedulable status").WithFailedPlugin("FakePreFilter"),
"2": framework.NewStatus(framework.UnschedulableAndUnresolvable, "injected unschedulable status").WithFailedPlugin("FakePreFilter"),
},
PreFilterMsg: "injected unschedulable status",
UnschedulablePlugins: sets.NewString("FakePreFilter"),
},
Expand Down Expand Up @@ -1948,7 +1957,11 @@ func TestSchedulerSchedulePod(t *testing.T) {
Pod: st.MakePod().Name("test-prefilter").UID("test-prefilter").Obj(),
NumAllNodes: 3,
Diagnosis: framework.Diagnosis{
NodeToStatusMap: framework.NodeToStatusMap{},
NodeToStatusMap: framework.NodeToStatusMap{
"node1": framework.NewStatus(framework.Unschedulable, "node(s) didn't satisfy plugin(s) [FakePreFilter2 FakePreFilter3] simultaneously"),
"node2": framework.NewStatus(framework.Unschedulable, "node(s) didn't satisfy plugin(s) [FakePreFilter2 FakePreFilter3] simultaneously"),
"node3": framework.NewStatus(framework.Unschedulable, "node(s) didn't satisfy plugin(s) [FakePreFilter2 FakePreFilter3] simultaneously"),
},
UnschedulablePlugins: sets.String{},
PreFilterMsg: "node(s) didn't satisfy plugin(s) [FakePreFilter2 FakePreFilter3] simultaneously",
},
Expand All @@ -1974,7 +1987,9 @@ func TestSchedulerSchedulePod(t *testing.T) {
Pod: st.MakePod().Name("test-prefilter").UID("test-prefilter").Obj(),
NumAllNodes: 1,
Diagnosis: framework.Diagnosis{
NodeToStatusMap: framework.NodeToStatusMap{},
NodeToStatusMap: framework.NodeToStatusMap{
"node1": framework.NewStatus(framework.Unschedulable, "node(s) didn't satisfy plugin FakePreFilter2"),
},
UnschedulablePlugins: sets.String{},
PreFilterMsg: "node(s) didn't satisfy plugin FakePreFilter2",
},
Expand Down

0 comments on commit ecb92c9

Please sign in to comment.