Repository: kubernetes-sigs/kueue

Issue:
CrashLoopBackOff when using config/alpha-enabled due to missing AdmissionFairSharing config
<!-- Please use this template while reporting a bug and provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. Thanks!

If the matter is security related, please disclose it privately via https://kubernetes.io/security/
-->

**What happened**:

When deploying Kueue with `config/alpha-enabled` (which uses `--feature-gates=AllAlpha=true`), the controller crashes with a nil pointer dereference resulting in CrashLoopBackOff

Reference thread : https://github.com/kubernetes-sigs/kueue/pull/6597#issuecomment-3220069521


The issue occurs because:
1. `AllAlpha=true` flag enables the AdmissionFairSharing feature
2. No `admissionFairSharing` configuration is provided in the default config (commented out in `kueue-manager-config` ConfigMap)
3. `cfg.AdmissionFairSharing = nil` (no config provided)
4. Scheduler calls `CalculateEntryPenalty(requests, nil)`
5. Crash at line 49: `afs.UsageSamplingInterval.Seconds()` when `afs` is nil

**What you expected to happen**:

The controller should start successfully when using `config/alpha-enabled`, either by:
- Providing a default configuration for AdmissionFairSharing when the feature is enabled
- Gracefully handling the nil configuration case

**How to reproduce it (as minimally and precisely as possible)**:

1. Deploy Kueue using alpha-enabled configuration:
   ```bash
   kubectl apply -k config/alpha-enabled
   ```
2. Observe controller pod in CrashLoopBackOff state:
   ```bash
   kubectl get pods -n kueue-system
   kubectl logs -n kueue-system -l app=kueue-controller-manager
   ```

**Anything else we need to know?**:

**Workarounds that work:**

1. Add nil check in `pkg/util/admissionfairsharing/admission_fair_sharing.go`:
   ```go
   func CalculateEntryPenalty(totalRequests corev1.ResourceList, afs *config.AdmissionFairSharing) corev1.ResourceList {
   	// Return empty penalty if no admission fair sharing config is provided
   	if afs == nil {
   		return corev1.ResourceList{}
   	}
   	alpha := CalculateAlphaRate(
   		afs.UsageSamplingInterval.Seconds(),
   		afs.UsageHalfLifeTime.Seconds(),
   	)
   	return resource.MulByFloat(totalRequests, alpha)
   }
   ```

2. Or provide explicit AdmissionFairSharing configuration in `kueue-manager-config` ConfigMap:
   ```yaml
   admissionFairSharing:
     usageHalfLifeTime: "168h"
     usageSamplingInterval: "5m"
     resourceWeights:
       cpu: 1
       memory: 1
   ```

This affects TrainJob v2 integration testing as the controller cannot start with alpha features enabled

Discussion:
@kannon92: Thank you for the report.
Would you be interested in proposing a patch?@kannon92: > This affects TrainJob v2 integration testing as the controller cannot start with alpha features enabled

Looking at that PR, there isn't any reason why you have to enable the alpha features. TrainJob is not under a feature gate.@kannon92: cc @PBundyra @mbobrovskyi: /assign

Solution patch:
diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go
index c2a86a1fd..4c1d61087 100644
--- a/pkg/scheduler/scheduler.go
+++ b/pkg/scheduler/scheduler.go
@@ -623,7 +623,7 @@ func (s *Scheduler) admit(ctx context.Context, e *entry, cq *cache.ClusterQueueS
 	e.status = assumed
 	log.V(2).Info("Workload assumed in the cache")
 
-	if features.Enabled(features.AdmissionFairSharing) {
+	if s.admissionFairSharing != nil && features.Enabled(features.AdmissionFairSharing) {
 		s.updateEntryPenalty(log, e, add)
 
 		// Trigger LocalQueue reconciler to apply any pending penalties
@@ -642,7 +642,7 @@ func (s *Scheduler) admit(ctx context.Context, e *entry, cq *cache.ClusterQueueS
 		// Ignore errors because the workload or clusterQueue could have been deleted
 		// by an event.
 		_ = s.cache.ForgetWorkload(log, newWorkload)
-		if features.Enabled(features.AdmissionFairSharing) {
+		if s.admissionFairSharing != nil && features.Enabled(features.AdmissionFairSharing) {
 			s.updateEntryPenalty(log, e, subtract)
 		}
 		if apierrors.IsNotFound(err) {



Test patch:
diff --git a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go
index cceaa53e3..299550600 100644
--- a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go
+++ b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go
@@ -794,3 +794,101 @@ var _ = ginkgo.Describe("Scheduler", ginkgo.Ordered, ginkgo.ContinueOnFailure, f
 		})
 	})
 })
+
+var _ = ginkgo.Describe("Scheduler with AdmissionFairSharing = nil", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {
+	var (
+		defaultFlavor *kueue.ResourceFlavor
+		ns            *corev1.Namespace
+
+		cohorts []*kueue.Cohort
+		cqs     []*kueue.ClusterQueue
+		lqs     []*kueue.LocalQueue
+		wls     []*kueue.Workload
+	)
+
+	var createWorkloadWithPriority = func(queue kueue.LocalQueueName, cpuRequests string, priority int32) *kueue.Workload {
+		wl := testing.MakeWorkloadWithGeneratedName("workload-", ns.Name).
+			Priority(priority).
+			Queue(queue).
+			Request(corev1.ResourceCPU, cpuRequests).Obj()
+		wls = append(wls, wl)
+		util.MustCreate(ctx, k8sClient, wl)
+		return wl
+	}
+
+	var createWorkload = func(queue kueue.LocalQueueName, cpuRequests string) *kueue.Workload {
+		return createWorkloadWithPriority(queue, cpuRequests, 0)
+	}
+
+	ginkgo.BeforeAll(func() {
+		fwk.StartManager(ctx, cfg, managerAndSchedulerSetup(nil))
+	})
+
+	ginkgo.AfterAll(func() {
+		fwk.StopManager(ctx)
+	})
+
+	ginkgo.BeforeEach(func() {
+		defaultFlavor = testing.MakeResourceFlavor("default").Obj()
+		util.MustCreate(ctx, k8sClient, defaultFlavor)
+
+		ns = util.CreateNamespaceFromPrefixWithLog(ctx, k8sClient, "core-")
+	})
+
+	ginkgo.AfterEach(func() {
+		for _, wl := range wls {
+			util.ExpectObjectToBeDeleted(ctx, k8sClient, wl, true)
+		}
+		for _, lq := range lqs {
+			util.ExpectObjectToBeDeleted(ctx, k8sClient, lq, true)
+		}
+		gomega.Expect(util.DeleteNamespace(ctx, k8sClient, ns)).To(gomega.Succeed())
+		for _, cq := range cqs {
+			util.ExpectObjectToBeDeleted(ctx, k8sClient, cq, true)
+		}
+		for _, cohort := range cohorts {
+			util.ExpectObjectToBeDeleted(ctx, k8sClient, cohort, true)
+		}
+		util.ExpectObjectToBeDeleted(ctx, k8sClient, defaultFlavor, true)
+	})
+
+	ginkgo.When("Using AdmissionFairSharing at Cohort level", func() {
+		var (
+			cq1 *kueue.ClusterQueue
+			lqA *kueue.LocalQueue
+		)
+
+		ginkgo.BeforeEach(func() {
+			features.SetFeatureGateDuringTest(ginkgo.GinkgoTB(), features.AdmissionFairSharing, true)
+
+			cq1 = testing.MakeClusterQueue("cq1").
+				Cohort("all").
+				ResourceGroup(*testing.MakeFlavorQuotas(defaultFlavor.Name).Resource(corev1.ResourceCPU, "32").Obj()).
+				Preemption(kueue.ClusterQueuePreemption{WithinClusterQueue: kueue.PreemptionPolicyNever}).
+				QueueingStrategy(kueue.StrictFIFO).
+				AdmissionMode(kueue.UsageBasedAdmissionFairSharing).
+				Obj()
+			util.MustCreate(ctx, k8sClient, cq1)
+			cqs = append(cqs, cq1)
+
+			lqA = testing.MakeLocalQueue("lq-a", ns.Name).
+				FairSharing(&kueue.FairSharing{Weight: ptr.To(resource.MustParse("1"))}).
+				ClusterQueue(cq1.Name).Obj()
+			lqs = append(lqs, lqA)
+			util.MustCreate(ctx, k8sClient, lqA)
+		})
+
+		ginkgo.It("should ignore FairSharing", func() {
+			ginkgo.By("Creating a workload")
+			wl := createWorkload(kueue.LocalQueueName(lqA.Name), "32")
+
+			ginkgo.By("Admitting the workload")
+			util.ExpectWorkloadsToBeAdmitted(ctx, k8sClient, wl)
+			util.ExpectReservingActiveWorkloadsMetric(cq1, 1)
+
+			ginkgo.By("Checking that FairSharing status is nil")
+			gomega.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(lqA), lqA)).To(gomega.Succeed())
+			gomega.Expect(lqA.Status.FairSharing).Should(gomega.BeNil())
+		})
+	})
+})