util: make inode metrics optional in FilesystemNodeGetVolumeStats()

CephFS does not have a concept of "free inodes", inodes get allocated on-demand in the filesystem. This confuses alerting managers that expect a (high) number of free inodes, and warnings get produced if the number of free inodes is not high enough. This causes alerts to always get reported for CephFS. To prevent the false-positive alerts from happening, the NodeGetVolumeStats procedure for CephFS (and CephNFS) will not contain inodes in the reply anymore. See-also: https://bugzilla.redhat.com/2128263 Signed-off-by: Niels de Vos <ndevos@redhat.com>
ceph · Oct 13, 2022 · 60bcc6a · 60bcc6a
1 parent 386d3dd
commit 60bcc6a
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 27 deletions.
diff --git a/internal/cephfs/nodeserver.go b/internal/cephfs/nodeserver.go
@@ -637,7 +637,7 @@ func (ns *NodeServer) NodeGetVolumeStats(
 	}
 
 	if stat.Mode().IsDir() {
-		return csicommon.FilesystemNodeGetVolumeStats(ctx, ns.Mounter, targetPath)
+		return csicommon.FilesystemNodeGetVolumeStats(ctx, ns.Mounter, targetPath, false)
 	}
 
 	return nil, status.Errorf(codes.InvalidArgument, "targetpath %q is not a directory or device", targetPath)

diff --git a/internal/csi-common/utils.go b/internal/csi-common/utils.go
@@ -241,6 +241,7 @@ func FilesystemNodeGetVolumeStats(
 	ctx context.Context,
 	mounter mount.Interface,
 	targetPath string,
+	includeInodes bool,
 ) (*csi.NodeGetVolumeStatsResponse, error) {
 	isMnt, err := util.IsMountPoint(mounter, targetPath)
 	if err != nil {
@@ -274,38 +275,44 @@ func FilesystemNodeGetVolumeStats(
 	if !ok {
 		log.ErrorLog(ctx, "failed to fetch used bytes")
 	}
-	inodes, ok := (*(volMetrics.Inodes)).AsInt64()
-	if !ok {
-		log.ErrorLog(ctx, "failed to fetch available inodes")
-
-		return nil, status.Error(codes.Unknown, "failed to fetch available inodes")
-	}
-	inodesFree, ok := (*(volMetrics.InodesFree)).AsInt64()
-	if !ok {
-		log.ErrorLog(ctx, "failed to fetch free inodes")
-	}
 
-	inodesUsed, ok := (*(volMetrics.InodesUsed)).AsInt64()
-	if !ok {
-		log.ErrorLog(ctx, "failed to fetch used inodes")
-	}
-
-	return &csi.NodeGetVolumeStatsResponse{
+	res := &csi.NodeGetVolumeStatsResponse{
 		Usage: []*csi.VolumeUsage{
 			{
 				Available: requirePositive(available),
 				Total:     requirePositive(capacity),
 				Used:      requirePositive(used),
 				Unit:      csi.VolumeUsage_BYTES,
 			},
-			{
-				Available: requirePositive(inodesFree),
-				Total:     requirePositive(inodes),
-				Used:      requirePositive(inodesUsed),
-				Unit:      csi.VolumeUsage_INODES,
-			},
 		},
-	}, nil
+	}
+
+	if includeInodes {
+		inodes, ok := (*(volMetrics.Inodes)).AsInt64()
+		if !ok {
+			log.ErrorLog(ctx, "failed to fetch available inodes")
+
+			return nil, status.Error(codes.Unknown, "failed to fetch available inodes")
+		}
+		inodesFree, ok := (*(volMetrics.InodesFree)).AsInt64()
+		if !ok {
+			log.ErrorLog(ctx, "failed to fetch free inodes")
+		}
+
+		inodesUsed, ok := (*(volMetrics.InodesUsed)).AsInt64()
+		if !ok {
+			log.ErrorLog(ctx, "failed to fetch used inodes")
+		}
+
+		res.Usage = append(res.Usage, &csi.VolumeUsage{
+			Available: requirePositive(inodesFree),
+			Total:     requirePositive(inodes),
+			Used:      requirePositive(inodesUsed),
+			Unit:      csi.VolumeUsage_INODES,
+		})
+	}
+
+	return res, nil
 }
 
 // requirePositive returns the value for `x` when it is greater or equal to 0,

diff --git a/internal/csi-common/utils_test.go b/internal/csi-common/utils_test.go
@@ -88,7 +88,7 @@ func TestFilesystemNodeGetVolumeStats(t *testing.T) {
 
 	// retry until a mountpoint is found
 	for {
-		stats, err := FilesystemNodeGetVolumeStats(context.TODO(), mount.New(""), cwd)
+		stats, err := FilesystemNodeGetVolumeStats(context.TODO(), mount.New(""), cwd, true)
 		if err != nil && cwd != "/" && strings.HasSuffix(err.Error(), "is not mounted") {
 			// try again with the parent directory
 			cwd = filepath.Dir(cwd)

diff --git a/internal/nfs/nodeserver/nodeserver.go b/internal/nfs/nodeserver/nodeserver.go
@@ -182,7 +182,7 @@ func (ns *NodeServer) NodeGetVolumeStats(
 	}
 
 	if stat.Mode().IsDir() {
-		return csicommon.FilesystemNodeGetVolumeStats(ctx, ns.Mounter, targetPath)
+		return csicommon.FilesystemNodeGetVolumeStats(ctx, ns.Mounter, targetPath, false)
 	}
 
 	return nil, status.Errorf(codes.InvalidArgument,

diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go
@@ -1240,7 +1240,7 @@ func (ns *NodeServer) NodeGetVolumeStats(
 	}
 
 	if stat.Mode().IsDir() {
-		return csicommon.FilesystemNodeGetVolumeStats(ctx, ns.Mounter, targetPath)
+		return csicommon.FilesystemNodeGetVolumeStats(ctx, ns.Mounter, targetPath, true)
 	} else if (stat.Mode() & os.ModeDevice) == os.ModeDevice {
 		return blockNodeGetVolumeStats(ctx, targetPath)
 	}