Volume QA part II: fixes and tests

cmd/nerdctl/namespace.go

@@ -113,11 +113,11 @@ func namespaceLsAction(cmd *cobra.Command, args []string) error {
 		}
 		numImages = len(images)
 
-		volStore, err := volumestore.Path(dataStore, ns)
+		volStore, err := volumestore.New(dataStore, ns)
 		if err != nil {
 			log.L.Warn(err)
 		} else {
-			volEnts, err := os.ReadDir(volStore)
+			volEnts, err := volStore.List(false)
 			if err != nil {
 				if !os.IsNotExist(err) {
 					log.L.Warn(err)

cmd/nerdctl/system_prune_linux_test.go

@@ -32,6 +32,10 @@ import (
 
 func TestSystemPrune(t *testing.T) {
 	testutil.RequiresBuild(t)
+	// FIXME: using a dedicated namespace does not work with rootful (because of buildkit running)
+	// t.Parallel()
+	// namespaceID := testutil.Identifier(t)
+	// base := testutil.NewBaseWithNamespace(t, namespaceID)
 	base := testutil.NewBase(t)
 	base.Cmd("container", "prune", "-f").AssertOK()
 	base.Cmd("network", "prune", "-f").AssertOK()

cmd/nerdctl/volume_create.go

@@ -17,6 +17,9 @@
 package main
 
 import (
+	"fmt"
+
+	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/containerd/nerdctl/v2/pkg/cmd/volume"
 
@@ -45,6 +48,12 @@ func processVolumeCreateOptions(cmd *cobra.Command) (types.VolumeCreateOptions,
 	if err != nil {
 		return types.VolumeCreateOptions{}, err
 	}
+	for _, label := range labels {
+		if label == "" {
+			return types.VolumeCreateOptions{}, fmt.Errorf("labels cannot be empty (%w)", errdefs.ErrInvalidArgument)
+		}
+	}
+
 	return types.VolumeCreateOptions{
 		GOptions: globalOptions,
 		Labels:   labels,

cmd/nerdctl/volume_create_test.go

@@ -19,40 +19,151 @@ package main
 import (
 	"testing"
 
+	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/nerdctl/v2/pkg/testutil"
-	"gotest.tools/v3/assert"
+	"gotest.tools/v3/icmd"
 )
 
-// Test TestVolumeCreate for creating volume with given name.
 func TestVolumeCreate(t *testing.T) {
-	base := testutil.NewBase(t)
-	testVolume := testutil.Identifier(t)
+	t.Parallel()
 
-	base.Cmd("volume", "create", testVolume).AssertOK()
-	defer base.Cmd("volume", "rm", "-f", testVolume).Run()
+	base := testutil.NewBase(t)
 
-	base.Cmd("volume", "list").AssertOutContains(testVolume)
-}
+	malformed := errdefs.ErrInvalidArgument.Error()
+	atMost := "at most 1 arg"
+	exitCodeVariant := 1
+	if base.Target == testutil.Docker {
+		malformed = "invalid"
+		exitCodeVariant = 125
+	}
 
-// Test TestVolumeCreateTooManyArgs for creating volume with too many args.
-func TestVolumeCreateTooManyArgs(t *testing.T) {
-	base := testutil.NewBase(t)
+	testCases := []struct {
+		description        string
+		command            func(tID string) *testutil.Cmd
+		tearUp             func(tID string)
+		tearDown           func(tID string)
+		expected           func(tID string) icmd.Expected
+		inspect            func(t *testing.T, stdout string, stderr string)
+		dockerIncompatible bool
+	}{
+		{
+			description: "arg missing should create anonymous volume",
+			command: func(tID string) *testutil.Cmd {
+				return base.Cmd("volume", "create")
+			},
+			expected: func(tID string) icmd.Expected {
+				return icmd.Expected{
+					ExitCode: 0,
+				}
+			},
+		},
+		{
+			description: "invalid identifier should fail",
+			command: func(tID string) *testutil.Cmd {
+				return base.Cmd("volume", "create", "∞")
+			},
+			expected: func(tID string) icmd.Expected {
+				return icmd.Expected{
+					ExitCode: 1,
+					Err:      malformed,
+				}
+			},
+		},
+		{
+			description: "too many args should fail",
+			command: func(tID string) *testutil.Cmd {
+				return base.Cmd("volume", "create", "too", "many")
+			},
+			expected: func(tID string) icmd.Expected {
+				return icmd.Expected{
+					ExitCode: 1,
+					Err:      atMost,
+				}
+			},
+		},
+		{
+			description: "success",
+			command: func(tID string) *testutil.Cmd {
+				return base.Cmd("volume", "create", tID)
+			},
+			tearDown: func(tID string) {
+				base.Cmd("volume", "rm", "-f", tID).Run()
+			},
+			expected: func(tID string) icmd.Expected {
+				return icmd.Expected{
+					ExitCode: 0,
+					Out:      tID,
+				}
+			},
+		},
+		{
+			description: "success with labels",
+			command: func(tID string) *testutil.Cmd {
+				return base.Cmd("volume", "create", "--label", "foo1=baz1", "--label", "foo2=baz2", tID)
+			},
+			tearDown: func(tID string) {
+				base.Cmd("volume", "rm", "-f", tID).Run()
+			},
+			expected: func(tID string) icmd.Expected {
+				return icmd.Expected{
+					ExitCode: 0,
+					Out:      tID,
+				}
+			},
+		},
+		{
+			description: "invalid labels",
+			command: func(tID string) *testutil.Cmd {
+				// See https://github.com/containerd/nerdctl/issues/3126
+				return base.Cmd("volume", "create", "--label", "a", "--label", "", tID)
+			},
+			tearDown: func(tID string) {
+				base.Cmd("volume", "rm", "-f", tID).Run()
+			},
+			expected: func(tID string) icmd.Expected {
+				return icmd.Expected{
+					ExitCode: exitCodeVariant,
+					Err:      malformed,
+				}
+			},
+		},
+		{
+			description: "creating already existing volume should succeed",
+			command: func(tID string) *testutil.Cmd {
+				base.Cmd("volume", "create", tID).AssertOK()
+				return base.Cmd("volume", "create", tID)
+			},
+			tearDown: func(tID string) {
+				base.Cmd("volume", "rm", "-f", tID).Run()
+			},
+			expected: func(tID string) icmd.Expected {
+				return icmd.Expected{
+					ExitCode: 0,
+					Out:      tID,
+				}
+			},
+		},
+	}
 
-	base.Cmd("volume", "create", "too", "many").AssertFail()
-}
+	for _, test := range testCases {
+		currentTest := test
+		t.Run(currentTest.description, func(tt *testing.T) {
+			tt.Parallel()
 
-// Test TestVolumeCreateWithLabels for creating volume with given labels.
-func TestVolumeCreateWithLabels(t *testing.T) {
-	base := testutil.NewBase(t)
-	testVolume := testutil.Identifier(t)
+			tID := testutil.Identifier(tt)
 
-	base.Cmd("volume", "create", testVolume, "--label", "foo1=baz1", "--label", "foo2=baz2").AssertOK()
-	defer base.Cmd("volume", "rm", "-f", testVolume).Run()
+			if currentTest.tearDown != nil {
+				currentTest.tearDown(tID)
+				tt.Cleanup(func() {
+					currentTest.tearDown(tID)
+				})
+			}
+			if currentTest.tearUp != nil {
+				currentTest.tearUp(tID)
+			}
 
-	inspect := base.InspectVolume(testVolume)
-	inspectNerdctlLabels := *inspect.Labels
-	expected := make(map[string]string, 2)
-	expected["foo1"] = "baz1"
-	expected["foo2"] = "baz2"
-	assert.DeepEqual(base.T, expected, inspectNerdctlLabels)
+			cmd := currentTest.command(tID)
+			cmd.Assert(currentTest.expected(tID))
+		})
+	}
 }

cmd/nerdctl/volume_inspect.go

@@ -66,7 +66,7 @@ func volumeInspectAction(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	return volume.Inspect(args, options)
+	return volume.Inspect(cmd.Context(), args, options)
 }
 
 func volumeInspectShellComplete(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {