search

kubewharf / kubeadmiral

Multi-Cluster Kubernetes Orchestration
Apache License 2.0
658 stars 89 forks source link

feat: add prioritySort plugin #291

Closed Poor12 closed 7 months ago

codecov[bot] commented 7 months ago

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (6d35318) 31.15% compared to head (92a922c) 31.40%.

Files Patch % Lines
pkg/controllers/scheduler/schedulingunit.go 82.35% 8 Missing and 1 partial :warning:
...ntrollers/scheduler/framework/runtime/framework.go 16.66% 5 Missing :warning:
...er/framework/plugins/prioritysort/priority_sort.go 92.59% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #291 +/- ## ========================================== + Coverage 31.15% 31.40% +0.24% ========================================== Files 122 123 +1 Lines 14266 14343 +77 ========================================== + Hits 4445 4504 +59 - Misses 9406 9426 +20 + Partials 415 413 -2 ``` | [Flag](https://app.codecov.io/gh/kubewharf/kubeadmiral/pull/291/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubewharf) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/kubewharf/kubeadmiral/pull/291/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubewharf) | `31.40% <82.41%> (+0.24%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubewharf#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

limhawjia commented 7 months ago

This PR changes the scheduler's framework to support having multiple select plugins. However, it changes the select plugin interface such that it can now override the cluster scores list. I have some reservations about this.

  1. There is now an overlap of responsibilities with the scoring stage.
  2. It gives a single select plugin too much power. The scores returned by a select plugin will completely replace any existing ones, possibly rendering the work done by previous plugins or even the whole scoring stage obsolete.
Poor12 commented 7 months ago

/cc @gary-lgy