util: NodeGetVolumeStatsResponse.Usage may not contain negative values

Following the CSI specification, values that are included in the
VolumeUsage MUST NOT be negative. However, CephFS seems to return -1 for
the number of inodes that are available. Instead of returning a
negative value, set it to 0 so that it will not get included in the
encoded JSON response.

Updates: #2579
See-also: 5b0d454015/spec.md (L2477-L2487)
Signed-off-by: Niels de Vos <ndevos@redhat.com>

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,
+				Available: requirePositive(available),
-				Total:     capacity,
+				Total:     requirePositive(capacity),
-				Used:      used,
+				Used:      requirePositive(used),
 				Unit:      csi.VolumeUsage_BYTES,
 			},
 			{
-				Available: inodesFree,
+				Available: requirePositive(inodesFree),
-				Total:     inodes,
+				Total:     requirePositive(inodes),
-				Used:      inodesUsed,
+				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
+}

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))
+}