scheduler: reservation transformer ends early with reservation affini…

…ty (#1717) Signed-off-by: Joseph <[email protected]>
koordinator-sh · Oct 19, 2023 · 4b4ba9a · 4b4ba9a
1 parent 13f9efa
commit 4b4ba9a
Show file tree

Hide file tree

Showing 2 changed files with 138 additions and 10 deletions.
diff --git a/pkg/scheduler/plugins/reservation/transformer.go b/pkg/scheduler/plugins/reservation/transformer.go
@@ -129,6 +129,13 @@ func (pl *Plugin) prepareMatchReservationState(ctx context.Context, cycleState *
 			return
 		}
 
+		// The Pod declares a ReservationAffinity, which means that the Pod must reuse the Reservation resources,
+		// but there are no matching Reservations, which means that the node itself does not need to be processed.
+		// We can end early to avoid meaningless operations.
+		if reservationAffinity != nil && len(matched) == 0 {
+			return
+		}
+
 		if err := extender.Scheduler().GetCache().InvalidNodeInfo(node.Name); err != nil {
 			klog.ErrorS(err, "Failed to InvalidNodeInfo", "pod", klog.KObj(pod), "node", node.Name)
 			errCh.SendErrorWithCancel(err, cancel)

diff --git a/pkg/scheduler/plugins/reservation/transformer_test.go b/pkg/scheduler/plugins/reservation/transformer_test.go
@@ -441,7 +441,7 @@ func Test_matchReservation(t *testing.T) {
 	}
 }
 
-func TestBeforePreFilterWithNodeAffinity(t *testing.T) {
+func TestBeforePreFilterWithReservationAffinity(t *testing.T) {
 	node := &corev1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "node1",
@@ -460,6 +460,9 @@ func TestBeforePreFilterWithNodeAffinity(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{
 			UID:  uuid.NewUUID(),
 			Name: "reservation8C16G",
+			Labels: map[string]string{
+				"reservation-a": "true",
+			},
 		},
 		Spec: schedulingv1alpha1.ReservationSpec{
 			Owners: []schedulingv1alpha1.ReservationOwner{
@@ -473,20 +476,138 @@ func TestBeforePreFilterWithNodeAffinity(t *testing.T) {
 			},
 			Template: &corev1.PodTemplateSpec{
 				Spec: corev1.PodSpec{
-					Affinity: &corev1.Affinity{
-						PodAntiAffinity: &corev1.PodAntiAffinity{
-							RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{
+					Containers: []corev1.Container{
+						{
+							Resources: corev1.ResourceRequirements{
+								Requests: corev1.ResourceList{
+									corev1.ResourceCPU:    resource.MustParse("8"),
+									corev1.ResourceMemory: resource.MustParse("16Gi"),
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		Status: schedulingv1alpha1.ReservationStatus{
+			Phase:    schedulingv1alpha1.ReservationAvailable,
+			NodeName: node.Name,
+			Allocatable: corev1.ResourceList{
+				corev1.ResourceCPU:    resource.MustParse("8"),
+				corev1.ResourceMemory: resource.MustParse("16Gi"),
+			},
+		},
+	}
+	var pods []*corev1.Pod
+	pods = append(pods, reservationutil.NewReservePod(matchedReservation))
+
+	tests := []struct {
+		name         string
+		rAffinity    *apiext.ReservationAffinity
+		wantRestored bool
+	}{
+		{
+			name:         "pod has no reservation affinity",
+			wantRestored: true,
+		},
+		{
+			name: "pod has reservation affinity and matched",
+			rAffinity: &apiext.ReservationAffinity{
+				RequiredDuringSchedulingIgnoredDuringExecution: &apiext.ReservationAffinitySelector{
+					ReservationSelectorTerms: []corev1.NodeSelectorTerm{
+						{
+							MatchExpressions: []corev1.NodeSelectorRequirement{
 								{
-									LabelSelector: &metav1.LabelSelector{
-										MatchLabels: map[string]string{
-											"app": "test-app-3",
-										},
-									},
-									TopologyKey: corev1.LabelHostname,
+									Key:      "reservation-a",
+									Operator: corev1.NodeSelectorOpIn,
+									Values:   []string{"true"},
 								},
 							},
 						},
 					},
+				},
+			},
+			wantRestored: true,
+		},
+		{
+			name: "pod has reservation affinity but failed to match",
+			rAffinity: &apiext.ReservationAffinity{
+				RequiredDuringSchedulingIgnoredDuringExecution: &apiext.ReservationAffinitySelector{
+					ReservationSelectorTerms: []corev1.NodeSelectorTerm{
+						{
+							MatchExpressions: []corev1.NodeSelectorRequirement{
+								{
+									Key:      "reservation-a",
+									Operator: corev1.NodeSelectorOpIn,
+									Values:   []string{"false"},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantRestored: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			suit := newPluginTestSuitWith(t, pods, []*corev1.Node{node})
+			p, err := suit.pluginFactory()
+			assert.NoError(t, err)
+			pl := p.(*Plugin)
+
+			pl.reservationCache.updateReservation(matchedReservation)
+
+			testPod := &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"test-reservation": "true",
+					},
+				},
+			}
+			if tt.rAffinity != nil {
+				assert.NoError(t, apiext.SetReservationAffinity(testPod, tt.rAffinity))
+			}
+			cycleState := framework.NewCycleState()
+			_, restored, status := pl.BeforePreFilter(context.TODO(), cycleState, testPod)
+			assert.Equal(t, tt.wantRestored, restored)
+			assert.True(t, status.IsSuccess())
+		})
+	}
+}
+
+func TestBeforePreFilterWithNodeAffinity(t *testing.T) {
+	node := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "node1",
+			Labels: map[string]string{
+				"test": "true",
+			},
+		},
+		Status: corev1.NodeStatus{
+			Allocatable: corev1.ResourceList{
+				corev1.ResourceCPU:    resource.MustParse("32"),
+				corev1.ResourceMemory: resource.MustParse("64Gi"),
+			},
+		},
+	}
+	matchedReservation := &schedulingv1alpha1.Reservation{
+		ObjectMeta: metav1.ObjectMeta{
+			UID:  uuid.NewUUID(),
+			Name: "reservation8C16G",
+		},
+		Spec: schedulingv1alpha1.ReservationSpec{
+			Owners: []schedulingv1alpha1.ReservationOwner{
+				{
+					LabelSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"test-reservation": "true",
+						},
+					},
+				},
+			},
+			Template: &corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{
 						{
 							Resources: corev1.ResourceRequirements{