diff --git a/internal/csi-common/utils.go b/internal/csi-common/utils.go index 020c67306..c1c4279f1 100644 --- a/internal/csi-common/utils.go +++ b/internal/csi-common/utils.go @@ -263,17 +263,30 @@ func FilesystemNodeGetVolumeStats(ctx context.Context, targetPath string) (*csi. return &csi.NodeGetVolumeStatsResponse{ Usage: []*csi.VolumeUsage{ { - Available: available, - Total: capacity, - Used: used, + Available: requirePositive(available), + Total: requirePositive(capacity), + Used: requirePositive(used), Unit: csi.VolumeUsage_BYTES, }, { - Available: inodesFree, - Total: inodes, - Used: inodesUsed, + Available: requirePositive(inodesFree), + Total: requirePositive(inodes), + Used: requirePositive(inodesUsed), Unit: csi.VolumeUsage_INODES, }, }, }, nil } + +// requirePositive returns the value for `x` when it is greater or equal to 0, +// or returns 0 in the acse `x` is negative. +// +// This is used for VolumeUsage entries in the NodeGetVolumeStatsResponse. The +// CSI spec does not allow negative values in the VolumeUsage objects. +func requirePositive(x int64) int64 { + if x >= 0 { + return x + } + + return 0 +} diff --git a/internal/csi-common/utils_test.go b/internal/csi-common/utils_test.go index e0761fb9f..ca5b844e2 100644 --- a/internal/csi-common/utils_test.go +++ b/internal/csi-common/utils_test.go @@ -17,9 +17,15 @@ limitations under the License. package csicommon import ( + "context" + "os" + "path/filepath" + "strings" "testing" "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var fakeID = "fake-id" @@ -70,3 +76,43 @@ func TestGetReqID(t *testing.T) { t.Errorf("getReqID() = %v, want empty string", got) } } + +func TestFilesystemNodeGetVolumeStats(t *testing.T) { + t.Parallel() + + // ideally this is tested on different filesystems, including CephFS, + // but we'll settle for the filesystem where the project is checked out + cwd, err := os.Getwd() + require.NoError(t, err) + + // retry until a mountpoint is found + for { + stats, err := FilesystemNodeGetVolumeStats(context.TODO(), cwd) + if err != nil && cwd != "/" && strings.HasSuffix(err.Error(), "is not mounted") { + // try again with the parent directory + cwd = filepath.Dir(cwd) + + continue + } + + require.NoError(t, err) + assert.NotEqual(t, len(stats.Usage), 0) + for _, usage := range stats.Usage { + assert.NotEqual(t, usage.Available, -1) + assert.NotEqual(t, usage.Total, -1) + assert.NotEqual(t, usage.Used, -1) + assert.NotEqual(t, usage.Unit, 0) + } + + // tests done, no need to retry again + break + } +} + +func TestRequirePositive(t *testing.T) { + t.Parallel() + + assert.Equal(t, requirePositive(0), int64(0)) + assert.Equal(t, requirePositive(-1), int64(0)) + assert.Equal(t, requirePositive(1), int64(1)) +}