Introduce FS at admission time

[PBundyra]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Changes described in the first phase of #4252

Which issue(s) this PR fixes:

Fixes #4136

Special notes for your reviewer:

I still need to comment some API fields and integration tests

Does this PR introduce a user-facing change?

Support a new fair sharing alpha feature `Admission Fair Sharing`, along with the new API. It orders workloads based on the recent usage coming from a LocalQueue the workload was submitted to. The recent usage is more important than the priority of workloads  

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	118ac5c
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/681b97748d89a5000795c375

[PBundyra]

/assign @gabesaba @pajakd
cc @mimowo @mwielgus

[k8s-ci-robot]

@PBundyra: GitHub didn't allow me to assign the following users: pajakd.

Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @gabesaba @pajakd
cc @mimowo @mwielgus

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-sigs/prow repository.

[PBundyra]

/retest

[pajakd]

Just to be sure about the scope here -- by "Changes described in the first phase of #4252" you mean CQ + LQ level support, right (so no cohorts yet)?

[PBundyra]

Just to be sure about the scope here -- by "Changes described in the first phase of #4252" you mean CQ + LQ level support, right (so no cohorts yet)?

Exactly

[gabesaba]

First pass. Let's iron out the API, and interaction with other FS mode, as this will be difficult to change later

[mimowo]

Actually this code does not know this comes from informer. For example we could change the impl of LqUsage to use cache, but we wouldn't remember to change the log. I would suggest to have a local function which logs and call it.

something like:

lqUsageOrLogError := func(...) usage {
   usage, err := a.LqUsage(ctx, c, fsResWeights)
   if err != nil {
      log.V(2).Error("Error determining LocalQueue usage", "localQueue", lq.Name)
   }
   return usage
}

also, errors should probably not be V(3).

[PBundyra]

I'll fix the message and the depth of the log but the local functions seems like a little overkill to me. I need to have information about the error not only for logging but also to decide if I can compare usages

[mimowo]

Ah, got it, so in case of errors you don't compare them at all, sounds reasonable. Ideally, the functions used in comparators should not return errors at all.

For example, we could retrieve the LqUsage before calling comparators and just store in memory for quick access. We can leave it for a follow up.

[k8s-ci-robot]

@PBundyra: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kueue-test-integration-main	88778ec	link	true	/test pull-kueue-test-integration-main

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-sigs/prow repository. I understand the commands that are listed here.

[mimowo]

/lgtm
/approve
Thank you, this is an important feature.

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 33fd40967855828f0da439e29de8d1485591560e

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mimowo, PBundyra

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 [mimowo]

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

[mimowo]

These are good comments, I guess the most important is to address the API remarks before the release, the other ones could probably be fixed as follow up issue(s). wdyt @tenzen-y ?

[mimowo]

@PBundyra please address the comments, I think the API updates have priority.

[PBundyra]

Sure, will do in a follow-up

[tenzen-y]

These are good comments, I guess the most important is to address the API remarks before the release, the other ones could probably be fixed as follow up issue(s). wdyt @tenzen-y ?

I think API markers and validations for ConfigAPIs are higher priority.

[PBundyra]

@PBundyra please address the comments, I think the API updates have priority.

Addressed here: #5203