KEP 973 workload-Priority

[Gekko0114]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add support for workload Priority

Which issue(s) this PR fixes:

Fixes #973

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Add support for workload Priority

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	2c000f0
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/650e94d865f4b300080f614e

[k8s-ci-robot]

Hi @Gekko0114. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[alculquicondor]

Is this ready for review or still a Draft?

[Gekko0114]

Hi @alculquicondor, @tenzen-y

Could you review this PR ?
Though I will add other tests, the basic implementation has done.

[alculquicondor]

/ok-to-test

[tenzen-y]

Sorry for the late. I will review this PR today.

[Gekko0114]

Sorry for the late. I will review this PR today.

Np, thank you so much!

[tenzen-y]

I left a comment for the first pass. I will review this PR again once you address @alculquicondor's comments regarding to immutable priorityClass.

[tenzen-y]

Hi @alculquicondor @tenzen-y,
Thank you so much for your review comments. I have made the requested changes. Could you review it again?
Additionally, I haven't written tests for preemption and the queue. Should I write tests for them as well?

I'm ok with not adding teets for the scheduler and preemptor since they depend only on priority values and priorityClassSources don't directly affect them.

WDYT? @alculquicondor

However, expanding integration tests for the job with the scheduler might be worth it.

kueue/test/integration/controller/jobs/job/job_controller_test.go

Line 706 in fd90aad

var _ = ginkgo.Describe("Job controller interacting with scheduler", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {

[Gekko0114]

Hi @alculquicondor , @tenzen-y ,

I've fixed all comments and added e2e and integration test cases. Could you review it again?

[alculquicondor]

Looking good overall.
I'll leave the final say to @tenzen-y.

[tenzen-y]

@Gekko0114 Otherwise LGTM
Could you open a PR to add documentation for WorkloadPriorityClass?

[tenzen-y]

Also, it would be better to squash all commits into one to avoid a huge commit message.

[Gekko0114]

Hi @tenzen-y , @alculquicondor

Thank you so much for your detailed review! I've learned a lot from this PR.
I've fixed all comments, could you review it again?

Could you open a PR to add documentation for WorkloadPriorityClass?

Sure, do you mean I should add docs under here ?

[tenzen-y]

Could you open a PR to add documentation for WorkloadPriorityClass?

Sure, do you mean I should add docs under here ?

Yes.

[tenzen-y]

@Gekko0114 Thank you for the hard work! I am really looking forward to this new feature :)

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 92c89dd9222d35070d9f6595565cac6ae742b070

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Gekko0114, tenzen-y

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [tenzen-y]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment