kubernetes-sigs / kueue

Kubernetes-native Job Queueing
https://kueue.sigs.k8s.io
Apache License 2.0
1.49k stars 267 forks source link

TAS: fix scenario when parallelism < completions #3559

Closed mimowo closed 2 weeks ago

mimowo commented 2 weeks ago

What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Part of #3450

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fix running Job when parallelism < completions, before the fix the replacement pods for the successfully
completed Pods were not ungated.
k8s-ci-robot commented 2 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mimowo

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: - ~~[pkg/controller/OWNERS](https://github.com/kubernetes-sigs/kueue/blob/main/pkg/controller/OWNERS)~~ [mimowo] - ~~[test/OWNERS](https://github.com/kubernetes-sigs/kueue/blob/main/test/OWNERS)~~ [mimowo] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
netlify[bot] commented 2 weeks ago

Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
Latest commit f126604c52f24abce9490d9e024b8e143182639f
Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/6737a67f0e59d2000838d29b
mbobrovskyi commented 2 weeks ago

/lgtm Thanks!

k8s-ci-robot commented 2 weeks ago

LGTM label has been added.

Git tree hash: b158606c1b7d5911f2412487e32c3dbcaba3b450

mimowo commented 2 weeks ago

/cherry-pick release-0.9

k8s-infra-cherrypick-robot commented 2 weeks ago

@mimowo: #3559 failed to apply on top of branch "release-0.9":

Applying: TAS: fix scenario when parallelism < completions
Using index info to reconstruct a base tree...
M   pkg/controller/tas/topology_ungater.go
A   test/e2e/tas/tas_test.go
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): test/e2e/tas/tas_test.go deleted in HEAD and modified in TAS: fix scenario when parallelism < completions. Version TAS: fix scenario when parallelism < completions of test/e2e/tas/tas_test.go left in tree.
Auto-merging pkg/controller/tas/topology_ungater.go
CONFLICT (content): Merge conflict in pkg/controller/tas/topology_ungater.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 TAS: fix scenario when parallelism < completions
In response to [this](https://github.com/kubernetes-sigs/kueue/pull/3559#issuecomment-2479847536): >/cherry-pick release-0.9 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.