From b5603b29d188c8cfd08173d02d5b5567d1656d97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Skocze=C5=84?= Date: Tue, 27 Aug 2024 13:30:31 +0000 Subject: [PATCH] Make LendingLimit enabled by default in the tests --- pkg/cache/cache_test.go | 99 +++++++++-- pkg/cache/clusterqueue_test.go | 57 ++++-- pkg/cache/snapshot_test.go | 163 +++++++++++++++++- pkg/metrics/metrics_test.go | 3 - .../flavorassigner/flavorassigner_test.go | 25 ++- pkg/scheduler/preemption/preemption_test.go | 25 ++- pkg/scheduler/scheduler_test.go | 27 ++- pkg/webhooks/clusterqueue_webhook_test.go | 26 +-- test/integration/scheduler/preemption_test.go | 7 +- test/integration/scheduler/scheduler_test.go | 4 +- 10 files changed, 354 insertions(+), 82 deletions(-) diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index b8dca3c517..c0e794b0d5 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -93,12 +93,12 @@ func TestCacheClusterQueueOperations(t *testing.T) { return nil } cases := []struct { - name string - operation func(*Cache) error - clientObjects []client.Object - wantClusterQueues map[string]*clusterQueue - wantCohorts map[string]sets.Set[string] - enableLendingLimit bool + name string + operation func(*Cache) error + clientObjects []client.Object + wantClusterQueues map[string]*clusterQueue + wantCohorts map[string]sets.Set[string] + disableLendingLimit bool }{ { name: "add", @@ -398,7 +398,6 @@ func TestCacheClusterQueueOperations(t *testing.T) { "one": sets.New("b"), "two": sets.New("a", "c", "e", "f"), }, - enableLendingLimit: true, }, { name: "shouldn't delete usage resources on update ClusterQueue", @@ -926,8 +925,7 @@ func TestCacheClusterQueueOperations(t *testing.T) { wantCohorts: map[string]sets.Set[string]{}, }, { - name: "add CQ with multiple resource groups and flavors", - enableLendingLimit: true, + name: "add CQ with multiple resource groups and flavors", operation: func(cache *Cache) error { cq := utiltesting.MakeClusterQueue("foo"). ResourceGroup( @@ -996,11 +994,92 @@ func TestCacheClusterQueueOperations(t *testing.T) { }, }, }, + { + name: "should not populate the fields with lendingLimit when feature disabled", + disableLendingLimit: true, + operation: func(cache *Cache) error { + cq := utiltesting.MakeClusterQueue("foo"). + ResourceGroup( + kueue.FlavorQuotas{ + Name: "on-demand", + Resources: []kueue.ResourceQuota{ + { + Name: corev1.ResourceCPU, + NominalQuota: resource.MustParse("10"), + LendingLimit: ptr.To(resource.MustParse("8")), + }, + { + Name: corev1.ResourceMemory, + NominalQuota: resource.MustParse("10Gi"), + LendingLimit: ptr.To(resource.MustParse("8Gi")), + }, + }, + }, + kueue.FlavorQuotas{ + Name: "spot", + Resources: []kueue.ResourceQuota{ + { + Name: corev1.ResourceCPU, + NominalQuota: resource.MustParse("20"), + LendingLimit: ptr.To(resource.MustParse("20")), + }, + { + Name: corev1.ResourceMemory, + NominalQuota: resource.MustParse("20Gi"), + LendingLimit: ptr.To(resource.MustParse("20Gi")), + }, + }, + }, + ). + ResourceGroup( + kueue.FlavorQuotas{ + Name: "license", + Resources: []kueue.ResourceQuota{ + { + Name: "license", + NominalQuota: resource.MustParse("8"), + LendingLimit: ptr.To(resource.MustParse("4")), + }, + }, + }, + ). + Obj() + return cache.AddClusterQueue(context.Background(), cq) + }, + wantClusterQueues: map[string]*clusterQueue{ + "foo": { + Name: "foo", + NamespaceSelector: labels.Everything(), + Status: pending, + Preemption: defaultPreemption, + AllocatableResourceGeneration: 1, + FlavorFungibility: defaultFlavorFungibility, + FairWeight: oneQuantity, + GuaranteedQuota: nil, + Usage: resources.FlavorResourceQuantities{ + {Flavor: "on-demand", Resource: corev1.ResourceCPU}: 0, + {Flavor: "on-demand", Resource: corev1.ResourceMemory}: 0, + {Flavor: "spot", Resource: corev1.ResourceCPU}: 0, + {Flavor: "spot", Resource: corev1.ResourceMemory}: 0, + {Flavor: "license", Resource: "license"}: 0, + }, + AdmittedUsage: resources.FlavorResourceQuantities{ + {Flavor: "on-demand", Resource: corev1.ResourceCPU}: 0, + {Flavor: "on-demand", Resource: corev1.ResourceMemory}: 0, + {Flavor: "spot", Resource: corev1.ResourceCPU}: 0, + {Flavor: "spot", Resource: corev1.ResourceMemory}: 0, + {Flavor: "license", Resource: "license"}: 0, + }, + }, + }, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, tc.enableLendingLimit)() + if tc.disableLendingLimit { + defer features.SetFeatureGateDuringTest(t, features.LendingLimit, false)() + } cache := New(utiltesting.NewFakeClient(tc.clientObjects...)) if err := tc.operation(cache); err != nil { t.Errorf("Unexpected error during test operation: %s", err) diff --git a/pkg/cache/clusterqueue_test.go b/pkg/cache/clusterqueue_test.go index 807867f52e..c2f0d84755 100644 --- a/pkg/cache/clusterqueue_test.go +++ b/pkg/cache/clusterqueue_test.go @@ -96,11 +96,11 @@ func TestClusterQueueUpdateWithFlavors(t *testing.T) { func TestFitInCohort(t *testing.T) { cases := map[string]struct { - request resources.FlavorResourceQuantities - wantFit bool - usage resources.FlavorResourceQuantities - clusterQueue []*kueue.ClusterQueue - enableLendingLimit bool + request resources.FlavorResourceQuantities + wantFit bool + usage resources.FlavorResourceQuantities + clusterQueue []*kueue.ClusterQueue + disableLendingLimit bool }{ "full cohort, empty request": { wantFit: true, @@ -262,7 +262,7 @@ func TestFitInCohort(t *testing.T) { Obj(), }, }, - "lendingLimit enabled can't fit": { + "lendingLimit can't fit": { request: resources.FlavorResourceQuantities{ {Flavor: "f1", Resource: corev1.ResourceCPU}: 3_000, }, @@ -295,9 +295,43 @@ func TestFitInCohort(t *testing.T) { Cohort("C"). Obj(), }, - enableLendingLimit: true, }, - "lendingLimit enabled can fit": { + "lendingLimit should not affect the fit when feature disabled": { + disableLendingLimit: true, + request: resources.FlavorResourceQuantities{ + {Flavor: "f1", Resource: corev1.ResourceCPU}: 3_000, + }, + wantFit: true, + usage: resources.FlavorResourceQuantities{ + {Flavor: "f1", Resource: corev1.ResourceCPU}: 2_000, + }, + clusterQueue: []*kueue.ClusterQueue{ + utiltesting. + MakeClusterQueue("CQ"). + ResourceGroup( + utiltesting.MakeFlavorQuotas("f1"). + ResourceQuotaWrapper(corev1.ResourceCPU). + NominalQuota("2"). + Append(). + FlavorQuotas, + ). + Cohort("C"). + Obj(), + utiltesting. + MakeClusterQueue("CQ-B"). + ResourceGroup( + utiltesting.MakeFlavorQuotas("f1"). + ResourceQuotaWrapper(corev1.ResourceCPU). + NominalQuota("3"). + LendingLimit("2"). + Append(). + FlavorQuotas, + ). + Cohort("C"). + Obj(), + }, + }, + "lendingLimit can fit": { request: resources.FlavorResourceQuantities{ {Flavor: "f1", Resource: corev1.ResourceCPU}: 3_000, }, @@ -330,13 +364,14 @@ func TestFitInCohort(t *testing.T) { Cohort("C"). Obj(), }, - enableLendingLimit: true, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, tc.enableLendingLimit)() + if tc.disableLendingLimit { + defer features.SetFeatureGateDuringTest(t, features.LendingLimit, false)() + } cache := New(utiltesting.NewFakeClient()) cache.AddOrUpdateResourceFlavor(utiltesting.MakeResourceFlavor("f1").Obj()) @@ -1018,8 +1053,6 @@ func TestDominantResourceShare(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, true)() - cache := New(utiltesting.NewFakeClient()) cache.AddOrUpdateResourceFlavor(utiltesting.MakeResourceFlavor("default").Obj()) cache.AddOrUpdateResourceFlavor(utiltesting.MakeResourceFlavor("on-demand").Obj()) diff --git a/pkg/cache/snapshot_test.go b/pkg/cache/snapshot_test.go index ac1c3dafa2..10456f7c2c 100644 --- a/pkg/cache/snapshot_test.go +++ b/pkg/cache/snapshot_test.go @@ -44,11 +44,11 @@ var snapCmpOpts = []cmp.Option{ func TestSnapshot(t *testing.T) { testCases := map[string]struct { - cqs []*kueue.ClusterQueue - rfs []*kueue.ResourceFlavor - wls []*kueue.Workload - wantSnapshot Snapshot - enableLendingLimit bool + cqs []*kueue.ClusterQueue + rfs []*kueue.ResourceFlavor + wls []*kueue.Workload + wantSnapshot Snapshot + disableLendingLimit bool }{ "empty": {}, "independent clusterQueues": { @@ -527,13 +527,161 @@ func TestSnapshot(t *testing.T) { }, } }(), - enableLendingLimit: true, + }, + "should not populate the snapshot with lendingLimit when feature disabled": { + disableLendingLimit: true, + cqs: []*kueue.ClusterQueue{ + utiltesting.MakeClusterQueue("a"). + Cohort("lending"). + ResourceGroup( + *utiltesting.MakeFlavorQuotas("arm").Resource(corev1.ResourceCPU, "10", "", "5").Obj(), + *utiltesting.MakeFlavorQuotas("x86").Resource(corev1.ResourceCPU, "20", "", "10").Obj(), + ). + FlavorFungibility(defaultFlavorFungibility). + Obj(), + utiltesting.MakeClusterQueue("b"). + Cohort("lending"). + ResourceGroup( + *utiltesting.MakeFlavorQuotas("arm").Resource(corev1.ResourceCPU, "10", "", "5").Obj(), + *utiltesting.MakeFlavorQuotas("x86").Resource(corev1.ResourceCPU, "20", "", "10").Obj(), + ). + Obj(), + }, + rfs: []*kueue.ResourceFlavor{ + utiltesting.MakeResourceFlavor("arm").NodeLabel("arch", "arm").Obj(), + utiltesting.MakeResourceFlavor("x86").NodeLabel("arch", "x86").Obj(), + }, + wls: []*kueue.Workload{ + utiltesting.MakeWorkload("alpha", ""). + PodSets(*utiltesting.MakePodSet("main", 5). + Request(corev1.ResourceCPU, "2").Obj()). + ReserveQuota(utiltesting.MakeAdmission("a", "main"). + Assignment(corev1.ResourceCPU, "arm", "10000m"). + AssignmentPodCount(5).Obj()). + Obj(), + utiltesting.MakeWorkload("beta", ""). + PodSets(*utiltesting.MakePodSet("main", 5). + Request(corev1.ResourceCPU, "1").Obj()). + ReserveQuota(utiltesting.MakeAdmission("a", "main"). + Assignment(corev1.ResourceCPU, "arm", "5000m"). + AssignmentPodCount(5).Obj()). + Obj(), + utiltesting.MakeWorkload("gamma", ""). + PodSets(*utiltesting.MakePodSet("main", 5). + Request(corev1.ResourceCPU, "2").Obj()). + ReserveQuota(utiltesting.MakeAdmission("a", "main"). + Assignment(corev1.ResourceCPU, "x86", "10000m"). + AssignmentPodCount(5).Obj()). + Obj(), + }, + wantSnapshot: func() Snapshot { + cohort := &CohortSnapshot{ + Name: "lending", + AllocatableResourceGeneration: 2, + RequestableResources: resources.FlavorResourceQuantities{ + {Flavor: "arm", Resource: corev1.ResourceCPU}: 20_000, + {Flavor: "x86", Resource: corev1.ResourceCPU}: 40_000, + }, + Usage: resources.FlavorResourceQuantities{ + {Flavor: "arm", Resource: corev1.ResourceCPU}: 15_000, + {Flavor: "x86", Resource: corev1.ResourceCPU}: 10_000, + }, + Lendable: map[corev1.ResourceName]int64{ + corev1.ResourceCPU: 60_000, + }, + } + return Snapshot{ + ClusterQueues: map[string]*ClusterQueueSnapshot{ + "a": { + Name: "a", + Cohort: cohort, + AllocatableResourceGeneration: 1, + ResourceGroups: []ResourceGroup{ + { + CoveredResources: sets.New(corev1.ResourceCPU), + Flavors: []kueue.ResourceFlavorReference{"arm", "x86"}, + LabelKeys: sets.New("arch"), + }, + }, + Quotas: map[resources.FlavorResource]*ResourceQuota{ + {Flavor: "arm", Resource: corev1.ResourceCPU}: {Nominal: 10_000, BorrowingLimit: nil, LendingLimit: nil}, + {Flavor: "x86", Resource: corev1.ResourceCPU}: {Nominal: 20_000, BorrowingLimit: nil, LendingLimit: nil}, + }, + FlavorFungibility: defaultFlavorFungibility, + FairWeight: oneQuantity, + Usage: resources.FlavorResourceQuantities{ + {Flavor: "arm", Resource: corev1.ResourceCPU}: 15_000, + {Flavor: "x86", Resource: corev1.ResourceCPU}: 10_000, + }, + Workloads: map[string]*workload.Info{ + "/alpha": workload.NewInfo(utiltesting.MakeWorkload("alpha", ""). + PodSets(*utiltesting.MakePodSet("main", 5). + Request(corev1.ResourceCPU, "2").Obj()). + ReserveQuota(utiltesting.MakeAdmission("a", "main"). + Assignment(corev1.ResourceCPU, "arm", "10000m"). + AssignmentPodCount(5).Obj()). + Obj()), + "/beta": workload.NewInfo(utiltesting.MakeWorkload("beta", ""). + PodSets(*utiltesting.MakePodSet("main", 5). + Request(corev1.ResourceCPU, "1").Obj()). + ReserveQuota(utiltesting.MakeAdmission("a", "main"). + Assignment(corev1.ResourceCPU, "arm", "5000m"). + AssignmentPodCount(5).Obj()). + Obj()), + "/gamma": workload.NewInfo(utiltesting.MakeWorkload("gamma", ""). + PodSets(*utiltesting.MakePodSet("main", 5). + Request(corev1.ResourceCPU, "2").Obj()). + ReserveQuota(utiltesting.MakeAdmission("a", "main"). + Assignment(corev1.ResourceCPU, "x86", "10000m"). + AssignmentPodCount(5).Obj()). + Obj()), + }, + Preemption: defaultPreemption, + NamespaceSelector: labels.Everything(), + Status: active, + GuaranteedQuota: nil, + }, + "b": { + Name: "b", + Cohort: cohort, + AllocatableResourceGeneration: 1, + ResourceGroups: []ResourceGroup{ + { + CoveredResources: sets.New(corev1.ResourceCPU), + Flavors: []kueue.ResourceFlavorReference{"arm", "x86"}, + LabelKeys: sets.New("arch"), + }, + }, + Quotas: map[resources.FlavorResource]*ResourceQuota{ + {Flavor: "arm", Resource: corev1.ResourceCPU}: {Nominal: 10_000, BorrowingLimit: nil, LendingLimit: nil}, + {Flavor: "x86", Resource: corev1.ResourceCPU}: {Nominal: 20_000, BorrowingLimit: nil, LendingLimit: nil}, + }, + FlavorFungibility: defaultFlavorFungibility, + FairWeight: oneQuantity, + Usage: resources.FlavorResourceQuantities{ + {Flavor: "arm", Resource: corev1.ResourceCPU}: 0, + {Flavor: "x86", Resource: corev1.ResourceCPU}: 0, + }, + Preemption: defaultPreemption, + NamespaceSelector: labels.Everything(), + Status: active, + GuaranteedQuota: nil, + }, + }, + ResourceFlavors: map[kueue.ResourceFlavorReference]*kueue.ResourceFlavor{ + "arm": utiltesting.MakeResourceFlavor("arm").NodeLabel("arch", "arm").Obj(), + "x86": utiltesting.MakeResourceFlavor("x86").NodeLabel("arch", "x86").Obj(), + }, + } + }(), }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, tc.enableLendingLimit)() + if tc.disableLendingLimit { + defer features.SetFeatureGateDuringTest(t, features.LendingLimit, false)() + } cache := New(utiltesting.NewFakeClient()) for _, cq := range tc.cqs { // Purposely do not make a copy of clusterQueues. Clones of necessary fields are @@ -824,7 +972,6 @@ func TestSnapshotAddRemoveWorkload(t *testing.T) { } func TestSnapshotAddRemoveWorkloadWithLendingLimit(t *testing.T) { - _ = features.SetEnable(features.LendingLimit, true) flavors := []*kueue.ResourceFlavor{ utiltesting.MakeResourceFlavor("default").Obj(), } diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index fbc5bff4e5..0bfe628c26 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/metrics/metrics_test.go @@ -22,7 +22,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/prometheus/client_golang/prometheus" - "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/util/testing/metrics" ) @@ -47,7 +46,6 @@ func TestGenerateExponentialBuckets(t *testing.T) { } func TestReportAndCleanupClusterQueueMetrics(t *testing.T) { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, true)() ReportClusterQueueQuotas("cohort", "queue", "flavor", "res", 5, 10, 3) ReportClusterQueueQuotas("cohort", "queue", "flavor2", "res", 1, 2, 1) @@ -74,7 +72,6 @@ func TestReportAndCleanupClusterQueueMetrics(t *testing.T) { } func TestReportAndCleanupClusterQueueQuotas(t *testing.T) { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, true)() ReportClusterQueueQuotas("cohort", "queue", "flavor", "res", 5, 10, 3) ReportClusterQueueQuotas("cohort", "queue", "flavor", "res2", 5, 10, 3) ReportClusterQueueQuotas("cohort", "queue", "flavor2", "res", 1, 2, 1) diff --git a/pkg/scheduler/flavorassigner/flavorassigner_test.go b/pkg/scheduler/flavorassigner/flavorassigner_test.go index 96c604800c..e0684224f1 100644 --- a/pkg/scheduler/flavorassigner/flavorassigner_test.go +++ b/pkg/scheduler/flavorassigner/flavorassigner_test.go @@ -66,15 +66,15 @@ func TestAssignFlavors(t *testing.T) { } cases := map[string]struct { - wlPods []kueue.PodSet - wlReclaimablePods []kueue.ReclaimablePod - clusterQueue kueue.ClusterQueue - clusterQueueUsage resources.FlavorResourceQuantities - cohortResources *cohortResources - wantRepMode FlavorAssignmentMode - wantAssignment Assignment - enableLendingLimit bool - enableFairSharing bool + wlPods []kueue.PodSet + wlReclaimablePods []kueue.ReclaimablePod + clusterQueue kueue.ClusterQueue + clusterQueueUsage resources.FlavorResourceQuantities + cohortResources *cohortResources + wantRepMode FlavorAssignmentMode + wantAssignment Assignment + disableLendingLimit bool + enableFairSharing bool }{ "single flavor, fits": { wlPods: []kueue.PodSet{ @@ -1726,7 +1726,6 @@ func TestAssignFlavors(t *testing.T) { {Flavor: "two", Resource: corev1.ResourcePods}: 1, }, }, - enableLendingLimit: true, }, "lend try next flavor, found the first flavor": { wlPods: []kueue.PodSet{ @@ -1781,7 +1780,6 @@ func TestAssignFlavors(t *testing.T) { {Flavor: "one", Resource: corev1.ResourcePods}: 1, }, }, - enableLendingLimit: true, }, "quota exhausted, but can preempt in cohort and ClusterQueue": { wlPods: []kueue.PodSet{ @@ -1830,7 +1828,6 @@ func TestAssignFlavors(t *testing.T) { {Flavor: "one", Resource: corev1.ResourcePods}: 1, }, }, - enableLendingLimit: true, }, "when borrowing while preemption is needed for flavor one, fair sharing enabled, reclaimWithinCohort=Any": { enableFairSharing: true, @@ -1927,7 +1924,9 @@ func TestAssignFlavors(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, tc.enableLendingLimit)() + if tc.disableLendingLimit { + defer features.SetFeatureGateDuringTest(t, features.LendingLimit, false)() + } log := testr.NewWithOptions(t, testr.Options{ Verbosity: 2, }) diff --git a/pkg/scheduler/preemption/preemption_test.go b/pkg/scheduler/preemption/preemption_test.go index 705341d4f3..7a4ba86964 100644 --- a/pkg/scheduler/preemption/preemption_test.go +++ b/pkg/scheduler/preemption/preemption_test.go @@ -276,12 +276,12 @@ func TestPreemption(t *testing.T) { Obj(), } cases := map[string]struct { - admitted []kueue.Workload - incoming *kueue.Workload - targetCQ string - assignment flavorassigner.Assignment - wantPreempted sets.Set[string] - enableLendingLimit bool + admitted []kueue.Workload + incoming *kueue.Workload + targetCQ string + assignment flavorassigner.Assignment + wantPreempted sets.Set[string] + disableLendingLimit bool }{ "preempt lowest priority": { admitted: []kueue.Workload{ @@ -1162,8 +1162,7 @@ func TestPreemption(t *testing.T) { Mode: flavorassigner.Preempt, }, }), - wantPreempted: sets.New(targetKeyReason("/lend2-mid", kueue.InCohortReclamationReason)), - enableLendingLimit: true, + wantPreempted: sets.New(targetKeyReason("/lend2-mid", kueue.InCohortReclamationReason)), }, "preempt from all ClusterQueues in cohort-lend": { admitted: []kueue.Workload{ @@ -1196,8 +1195,7 @@ func TestPreemption(t *testing.T) { Mode: flavorassigner.Preempt, }, }), - wantPreempted: sets.New(targetKeyReason("/lend1-low", kueue.InClusterQueueReason), targetKeyReason("/lend2-low", kueue.InCohortReclamationReason)), - enableLendingLimit: true, + wantPreempted: sets.New(targetKeyReason("/lend1-low", kueue.InClusterQueueReason), targetKeyReason("/lend2-low", kueue.InCohortReclamationReason)), }, "cannot preempt from other ClusterQueues if exceeds requestable quota including lending limit": { admitted: []kueue.Workload{ @@ -1217,8 +1215,7 @@ func TestPreemption(t *testing.T) { Mode: flavorassigner.Preempt, }, }), - wantPreempted: nil, - enableLendingLimit: true, + wantPreempted: nil, }, "preemptions from cq when target queue is exhausted for the single requested resource": { admitted: []kueue.Workload{ @@ -1422,7 +1419,9 @@ func TestPreemption(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, tc.enableLendingLimit)() + if tc.disableLendingLimit { + defer features.SetFeatureGateDuringTest(t, features.LendingLimit, false)() + } ctx, log := utiltesting.ContextWithLog(t) cl := utiltesting.NewClientBuilder(). WithLists(&kueue.WorkloadList{Items: tc.admitted}). diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index 6bb40b3dfa..ff6a2faecf 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -252,7 +252,7 @@ func TestSchedule(t *testing.T) { } cases := map[string]struct { // Features - enableLendingLimit bool + disableLendingLimit bool disablePartialAdmission bool enableFairSharing bool @@ -747,7 +747,26 @@ func TestSchedule(t *testing.T) { wantInadmissibleLeft: map[string][]string{ "lend-b": {"lend/b"}, }, - enableLendingLimit: true, + }, + "lendingLimit should not affect assignments when feature disabled": { + disableLendingLimit: true, + workloads: []kueue.Workload{ + *utiltesting.MakeWorkload("a", "lend"). + Request(corev1.ResourceCPU, "2"). + ReserveQuota(utiltesting.MakeAdmission("lend-b").Assignment(corev1.ResourceCPU, "default", "2000m").Obj()). + Obj(), + *utiltesting.MakeWorkload("b", "lend"). + Queue("lend-b-queue"). + Request(corev1.ResourceCPU, "3"). + Obj(), + }, + wantAssignments: map[string]kueue.Admission{ + "lend/a": *utiltesting.MakeAdmission("lend-b").Assignment(corev1.ResourceCPU, "default", "2000m").Obj(), + "lend/b": *utiltesting.MakeAdmission("lend-b").Assignment(corev1.ResourceCPU, "default", "3000m").Obj(), + }, + wantScheduled: []string{ + "lend/b", + }, }, "preempt workloads in ClusterQueue and cohort": { workloads: []kueue.Workload{ @@ -2506,8 +2525,8 @@ func TestSchedule(t *testing.T) { for name, tc := range cases { tc.multiplePreemptions.runTest(name, t, func(t *testing.T) { - if tc.enableLendingLimit { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, true)() + if tc.disableLendingLimit { + defer features.SetFeatureGateDuringTest(t, features.LendingLimit, false)() } if tc.disablePartialAdmission { defer features.SetFeatureGateDuringTest(t, features.PartialAdmission, false)() diff --git a/pkg/webhooks/clusterqueue_webhook_test.go b/pkg/webhooks/clusterqueue_webhook_test.go index 3c8f9d5b5e..8e27f1f946 100644 --- a/pkg/webhooks/clusterqueue_webhook_test.go +++ b/pkg/webhooks/clusterqueue_webhook_test.go @@ -36,10 +36,10 @@ func TestValidateClusterQueue(t *testing.T) { resourceGroupsPath := specPath.Child("resourceGroups") testcases := []struct { - name string - clusterQueue *kueue.ClusterQueue - wantErr field.ErrorList - enableLendingLimit bool + name string + clusterQueue *kueue.ClusterQueue + wantErr field.ErrorList + disableLendingLimit bool }{ { name: "built-in resources with qualified names", @@ -137,7 +137,6 @@ func TestValidateClusterQueue(t *testing.T) { *testingutil.MakeFlavorQuotas("x86").Resource("cpu", "1", "", "0").Obj()). Cohort("cohort"). Obj(), - enableLendingLimit: true, }, { name: "flavor quota with negative lendingLimit", @@ -149,7 +148,6 @@ func TestValidateClusterQueue(t *testing.T) { wantErr: field.ErrorList{ field.Invalid(resourceGroupsPath.Index(0).Child("flavors").Index(0).Child("resources").Index(0).Child("lendingLimit"), "-1", ""), }, - enableLendingLimit: true, }, { name: "flavor quota with lendingLimit and empty cohort", @@ -160,7 +158,6 @@ func TestValidateClusterQueue(t *testing.T) { wantErr: field.ErrorList{ field.Invalid(resourceGroupsPath.Index(0).Child("flavors").Index(0).Child("resources").Index(0).Child("lendingLimit"), "1", limitIsEmptyErrorMsg), }, - enableLendingLimit: true, }, { name: "flavor quota with lendingLimit greater than nominalQuota", @@ -172,7 +169,14 @@ func TestValidateClusterQueue(t *testing.T) { wantErr: field.ErrorList{ field.Invalid(resourceGroupsPath.Index(0).Child("flavors").Index(0).Child("resources").Index(0).Child("lendingLimit"), "2", lendingLimitErrorMsg), }, - enableLendingLimit: true, + }, + { + name: "flavor quota with lendingLimit and empty cohort, but feature disabled", + disableLendingLimit: true, + clusterQueue: testingutil.MakeClusterQueue("cluster-queue"). + ResourceGroup( + *testingutil.MakeFlavorQuotas("x86").Resource("cpu", "1", "", "1").Obj()). + Obj(), }, { name: "empty queueing strategy is supported", @@ -318,12 +322,14 @@ func TestValidateClusterQueue(t *testing.T) { }, }, }, - }, + }, // TODO: TC with lending limit disabled } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - defer features.SetFeatureGateDuringTest(t, features.LendingLimit, tc.enableLendingLimit)() + if tc.disableLendingLimit { + defer features.SetFeatureGateDuringTest(t, features.LendingLimit, false)() + } gotErr := ValidateClusterQueue(tc.clusterQueue) if diff := cmp.Diff(tc.wantErr, gotErr, cmpopts.IgnoreFields(field.Error{}, "Detail", "BadValue")); diff != "" { t.Errorf("ValidateResources() mismatch (-want +got):\n%s", diff) diff --git a/test/integration/scheduler/preemption_test.go b/test/integration/scheduler/preemption_test.go index 1a1d98d34a..50c305935f 100644 --- a/test/integration/scheduler/preemption_test.go +++ b/test/integration/scheduler/preemption_test.go @@ -722,18 +722,13 @@ var _ = ginkgo.Describe("Preemption", func() { }) }) - ginkgo.Context("When lending limit enabled", func() { + ginkgo.Context("With lending limit", func() { var ( prodCQ *kueue.ClusterQueue devCQ *kueue.ClusterQueue ) - ginkgo.BeforeEach(func() { - _ = features.SetEnable(features.LendingLimit, true) - }) - ginkgo.AfterEach(func() { - _ = features.SetEnable(features.LendingLimit, false) gomega.Expect(util.DeleteNamespace(ctx, k8sClient, ns)).To(gomega.Succeed()) util.ExpectObjectToBeDeleted(ctx, k8sClient, prodCQ, true) if devCQ != nil { diff --git a/test/integration/scheduler/scheduler_test.go b/test/integration/scheduler/scheduler_test.go index 94fd8859c9..1131793347 100644 --- a/test/integration/scheduler/scheduler_test.go +++ b/test/integration/scheduler/scheduler_test.go @@ -1407,19 +1407,17 @@ var _ = ginkgo.Describe("Scheduler", func() { }) }) - ginkgo.When("Using cohorts for sharing when LendingLimit enabled", func() { + ginkgo.When("Using cohorts for sharing with LendingLimit", func() { var ( prodCQ *kueue.ClusterQueue devCQ *kueue.ClusterQueue ) ginkgo.BeforeEach(func() { - _ = features.SetEnable(features.LendingLimit, true) gomega.Expect(k8sClient.Create(ctx, onDemandFlavor)).Should(gomega.Succeed()) }) ginkgo.AfterEach(func() { - _ = features.SetEnable(features.LendingLimit, false) gomega.Expect(util.DeleteNamespace(ctx, k8sClient, ns)).To(gomega.Succeed()) util.ExpectObjectToBeDeleted(ctx, k8sClient, prodCQ, true) if devCQ != nil {