Skip to content

Commit ebb3de8

Browse files
saintubeshenxin
saintube
and
shenxin
authored
scheduler: add OmitNodeLabelsForReservation, expose GetQuotaInformer (#2412)
Signed-off-by: saintube <[email protected]> Co-authored-by: shenxin <[email protected]>
1 parent c0c7698 commit ebb3de8

File tree

8 files changed

+161
-165
lines changed

8 files changed

+161
-165
lines changed

‎pkg/features/scheduler_features.go

+7
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ const (
6464
// LazyReservationRestore is used to restore reserved resources lazily to improve the scheduling performance.
6565
LazyReservationRestore featuregate.Feature = "LazyReservationRestore"
6666

67+
// owner: @saintube @ZiMengSheng
68+
// alpha: v1.6
69+
//
70+
// OmitNodeLabelsForReservation is used to omit node labels while matching reservation affinity.
71+
OmitNodeLabelsForReservation featuregate.Feature = "OmitNodeLabelsForReservation"
72+
6773
CSIStorageCapacity featuregate.Feature = "CSIStorageCapacity"
6874

6975
GenericEphemeralVolume featuregate.Feature = "GenericEphemeralVolume"
@@ -85,6 +91,7 @@ var defaultSchedulerFeatureGates = map[featuregate.Feature]featuregate.FeatureSp
8591
DisableDefaultQuota: {Default: false, PreRelease: featuregate.Alpha},
8692
SupportParentQuotaSubmitPod: {Default: false, PreRelease: featuregate.Alpha},
8793
LazyReservationRestore: {Default: false, PreRelease: featuregate.Alpha},
94+
OmitNodeLabelsForReservation: {Default: false, PreRelease: featuregate.Alpha},
8895
CSIStorageCapacity: {Default: true, PreRelease: featuregate.GA}, // remove in 1.26
8996
GenericEphemeralVolume: {Default: true, PreRelease: featuregate.GA},
9097
PodDisruptionBudget: {Default: true, PreRelease: featuregate.GA},

‎pkg/scheduler/frameworkext/reservation_info.go

+21-11
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"k8s.io/apimachinery/pkg/types"
2323
utilerrors "k8s.io/apimachinery/pkg/util/errors"
2424
quotav1 "k8s.io/apiserver/pkg/quota/v1"
25+
k8sfeature "k8s.io/apiserver/pkg/util/feature"
2526
corev1helpers "k8s.io/component-helpers/scheduling/corev1"
2627
"k8s.io/klog/v2"
2728
k8spodutil "k8s.io/kubernetes/pkg/api/v1/pod"
@@ -30,6 +31,7 @@ import (
3031

3132
apiext "github.com/koordinator-sh/koordinator/apis/extension"
3233
schedulingv1alpha1 "github.com/koordinator-sh/koordinator/apis/scheduling/v1alpha1"
34+
"github.com/koordinator-sh/koordinator/pkg/features"
3335
"github.com/koordinator-sh/koordinator/pkg/util"
3436
reservationutil "github.com/koordinator-sh/koordinator/pkg/util/reservation"
3537
)
@@ -47,6 +49,7 @@ type ReservationInfo struct {
4749
AssignedPods map[types.UID]*PodRequirement
4850
OwnerMatchers []reservationutil.ReservationOwnerMatcher
4951
ParseError error
52+
Labels map[string]string
5053
}
5154

5255
type PodRequirement struct {
@@ -303,22 +306,29 @@ func (ri *ReservationInfo) GetTaints() []corev1.Taint {
303306

304307
// MatchReservationAffinity returns the statuses of whether the reservation affinity matches, whether the reservation
305308
// taints are tolerated, and whether the reservation name matches.
306-
func (ri *ReservationInfo) MatchReservationAffinity(reservationAffinity *reservationutil.RequiredReservationAffinity, nodeLabels map[string]string) bool {
309+
func (ri *ReservationInfo) MatchReservationAffinity(reservationAffinity *reservationutil.RequiredReservationAffinity, node *corev1.Node) bool {
307310
if reservationAffinity != nil {
308-
// NOTE: There are some special scenarios.
309-
// For example, the AZ where the Pod wants to select the Reservation is cn-hangzhou, but the Reservation itself
310-
// does not have this information, so it needs to perceive the label of the Node when Matching Affinity.
311311
fakeNode := &corev1.Node{
312312
ObjectMeta: metav1.ObjectMeta{
313-
Name: ri.GetName(),
314-
Labels: map[string]string{},
313+
Name: ri.GetName(),
315314
},
316315
}
317-
for k, v := range nodeLabels {
318-
fakeNode.Labels[k] = v
319-
}
320-
for k, v := range ri.GetObject().GetLabels() {
321-
fakeNode.Labels[k] = v
316+
reservationLabels := ri.GetObject().GetLabels()
317+
if k8sfeature.DefaultFeatureGate.Enabled(features.OmitNodeLabelsForReservation) {
318+
// In this case, we suppose the necessary node labels have been patched to the reservation.
319+
fakeNode.Labels = reservationLabels
320+
} else {
321+
// NOTE: There are some special scenarios.
322+
// For example, the AZ where the Pod wants to select the Reservation is cn-hangzhou, but the Reservation itself
323+
// does not have this information, so it needs to perceive the label of the Node when Matching Affinity.
324+
nodeLabels := node.GetLabels()
325+
fakeNode.Labels = make(map[string]string, len(nodeLabels)+len(reservationLabels))
326+
for k, v := range nodeLabels {
327+
fakeNode.Labels[k] = v
328+
}
329+
for k, v := range reservationLabels {
330+
fakeNode.Labels[k] = v
331+
}
322332
}
323333
return reservationAffinity.MatchAffinity(fakeNode)
324334
}

‎pkg/scheduler/frameworkext/reservation_info_test.go

+126
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package frameworkext
1818

1919
import (
20+
"encoding/json"
2021
"sort"
2122
"testing"
2223
"time"
@@ -733,6 +734,7 @@ func TestReservationInfoRefreshAvailable(t *testing.T) {
733734
},
734735
},
735736
},
737+
736738
Owners: []schedulingv1alpha1.ReservationOwner{
737739
{
738740
LabelSelector: &metav1.LabelSelector{
@@ -743,6 +745,7 @@ func TestReservationInfoRefreshAvailable(t *testing.T) {
743745
},
744746
},
745747
},
748+
746749
Status: schedulingv1alpha1.ReservationStatus{
747750
NodeName: "test-node",
748751
Phase: schedulingv1alpha1.ReasonReservationAvailable,
@@ -792,3 +795,126 @@ func TestReservationInfoRefreshAvailable(t *testing.T) {
792795
assert.Equal(t, expectedRInfo, rInfo)
793796
})
794797
}
798+
799+
func TestReservationInfoMatchReservationAffinity(t *testing.T) {
800+
tests := []struct {
801+
name string
802+
reservation *schedulingv1alpha1.Reservation
803+
reservationAffinity *apiext.ReservationAffinity
804+
want bool
805+
}{
806+
{
807+
name: "nothing to match",
808+
reservation: &schedulingv1alpha1.Reservation{
809+
Spec: schedulingv1alpha1.ReservationSpec{
810+
Owners: []schedulingv1alpha1.ReservationOwner{
811+
{
812+
LabelSelector: &metav1.LabelSelector{
813+
MatchLabels: map[string]string{
814+
"app": "test",
815+
},
816+
},
817+
},
818+
},
819+
},
820+
},
821+
want: true,
822+
},
823+
{
824+
name: "match reservation affinity",
825+
reservation: &schedulingv1alpha1.Reservation{
826+
ObjectMeta: metav1.ObjectMeta{
827+
Labels: map[string]string{
828+
"reservation-type": "reservation-test",
829+
},
830+
},
831+
Spec: schedulingv1alpha1.ReservationSpec{
832+
Owners: []schedulingv1alpha1.ReservationOwner{
833+
{
834+
LabelSelector: &metav1.LabelSelector{
835+
MatchLabels: map[string]string{
836+
"app": "test",
837+
},
838+
},
839+
},
840+
},
841+
},
842+
},
843+
reservationAffinity: &apiext.ReservationAffinity{
844+
RequiredDuringSchedulingIgnoredDuringExecution: &apiext.ReservationAffinitySelector{
845+
ReservationSelectorTerms: []corev1.NodeSelectorTerm{
846+
{
847+
MatchExpressions: []corev1.NodeSelectorRequirement{
848+
{
849+
Key: "reservation-type",
850+
Operator: corev1.NodeSelectorOpIn,
851+
Values: []string{"reservation-test"},
852+
},
853+
},
854+
},
855+
},
856+
},
857+
},
858+
want: true,
859+
},
860+
{
861+
name: "not match reservation affinity",
862+
reservation: &schedulingv1alpha1.Reservation{
863+
ObjectMeta: metav1.ObjectMeta{
864+
Labels: map[string]string{
865+
"reservation-type": "reservation-test-not-match",
866+
},
867+
},
868+
Spec: schedulingv1alpha1.ReservationSpec{
869+
Owners: []schedulingv1alpha1.ReservationOwner{
870+
{
871+
LabelSelector: &metav1.LabelSelector{
872+
MatchLabels: map[string]string{
873+
"app": "test",
874+
},
875+
},
876+
},
877+
},
878+
},
879+
},
880+
reservationAffinity: &apiext.ReservationAffinity{
881+
RequiredDuringSchedulingIgnoredDuringExecution: &apiext.ReservationAffinitySelector{
882+
ReservationSelectorTerms: []corev1.NodeSelectorTerm{
883+
{
884+
MatchExpressions: []corev1.NodeSelectorRequirement{
885+
{
886+
Key: "reservation-type",
887+
Operator: corev1.NodeSelectorOpIn,
888+
Values: []string{"reservation-test"},
889+
},
890+
},
891+
},
892+
},
893+
},
894+
},
895+
want: false,
896+
},
897+
}
898+
for _, tt := range tests {
899+
t.Run(tt.name, func(t *testing.T) {
900+
pod := &corev1.Pod{
901+
ObjectMeta: metav1.ObjectMeta{
902+
Name: "test-pod",
903+
},
904+
}
905+
if tt.reservationAffinity != nil {
906+
affinityData, err := json.Marshal(tt.reservationAffinity)
907+
assert.NoError(t, err)
908+
if pod.Annotations == nil {
909+
pod.Annotations = map[string]string{}
910+
}
911+
pod.Annotations[apiext.AnnotationReservationAffinity] = string(affinityData)
912+
}
913+
reservationAffinity, err := reservationutil.GetRequiredReservationAffinity(pod)
914+
assert.NoError(t, err)
915+
rInfo := NewReservationInfo(tt.reservation)
916+
got := rInfo.MatchReservationAffinity(reservationAffinity, &corev1.Node{})
917+
assert.Equal(t, tt.want, got)
918+
})
919+
}
920+
}

‎pkg/scheduler/plugins/elasticquota/plugin.go

+4
Original file line numberDiff line numberDiff line change
@@ -359,3 +359,7 @@ func (g *Plugin) Unreserve(ctx context.Context, state *framework.CycleState, p *
359359
}
360360
mgr.UnreservePod(quotaName, p)
361361
}
362+
363+
func (g *Plugin) GetQuotaInformer() cache.SharedIndexInformer { // expose for extensions
364+
return g.quotaInformer
365+
}

‎pkg/scheduler/plugins/reservation/plugin.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ func (pl *Plugin) PreFilter(ctx context.Context, cycleState *framework.CycleStat
307307
return nil, framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonReservationAffinity)
308308
}
309309
preResult = &framework.PreFilterResult{
310-
NodeNames: sets.Set[string]{},
310+
NodeNames: make(sets.Set[string], len(state.nodeReservationStates)),
311311
}
312312
for nodeName := range state.nodeReservationStates {
313313
preResult.NodeNames.Insert(nodeName)

‎pkg/scheduler/plugins/reservation/transformer.go

+1-28
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func (pl *Plugin) prepareMatchReservationState(ctx context.Context, cycleState *
133133
diagnosisState.taintsUnmatched++
134134
taintKey := getDiagnosisTaintKey(&firstUnmatchedTaint)
135135
diagnosisState.taintsUnmatchedReasons[taintKey]++
136-
} else if !matchReservationAffinity(node, rInfo, reservationAffinity) { // ReservationAffinity unmatched
136+
} else if !rInfo.MatchReservationAffinity(reservationAffinity, node) { // ReservationAffinity unmatched
137137
diagnosisState.affinityUnmatched++
138138
} else if !extension.ExactMatchReservation(podRequests, rInfo.Allocatable, exactMatchReservationSpec) { // exactMatchSpec unmatched
139139
diagnosisState.notExactMatched++
@@ -543,33 +543,6 @@ func calculateResource(pod *corev1.Pod) (res framework.Resource, non0CPU int64,
543543
return
544544
}
545545

546-
// matchReservationAffinity returns the statuses of whether the reservation affinity matches, whether the reservation
547-
// taints are tolerated, and whether the reservation name matches.
548-
func matchReservationAffinity(node *corev1.Node, reservation *frameworkext.ReservationInfo, reservationAffinity *reservationutil.RequiredReservationAffinity) bool {
549-
if reservationAffinity != nil {
550-
// NOTE: There are some special scenarios.
551-
// For example, the AZ where the Pod wants to select the Reservation is cn-hangzhou, but the Reservation itself
552-
// does not have this information, so it needs to perceive the label of the Node when Matching Affinity.
553-
// FIXME(saintube): clean up the default node labels casting and preserve optional labels
554-
// https://github.com/koordinator-sh/koordinator/issues/2208
555-
reservationLabels := reservation.GetObject().GetLabels()
556-
fakeNode := &corev1.Node{
557-
ObjectMeta: metav1.ObjectMeta{
558-
Name: reservation.GetName(),
559-
Labels: make(map[string]string, len(node.Labels)+len(reservationLabels)),
560-
},
561-
}
562-
for k, v := range node.Labels {
563-
fakeNode.Labels[k] = v
564-
}
565-
for k, v := range reservationLabels {
566-
fakeNode.Labels[k] = v
567-
}
568-
return reservationAffinity.MatchAffinity(fakeNode)
569-
}
570-
return true
571-
}
572-
573546
func parseSpecificNodesFromAffinity(pod *corev1.Pod) (sets.String, *framework.Status) {
574547
affinity := pod.Spec.Affinity
575548
if affinity == nil ||

0 commit comments

Comments
 (0)