From b7703faf37f5905f8e1c83f0c15a01df6cdbb181 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Mon, 3 Oct 2022 17:46:52 +0200 Subject: [PATCH] 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 --- internal/cephfs/nodeserver.go | 2 +- internal/csi-common/utils.go | 53 +++++++++++++++------------ internal/csi-common/utils_test.go | 2 +- internal/nfs/nodeserver/nodeserver.go | 2 +- internal/rbd/nodeserver.go | 2 +- 5 files changed, 34 insertions(+), 27 deletions(-) diff --git a/internal/cephfs/nodeserver.go b/internal/cephfs/nodeserver.go index cb540541f62..629467d674e 100644 --- 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 index feb2386f8f2..a2eaa741f8c 100644 --- 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,23 +275,8 @@ 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), @@ -298,14 +284,35 @@ func FilesystemNodeGetVolumeStats( 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 index 42958f74d18..e5687986aba 100644 --- 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 index cb0c70275c9..c4e7ca84565 100644 --- 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 index 9c863920c32..9b8c17fa3db 100644 --- 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) }