You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by "zhuqi-lucas (via GitHub)" <gi...@apache.org> on 2023/08/25 08:26:08 UTC
[GitHub] [yunikorn-k8shim] zhuqi-lucas opened a new pull request, #660: [YUNIKORN-1928] Remove the unused skip logic for auto scaling container.
zhuqi-lucas opened a new pull request, #660:
URL: https://github.com/apache/yunikorn-k8shim/pull/660
### What is this PR for?
In shim side we have skip autoscaling logic both for yunikorn mode and plugin mode.
```
switch request.State {
case si.UpdateContainerSchedulingStateRequest_SKIPPED:
// auto-scaler scans pods whose pod condition is PodScheduled=false && reason=Unschedulable
// if the pod is skipped because the queue quota has been exceed, we do not trigger the auto-scaling
task.SetTaskSchedulingState(interfaces.TaskSchedSkipped)
if ctx.updatePodCondition(task,
&v1.PodCondition{
Type: v1.PodScheduled,
Status: v1.ConditionFalse,
Reason: "SchedulingSkipped",
Message: request.Reason,
}) {
events.GetRecorder().Eventf(task.pod.DeepCopy(), nil,
v1.EventTypeNormal, "PodUnschedulable", "PodUnschedulable",
"Task %s is skipped from scheduling because the queue quota has been exceed", task.alias)
}
```
We need to remove the unused misleading logic.
### What type of PR is it?
* [ ] - Bug Fix
* [ ] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
* Open an issue on Jira https://issues.apache.org/jira/browse/YUNIKORN/
* Put link here, and add [YUNIKORN-*Jira number*] in PR title, eg. `[YUNIKORN-2] Gang scheduling interface parameters`
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] craigcondit closed pull request #660: [YUNIKORN-1928] Remove the unused skip logic for auto scaling container.
Posted by "craigcondit (via GitHub)" <gi...@apache.org>.
craigcondit closed pull request #660: [YUNIKORN-1928] Remove the unused skip logic for auto scaling container.
URL: https://github.com/apache/yunikorn-k8shim/pull/660
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] codecov[bot] commented on pull request #660: [YUNIKORN-1928] Remove the unused skip logic for auto scaling container.
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #660:
URL: https://github.com/apache/yunikorn-k8shim/pull/660#issuecomment-1692991467
## [Codecov](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/660?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#660](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/660?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (397a632) into [master](https://app.codecov.io/gh/apache/yunikorn-k8shim/commit/5b35abcd16f4c489f2aaaa3197f9c2a685db393b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5b35abc) will **increase** coverage by `0.13%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #660 +/- ##
==========================================
+ Coverage 71.44% 71.57% +0.13%
==========================================
Files 51 51
Lines 8128 8113 -15
==========================================
Hits 5807 5807
+ Misses 2123 2108 -15
Partials 198 198
```
| [Files Changed](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/660?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/interfaces/task\_sched\_state.go](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/660?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2FwcG1nbXQvaW50ZXJmYWNlcy90YXNrX3NjaGVkX3N0YXRlLmdv) | `100.00% <ø> (ø)` | |
| [pkg/cache/context.go](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/660?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `45.86% <ø> (+0.79%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] craigcondit commented on pull request #660: [YUNIKORN-1928] Remove the unused skip logic for auto scaling container.
Posted by "craigcondit (via GitHub)" <gi...@apache.org>.
craigcondit commented on PR #660:
URL: https://github.com/apache/yunikorn-k8shim/pull/660#issuecomment-1693930625
This logic is 100% needed and should not be removed. It is used to flag pods that have been skipped so that they may be treated differently by the scheduler plugin logic. Unlike the standalone scheduler mode (which does not need this), plugin mode does, because the default scheduler itself is the one responsible for setting the Pod state. Without this, PreEnqueue functionality is broken.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org