Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

read to EOL for pod and container id in cgroup #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zwpaper
Copy link

@zwpaper zwpaper commented Feb 16, 2022

fix containerd cgroupfs path use :

refactor the reading logic to simplify it.

fix containerd cgroupfs path use `:`

Signed-off-by: ZhangWei <[email protected]>
if (!strcmp(token, "memory")) {
cgroup_ptr = strtok_r(NULL, ":", &last_ptr);
cgroup_ptr = strtok_r(NULL, "\n", &last_ptr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fgets never return EOL character , why search for EOL?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strtok_r is used here instead of fgets you mentioned.

containerd will use : as file name separator if cgroupfs is used so that it will read a wrong pod id and container id if search to :.

* /kubepods/besteffort/pod27882189_b4d9_11e9_b287_ec0d9ae89a20/docker-4aa615892ab2a014d52178bdf3da1c4a45c8ddfb5171dd6e39dc910f96693e14
*
* containerd
* /system.slice/containerd.service/kubepods-besteffort-pod019c1fe8_0d92_4aa0_b61c_4df58bdde71c.slice:cri-containerd:9e073649debeec6d511391c9ec7627ee67ce3a3fb508b0fa0437a97f8e58ba98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which version of kubernetes generates this cgroup pattern?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try v1.21.x with containerd and cgroupfs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants