dongjoon-hyun commented on PR #851:
URL:
https://github.com/apache/spark-kubernetes-operator/pull/851#issuecomment-5751418533
Thanks again, @peter-toth! Rebased on `main` (which now carries #850) and
addressed 6, 7 and 8.
- **6.** Added tests for each condition of `isPriorityClassChangeAllowed`,
and I re-ran the mutations: removing the follow rule fails
`pendingWorkloadWithPodPriorityClassDoesNotFollowTheLabel` and
`pendingWorkloadKeepsItsPodPriorityClassName`, removing `current != null` fails
`quotaReservedWorkloadWithoutPriorityClassDoesNotGainOne`, removing the
`desired` group check fails
`quotaReservedWorkloadDoesNotSwitchPriorityClassGroup`, and removing `desired
!= null` fails `quotaReservedWorkloadKeepsFrozenPriorityClass`.
`priorityClass(...)` and `forbiddenClient()` are extracted as you suggested.
- **7.** Good catch, fixed. `isPriorityClassChangeAllowed` now starts with
part 1 of Kueue's gate: a Workload whose ref is a `scheduling.k8s.io`
`PriorityClass` is never rewritten, so a `globalDefault` change no longer
re-prices a pending Workload. That also simplifies the quota-reserved check,
which now only has to accept a `kueue.x-k8s.io` ref on both sides.
- **8.** Reworded to "before the `Workload` reserves quota", and spelled out
both the freeze and the fact that a `PriorityClass`-backed `Workload` does not
follow the label.
- **2.** Thank you for the correction on the sequencing, I had the wrong PR.
I will do the get/create split as a follow-up after #847 lands, with the
two-part gate from finding 7.
One note on the rebase: with `resolvePodSetFlavors` from #850 in the same
class, PMD now reports `GodClass` for `KueueWorkloadUtils`, so I added
`@SuppressWarnings("PMD.GodClass")` like `KueueWorkloadFactory` has. If you
would rather see the priority resolution split into its own class, I am happy
to do that instead.
--
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]
