Repository: kubernetes-sigs/kueue

Issue:
TAS: support ProvisioningRequests
**What would you like to be added**:

Support for ProvisioningRequests .

**Why is this needed**:

To make TAS work with autoscaling.

**Completion requirements**:

This enhancement requires the following artifacts:

- [ ] Design doc
- [ ] API change
- [ ] Docs update

The artifacts should be linked in subsequent comments.


Discussion:
@mimowo: cc @tenzen-y @mwielgus @mwysokin 
/assign@sohankunkerkar: @mimowo, can I pick up some of the stuff here? I'm happy to shadow you if possible.@mimowo: Hi @sohankunkerkar , sounds great. Currently I'm prototyping the following high-level idea:

If the workload requires TAS and ProvRequest, then:
1. skip computation of TAS assignment in the first pass of scheduler, just assign let it assign QuotaReservation
2. force scheduler to do another pass on the workload when QuotaReserved and all checks are green (incl. ProvReq)
3. the second (delayed) scheduling pass will be consistent with the choices about quota reservation, but more detailed (will include TAS assignments)

When I have something working in a publishable state (hopefully tomorrow) I will push it as a draft PR. I will appreciate working together on the design details, review, testing and other spin off tasks.@sohankunkerkar: @mimowo Is there anything I can help you with apart from reviewing the KEP? I’d love to get more involved!@mimowo: @sohankunkerkar sure, some early feedback on the impl would be nice. FYI I'm ooo next week.@mimowo: Since I'm ooo next week, aside from reviewing, if you want to code a little bit, then we still need the requeuing with exponential delay. Feel free to prototype on your branch (PR based on top of mine). For that I was thinking of either extend the Workload.Info struct with `secondPass {requeueCount, requeueAt}`, or embedded mapping inside the queue. Be aware that the feature is a priority for 0.12, so addressing remarks and merging both PRs will be time sensitive.
@mimowo: Also, I have another ask, and more isolated and important - prepare a PR which extracts the fix in this code fragment: https://github.com/kubernetes-sigs/kueue/pull/5052/files#diff-fb3d6b36d3cd727e1dd2353fb86c0aee0bfaae1c900e0f005ab913620021d551R213-R228

This bug was discovered by this test: https://github.com/kubernetes-sigs/kueue/pull/5052/files#diff-95ef2b4868044a4fbcc96fcda6df858f3b79548d2e4548fafcd9280114928dfcR1705.

Basically, after restart sometimes the cache is not yet populated, and cq might be still nil. The idea of the fix is to skip based on NoFit, before we call cq.Parent(). 

Ideally, I would like to extract the fix to a separate PR with dedicated test (either TAS or pure quota), so that we can cherry-pick if previous releases are affected., and I think 0.11 is affected: https://github.com/kubernetes-sigs/kueue/blob/ebc7ad9b840fa40ed20e070197fc08e5e5d980b1/pkg/scheduler/scheduler.go#L216. So, ideally, we can write a test which is independent of the new code changes.@gabesaba: > Also, I have another ask, and more isolated and important - prepare a PR which extracts the fix in this code fragment: https://github.com/kubernetes-sigs/kueue/pull/5052/files#diff-fb3d6b36d3cd727e1dd2353fb86c0aee0bfaae1c900e0f005ab913620021d551R213-R228
> 
> This bug was discovered by this test: https://github.com/kubernetes-sigs/kueue/pull/5052/files#diff-95ef2b4868044a4fbcc96fcda6df858f3b79548d2e4548fafcd9280114928dfcR1705.
> 
> Basically, after restart sometimes the cache is not yet populated, and cq might be still nil. The idea of the fix is to skip based on NoFit, before we call cq.Parent().
> 
> Ideally, I would like to extract the fix to a separate PR with dedicated test (either TAS or pure quota), so that we can cherry-pick if previous releases are affected., and I think 0.11 is affected:
> 
> [kueue/pkg/scheduler/scheduler.go](https://github.com/kubernetes-sigs/kueue/blob/ebc7ad9b840fa40ed20e070197fc08e5e5d980b1/pkg/scheduler/scheduler.go#L216)
> 
> Line 216 in [ebc7ad9](/kubernetes-sigs/kueue/commit/ebc7ad9b840fa40ed20e070197fc08e5e5d980b1)
> 
>  if cq.HasParent() { 
> . So, ideally, we can write a test which is independent of the new code changes.

I think this failure is related: https://github.com/kubernetes-sigs/kueue/pull/5287#issuecomment-2893436998

Did we fix for FairSharing too?@mimowo: > Did we fix for FairSharing too?

Fixing for FairSharing was not considered. It might have been fixed as a side effect of the PR: https://github.com/kubernetes-sigs/kueue/pull/5138, but I didn't check.@gabesaba: I observe that no unit tests fail (running integration tests now, but I suspect they won't either) when we remove this line https://github.com/kubernetes-sigs/kueue/blob/7da8e0a2ea64c026ed3daf28aec63d711c1d1f72/pkg/scheduler/scheduler.go#L361-L362

I think that should be the centralized place that we do this validation, rather than having some ad-hoc check later in the scheduling code@gabesaba: One idea is that once we get entries (step 3), we split that list into "validEntries" and "invalidEntries". We only pass validEntries to the iterator

https://github.com/kubernetes-sigs/kueue/blob/7da8e0a2ea64c026ed3daf28aec63d711c1d1f72/pkg/scheduler/scheduler.go#L198-L202

and requeue invalidEntries@mimowo: @gabesaba I suggest you open an issue for this proposal(s). This does not refer directly to the issue we are currently discussing in. It was just discovered while working on the issue.@gabesaba: > [@gabesaba](https://github.com/gabesaba) I suggest you open an issue for this proposal(s). This does not refer directly to the issue we are currently discussing in. It was just discovered while working on the issue.

Ack. Created https://github.com/kubernetes-sigs/kueue/issues/5291

Solution patch:
diff --git a/.dockerignore b/.dockerignore
new file mode 100644
index 000000000..2f1a5fe9e
--- /dev/null
+++ b/.dockerignore
@@ -0,0 +1,45 @@
+# assorted source directories not built with dockerfile
+site
+charts
+keps
+examples
+.github
+.openvex
+
+# Below matches .gitignore
+
+# Binaries for programs and plugins
+*.exe
+*.exe~
+*.dll
+*.so
+*.dylib
+bin
+artifacts
+testbin/*
+.DS_Store
+# Test binary, build with `go test -c`
+*.test
+
+# Output of the go coverage tool, specifically when used with LiteIDE
+*.out
+
+# build_logs directory, where by default are downloaded artifacts from the CI job
+build_logs
+
+# editor and IDE paraphernalia
+.idea
+.vscode
+*.nix
+*.swp
+*.swo
+*~
+
+# Local Netlify folder
+.netlify
+
+# Directory with CRDs for dependencies
+dep-crds
+
+kueue.yaml
+node_modules
diff --git a/Dockerfile b/Dockerfile
index f7b2e2438..209fdcf39 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -10,7 +10,7 @@ RUN go mod download
 # copy the rest of the sources and build
 COPY . .
 ARG CGO_ENABLED TARGETARCH
-RUN make build GO_BUILD_ENV='CGO_ENABLED=${CGO_ENABLED} GOOS=linux GOARCH=${TARGETARCH}'
+RUN make build GO_BUILD_ENV="GOARCH=${TARGETARCH} CGO_ENABLED=${CGO_ENABLED}"
 
 # final image, implicitly --platform=${TARGETPLATFORM}
 FROM ${BASE_IMAGE}
diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go
index 5e536afc4..b1ad9dfbe 100644
--- a/pkg/scheduler/scheduler.go
+++ b/pkg/scheduler/scheduler.go
@@ -211,18 +211,20 @@ func (s *Scheduler) schedule(ctx context.Context) wait.SpeedSignal {
 	for iterator.hasNext() {
 		e := iterator.pop()
 
-		cq := snapshot.ClusterQueue(e.ClusterQueue)
 		log := log.WithValues("workload", klog.KObj(e.Obj), "clusterQueue", klog.KRef("", string(e.ClusterQueue)))
-		if cq.HasParent() {
-			log = log.WithValues("parentCohort", klog.KRef("", string(cq.Parent().GetName())), "rootCohort", klog.KRef("", string(cq.Parent().Root().GetName())))
-		}
-		ctx := ctrl.LoggerInto(ctx, log)
 
 		mode := e.assignment.RepresentativeMode()
 		if mode == flavorassigner.NoFit {
 			log.V(3).Info("Skipping workload as FlavorAssigner assigned NoFit mode")
 			continue
 		}
+
+		cq := snapshot.ClusterQueue(e.ClusterQueue)
+		if cq != nil && cq.HasParent() {
+			log = log.WithValues("parentCohort", klog.KRef("", string(cq.Parent().GetName())), "rootCohort", klog.KRef("", string(cq.Parent().Root().GetName())))
+		}
+		ctx := ctrl.LoggerInto(ctx, log)
+
 		log.V(2).Info("Attempting to schedule workload")
 
 		if mode == flavorassigner.Preempt && len(e.preemptionTargets) == 0 {



Test patch:
diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go
index 3b1963b16..7c56536a0 100644
--- a/pkg/scheduler/scheduler_test.go
+++ b/pkg/scheduler/scheduler_test.go
@@ -363,6 +363,21 @@ func TestSchedule(t *testing.T) {
 				},
 			},
 		},
+		"skip workload with missing or deleted ClusterQueue (NoFit)": {
+			workloads: []kueue.Workload{
+				*utiltesting.MakeWorkload("missing-cq-workload", "sales").
+					Queue("non-existent-queue").
+					PodSets(*utiltesting.MakePodSet("set", 1).
+						Request(corev1.ResourceCPU, "1").
+						Obj()).
+					Generation(1).
+					Obj(),
+			},
+			// Expect no panics and workload skipped.
+			wantLeft:     nil,
+			wantEvents:   nil,
+			eventCmpOpts: ignoreEventMessageCmpOpts,
+		},
 		"workload fits in single clusterQueue, with check state pending": {
 			workloads: []kueue.Workload{
 				*utiltesting.MakeWorkload("foo", "sales").