Scheduler doesn't reschedule vpods that are scheduled on unscehdulabl…

…e pods (#6726) To detect whether scheduling is already done, it uses the existing placements fields, however, if one or more placements are for unschedulable pods, it won't reschedule them. See added unit tests for example scenarios. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
knative · Feb 7, 2023 · 1092472 · 1092472
1 parent 40517be
commit 1092472
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 2 deletions.
diff --git a/pkg/scheduler/state/state.go b/pkg/scheduler/state/state.go
@@ -135,6 +135,15 @@ func (s *State) GetPodInfo(podName string) (zoneName string, nodeName string, er
 	return zoneName, nodeName, nil
 }
 
+func (s *State) IsSchedulablePod(ordinal int32) bool {
+	for _, x := range s.SchedulablePods {
+		if x == ordinal {
+			return true
+		}
+	}
+	return false
+}
+
 // stateBuilder reconstruct the state from scratch, by listing vpods
 type stateBuilder struct {
 	ctx               context.Context

diff --git a/pkg/scheduler/statefulset/scheduler.go b/pkg/scheduler/statefulset/scheduler.go
@@ -177,10 +177,20 @@ func (s *StatefulSetScheduler) scheduleVPod(vpod scheduler.VPod) ([]duckv1alpha1
 		return nil, err
 	}
 
-	placements := vpod.GetPlacements()
-	existingPlacements := placements
+	existingPlacements := vpod.GetPlacements()
 	var left int32
 
+	// Remove unschedulable pods from placements
+	var placements []duckv1alpha1.Placement
+	if len(existingPlacements) > 0 {
+		placements = make([]duckv1alpha1.Placement, 0, len(existingPlacements))
+		for _, p := range existingPlacements {
+			if state.IsSchedulablePod(st.OrdinalFromPodName(p.PodName)) {
+				placements = append(placements, *p.DeepCopy())
+			}
+		}
+	}
+
 	// The scheduler when policy type is
 	// Policy: MAXFILLUP (SchedulerPolicyType == MAXFILLUP)
 	// - allocates as many vreplicas as possible to the same pod(s)

diff --git a/pkg/scheduler/statefulset/scheduler_test.go b/pkg/scheduler/statefulset/scheduler_test.go
@@ -87,6 +87,32 @@ func TestStatefulsetScheduler(t *testing.T) {
 			expected:            []duckv1alpha1.Placement{{PodName: "statefulset-name-0", VReplicas: 3}},
 			schedulerPolicyType: scheduler.MAXFILLUP,
 		},
+		{
+			name:      "one replica, 8 vreplicas, already scheduled on unschedulable pod, add replicas",
+			vreplicas: 8,
+			replicas:  int32(1),
+			placements: []duckv1alpha1.Placement{
+				{PodName: "statefulset-name-0", VReplicas: 3},
+				{PodName: "statefulset-name-2", VReplicas: 5},
+			},
+			expected: []duckv1alpha1.Placement{
+				{PodName: "statefulset-name-0", VReplicas: 8},
+			},
+			schedulerPolicyType: scheduler.MAXFILLUP,
+		},
+		{
+			name:      "one replica, 1 vreplicas, already scheduled on unschedulable pod, remove replicas",
+			vreplicas: 1,
+			replicas:  int32(1),
+			placements: []duckv1alpha1.Placement{
+				{PodName: "statefulset-name-0", VReplicas: 3},
+				{PodName: "statefulset-name-2", VReplicas: 5},
+			},
+			expected: []duckv1alpha1.Placement{
+				{PodName: "statefulset-name-0", VReplicas: 1},
+			},
+			schedulerPolicyType: scheduler.MAXFILLUP,
+		},
 		{
 			name:                "one replica, 15 vreplicas, unschedulable",
 			vreplicas:           15,