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

fix: provide runtime cgroups to kubelet #3804

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

ginglis13
Copy link
Contributor

@ginglis13 ginglis13 commented Mar 1, 2024

Issue number:

Closes #3776

Description of changes:

Pass the cgroup where containerd is running to kubelet to enable container resource and performance monitoring via tools like cadvisor.

Testing done:

Tested on aws-k8s-1.23-aarch64 and aws-k8s-1.29-aarch64:

$ kubectl get --raw "/api/v1/nodes/${NODE_NAME}/proxy/stats/summary" | jq -r '.node.systemContainers[] | .name'
pods
kubelet
runtime

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@ginglis13 ginglis13 marked this pull request as ready for review March 2, 2024 01:52
@Donnerdiebel
Copy link

Hi all!

Any chances that this will get merged in the near future?

We just ran into this problem and deploying additional cAdvisors to be able to collect runtime metrics when using bottlerocket amis on EKS is not exactly outr preferred solution.
Anyways, thanks for taking a look into this and creating the fix.

All the best!

@ginglis13
Copy link
Contributor Author

Hi all!

Any chances that this will get merged in the near future?

We just ran into this problem and deploying additional cAdvisors to be able to collect runtime metrics when using bottlerocket amis on EKS is not exactly outr preferred solution. Anyways, thanks for taking a look into this and creating the fix.

All the best!

Hey @Donnerdiebel , thanks for the bump. I've got some additional testing to do for this change and haven't gotten around to it... I'll try to have an update by the end of the week 😄

@gthao313
Copy link
Member

The change and test looks good to me.
Just have a nit for future commit message. It would be recommended to give info like what package you have changes on.
For example, fix: provide runtime cgroups to kubelet can be kubernetes: provide runtime cgroups to kubelet. : )

Pass the cgroup where containerd is running to kubelet to
enable container resource and performance monitoring via tools
like cadvisor.

Signed-off-by: Gavin Inglis <[email protected]>
@ginglis13
Copy link
Contributor Author

^ reword commit message

@ginglis13 ginglis13 merged commit 99cb487 into bottlerocket-os:develop Mar 22, 2024
50 checks passed
@dhumphries-sainsburys
Copy link

Do we know when this is going to be released to a tag as doesn't seem to be there yet?

@ginglis13
Copy link
Contributor Author

Hi @dhumphries-sainsburys, you are correct that this fix has not yet made it into an official release of Bottlerocket. It will be included in the next release, which the team is still working to plan and prioritize.

@saikumarch7548
Copy link

when is the next release planned @ginglis13

@yeazelm yeazelm mentioned this pull request Apr 25, 2024
6 tasks
@ginglis13
Copy link
Contributor Author

Hi @saikumarch7548, this change has been included in the latest release of Bottlerocket, v1.19.5: https://github.com/bottlerocket-os/bottlerocket/releases/tag/v1.19.5

@saikumarch7548
Copy link

@ginglis13 , Thank you , we have tested it in our product and it works

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.

Missing runtime metrics from cAdvisor
6 participants