From c8c32ad491b82ef1c2fef31fc95541b9956ea765 Mon Sep 17 00:00:00 2001 From: Michal Szadkowski Date: Thu, 17 Oct 2024 10:52:06 +0200 Subject: [PATCH] Wrap AdmissionChecksSingleInstanceInClusterQueue and FlavorIndependentAdmissionCheck status conditions with a feature gate. Preserve the logic without the status conditions. --- pkg/cache/cache.go | 13 ++- pkg/cache/clusterqueue.go | 48 ++++++++--- pkg/cache/clusterqueue_test.go | 84 +++++++++++++----- .../multikueue/admissioncheck.go | 43 +++++----- .../multikueue/admissioncheck_test.go | 46 +++++++++- pkg/features/kube_features.go | 8 ++ site/content/en/docs/installation/_index.md | 1 + .../core/clusterqueue_controller_test.go | 85 +++++++++++++++++++ .../integration/multikueue/multikueue_test.go | 21 ++--- 9 files changed, 281 insertions(+), 68 deletions(-) diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 2dd1f70bfa..b253ddb6d4 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -36,6 +36,7 @@ import ( kueuealpha "sigs.k8s.io/kueue/apis/kueue/v1alpha1" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" utilindexer "sigs.k8s.io/kueue/pkg/controller/core/indexer" + "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/hierarchy" "sigs.k8s.io/kueue/pkg/metrics" "sigs.k8s.io/kueue/pkg/resources" @@ -240,11 +241,19 @@ func (c *Cache) DeleteResourceFlavor(rf *kueue.ResourceFlavor) sets.Set[string] func (c *Cache) AddOrUpdateAdmissionCheck(ac *kueue.AdmissionCheck) sets.Set[string] { c.Lock() defer c.Unlock() + + // TBD: This might not be needed, but we could keep it for readability + // because SingleInstanceInClusterQueue and FlavorIndependent will be false if AdmissionCheckValidationRules is off + singleInstanceInClusterQueue, flavorIndependent := false, false + if features.Enabled(features.AdmissionCheckValidationRules) { + singleInstanceInClusterQueue = apimeta.IsStatusConditionTrue(ac.Status.Conditions, kueue.AdmissionChecksSingleInstanceInClusterQueue) + flavorIndependent = apimeta.IsStatusConditionTrue(ac.Status.Conditions, kueue.FlavorIndependentAdmissionCheck) + } c.admissionChecks[ac.Name] = AdmissionCheck{ Active: apimeta.IsStatusConditionTrue(ac.Status.Conditions, kueue.AdmissionCheckActive), Controller: ac.Spec.ControllerName, - SingleInstanceInClusterQueue: apimeta.IsStatusConditionTrue(ac.Status.Conditions, kueue.AdmissionChecksSingleInstanceInClusterQueue), - FlavorIndependent: apimeta.IsStatusConditionTrue(ac.Status.Conditions, kueue.FlavorIndependentAdmissionCheck), + SingleInstanceInClusterQueue: singleInstanceInClusterQueue, + FlavorIndependent: flavorIndependent, } return c.updateClusterQueues() diff --git a/pkg/cache/clusterqueue.go b/pkg/cache/clusterqueue.go index f0f94a1851..9f0a5085be 100644 --- a/pkg/cache/clusterqueue.go +++ b/pkg/cache/clusterqueue.go @@ -34,6 +34,7 @@ import ( "k8s.io/utils/ptr" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" + "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/hierarchy" "sigs.k8s.io/kueue/pkg/metrics" "sigs.k8s.io/kueue/pkg/resources" @@ -265,16 +266,28 @@ func (c *clusterQueue) inactiveReason() (string, string) { reasons = append(reasons, kueue.ClusterQueueActiveReasonAdmissionCheckInactive) messages = append(messages, fmt.Sprintf("references inactive AdmissionCheck(s): %v", c.inactiveAdmissionChecks)) } - if len(c.multipleSingleInstanceControllersChecks) > 0 { - reasons = append(reasons, kueue.ClusterQueueActiveReasonMultipleSingleInstanceControllerAdmissionChecks) - for _, controller := range utilmaps.SortedKeys(c.multipleSingleInstanceControllersChecks) { - messages = append(messages, fmt.Sprintf("only one AdmissionCheck of %v can be referenced for controller %q", c.multipleSingleInstanceControllersChecks[controller], controller)) + if features.Enabled(features.AdmissionCheckValidationRules) { + if len(c.multipleSingleInstanceControllersChecks) > 0 { + reasons = append(reasons, kueue.ClusterQueueActiveReasonMultipleSingleInstanceControllerAdmissionChecks) + for _, controller := range utilmaps.SortedKeys(c.multipleSingleInstanceControllersChecks) { + messages = append(messages, fmt.Sprintf("only one AdmissionCheck of %v can be referenced for controller %q", c.multipleSingleInstanceControllersChecks[controller], controller)) + } + } + + if len(c.flavorIndependentAdmissionCheckAppliedPerFlavor) > 0 { + reasons = append(reasons, kueue.ClusterQueueActiveReasonFlavorIndependentAdmissionCheckAppliedPerFlavor) + messages = append(messages, fmt.Sprintf("AdmissionCheck(s): %v cannot be set at flavor level", c.flavorIndependentAdmissionCheckAppliedPerFlavor)) + } + } else { + if len(c.multipleSingleInstanceControllersChecks) > 0 { + reasons = append(reasons, kueue.ClusterQueueActiveReasonMultipleSingleInstanceControllerAdmissionChecks) + messages = append(messages, "Cannot use multiple MultiKueue AdmissionChecks on the same ClusterQueue") } - } - if len(c.flavorIndependentAdmissionCheckAppliedPerFlavor) > 0 { - reasons = append(reasons, kueue.ClusterQueueActiveReasonFlavorIndependentAdmissionCheckAppliedPerFlavor) - messages = append(messages, fmt.Sprintf("AdmissionCheck(s): %v cannot be set at flavor level", c.flavorIndependentAdmissionCheckAppliedPerFlavor)) + if len(c.flavorIndependentAdmissionCheckAppliedPerFlavor) > 0 { + reasons = append(reasons, kueue.ClusterQueueActiveReasonFlavorIndependentAdmissionCheckAppliedPerFlavor) + messages = append(messages, "MultiKueue AdmissionCheck cannot be specified per flavor") + } } if len(reasons) == 0 { @@ -333,11 +346,22 @@ func (c *clusterQueue) updateWithAdmissionChecks(checks map[string]AdmissionChec inactive = append(inactive, acName) } checksPerController[ac.Controller] = append(checksPerController[ac.Controller], acName) - if ac.SingleInstanceInClusterQueue { + + if features.Enabled(features.AdmissionCheckValidationRules) { + if ac.SingleInstanceInClusterQueue { + singleInstanceControllers.Insert(ac.Controller) + } + if ac.FlavorIndependent && flavors.Len() != 0 { + flavorIndependentCheckOnFlavors = append(flavorIndependentCheckOnFlavors, acName) + } + } else if ac.Controller == kueue.MultiKueueControllerName { + // MultiKueue Admission Checks have extra constraints + // disallow to use multiple MultiKueue AdmissionChecks on the same ClusterQueue + // MultiKueue AdmissionCheck cannot be specified per flavor singleInstanceControllers.Insert(ac.Controller) - } - if ac.FlavorIndependent && flavors.Len() != 0 { - flavorIndependentCheckOnFlavors = append(flavorIndependentCheckOnFlavors, acName) + if flavors.Len() != 0 { + flavorIndependentCheckOnFlavors = append(flavorIndependentCheckOnFlavors, acName) + } } } } diff --git a/pkg/cache/clusterqueue_test.go b/pkg/cache/clusterqueue_test.go index 0a3a641376..2a29659f9e 100644 --- a/pkg/cache/clusterqueue_test.go +++ b/pkg/cache/clusterqueue_test.go @@ -496,13 +496,14 @@ func TestClusterQueueUpdateWithAdmissionCheck(t *testing.T) { Obj() testcases := []struct { - name string - cq *kueue.ClusterQueue - cqStatus metrics.ClusterQueueStatus - admissionChecks map[string]AdmissionCheck - wantStatus metrics.ClusterQueueStatus - wantReason string - wantMessage string + name string + cq *kueue.ClusterQueue + cqStatus metrics.ClusterQueueStatus + admissionChecks map[string]AdmissionCheck + wantStatus metrics.ClusterQueueStatus + wantReason string + wantMessage string + acValidationRulesEnabled bool }{ { name: "Pending clusterQueue updated valid AC list", @@ -629,7 +630,7 @@ func TestClusterQueueUpdateWithAdmissionCheck(t *testing.T) { wantMessage: "Can't admit new workloads: references inactive AdmissionCheck(s): [check3].", }, { - name: "Active clusterQueue updated with duplicate single instance AC Controller", + name: "Active clusterQueue updated with duplicate single instance AC Controller - AdmissionCheckValidationRules enabled", cq: cqWithAC, cqStatus: active, admissionChecks: map[string]AdmissionCheck{ @@ -648,12 +649,13 @@ func TestClusterQueueUpdateWithAdmissionCheck(t *testing.T) { SingleInstanceInClusterQueue: true, }, }, - wantStatus: pending, - wantReason: "MultipleSingleInstanceControllerAdmissionChecks", - wantMessage: `Can't admit new workloads: only one AdmissionCheck of [check2 check3] can be referenced for controller "controller2".`, + wantStatus: pending, + wantReason: "MultipleSingleInstanceControllerAdmissionChecks", + wantMessage: `Can't admit new workloads: only one AdmissionCheck of [check2 check3] can be referenced for controller "controller2".`, + acValidationRulesEnabled: true, }, { - name: "Active clusterQueue with an AC strategy updated with duplicate single instance AC Controller", + name: "Active clusterQueue with an AC strategy updated with duplicate single instance AC Controller - AdmissionCheckValidationRules enabled", cq: cqWithACStrategy, cqStatus: active, admissionChecks: map[string]AdmissionCheck{ @@ -672,12 +674,13 @@ func TestClusterQueueUpdateWithAdmissionCheck(t *testing.T) { SingleInstanceInClusterQueue: true, }, }, - wantStatus: pending, - wantReason: "MultipleSingleInstanceControllerAdmissionChecks", - wantMessage: `Can't admit new workloads: only one AdmissionCheck of [check2 check3] can be referenced for controller "controller2".`, + wantStatus: pending, + wantReason: "MultipleSingleInstanceControllerAdmissionChecks", + wantMessage: `Can't admit new workloads: only one AdmissionCheck of [check2 check3] can be referenced for controller "controller2".`, + acValidationRulesEnabled: true, }, { - name: "Active clusterQueue with a FlavorIndependent AC applied per ResourceFlavor", + name: "Active clusterQueue with a FlavorIndependent AC applied per ResourceFlavor - AdmissionCheckValidationRules enabled", cq: cqWithACPerFlavor, cqStatus: pending, admissionChecks: map[string]AdmissionCheck{ @@ -687,9 +690,10 @@ func TestClusterQueueUpdateWithAdmissionCheck(t *testing.T) { FlavorIndependent: true, }, }, - wantStatus: pending, - wantReason: "FlavorIndependentAdmissionCheckAppliedPerFlavor", - wantMessage: "Can't admit new workloads: AdmissionCheck(s): [check1] cannot be set at flavor level.", + wantStatus: pending, + wantReason: "FlavorIndependentAdmissionCheckAppliedPerFlavor", + wantMessage: "Can't admit new workloads: AdmissionCheck(s): [check1] cannot be set at flavor level.", + acValidationRulesEnabled: true, }, { name: "Terminating clusterQueue updated with valid AC list", @@ -771,10 +775,52 @@ func TestClusterQueueUpdateWithAdmissionCheck(t *testing.T) { wantReason: "Terminating", wantMessage: "Can't admit new workloads; clusterQueue is terminating", }, + { + name: "Active clusterQueue with an AC strategy updated with duplicate single instance AC Controller", + cq: cqWithACStrategy, + cqStatus: active, + admissionChecks: map[string]AdmissionCheck{ + "check1": { + Active: true, + Controller: "controller1", + SingleInstanceInClusterQueue: true, + }, + "check2": { + Active: true, + Controller: "controller2", + }, + "check3": { + Active: true, + Controller: "controller2", + SingleInstanceInClusterQueue: true, + }, + }, + wantStatus: pending, + wantReason: "MultipleSingleInstanceControllerAdmissionChecks", + wantMessage: "Can't admit new workloads: Cannot use multiple MultiKueue AdmissionChecks on the same ClusterQueue.", + }, + { + name: "Active clusterQueue with a FlavorIndependent AC applied per ResourceFlavor", + cq: cqWithACPerFlavor, + cqStatus: pending, + admissionChecks: map[string]AdmissionCheck{ + "check1": { + Active: true, + Controller: "controller1", + FlavorIndependent: true, + }, + }, + wantStatus: pending, + wantReason: "FlavorIndependentAdmissionCheckAppliedPerFlavor", + wantMessage: "Can't admit new workloads: MultiKueue AdmissionCheck cannot be specified per flavor.", + }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { + if tc.acValidationRulesEnabled { + features.SetFeatureGateDuringTest(t, features.AdmissionCheckValidationRules, true) + } cache := New(utiltesting.NewFakeClient()) cq, err := cache.newClusterQueue(tc.cq) if err != nil { diff --git a/pkg/controller/admissionchecks/multikueue/admissioncheck.go b/pkg/controller/admissionchecks/multikueue/admissioncheck.go index b563e21586..6c41d19477 100644 --- a/pkg/controller/admissionchecks/multikueue/admissioncheck.go +++ b/pkg/controller/admissionchecks/multikueue/admissioncheck.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" + "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/util/admissioncheck" ) @@ -126,28 +127,30 @@ func (a *ACReconciler) Reconcile(ctx context.Context, req reconcile.Request) (re apimeta.SetStatusCondition(&ac.Status.Conditions, newCondition) needsUpdate = true } - if !apimeta.IsStatusConditionTrue(ac.Status.Conditions, kueue.AdmissionChecksSingleInstanceInClusterQueue) { - apimeta.SetStatusCondition(&ac.Status.Conditions, metav1.Condition{ - Type: kueue.AdmissionChecksSingleInstanceInClusterQueue, - Status: metav1.ConditionTrue, - Reason: SingleInstanceReason, - Message: SingleInstanceMessage, - ObservedGeneration: ac.Generation, - }) - needsUpdate = true - } - if !apimeta.IsStatusConditionTrue(ac.Status.Conditions, kueue.FlavorIndependentAdmissionCheck) { - apimeta.SetStatusCondition(&ac.Status.Conditions, metav1.Condition{ - Type: kueue.FlavorIndependentAdmissionCheck, - Status: metav1.ConditionTrue, - Reason: FlavorIndependentCheckReason, - Message: FlavorIndependentCheckMessage, - ObservedGeneration: ac.Generation, - }) - needsUpdate = true - } + if features.Enabled(features.AdmissionCheckValidationRules) { + if !apimeta.IsStatusConditionTrue(ac.Status.Conditions, kueue.AdmissionChecksSingleInstanceInClusterQueue) { + apimeta.SetStatusCondition(&ac.Status.Conditions, metav1.Condition{ + Type: kueue.AdmissionChecksSingleInstanceInClusterQueue, + Status: metav1.ConditionTrue, + Reason: SingleInstanceReason, + Message: SingleInstanceMessage, + ObservedGeneration: ac.Generation, + }) + needsUpdate = true + } + if !apimeta.IsStatusConditionTrue(ac.Status.Conditions, kueue.FlavorIndependentAdmissionCheck) { + apimeta.SetStatusCondition(&ac.Status.Conditions, metav1.Condition{ + Type: kueue.FlavorIndependentAdmissionCheck, + Status: metav1.ConditionTrue, + Reason: FlavorIndependentCheckReason, + Message: FlavorIndependentCheckMessage, + ObservedGeneration: ac.Generation, + }) + needsUpdate = true + } + } if needsUpdate { err := a.client.Status().Update(ctx, ac) if err != nil { diff --git a/pkg/controller/admissionchecks/multikueue/admissioncheck_test.go b/pkg/controller/admissionchecks/multikueue/admissioncheck_test.go index 34536bdca0..5ea1dcbbca 100644 --- a/pkg/controller/admissionchecks/multikueue/admissioncheck_test.go +++ b/pkg/controller/admissionchecks/multikueue/admissioncheck_test.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" + "sigs.k8s.io/kueue/pkg/features" utiltesting "sigs.k8s.io/kueue/pkg/util/testing" ) @@ -36,8 +37,9 @@ func TestReconcile(t *testing.T) { reconcileFor string configs []kueue.MultiKueueConfig - wantChecks []kueue.AdmissionCheck - wantError error + wantChecks []kueue.AdmissionCheck + wantError error + acValidationRulesEnabled bool }{ "missing admissioncheck": { reconcileFor: "missing-ac", @@ -66,6 +68,7 @@ func TestReconcile(t *testing.T) { }). Obj(), }, + acValidationRulesEnabled: true, }, "unmanaged": { reconcileFor: "ac1", @@ -107,6 +110,7 @@ func TestReconcile(t *testing.T) { }). Obj(), }, + acValidationRulesEnabled: true, }, "inactive cluster": { reconcileFor: "ac1", @@ -140,6 +144,7 @@ func TestReconcile(t *testing.T) { }). Obj(), }, + acValidationRulesEnabled: true, }, "all clusters missing or inactive": { reconcileFor: "ac1", @@ -176,6 +181,7 @@ func TestReconcile(t *testing.T) { }). Obj(), }, + acValidationRulesEnabled: true, }, "partially active": { reconcileFor: "ac1", @@ -212,6 +218,7 @@ func TestReconcile(t *testing.T) { }). Obj(), }, + acValidationRulesEnabled: true, }, "active": { reconcileFor: "ac1", @@ -245,11 +252,46 @@ func TestReconcile(t *testing.T) { }). Obj(), }, + acValidationRulesEnabled: true, + }, + "active AdmissionCheckValidationRules disabled": { + reconcileFor: "ac1", + checks: []kueue.AdmissionCheck{ + *utiltesting.MakeAdmissionCheck("ac1"). + ControllerName(kueue.MultiKueueControllerName). + Parameters(kueue.GroupVersion.Group, "MultiKueueConfig", "config1"). + Generation(1). + Obj(), + }, + configs: []kueue.MultiKueueConfig{ + *utiltesting.MakeMultiKueueConfig("config1").Clusters("worker1").Obj(), + }, + clusters: []kueue.MultiKueueCluster{ + *utiltesting.MakeMultiKueueCluster("worker1"). + Active(metav1.ConditionTrue, "ByTest", "by test", 1). + Obj(), + }, + wantChecks: []kueue.AdmissionCheck{ + *utiltesting.MakeAdmissionCheck("ac1"). + ControllerName(kueue.MultiKueueControllerName). + Parameters(kueue.GroupVersion.Group, "MultiKueueConfig", "config1"). + Condition(metav1.Condition{ + Type: kueue.AdmissionCheckActive, + Status: metav1.ConditionTrue, + Reason: "Active", + Message: `The admission check is active`, + ObservedGeneration: 1, + }). + Obj(), + }, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { + if tc.acValidationRulesEnabled { + features.SetFeatureGateDuringTest(t, features.AdmissionCheckValidationRules, true) + } builder, ctx := getClientBuilder() builder = builder.WithLists( diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 84cd861d2e..bfa7df658b 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -125,6 +125,13 @@ const ( // Summarize the resource requests of non-admitted Workloads in Workload.Status.resourceRequest // to improve observability WorkloadResourceRequestsSummary featuregate.Feature = "WorkloadResourceRequestsSummary" + + // owner: @mszadkow + // alpha: v0.9 + // Deprecated: v0.9 + // + // Enable additional AdmissionCheck validation rules that will appear in status conditions. + AdmissionCheckValidationRules featuregate.Feature = "AdmissionCheckValidationRules" ) func init() { @@ -151,6 +158,7 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ TopologyAwareScheduling: {Default: false, PreRelease: featuregate.Alpha}, ConfigurableResourceTransformations: {Default: false, PreRelease: featuregate.Alpha}, WorkloadResourceRequestsSummary: {Default: false, PreRelease: featuregate.Alpha}, + AdmissionCheckValidationRules: {Default: false, PreRelease: featuregate.Alpha}, } func SetFeatureGateDuringTest(tb testing.TB, f featuregate.Feature, value bool) { diff --git a/site/content/en/docs/installation/_index.md b/site/content/en/docs/installation/_index.md index e7cb507181..854274de0d 100644 --- a/site/content/en/docs/installation/_index.md +++ b/site/content/en/docs/installation/_index.md @@ -255,6 +255,7 @@ The currently supported features are: | `TopologyAwareScheduling` | `false` | Alpha | 0.9 | | | `ConfigurableResourceTransformations` | `false` | Alpha | 0.9 | | | `WorkloadResourceRequestsSummary` | `false` | Alpha | 0.9 | | +| `AdmissionCheckValidationRules` | `false` | Alpha | 0.9 | 0.9 | ## What's next diff --git a/test/integration/controller/core/clusterqueue_controller_test.go b/test/integration/controller/core/clusterqueue_controller_test.go index cd0c020c30..ddee8b846e 100644 --- a/test/integration/controller/core/clusterqueue_controller_test.go +++ b/test/integration/controller/core/clusterqueue_controller_test.go @@ -758,6 +758,91 @@ var _ = ginkgo.Describe("ClusterQueue controller", ginkgo.Ordered, ginkgo.Contin }, }, util.IgnoreConditionTimestampsAndObservedGeneration)) }) + + ginkgo.It("Should prevent workload admission due to multikueue contraints", func() { + cpuArchAFlavor = testing.MakeResourceFlavor(flavorCPUArchA).Obj() + gomega.Expect(k8sClient.Create(ctx, cpuArchAFlavor)).To(gomega.Succeed()) + + cpuArchBFlavor = testing.MakeResourceFlavor(flavorCPUArchB).Obj() + gomega.Expect(k8sClient.Create(ctx, cpuArchBFlavor)).To(gomega.Succeed()) + + check1 = testing.MakeAdmissionCheck("check1").ControllerName(kueue.MultiKueueControllerName).Obj() + gomega.Expect(k8sClient.Create(ctx, check1)).To(gomega.Succeed()) + util.SetAdmissionCheckActive(ctx, k8sClient, check1, metav1.ConditionTrue) + + check2 = testing.MakeAdmissionCheck("check2").ControllerName(kueue.MultiKueueControllerName).Obj() + gomega.Expect(k8sClient.Create(ctx, check2)).To(gomega.Succeed()) + util.SetAdmissionCheckActive(ctx, k8sClient, check2, metav1.ConditionTrue) + + ginkgo.By("Multiple MultiKueue admission checks for the same cluster queue") + + gomega.Eventually(func() []metav1.Condition { + var updatedCq kueue.ClusterQueue + gomega.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cq), &updatedCq)).To(gomega.Succeed()) + return updatedCq.Status.Conditions + }, util.Timeout, util.Interval).Should(gomega.BeComparableTo([]metav1.Condition{ + { + Type: kueue.ClusterQueueActive, + Status: metav1.ConditionFalse, + Reason: "MultipleSingleInstanceControllerAdmissionChecks", + Message: "Can't admit new workloads: Cannot use multiple MultiKueue AdmissionChecks on the same ClusterQueue.", + }, + }, util.IgnoreConditionTimestampsAndObservedGeneration)) + + ginkgo.By("Only one Multikueue flavor dependent admission check assigned to cluster queue") + gomega.Eventually(func() error { + updatedCq := &kueue.ClusterQueue{} + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(cq), updatedCq) + if err != nil { + return err + } + updatedCq.Spec.AdmissionChecks = nil + updatedCq.Spec.AdmissionChecksStrategy = &kueue.AdmissionChecksStrategy{ + AdmissionChecks: []kueue.AdmissionCheckStrategyRule{ + *testing.MakeAdmissionCheckStrategyRule("check1", flavorCPUArchA).Obj(), + }, + } + return k8sClient.Update(ctx, updatedCq) + }, util.Timeout, util.Interval).Should(gomega.Succeed()) + + gomega.Eventually(func() []metav1.Condition { + var updatedCq kueue.ClusterQueue + gomega.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cq), &updatedCq)).To(gomega.Succeed()) + return updatedCq.Status.Conditions + }, util.Timeout, util.Interval).Should(gomega.BeComparableTo([]metav1.Condition{ + { + Type: kueue.ClusterQueueActive, + Status: metav1.ConditionFalse, + Reason: "FlavorIndependentAdmissionCheckAppliedPerFlavor", + Message: "Can't admit new workloads: MultiKueue AdmissionCheck cannot be specified per flavor.", + }, + }, util.IgnoreConditionTimestampsAndObservedGeneration)) + + ginkgo.By("Only one Multikueue flavor independent admission check assigned to cluster queue") + gomega.Eventually(func() error { + updatedCq := &kueue.ClusterQueue{} + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(cq), updatedCq) + if err != nil { + return err + } + updatedCq.Spec.AdmissionChecks = []string{"check1"} + updatedCq.Spec.AdmissionChecksStrategy = nil + return k8sClient.Update(ctx, updatedCq) + }, util.Timeout, util.Interval).Should(gomega.Succeed()) + + gomega.Eventually(func() []metav1.Condition { + var updatedCq kueue.ClusterQueue + gomega.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cq), &updatedCq)).To(gomega.Succeed()) + return updatedCq.Status.Conditions + }, util.Timeout, util.Interval).Should(gomega.BeComparableTo([]metav1.Condition{ + { + Type: kueue.ClusterQueueActive, + Status: metav1.ConditionTrue, + Reason: "Ready", + Message: "Can admit new workloads", + }, + }, util.IgnoreConditionTimestampsAndObservedGeneration)) + }) }) ginkgo.When("Deleting clusterQueues", func() { diff --git a/test/integration/multikueue/multikueue_test.go b/test/integration/multikueue/multikueue_test.go index 35f8d7f454..4a01fb3ff1 100644 --- a/test/integration/multikueue/multikueue_test.go +++ b/test/integration/multikueue/multikueue_test.go @@ -187,6 +187,10 @@ var _ = ginkgo.Describe("Multikueue", ginkgo.Ordered, ginkgo.ContinueOnFailure, gomega.Expect(worker2TestCluster.client.Create(worker2TestCluster.ctx, worker2Cq)).Should(gomega.Succeed()) worker2Lq = utiltesting.MakeLocalQueue(worker2Cq.Name, worker2Ns.Name).ClusterQueue(worker2Cq.Name).Obj() gomega.Expect(worker2TestCluster.client.Create(worker2TestCluster.ctx, worker2Lq)).Should(gomega.Succeed()) + + ginkgo.By("Disabling AdmissionCheckValidationRules feature", func() { + gomega.Expect(features.SetEnable(features.AdmissionCheckValidationRules, false)).To(gomega.Succeed()) + }) }) ginkgo.AfterEach(func() { @@ -203,7 +207,7 @@ var _ = ginkgo.Describe("Multikueue", ginkgo.Ordered, ginkgo.ContinueOnFailure, util.ExpectObjectToBeDeleted(managerTestCluster.ctx, managerTestCluster.client, managerMultikueueSecret1, true) util.ExpectObjectToBeDeleted(managerTestCluster.ctx, managerTestCluster.client, managerMultikueueSecret2, true) }) - ginkgo.It("Should properly manage the active condition of AdmissionChecks and MultiKueueClusters, kubeconfig provided by secret", func() { + ginkgo.It("Should properly manage the active condition of AdmissionChecks and MultiKueueClusters, kubeconfig provided by secret, AdmissionCheckValidationRules disabled", func() { ac := utiltesting.MakeAdmissionCheck("testing-ac"). ControllerName(kueue.MultiKueueControllerName). Parameters(kueue.GroupVersion.Group, "MultiKueueConfig", "testing-config"). @@ -224,18 +228,6 @@ var _ = ginkgo.Describe("Multikueue", ginkgo.Ordered, ginkgo.ContinueOnFailure, Reason: "BadConfig", Message: `Cannot load the AdmissionChecks parameters: MultiKueueConfig.kueue.x-k8s.io "testing-config" not found`, }, util.IgnoreConditionTimestampsAndObservedGeneration), - gomega.BeComparableTo(metav1.Condition{ - Type: kueue.AdmissionChecksSingleInstanceInClusterQueue, - Status: metav1.ConditionTrue, - Reason: multikueue.SingleInstanceReason, - Message: multikueue.SingleInstanceMessage, - }, util.IgnoreConditionTimestampsAndObservedGeneration), - gomega.BeComparableTo(metav1.Condition{ - Type: kueue.FlavorIndependentAdmissionCheck, - Status: metav1.ConditionTrue, - Reason: multikueue.FlavorIndependentCheckReason, - Message: multikueue.FlavorIndependentCheckMessage, - }, util.IgnoreConditionTimestampsAndObservedGeneration), )) }, util.Timeout, util.Interval).Should(gomega.Succeed()) }) @@ -348,6 +340,9 @@ var _ = ginkgo.Describe("Multikueue", ginkgo.Ordered, ginkgo.ContinueOnFailure, }) ginkgo.It("Should properly manage the active condition of AdmissionChecks and MultiKueueClusters, kubeconfig provided by file", func() { + ginkgo.By("Enabling AdmissionCheckValidationRules feature", func() { + gomega.Expect(features.SetEnable(features.AdmissionCheckValidationRules, true)).To(gomega.Succeed()) + }) ac := utiltesting.MakeAdmissionCheck("testing-ac"). ControllerName(kueue.MultiKueueControllerName). Parameters(kueue.GroupVersion.Group, "MultiKueueConfig", "testing-config").