address comments

Author: qiutongs

container/common/fsHandler.go

@@ -40,7 +40,7 @@ type FsUsage struct {
 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
+	// Targets returns where the fs usage metric is collected,it maybe a directory, a file or some
 	// information about the snapshotter(for containerd)
 	Targets() []string
 }
@@ -139,8 +139,9 @@ func (fh *realFsHandler) Usage() FsUsage {
 }
 
 type fsUsageProvider struct {
-	fsInfo   fs.FsInfo
-	rootFs   string
+	fsInfo fs.FsInfo
+	rootFs string
+	// The directory consumed by the container but outside rootFs, e.g. directory of saving logs
 	extraDir string
 }
 
@@ -184,7 +185,7 @@ func (f *fsUsageProvider) Usage() (*FsUsage, error) {
 
 	// Combine errors into a single error to return
 	if rootErr != nil || extraErr != nil {
-		return nil, fmt.Errorf("rootDiskErr: %v, extraDiskErr: %v", rootErr, extraErr)
+		return nil, fmt.Errorf("failed to obtain filesystem usage; rootDiskErr: %v, extraDiskErr: %v", rootErr, extraErr)
 	}
 
 	return usage, nil

container/containerd/handler.go

@@ -22,8 +22,8 @@ import (
 	"time"
 
 	"github.com/containerd/containerd/errdefs"
+	criapi "github.com/google/cadvisor/cri-api/pkg/apis/runtime/v1alpha2"
 	"golang.org/x/net/context"
-	criapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 
 	"github.com/google/cadvisor/container"
 	"github.com/google/cadvisor/container/common"
@@ -176,15 +176,17 @@ func newContainerdContainerHandler(
 	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
+			return nil, fmt.Errorf("failed to obtain containerd snapshot mounts for disk usage metrics: %v", err)
 		}
 
-		// Default to top directory if the specific upperdir snapshot is not found
+		// Default to top directory
 		snapshotDir := "/var/lib/containerd"
-		// Example: upperdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs
-		if len(mounts) > 0 {
+		// TODO: only overlay snapshotters is handled as of now.
+		// Note: overlay returns single mount. https://github.com/containerd/containerd/blob/main/snapshots/overlay/overlay.go
+		if len(mounts) > 0 && mounts[0].Type == "overlay" {
 			for _, option := range mounts[0].Options {
-				if strings.Index(option, "upperdir=") == 0 {
+				// Example: upperdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/5001/fs
+				if strings.HasPrefix(option, "upperdir=") {
 					snapshotDir = option[len("upperdir="):]
 					break
 				}