koordlet: cgroup file read and write is privatized and consolidated into the resource executor package. #1024

Re-Grh · 2023-02-20T11:38:03Z

Ⅰ. Describe what this PR does

Ⅱ. Does this pull request fix one issue?

fix #1017

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

V. Checklist

I have written necessary docs and comments
I have added necessary unit tests and integration tests
All checks passed in make test

codecov · 2023-02-21T03:09:56Z

Codecov Report

Patch coverage: 61.09% and project coverage change: +0.03 🎉

Comparison is base (a40f96f) 67.41% compared to head (81bcd0a) 67.44%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1024      +/-   ##
==========================================
+ Coverage   67.41%   67.44%   +0.03%     
==========================================
  Files         260      260              
  Lines       28752    28662      -90     
==========================================
- Hits        19382    19330      -52     
+ Misses       8018     7982      -36     
+ Partials     1352     1350       -2

Flag	Coverage Δ
unittests	`67.44% <61.09%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/koordlet/resmanager/cpu_burst.go	`73.07% <0.00%> (ø)`
pkg/koordlet/runtimehooks/hooks/gpu/gpu.go	`65.62% <0.00%> (ø)`
...oordlet/runtimehooks/protocol/container_context.go	`5.36% <0.00%> (-0.12%)`	⬇️
.../koordlet/runtimehooks/protocol/kubeqos_context.go	`0.00% <0.00%> (ø)`
pkg/koordlet/runtimehooks/protocol/pod_context.go	`6.95% <0.00%> (-0.39%)`	⬇️
pkg/koordlet/runtimehooks/protocol/protocol.go	`3.57% <0.00%> (-0.36%)`	⬇️
pkg/koordlet/util/meminfo.go	`92.68% <ø> (+10.54%)`	⬆️
pkg/koordlet/util/stat.go	`53.42% <ø> (-4.27%)`	⬇️
pkg/koordlet/util/system/cgroup.go	`10.98% <ø> (-20.90%)`	⬇️
pkg/koordlet/util/system/util_test_tool.go	`50.00% <26.92%> (-8.91%)`	⬇️
... and 28 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

pkg/koordlet/resmanager/cgroup_reconcile_test.go

pkg/koordlet/resourceexecutor/cgroup.go

pkg/koordlet/resourceexecutor/psi.go

pkg/koordlet/runtimehooks/hooks/groupidentity/rule.go

pkg/koordlet/resmanager/cgroup_reconcile_test.go

pkg/koordlet/resourceexecutor/cgroup.go

pkg/koordlet/runtimehooks/hooks/groupidentity/rule.go

pkg/koordlet/resourceexecutor/cgroup.go

zwzhang0107 · 2023-02-24T06:33:28Z

pkg/koordlet/util/system/psi_file.go

 // parsePSIStats parses the specified file for pressure stall information.
-func parsePSIStats(r io.Reader) (PSIStats, error) {


other files and struct can also be moved into resourceexecutor

zwzhang0107 · 2023-02-24T06:36:05Z

pkg/koordlet/util/system/cgroup.go

-	}
-	return CgroupFileWrite(cgroupTaskDir, r, value)
-}
-
 func IsCgroupPathExist(parentDir string, r Resource) (bool, string) {


mv to resourceexecutor

zwzhang0107 · 2023-02-24T09:30:34Z

pkg/koordlet/util/container.go

@@ -193,16 +193,6 @@ func GetContainerCurMemLimitBytes(podParentDir string, c *corev1.ContainerStatus
 	return strconv.ParseInt(strings.TrimSpace(string(rawContent)), 10, 64)
 }

-// GetContainerCurTasks gets the TIDs of the given container's cgroup.
-// DEPRECATED: use resourceexecutor.CgroupReader instead.


check other deprecated func

zwzhang0107 · 2023-02-24T09:40:25Z

pkg/koordlet/runtimehooks/protocol/container_context.go

@@ -191,8 +191,10 @@ func (c *ContainerContext) injectForOrigin() {
 			klog.V(5).Infof("set container %v/%v/%v cpuset %v on cgroup parent %v",
 				c.Request.PodMeta.Namespace, c.Request.PodMeta.Name, c.Request.ContainerMeta.Name,
 				*c.Response.Resources.CPUSet, c.Request.CgroupParent)
-			audit.V(2).Container(c.Request.ContainerMeta.ID).Reason("runtime-hooks").Message(
-				"set container cpuset to %v", *c.Response.Resources.CPUSet).Do()
+			if updated {


independent pr

zwzhang0107 · 2023-03-03T10:12:05Z

pkg/koordlet/runtimehooks/hooks/groupidentity/rule.go

+		if err != nil {
+			klog.Infof("bvtupdater creat failed, dir %v, error %v", kubeQOSCgroupPath, err)
+		}
+		if err := bvtUpdater.Update(); err != nil {


set Update as private, use executor here

Re-Grh · 2023-03-06T02:30:27Z

/hold cancel

Signed-off-by: Re-Grh <1271013391@qq.com>

zwzhang0107 · 2023-03-06T08:30:54Z

pkg/koordlet/runtimehooks/protocol/protocol.go

@@ -101,7 +104,8 @@ func injectMemoryLimit(cgroupParent string, memoryLimit int64) error {
 	if err != nil {
 		return err
 	}
-	return updater.Update()
+	_, err = resourceexecutor.NewResourceUpdateExecutor().Update(false, updater)


pass resourceexecutor into server

zwzhang0107 · 2023-03-06T08:31:05Z

pkg/koordlet/runtimehooks/protocol/protocol.go

@@ -101,7 +104,8 @@ func injectMemoryLimit(cgroupParent string, memoryLimit int64) error {
 	if err != nil {
 		return err
 	}
-	return updater.Update()
+	_, err = resourceexecutor.NewResourceUpdateExecutor().Update(false, updater)


pass resourceexecutor into server

Signed-off-by: Re-Grh <1271013391@qq.com>

zwzhang0107 · 2023-03-07T09:48:02Z

pkg/koordlet/runtimehooks/reconciler/reconciler.go

@@ -204,12 +205,18 @@ type Reconciler interface {
 	Run(stopCh <-chan struct{}) error
 }

-func NewReconciler(s statesinformer.StatesInformer) Reconciler {
+type Options struct {
+	S        statesinformer.StatesInformer


StatesInformer

zwzhang0107 · 2023-03-07T09:53:44Z

pkg/koordlet/runtimehooks/protocol/protocol.go

 	cpuShareStr := strconv.FormatInt(cpuShares, 10)
 	updater, err := resourceexecutor.DefaultCgroupUpdaterFactory.New(sysutil.CPUSharesName, cgroupParent, cpuShareStr)
 	if err != nil {
 		return err
 	}
-	_, err = resourceexecutor.NewResourceUpdateExecutor().Update(false, updater)
+	_, err = e.Update(false, updater)


Signed-off-by: Re-Grh <1271013391@qq.com>

zwzhang0107 · 2023-03-07T11:14:44Z

/lgtm
/approve

koordinator-bot · 2023-03-07T11:14:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zwzhang0107

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/koordlet/OWNERS~~ [zwzhang0107]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

koordinator-bot bot requested review from jasonliu747 and saintube February 20, 2023 11:38

koordinator-bot bot added the size/XXL label Feb 20, 2023

Re-Grh force-pushed the cgrouprw branch from 211a247 to d713907 Compare February 21, 2023 02:57

saintube reviewed Feb 22, 2023

View reviewed changes

Re-Grh force-pushed the cgrouprw branch from 398988c to 79867bc Compare February 22, 2023 07:23

saintube reviewed Feb 23, 2023

View reviewed changes

saintube reviewed Feb 24, 2023

View reviewed changes

pkg/koordlet/resourceexecutor/cgroup.go Show resolved Hide resolved

Re-Grh force-pushed the cgrouprw branch 2 times, most recently from b81ddb0 to 072e99f Compare February 24, 2023 02:01

koordinator-bot bot added the do-not-merge/hold label Feb 24, 2023

zwzhang0107 reviewed Feb 24, 2023

View reviewed changes

Re-Grh force-pushed the cgrouprw branch from d7d33e7 to 82eb79c Compare February 24, 2023 09:17

zwzhang0107 reviewed Feb 24, 2023

View reviewed changes

Re-Grh force-pushed the cgrouprw branch 3 times, most recently from d2e9566 to d15b39b Compare March 3, 2023 06:05

zwzhang0107 reviewed Mar 3, 2023

View reviewed changes

koordinator-bot bot removed the do-not-merge/hold label Mar 6, 2023

Re-Grh added 4 commits March 6, 2023 16:23

cgroup-rw

f668124

Signed-off-by: Re-Grh <1271013391@qq.com>

cgroup-rw

d813d9c

Signed-off-by: Re-Grh <1271013391@qq.com>

cgroup-rw

97dc2db

Signed-off-by: Re-Grh <1271013391@qq.com>

cgroup

23f127a

Signed-off-by: Re-Grh <1271013391@qq.com>

Re-Grh force-pushed the cgrouprw branch from d15b39b to 23f127a Compare March 6, 2023 08:24

zwzhang0107 reviewed Mar 6, 2023

View reviewed changes

runtimehook

3a5b63d

Signed-off-by: Re-Grh <1271013391@qq.com>

zwzhang0107 reviewed Mar 7, 2023

View reviewed changes

runtimehook

69ba683

Signed-off-by: Re-Grh <1271013391@qq.com>

Re-Grh added 2 commits March 7, 2023 18:07

runtimehook

7c60c95

Signed-off-by: Re-Grh <1271013391@qq.com>

Merge branch 'main' into cgrouprw

81bcd0a

koordinator-bot bot assigned zwzhang0107 Mar 7, 2023

koordinator-bot bot added the lgtm label Mar 7, 2023

koordinator-bot bot added the approved label Mar 7, 2023

koordinator-bot bot merged commit a2880cf into koordinator-sh:main Mar 7, 2023

zwzhang0107 added this to the v1.2 milestone Apr 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

koordlet: cgroup file read and write is privatized and consolidated into the resource executor package. #1024

koordlet: cgroup file read and write is privatized and consolidated into the resource executor package. #1024

Re-Grh commented Feb 20, 2023 •

edited

codecov bot commented Feb 21, 2023 •

edited

zwzhang0107 Feb 24, 2023

zwzhang0107 Feb 24, 2023

zwzhang0107 Feb 24, 2023

zwzhang0107 Feb 24, 2023

zwzhang0107 Mar 3, 2023

Re-Grh commented Mar 6, 2023

zwzhang0107 Mar 6, 2023

zwzhang0107 Mar 6, 2023

zwzhang0107 Mar 7, 2023

zwzhang0107 Mar 7, 2023

zwzhang0107 commented Mar 7, 2023

koordinator-bot bot commented Mar 7, 2023

		// parsePSIStats parses the specified file for pressure stall information.
		func parsePSIStats(r io.Reader) (PSIStats, error) {

koordlet: cgroup file read and write is privatized and consolidated into the resource executor package. #1024

koordlet: cgroup file read and write is privatized and consolidated into the resource executor package. #1024

Conversation

Re-Grh commented Feb 20, 2023 • edited

Ⅰ. Describe what this PR does

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

V. Checklist

codecov bot commented Feb 21, 2023 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Re-Grh commented Mar 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwzhang0107 commented Mar 7, 2023

koordinator-bot bot commented Mar 7, 2023

Re-Grh commented Feb 20, 2023 •

edited

codecov bot commented Feb 21, 2023 •

edited