diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml index 65d14a3d35..1e9c98bd86 100644 --- a/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml +++ b/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml @@ -74,8 +74,9 @@ spec: description: ClusterQueueSpec defines the desired state of ClusterQueue properties: admissionCheckStrategy: - description: admissionCheckStrategy is the list of AdmissionChecks - that should run when a particular ResourceFlavor is used. + description: |- + admissionCheckStrategy is the list of AdmissionChecks that should run when a particular ResourceFlavor is used. + Cannot be used along with AdmissionChecks. items: properties: name: @@ -84,7 +85,7 @@ spec: onFlavors: description: |- forFlavors is a list of ResourceFlavors' metav1.Names that this AdmissionCheck should run for. - if empty the AdmissionCheck will run for all workloads submitted to the ClusterQueue. + If empty, the AdmissionCheck will run for all workloads submitted to the ClusterQueue. items: type: string type: array @@ -94,8 +95,9 @@ spec: type: object type: array admissionChecks: - description: admissionChecks lists the AdmissionChecks required by - this ClusterQueue + description: |- + admissionChecks lists the AdmissionChecks required by this ClusterQueue. + Cannot be used along with AdmissionCheckStrategy. items: type: string type: array diff --git a/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml b/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml index da9db9b3e8..d19342edca 100644 --- a/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml +++ b/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml @@ -59,8 +59,9 @@ spec: description: ClusterQueueSpec defines the desired state of ClusterQueue properties: admissionCheckStrategy: - description: admissionCheckStrategy is the list of AdmissionChecks - that should run when a particular ResourceFlavor is used. + description: |- + admissionCheckStrategy is the list of AdmissionChecks that should run when a particular ResourceFlavor is used. + Cannot be used along with AdmissionChecks. items: properties: name: @@ -69,7 +70,7 @@ spec: onFlavors: description: |- forFlavors is a list of ResourceFlavors' metav1.Names that this AdmissionCheck should run for. - if empty the AdmissionCheck will run for all workloads submitted to the ClusterQueue. + If empty, the AdmissionCheck will run for all workloads submitted to the ClusterQueue. items: type: string type: array @@ -79,8 +80,9 @@ spec: type: object type: array admissionChecks: - description: admissionChecks lists the AdmissionChecks required by - this ClusterQueue + description: |- + admissionChecks lists the AdmissionChecks required by this ClusterQueue. + Cannot be used along with AdmissionCheckStrategy. items: type: string type: array diff --git a/pkg/controller/core/workload_controller_test.go b/pkg/controller/core/workload_controller_test.go index 85aefd8a20..4655e7d5fd 100644 --- a/pkg/controller/core/workload_controller_test.go +++ b/pkg/controller/core/workload_controller_test.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/types" testingclock "k8s.io/utils/clock/testing" "k8s.io/utils/ptr" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -310,6 +309,7 @@ var ( kueue.Workload{}, "TypeMeta", "ObjectMeta.ResourceVersion", "Status.RequeueState.RequeueAt", ), cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"), + cmpopts.IgnoreFields(kueue.AdmissionCheckState{}, "LastTransitionTime"), cmpopts.SortSlices(func(a, b metav1.Condition) bool { return a.Type < b.Type }), } ) @@ -318,11 +318,61 @@ func TestReconcile(t *testing.T) { testStartTime := time.Now() cases := map[string]struct { workload *kueue.Workload + cq *kueue.ClusterQueue + lq *kueue.LocalQueue wantWorkload *kueue.Workload wantError error wantEvents []utiltesting.EventRecord reconcilerOpts []Option }{ + "assign Admission Checks from ClusterQueue.spec.AdmissionCheckStrategy": { + workload: utiltesting.MakeWorkload("wl", "ns"). + ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). + Queue("queue"). + Obj(), + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionCheckStrategy( + *utiltesting.MakeAdmissionCheckStrategy("ac1", "flavor1").Obj(), + *utiltesting.MakeAdmissionCheckStrategy("ac2").Obj()). + Obj(), + lq: utiltesting.MakeLocalQueue("queue", "ns").ClusterQueue("cq").Obj(), + wantWorkload: utiltesting.MakeWorkload("wl", "ns"). + ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). + Queue("queue"). + AdmissionChecks( + kueue.AdmissionCheckState{ + Name: "ac1", + State: kueue.CheckStatePending, + }, + kueue.AdmissionCheckState{ + Name: "ac2", + State: kueue.CheckStatePending, + }). + Obj(), + }, + "assign Admission Checks from ClusterQueue.spec.AdmissionChecks": { + workload: utiltesting.MakeWorkload("wl", "ns"). + ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). + Queue("queue"). + Obj(), + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionChecks("ac1", "ac2"). + Obj(), + lq: utiltesting.MakeLocalQueue("queue", "ns").ClusterQueue("cq").Obj(), + wantWorkload: utiltesting.MakeWorkload("wl", "ns"). + ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). + Queue("queue"). + AdmissionChecks( + kueue.AdmissionCheckState{ + Name: "ac1", + State: kueue.CheckStatePending, + }, + kueue.AdmissionCheckState{ + Name: "ac2", + State: kueue.CheckStatePending, + }). + Obj(), + }, "admit": { workload: utiltesting.MakeWorkload("wl", "ns"). ReserveQuota(utiltesting.MakeAdmission("q1").Obj()). @@ -545,6 +595,18 @@ func TestReconcile(t *testing.T) { ctx, ctxCancel := context.WithCancel(ctxWithLogger) defer ctxCancel() + if tc.cq != nil { + if err := cl.Create(ctx, tc.cq); err != nil { + t.Error("couldn't create the cluster queue", err) + } + if err := qManager.AddClusterQueue(ctx, tc.cq); err != nil { + t.Error("couldn't add the cluster queue to the cache", err) + } + if err := qManager.AddLocalQueue(ctx, tc.lq); err != nil { + t.Error("couldn't add the local queue to the cache", err) + } + } + _, gotError := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(tc.workload)}) if diff := cmp.Diff(tc.wantError, gotError); diff != "" { @@ -586,63 +648,3 @@ func TestReconcile(t *testing.T) { }) } } - -func TestAdmissionCheckStrategy(t *testing.T) { - cases := map[string]struct { - cq *kueue.ClusterQueue - wl *kueue.Workload - wantAdmissionChecks []string - }{ - "AdmissionCheckStrategy with a flavor": { - wl: utiltesting.MakeWorkload("wl", "ns"). - ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). - Obj(), - cq: utiltesting.MakeClusterQueue("cq"). - AdmissionCheckStrategy(*utiltesting.MakeAdmissionCheckStrategy("ac1", "flavor1").Obj()). - Obj(), - wantAdmissionChecks: []string{"ac1"}, - }, - "AdmissionCheckStrategy without a flavor": { - wl: utiltesting.MakeWorkload("wl", "ns"). - ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). - Obj(), - cq: utiltesting.MakeClusterQueue("cq"). - AdmissionCheckStrategy(*utiltesting.MakeAdmissionCheckStrategy("ac1").Obj()). - Obj(), - wantAdmissionChecks: []string{"ac1"}, - }, - "Two AdmissionCheckStrategies, one with flavor, one without flavor": { - wl: utiltesting.MakeWorkload("wl", "ns"). - ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). - Obj(), - cq: utiltesting.MakeClusterQueue("cq"). - AdmissionCheckStrategy( - *utiltesting.MakeAdmissionCheckStrategy("ac1", "flavor1").Obj(), - *utiltesting.MakeAdmissionCheckStrategy("ac2").Obj()). - Obj(), - wantAdmissionChecks: []string{"ac1", "ac2"}, - }, - "Workload has no Admission": { - wl: utiltesting.MakeWorkload("wl", "ns"). - Obj(), - cq: utiltesting.MakeClusterQueue("cq"). - AdmissionCheckStrategy( - *utiltesting.MakeAdmissionCheckStrategy("ac1", "flavor1").Obj(), - *utiltesting.MakeAdmissionCheckStrategy("ac2").Obj()). - Obj(), - wantAdmissionChecks: []string{}, - }, - } - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - ctx := context.Background() - log := ctrl.LoggerFrom(ctx) - gotAdmissionChecks := getChecksFromACStrategies(&log, tc.wl, tc.cq) - - if diff := cmp.Diff(tc.wantAdmissionChecks, gotAdmissionChecks); diff != "" { - t.Errorf("Unexpected AdmissionChecks, (want-/got+): %s", diff) - } - }) - - } -} diff --git a/pkg/workload/workload.go b/pkg/workload/workload.go index 3d039a33fb..fbea2c8aa2 100644 --- a/pkg/workload/workload.go +++ b/pkg/workload/workload.go @@ -556,4 +556,4 @@ func AdmissionChecksForWorkload(log *logr.Logger, wl *kueue.Workload, cq *kueue. } } return admissionCheckNames -} \ No newline at end of file +} diff --git a/pkg/workload/workload_test.go b/pkg/workload/workload_test.go index 97e81f57d9..3b5e5bd986 100644 --- a/pkg/workload/workload_test.go +++ b/pkg/workload/workload_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" config "sigs.k8s.io/kueue/apis/config/v1beta1" @@ -643,3 +644,63 @@ func TestResourceUsage(t *testing.T) { }) } } + +func TestAdmissionCheckStrategy(t *testing.T) { + cases := map[string]struct { + cq *kueue.ClusterQueue + wl *kueue.Workload + wantAdmissionChecks []string + }{ + "AdmissionCheckStrategy with a flavor": { + wl: utiltesting.MakeWorkload("wl", "ns"). + ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). + Obj(), + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionCheckStrategy(*utiltesting.MakeAdmissionCheckStrategy("ac1", "flavor1").Obj()). + Obj(), + wantAdmissionChecks: []string{"ac1"}, + }, + "AdmissionCheckStrategy without a flavor": { + wl: utiltesting.MakeWorkload("wl", "ns"). + ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). + Obj(), + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionCheckStrategy(*utiltesting.MakeAdmissionCheckStrategy("ac1").Obj()). + Obj(), + wantAdmissionChecks: []string{"ac1"}, + }, + "Two AdmissionCheckStrategies, one with flavor, one without flavor": { + wl: utiltesting.MakeWorkload("wl", "ns"). + ReserveQuota(utiltesting.MakeAdmission("cq").Assignment("cpu", "flavor1", "1").Obj()). + Obj(), + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionCheckStrategy( + *utiltesting.MakeAdmissionCheckStrategy("ac1", "flavor1").Obj(), + *utiltesting.MakeAdmissionCheckStrategy("ac2").Obj()). + Obj(), + wantAdmissionChecks: []string{"ac1", "ac2"}, + }, + "Workload has no Admission": { + wl: utiltesting.MakeWorkload("wl", "ns"). + Obj(), + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionCheckStrategy( + *utiltesting.MakeAdmissionCheckStrategy("ac1", "flavor1").Obj(), + *utiltesting.MakeAdmissionCheckStrategy("ac2").Obj()). + Obj(), + wantAdmissionChecks: nil, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + log := ctrl.LoggerFrom(ctx) + gotAdmissionChecks := AdmissionChecksForWorkload(&log, tc.wl, tc.cq) + + if diff := cmp.Diff(tc.wantAdmissionChecks, gotAdmissionChecks); diff != "" { + t.Errorf("Unexpected AdmissionChecks, (want-/got+): %s", diff) + } + }) + + } +} diff --git a/test/integration/scheduler/workload_controller_test.go b/test/integration/scheduler/workload_controller_test.go index a8fb5dda6f..0fdcb7bce5 100644 --- a/test/integration/scheduler/workload_controller_test.go +++ b/test/integration/scheduler/workload_controller_test.go @@ -80,7 +80,7 @@ var _ = ginkgo.Describe("Workload controller with scheduler", func() { check2 *kueue.AdmissionCheck check3 *kueue.AdmissionCheck reservationFlavor string = "reservation" - updatedWl kueue.Workload + updatedWl kueue.Workload flavorOnDemand string = "on-demand" resourceGPU corev1.ResourceName = "example.com/gpu" )