koordlet: support evict yarn container #2464
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2464 +/- ##
==========================================
+ Coverage 65.93% 66.06% +0.13%
==========================================
Files 477 484 +7
Lines 56194 56910 +716
==========================================
+ Hits 37049 37596 +547
- Misses 16461 16566 +105
- Partials 2684 2748 +64
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jimmysenior
force-pushed
the
feature/koordlet-support-evict-yarn-container
branch
from
76a95cf to
8b7980c
Compare
| @@ -20,6 +20,8 @@ import ( | |||
| clientset "k8s.io/client-go/kubernetes" | |||
| "k8s.io/client-go/tools/record" | |||
|
|
|||
| "github.com/koordinator-sh/koordinator/pkg/koordlet/qosmanager/plugins/copilot" | |||
There was a problem hiding this comment.
- The copilot agent seems not to be a QoS plugin, but an implementation of the
framework.Evictor. - The QoS plugin should not be referenced in
framework/options.go. Instead, try to register the plugin inplugins/register.go.
There was a problem hiding this comment.
copilot agent is not a implementation of the framework.Evictor, it is only a part of cpu evict or memory evict. It is inappropriate to register the plugin in plugins/register.go. It is essentially just used for communicating with Copilot, based on file sockets. Do you think it is feasible to place the copilot in the directory of koordinator/pkg/koordlet/qosmanager/helpers?
saintube
changed the title
|
|
||
| func isYarnNodeManager(pod *podEvictCPUInfo) bool { | ||
| targetKey := "app.kubernetes.io/component" | ||
| targetValue := "node-manager" |
There was a problem hiding this comment.
consider to be configurable?
jimmysenior
force-pushed
the
feature/koordlet-support-evict-yarn-container
branch
from
1798210 to
59a2d85
Compare
Signed-off-by: jimmysenior <[email protected]>
jimmysenior
force-pushed
the
feature/koordlet-support-evict-yarn-container
branch
from
157390b to
bfc4987
Compare
|
/lgtm |
| "testing" | ||
| "time" | ||
|
|
||
| "sigs.k8s.io/controller-runtime/pkg/manager/signals" |
There was a problem hiding this comment.
need goimports
imports are recommended to be organized like:
<buit-in pkgs>
<third-party pkgs>
<koordinator pkgs>| ) | ||
|
|
||
| func TestKillContainerByResource(t *testing.T) { | ||
| // 1. 配置 Socket 路径 |
| } | ||
| listener, err := net.Listen("unix", y.unixPath) | ||
| if err != nil { | ||
| fmt.Printf("Failed to listen UNIX socket: %v", err) |
There was a problem hiding this comment.
use klog.Fatalf instead of fmt.Printf
| hasKillPods = true | ||
| } else { | ||
| klog.V(5).Infof("cpuEvict pick pod %s to evict, failed", util.GetPodKey(bePod.pod)) | ||
| klog.Warningf("BatchCPU resource not found in resource list") |
Ⅰ. Describe what this PR does
suport evict container of yarn nodemanager
when koordlet config --evict-by-copilot-agent=true,the memory evict and cpu evict will send requet to component of copilot-agent through socket ,which pod to be evicted is yarn nodemanager. And the actual logic of evict will finished on the component of opilot-agent.
Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews
when need use this feature, need merge pr of yarn-copilot and charts also
koordinator-sh/yarn-copilot#99
koordinator-sh/charts#94
V. Checklist
make test