Add missing file system metrics in containerd handler #2936

qiutongs · 2021-09-08T21:09:46Z

Signed-off-by: Qiutong Song songqt01@gmail.com

Overview

File system metrics are not supported in cadvisor-containerd. This PR attempts to fix it.

The majority of code is copied from #2872

Fixed:

fs_inodes_free
fs_inodes_total
fs_usage_bytes
fs_limit_bytes

Testing

$ make test

Query prometheus endpoint

$ curl localhost:8080/metrics 2>/dev/null | grep fs_usage_bytes | grep nginx
container_fs_usage_bytes{container_label_io_cri_containerd_kind="container",container_label_io_kubernetes_container_name="nginx-pod",container_label_io_kubernetes_pod_name="nginx-pod",container_label_io_kubernetes_pod_namespace="default",container_label_io_kubernetes_pod_uid="9de12299-f7c6-4404-8483-b40636023b5e",container_label_k8s_app="",container_label_pod_template_hash="",container_label_run="",device="",id="/kubepods/besteffort/pod9de12299-f7c6-4404-8483-b40636023b5e/7c6e000abee310fc05bab29ef9ef58cf27a51e8652197dffcfa8c84656270a38",image="docker.io/library/nginx:latest",name="k8s_nginx-pod_nginx-pod_default_9de12299-f7c6-4404-8483-b40636023b5e_0"} 94208 1631309422620

Find the upperdir(writable layer)

$ mount | grep overlay | grep 7c6e000abee310fc05bab29ef9ef58cf27a51e8652197dffcfa8c84656270a38
overlay on /run/containerd/io.containerd.runtime.v2.task/k8s.io/7c6e000abee310fc05bab29ef9ef58cf27a51e8652197dffcfa8c84656270a38/rootfs type overlay (rw,relatime,lowerdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4802/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4801/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4800/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4799/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4797/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4451/fs,upperdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs,workdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/work)

$ sudo du -hs /var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs
92K     /var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs

94208 and 92k are close.

After counting the size of logs, the reported number of fs_usage_bytes is 98304. The log is roughly 4KB.

$ du -sh /var/log/pods/default_nginx-pod_9de12299-f7c6-4404-8483-b40636023b5e/nginx-pod/0.log 
4.0K    /var/log/pods/default_nginx-pod_9de12299-f7c6-4404-8483-b40636023b5e/nginx-pod/0.log

fs_limit_bytes is reported

container_fs_limit_bytes{container_label_io_cri_containerd_kind="container",container_label_io_kubernetes_container_name="nginx-pod",container_label_io_kubernetes_pod_name="nginx-pod",container_label_io_kubernetes_pod_namespace="default",container_label_io_kubernetes_pod_uid="9de12299-f7c6-4404-8483-b40636023b5e",container_label_k8s_app="",container_label_pod_template_hash="",container_label_run="",device="/dev/sda5",id="/kubepods/besteffort/pod9de12299-f7c6-4404-8483-b40636023b5e/7c6e000abee310fc05bab29ef9ef58cf27a51e8652197dffcfa8c84656270a38",image="docker.io/library/nginx:latest",name="k8s_nginx-pod_nginx-pod_default_9de12299-f7c6-4404-8483-b40636023b5e_0"} 4.1610903552e+10 1631657309819

(dlv) p snapshotDir
"/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs"
(dlv) p deviceInfo
*github.com/google/cadvisor/fs.DeviceInfo {
        Device: "/dev/sda5",
        Major: 8,
        Minor: 5,}

Open Questions

~~1. Should we count the log file as #2872 (comment)? e.g. /var/log/pods/default_nginx-pod_9de12299-f7c6-4404-8483-b40636023b5e/nginx-pod/0.log~~
2. Can we manually construct the rootfs path so that we can get the device name - /run/containerd/io.containerd.runtime.v2.task/k8s.io/7c6e000abee310fc05bab29ef9ef58cf27a51e8652197dffcfa8c84656270a38/rootfs? With the device name, we can calculate fs_limit_bytes.

Appendix

(dlv) p mounts
[]*github.com/containerd/containerd/api/types.Mount len: 1, cap: 1, [
        *{
                Type: "overlay",
                Source: "overlay",
                Target: "",
                Options: []string len: 4, cap: 4, [
                        "index=off",
                        "workdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/work",
                        "upperdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs",
                        "lowerdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4802/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4801/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4800/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4799/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4797/fs:/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/4451/fs",
                ],
                XXX_NoUnkeyedLiteral: struct {} {},
                XXX_unrecognized: []uint8 len: 0, cap: 0, nil,
                XXX_sizecache: 0,},
]

k8s-ci-robot · 2021-09-08T21:09:54Z

Hi @qiutongs. 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.

qiutongs · 2021-09-08T21:13:15Z

/assign yyrdl

bobbypage · 2021-09-09T21:39:43Z

/ok-to-test

bobbypage · 2021-09-16T00:41:24Z

container/containerd/handler.go

+		snapshotDir := "/var/lib/containerd"
+		// Example: upperdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs
+		if len(mounts) > 0 {
+			for _, option := range mounts[0].Options {


maybe check that types.Mount.Type == overlay or overlay2? Since only overlay will have upper/lowerdir?

Let me think about it.

That is a good call. Let me update.

bobbypage · 2021-09-16T00:42:20Z

container/containerd/handler.go

+		// Example: upperdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs
+		if len(mounts) > 0 {
+			for _, option := range mounts[0].Options {
+				if strings.Index(option, "upperdir=") == 0 {


maybe prefer strings.HasPrefix(option, "upperdir=")?

Oh right. I will change.

bobbypage · 2021-09-16T00:43:24Z

container/common/fsHandler.go

+type FsUsageProvider interface {
+	// Usage returns the fs usage
+	Usage() (*FsUsage, error)
+	// Targets returns where the fs usage metric is collected,it maybe a directory ,a file or some


super nit: space after comma and ,a

bobbypage · 2021-09-16T00:44:33Z

container/common/fsHandler.go

@@ -125,7 +113,8 @@ func (fh *realFsHandler) trackUsage() {
 			// if the long duration is persistent either because of slow
 			// disk or lots of containers.
 			longOp = longOp + time.Second
-			klog.V(2).Infof("fs: disk usage and inodes count on following dirs took %v: %v; will not log again for this container unless duration exceeds %v", duration, []string{fh.rootfs, fh.extraDir}, longOp)
+			klog.V(2).Infof(`fs: disk usage and inodes count on targets took %v: %v; `+


If this is vendored in kubelet V(2) logging verbosity is enabled by default. If this is can be noisy, consider dropping to (V3) or higher

Hmm, maybe this log is useful. After all, it was V(2) before.

container/common/fsHandler.go

bobbypage · 2021-09-16T00:47:34Z

container/common/fsHandler.go

+
+	// Combine errors into a single error to return
+	if rootErr != nil || extraErr != nil {
+		return nil, fmt.Errorf("rootDiskErr: %v, extraDiskErr: %v", rootErr, extraErr)


maybe add to error a little more context if this error get's surfaced in some log somewhere:

fmt.Errorf("failed to obtain filesystem usage; rootDiskErr: %v, extraDiskErr: %v", rootErr, extraErr)

bobbypage · 2021-09-16T00:49:56Z

container/containerd/handler.go

+	if includedMetrics.Has(container.DiskUsageMetrics) && cntr.Labels["io.cri-containerd.kind"] != "sandbox" {
+		mounts, err := client.SnapshotMounts(ctx, cntr.Snapshotter, cntr.SnapshotKey)
+		if err != nil {
+			return nil, err


add a little context here maybe?

return nil, fmt.Errorf("failed to obtain containerd snapshot mounts for disk usage metrics: %v", err)

bobbypage · 2021-09-16T00:50:55Z

container/containerd/handler.go

+		// Default to top directory if the specific upperdir snapshot is not found
+		snapshotDir := "/var/lib/containerd"
+		// Example: upperdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs
+		if len(mounts) > 0 {


the behavior here is to get the first mount and then break.

Is this intentional? Can we have multiple mounts? Should we sum them or is the top upper dir enough?

Yes, after checking containerd code, it only returns one mount for overlay.

bobbypage · 2021-09-16T00:54:29Z

container/containerd/handler.go

+			client:      client,
+			containerID: id,
+			// Path of logs, e.g. /var/log/pods/XXX
+			logPath: status.LogPath,


is this path to actual log file, or directory?

crio-o handler makes it seem like path returned there is actual log file, xref https://github.com/google/cadvisor/blob/master/container/crio/handler.go#L121-L128

It is the actual log file.

Signed-off-by: Qiutong Song <songqt01@gmail.com>

bobbypage · 2021-09-22T22:57:09Z

LGTM, thanks

sepich · 2021-11-17T14:12:19Z

Unfortunately this only accounts for

writable layer
log size

But does not count emptyDir size (/var/lib/kubelet/pods/<id>/volumes/kubernetes.io~empty-dir/).
Kublet on the other side does ephemeral-storage accounting for all 3.
https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#local-ephemeral-storage

google-cla bot added the cla: yes label Sep 8, 2021

k8s-ci-robot added the needs-ok-to-test label Sep 8, 2021

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Sep 9, 2021

qiutongs force-pushed the containerd-fix branch 2 times, most recently from 19c267b to 28c1a9d Compare September 14, 2021 22:39

bobbypage reviewed Sep 16, 2021

View reviewed changes

pingleig mentioned this pull request Sep 20, 2021

[k8s][containerd] container file system metrics is not supported by cadvisor for contianerd aws/amazon-cloudwatch-agent#192

Open

qiutongs added 5 commits September 22, 2021 14:32

Add missing file system metrics in containerd handler

8158195

Signed-off-by: Qiutong Song <songqt01@gmail.com>

fix lint errors

c1a5281

ignore sandbox container when getting file system metrics

3b22076

fix fs_limit, fs_inode_total, fs_inode_free in containerd handler

aaa0aea

Count the size of logs towards TotalUsageBytes

9811dc8

qiutongs force-pushed the containerd-fix branch from cd13162 to 6012838 Compare September 22, 2021 21:37

address comments

050a3bd

qiutongs force-pushed the containerd-fix branch from 6012838 to 050a3bd Compare September 22, 2021 21:41

bobbypage merged commit dcd18fa into google:containerd-cri Sep 22, 2021

qiutongs deleted the containerd-fix branch September 24, 2021 20:52

This was referenced Oct 5, 2021

Fix prometheus operator support lensapp/lens#3653

Merged

File system error:Metrics not available at the moment lensapp/lens#3372

Open

Add more file system metrics to the pod and container graphs lensapp/lens#3961

Merged

bobbypage mentioned this pull request Oct 11, 2021

Integrate CRI and containerd file system stats #2956

Merged

shannonxtreme mentioned this pull request Mar 8, 2022

Migrating off dockershim: document known issue with metrics kubernetes/website#30681

Closed

snuggie12 mentioned this pull request May 7, 2022

Disk usage metrics for containerd #2785

Open

rohitsakala mentioned this pull request Jul 20, 2022

Some metrics do not work for internal containerd rancher/rke2#1167

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing file system metrics in containerd handler #2936

Add missing file system metrics in containerd handler #2936

qiutongs commented Sep 8, 2021 •

edited

Loading

k8s-ci-robot commented Sep 8, 2021

qiutongs commented Sep 8, 2021

bobbypage commented Sep 9, 2021

bobbypage Sep 16, 2021

qiutongs Sep 16, 2021

qiutongs Sep 17, 2021

bobbypage Sep 16, 2021

qiutongs Sep 16, 2021

bobbypage Sep 16, 2021

bobbypage Sep 16, 2021

qiutongs Sep 16, 2021

bobbypage Sep 16, 2021

bobbypage Sep 16, 2021

bobbypage Sep 16, 2021

qiutongs Sep 17, 2021

bobbypage Sep 16, 2021

qiutongs Sep 16, 2021

bobbypage commented Sep 22, 2021

sepich commented Nov 17, 2021

Add missing file system metrics in containerd handler #2936

Add missing file system metrics in containerd handler #2936

Conversation

qiutongs commented Sep 8, 2021 • edited Loading

Overview

Testing

Open Questions

Appendix

k8s-ci-robot commented Sep 8, 2021

qiutongs commented Sep 8, 2021

bobbypage commented Sep 9, 2021

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

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

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

bobbypage commented Sep 22, 2021

sepich commented Nov 17, 2021

qiutongs commented Sep 8, 2021 •

edited

Loading