dongjoon-hyun commented on code in PR #881:
URL:
https://github.com/apache/spark-kubernetes-operator/pull/881#discussion_r4096621289
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/kueue/KueueWorkloadUtils.java:
##########
@@ -165,6 +165,18 @@ public static AdmissionResponse requestAdmission(
return new AdmissionResponse(AdmissionResult.STALE, workload);
}
WorkloadSpec spec = workload.getSpec();
+ boolean changed = false;
+ String desiredQueueName = desired.getSpec().getQueueName();
+ if (!Objects.equals(spec.getQueueName(), desiredQueueName)) {
Review Comment:
Thank you, @peter-toth. You are right. Fixed in e810f18. The queue move is
now skipped once the quota is reserved, like the priority class clause, so the
PUT is not rejected and the priority raise is not dropped. I added
`quotaReservedWorkloadKeepsItsQueue` and updated the doc sentence accordingly.
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/kueue/KueueWorkloadUtils.java:
##########
@@ -365,6 +376,49 @@ public static ReconcileProgress retryAfterRequestFailure(
return ReconcileProgress.completeAndDefaultRequeue();
}
+ /**
+ * Releases the pending Workload of a resource whose queue label was removed
while it waited for
+ * the admission. The resource now starts without Kueue, so Kueue would
otherwise admit the
+ * Workload later into quota which nothing uses. An admitted Workload is
kept, since the
+ * resources it was admitted for may be running already, and it is released
with them like the
+ * Workload of a queued resource. Like the admission request, a failed
release is retried before
+ * the resources are requested.
+ *
+ * @param context The context of the resource without a queue name label.
+ * @return The progress to return while the release fails, or empty to
proceed.
+ */
+ public static Optional<ReconcileProgress> releaseDequeuedWorkload(
+ final BaseContext<?> context) {
+ Optional<Workload> workload = context.getCachedKueueWorkload();
+ if (workload.isEmpty() || isAdmitted(workload.get())) {
Review Comment:
Agreed. Since this predates this PR, I will handle it in a follow-up JIRA
issue.
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/kueue/KueueWorkloadUtils.java:
##########
@@ -165,6 +165,18 @@ public static AdmissionResponse requestAdmission(
return new AdmissionResponse(AdmissionResult.STALE, workload);
}
WorkloadSpec spec = workload.getSpec();
+ boolean changed = false;
+ String desiredQueueName = desired.getSpec().getQueueName();
+ if (!Objects.equals(spec.getQueueName(), desiredQueueName)) {
+ // Like Kueue, a queue label changed while waiting moves the Workload to
the new queue in
+ // place, since it holds no quota yet.
+ log.info(
+ "Moving the pending Kueue Workload {} to queue {}.",
+ workload.getMetadata().getName(),
+ desiredQueueName);
+ spec.setQueueName(desiredQueueName);
Review Comment:
Fixed in e810f18. The moved `Workload` now gets the new
`kueue.x-k8s.io/queue-name` label in the same update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
