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 cb540541f..629467d67 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 feb2386f8..a2eaa741f 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 42958f74d..e5687986a 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 cb0c70275..c4e7ca845 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 9c863920c..9b8c17fa3 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) }