allow for cadvisor to detect disk stats when the read-only and writeable layers are on separate disks. #3395

kannon92 · 2023-09-13T20:12:09Z

We are working on splitting the image disk and we realize that some upstream changes are needed in cadvisor in order to account for crio splitting the containers between a read-only and writeable layer.

This is only mergeable once the K8s KEP is approved and the CRIO changes are merged.

Pre Reqs:

Note we vendored these cadisvor changes in the k8s/k8s PR to test this idea out.

k8s-ci-robot · 2023-09-13T20:12:20Z

Hi @kannon92. Thanks for your PR.

I'm waiting for a google member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

iwankgb · 2023-10-07T17:46:35Z

/ok-to-test

haircommander · 2023-10-18T16:36:09Z

nit: I think the title could be a touch more informative

haircommander · 2023-10-18T16:39:09Z

fs/fs.go

 	if context.Crio.Root != "" {
 		crioPath := context.Crio.Root
 		crioImagePaths := map[string]struct{}{
 			"/": {},
 		}
+		for _, dir := range []string{"devicemapper", "btrfs", "aufs", "overlay", "zfs"} {


why not use StorageDriver as well instead of iterating through all of them?

Can you link or explain more on this?

the storage driver field will be one of these values, and will also be the dir that's being used. instead of iterating through the options, I think you could just use that value

Pushed a change for this.

kannon92 · 2023-10-18T21:36:22Z

nit: I think the title could be a touch more informative

updated.

kannon92 · 2023-12-15T14:06:47Z

/retest

dims

LGTM

kannon92 · 2023-12-18T22:11:27Z

@dims can you advise on how to fix this CI? It looks like I have a bunch of jobs that are stuck.

dims · 2023-12-18T22:43:32Z

@kannon92 try quashing the commits into one and pushing an update. given there is a ok-to-test label now, let's hope that succeeds.

kannon92 · 2023-12-18T22:49:21Z

@kannon92 try quashing the commits into one and pushing an update. given there is a ok-to-test label now, let's hope that succeeds.

Thank you for the suggestion. Did exactly that we will see.

dims · 2023-12-18T22:56:06Z

i see a bunch kick off after i hit Approve and Run https://github.com/google/cadvisor/pull/3395/checks

kannon92 · 2023-12-19T17:14:16Z

@dims sorry to bother you so much. Do you know who/how to get this merged?

Looks like all the jobs finished but PR requires something with write access to merge.

dims · 2023-12-19T17:51:59Z

@kannon92 we'll need a review from @bobbypage

kannon92 · 2023-12-19T21:49:05Z

/cc @bobbypage

iwankgb

Sorry for the delay

bobbypage · 2023-12-21T22:51:06Z

lgtm, but I'm curious about if we need separate changes for containerd as the changes are specific to cri-o?

kannon92 · 2023-12-22T03:19:40Z

lgtm, but I'm curious about if we need separate changes for containerd as the changes are specific to cri-o?

Im not sure of the work needed for containerd to be honest. We have discussed with containerd about this feature but im not sure if/when they may implement support for it. So it’s hard to say what work will be needed. Currently containerd will be not supported for this feature due to lack of support for this feature in general (no way to separate writeable / read only layers).

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [github.com/google/cadvisor](https://togithub.com/google/cadvisor) | `v0.48.1` -> `v0.49.1` | [![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgoogle%2fcadvisor/v0.49.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fgoogle%2fcadvisor/v0.49.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fgoogle%2fcadvisor/v0.48.1/v0.49.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgoogle%2fcadvisor/v0.48.1/v0.49.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>google/cadvisor (github.com/google/cadvisor)</summary> ### [`v0.49.1`](https://togithub.com/google/cadvisor/releases/tag/v0.49.1) [Compare Source](https://togithub.com/google/cadvisor/compare/v0.49.0...v0.49.1) #### What's Changed - Cherrpick [#3485](https://togithub.com/google/cadvisor/issues/3485) to release-v0.49- Remove s390x support by [@bobbypage](https://togithub.com/bobbypage) in [https://github.com/google/cadvisor/pull/3486](https://togithub.com/google/cadvisor/pull/3486) - build docker - add --provenance=false flag by [@bobbypage](https://togithub.com/bobbypage) in [https://github.com/google/cadvisor/pull/3488](https://togithub.com/google/cadvisor/pull/3488) **Full Changelog**: google/cadvisor@v0.49.0...v0.49.1 Multi Arch Container Image: gcr.io/cadvisor/cadvisor:v0.49.1 Architecture Specific Container Images: gcr.io/cadvisor/cadvisor-arm:v0.49.1 gcr.io/cadvisor/cadvisor-arm64:v0.49.1 gcr.io/cadvisor/cadvisor-amd64:v0.49.1 Binaries: SHA256 (./cadvisor-v0.49.1-linux-arm) = 5f4128a60c277a5f5182b22ea93c786b1016465934d908c37a4f5ce9d1dfd2b1 SHA256 (./cadvisor-v0.49.1-linux-amd64) = 1d5cc701a3fcdf1e8ed1c86da5304b896a6997d9e6673139e78a6f87812495b0 SHA256 (./cadvisor-v0.49.1-linux-arm64) = c535f46d789599f25c7c680af193d4402da27a98d9828eb2ec916af6256e0c0c ### [`v0.49.0`](https://togithub.com/google/cadvisor/releases/tag/v0.49.0) [Compare Source](https://togithub.com/google/cadvisor/compare/v0.48.1...v0.49.0) #### What's Changed - allow for cadvisor to detect disk stats when the read-only and writeable layers are on separate disks. by [@kannon92](https://togithub.com/kannon92) in [https://github.com/google/cadvisor/pull/3395](https://togithub.com/google/cadvisor/pull/3395) - build(deps): bump golang.org/x/crypto from 0.14.0 to 0.17.0 in /cmd by [@dependabot](https://togithub.com/dependabot) in [https://github.com/google/cadvisor/pull/3438](https://togithub.com/google/cadvisor/pull/3438) - upgrade actions version in github workflow by [@nnnkkk7](https://togithub.com/nnnkkk7) in [https://github.com/google/cadvisor/pull/3443](https://togithub.com/google/cadvisor/pull/3443) - Remove mentions of accelerator from the docs by [@bobrik](https://togithub.com/bobrik) in [https://github.com/google/cadvisor/pull/3458](https://togithub.com/google/cadvisor/pull/3458) - Reduce kubelet logs 'Failed to create existing container' when kubelet is using crio by [@dsxing](https://togithub.com/dsxing) in [https://github.com/google/cadvisor/pull/3457](https://togithub.com/google/cadvisor/pull/3457) - Add note about WebUI auth by [@bobbypage](https://togithub.com/bobbypage) in [https://github.com/google/cadvisor/pull/3463](https://togithub.com/google/cadvisor/pull/3463) - Remove section about canary image by [@discapes](https://togithub.com/discapes) in [https://github.com/google/cadvisor/pull/3472](https://togithub.com/google/cadvisor/pull/3472) - Bump golang to 1.22 by [@bobbypage](https://togithub.com/bobbypage) in [https://github.com/google/cadvisor/pull/3474](https://togithub.com/google/cadvisor/pull/3474) - Bump deps (runc, docker, grpc, golang/x/net) by [@bobbypage](https://togithub.com/bobbypage) in [https://github.com/google/cadvisor/pull/3477](https://togithub.com/google/cadvisor/pull/3477) - Bump to alpine 3.18 and disable libipmctl by [@bobbypage](https://togithub.com/bobbypage) in [https://github.com/google/cadvisor/pull/3483](https://togithub.com/google/cadvisor/pull/3483) #### New Contributors - [@kannon92](https://togithub.com/kannon92) made their first contribution in [https://github.com/google/cadvisor/pull/3395](https://togithub.com/google/cadvisor/pull/3395) - [@nnnkkk7](https://togithub.com/nnnkkk7) made their first contribution in [https://github.com/google/cadvisor/pull/3443](https://togithub.com/google/cadvisor/pull/3443) - [@dsxing](https://togithub.com/dsxing) made their first contribution in [https://github.com/google/cadvisor/pull/3457](https://togithub.com/google/cadvisor/pull/3457) - [@discapes](https://togithub.com/discapes) made their first contribution in [https://github.com/google/cadvisor/pull/3472](https://togithub.com/google/cadvisor/pull/3472) **Full Changelog**: google/cadvisor@v0.48.1...v0.49.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).  --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com> Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>

k8s-ci-robot added the needs-ok-to-test label Sep 13, 2023

kannon92 mentioned this pull request Oct 2, 2023

Kubelet support for Split Image Filesystem kubernetes/enhancements#4191

Open

15 tasks

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Oct 7, 2023

kannon92 mentioned this pull request Oct 18, 2023

update the crio api to reflect image store cri-o/cri-o#7401

Merged

haircommander reviewed Oct 18, 2023

View reviewed changes

kannon92 changed the title ~~add cadvisor and crio upstream changes~~ allow for cadvisor to detect disk stats when the read-only and writeable layers are on separate disks. Oct 18, 2023

kannon92 requested a review from haircommander December 14, 2023 17:48

dims approved these changes Dec 15, 2023

View reviewed changes

add cadvisor and crio upstream changes

db33cc7

kannon92 force-pushed the crio-split-disk branch from 30e7101 to db33cc7 Compare December 18, 2023 22:48

iwankgb approved these changes Dec 21, 2023

View reviewed changes

iwankgb merged commit db3c917 into google:master Dec 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow for cadvisor to detect disk stats when the read-only and writeable layers are on separate disks. #3395

allow for cadvisor to detect disk stats when the read-only and writeable layers are on separate disks. #3395

kannon92 commented Sep 13, 2023 •

edited

Loading

k8s-ci-robot commented Sep 13, 2023

iwankgb commented Oct 7, 2023

haircommander commented Oct 18, 2023

haircommander Oct 18, 2023

kannon92 Nov 1, 2023

haircommander Nov 1, 2023

kannon92 Nov 1, 2023

kannon92 Nov 1, 2023

kannon92 commented Oct 18, 2023

kannon92 commented Dec 15, 2023

dims left a comment

kannon92 commented Dec 18, 2023

dims commented Dec 18, 2023

kannon92 commented Dec 18, 2023

dims commented Dec 18, 2023

kannon92 commented Dec 19, 2023

dims commented Dec 19, 2023

kannon92 commented Dec 19, 2023

iwankgb left a comment

bobbypage commented Dec 21, 2023

kannon92 commented Dec 22, 2023

allow for cadvisor to detect disk stats when the read-only and writeable layers are on separate disks. #3395

allow for cadvisor to detect disk stats when the read-only and writeable layers are on separate disks. #3395

Conversation

kannon92 commented Sep 13, 2023 • edited Loading

k8s-ci-robot commented Sep 13, 2023

iwankgb commented Oct 7, 2023

haircommander commented Oct 18, 2023

haircommander Oct 18, 2023

Choose a reason for hiding this comment

kannon92 Nov 1, 2023

Choose a reason for hiding this comment

haircommander Nov 1, 2023

Choose a reason for hiding this comment

kannon92 Nov 1, 2023

Choose a reason for hiding this comment

kannon92 Nov 1, 2023

Choose a reason for hiding this comment

kannon92 commented Oct 18, 2023

kannon92 commented Dec 15, 2023

dims left a comment

Choose a reason for hiding this comment

kannon92 commented Dec 18, 2023

dims commented Dec 18, 2023

kannon92 commented Dec 18, 2023

dims commented Dec 18, 2023

kannon92 commented Dec 19, 2023

dims commented Dec 19, 2023

kannon92 commented Dec 19, 2023

iwankgb left a comment

Choose a reason for hiding this comment

bobbypage commented Dec 21, 2023

kannon92 commented Dec 22, 2023

kannon92 commented Sep 13, 2023 •

edited

Loading