[PartialAdmission] Fix preemption while partially admitting.

[trasc]

What type of PR is this?

/kind bug

What this PR does / why we need it:

• Fix the flow that allows a workload to be partially admitted while preempting lower priority workloads.
• Add an unit test for preemption while partially admitting.

Which issue(s) this PR fixes:

Fixes #2799

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fix some scenarios for partial admission which are affected by wrong calculation of resources
used by the incoming workload which is partially admitted and preempting.

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue ready!

Name	Link
🔨 Latest commit	6fa2dca
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/66e9896ab4da4400089409d6
😎 Deploy Preview	https://deploy-preview-2826--kubernetes-sigs-kueue.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[trasc]

/cc @gabesaba

[mimowo]

Please add a release note describing what's changed from the user's perspective.

[trasc]

Please add a release note describing what's changed from the user's perspective.

done

[gabesaba]

/assign

[trasc]

@gabesaba @mimowo is there anything else we should do on this?

[mimowo]

I support the ask in comment.

[mimowo]

@trasc Could you describe what the scenario is fixed here, and what is the consequence from the user perspective when it happens?

Please update the release note with a short description of the problematic scenario.

It will also help us to guide the decision about cherry-picking.

[mimowo]

@gabesaba is it fair to close the original issue #2799 when this PR is merged?

[trasc]

@trasc Could you describe what the scenario is fixed here, and what is the consequence from the user perspective when it happens?

Please update the release note with a short description of the problematic scenario.

It will also help us to guide the decision about cherry-picking.

The scenario is preemption during partial admission.

[mimowo]

The scenario is preemption during partial admission.

Can you provide a little bit more details? In particular: is the preemption not working at all / erroring / panicing / or working sub-optimally.
Also: where the issues deterministic on every preemption, or occasional?

[mimowo]

/reopen
misclicked by mistake

[k8s-ci-robot]

@mimowo: Reopened this PR.

In response to this:

/reopen
misclicked by mistake

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.

[alculquicondor]

/cc

[mimowo]

The scenario is preemption during partial admission.

I'm testing locally the impact of the bug and it seems to go beyond preemption.

For example:

1. I have a CQ allowing for 9 pods (preemption disabled completely)
2. I created 2 small jobs, each consuming 3 pods, so 6 pods total.
3. a big Job comes in requesting 9 pods, but using partial admission to say min 3 pods
4. 3 pods are created (correctly), but the CQ misreports usage as 15 units (6 + 9).
5. When, the small jobs are deleted (or finished), the big Job continues to use 3 pods, but reports consuming 9 resources
6. I create new small jobs requesting 3 units which should fit in, but they are suspended

I'm yet going to determine the scope of the problem and fix, but it seems serious enough to justify cherry-picking.

[trasc]

The scenario is preemption during partial admission.

I'm testing locally the impact of the bug and it seems to go beyond preemption.

For example:

1. I have a CQ allowing for 9 pods (preemption disabled completely)
2. I created 2 small jobs, each consuming 3 pods, so 6 pods total.
3. a big Job comes in requesting 9 pods, but using partial admission to say min 3 pods
4. 3 pods are created (correctly), but the CQ misreports usage as 15 units (6 + 9).
5. When, the small jobs are deleted (or finished), the big Job continues to use 3 pods, but reports consuming 9 resources
6. I create new small jobs requesting 3 units which should fit in, but they are suspended

I'm yet going to determine the scope of the problem and fix, but it seems serious enough to justify cherry-picking.

I think this is a different issue. Just open the issue in GH and feel free to assign it to me.

[mimowo]

Ok, there you go: #3108

[mimowo]

Regarding the fix here I played a bit more, and observed, that the feature generally worked before (no panics, no errors), but there are differences in some scenarios, and they are really hard for me to briefly (also the other issue is interfering to test properly, because the other issue is also present with preemption enabled).

Still, IIUC the culprit for the issue is wrong calculation of resources requested by the incoming workload. I would like to capture this in the release note as below.

/release-note-edit
```release-note
Fix some scenarios for partial admission which are affected by wrong calculation of resources
used by the incoming workload which is partially admitted and preempting.

Going forward it would be super helpful to have an integration test(s) for this feature, otherwise it is hard to judge how the unit tests translate into the e2e behavior, imo.

Regarding the cherry-pick, no strong view here, but I think we don't need to, given that we don't have customers / users affected by the issue, or demanding the fix. Additionally, the other issue opened above still interferes to fully evaluate the fix. We can re-evaluate cherry-picking them together, or when we have users demanding the fix.

[mimowo]

LGTM, except for the point raised here, but I can accept as is, just waiting a bit for @alculquicondor in case he has something to add or other ideas to improve.

[mimowo]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: bfea979511531e090af34379b6573419f2bde216

[trasc]

/cherrypick release-0.8

[k8s-infra-cherrypick-robot]

@trasc: new pull request created: #3205

In response to this:

/cherrypick release-0.8

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.