Skip to content

Commit

Permalink
util: make inode metrics optional in FilesystemNodeGetVolumeStats()
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
nixpanic authored and mergify[bot] committed Oct 13, 2022
1 parent 386d3dd commit b7703fa
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 27 deletions.
2 changes: 1 addition & 1 deletion internal/cephfs/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
53 changes: 30 additions & 23 deletions internal/csi-common/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion internal/csi-common/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/nfs/nodeserver/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion internal/rbd/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit b7703fa

Please sign in to comment.