From 7f23e6ed7ed7771c6f9e504dd4e8d5a7c68232de Mon Sep 17 00:00:00 2001 From: apostasie Date: Thu, 20 Jun 2024 16:37:58 -0700 Subject: [PATCH] Volume QA: inspect, prune, create (fixes and tests) Signed-off-by: apostasie --- cmd/nerdctl/system_prune_linux_test.go | 5 +- cmd/nerdctl/volume_create.go | 9 + cmd/nerdctl/volume_create_test.go | 142 ++++++++++--- cmd/nerdctl/volume_inspect.go | 2 +- cmd/nerdctl/volume_inspect_test.go | 256 ++++++++++++++++++++--- cmd/nerdctl/volume_list_test.go | 12 +- cmd/nerdctl/volume_namespace_test.go | 139 ++++++++++++ cmd/nerdctl/volume_prune_linux_test.go | 119 +++++++++-- cmd/nerdctl/volume_remove_linux_test.go | 19 +- pkg/cmd/volume/create.go | 4 - pkg/cmd/volume/inspect.go | 29 ++- pkg/cmd/volume/prune.go | 10 +- pkg/cmd/volume/rm.go | 3 +- pkg/mountutil/mountutil.go | 15 +- pkg/mountutil/volumestore/volumestore.go | 68 +++--- 15 files changed, 692 insertions(+), 140 deletions(-) create mode 100644 cmd/nerdctl/volume_namespace_test.go diff --git a/cmd/nerdctl/system_prune_linux_test.go b/cmd/nerdctl/system_prune_linux_test.go index c1e91658f31..4c01bb59bff 100644 --- a/cmd/nerdctl/system_prune_linux_test.go +++ b/cmd/nerdctl/system_prune_linux_test.go @@ -31,8 +31,11 @@ import ( ) func TestSystemPrune(t *testing.T) { + t.Parallel() + testutil.RequiresBuild(t) - base := testutil.NewBase(t) + namespaceID := testutil.Identifier(t) + base := testutil.NewBaseWithNamespace(t, namespaceID) base.Cmd("container", "prune", "-f").AssertOK() base.Cmd("network", "prune", "-f").AssertOK() base.Cmd("volume", "prune", "-f").AssertOK() diff --git a/cmd/nerdctl/volume_create.go b/cmd/nerdctl/volume_create.go index 972d5fc0359..9c8bfda26f1 100644 --- a/cmd/nerdctl/volume_create.go +++ b/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, diff --git a/cmd/nerdctl/volume_create_test.go b/cmd/nerdctl/volume_create_test.go index f8f5997b501..40643ef723c 100644 --- a/cmd/nerdctl/volume_create_test.go +++ b/cmd/nerdctl/volume_create_test.go @@ -19,40 +19,132 @@ 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() + 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 + }{ + { + description: "arg missing", + 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", + 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", + 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: "at most 1 arg", + } + }, + }, + { + 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, + } + }, + }, + } - 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)) + }) + } } diff --git a/cmd/nerdctl/volume_inspect.go b/cmd/nerdctl/volume_inspect.go index 0eee9c41231..302b047f734 100644 --- a/cmd/nerdctl/volume_inspect.go +++ b/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) { diff --git a/cmd/nerdctl/volume_inspect_test.go b/cmd/nerdctl/volume_inspect_test.go index 24fff144a77..26f3d52d75e 100644 --- a/cmd/nerdctl/volume_inspect_test.go +++ b/cmd/nerdctl/volume_inspect_test.go @@ -18,48 +18,246 @@ package main import ( "crypto/rand" + "encoding/json" + "fmt" "os" "path/filepath" "testing" + "github.com/containerd/containerd/errdefs" + "github.com/containerd/nerdctl/v2/pkg/inspecttypes/native" "github.com/containerd/nerdctl/v2/pkg/testutil" "gotest.tools/v3/assert" + "gotest.tools/v3/icmd" ) -func TestVolumeInspectContainsLabels(t *testing.T) { - t.Parallel() - testVolume := testutil.Identifier(t) - - base := testutil.NewBase(t) - base.Cmd("volume", "create", "--label", "tag=testVolume", testVolume).AssertOK() - defer base.Cmd("volume", "rm", "-f", testVolume).Run() - - inspect := base.InspectVolume(testVolume) - inspectNerdctlLabels := (*inspect.Labels) - expected := make(map[string]string, 1) - expected["tag"] = "testVolume" - assert.DeepEqual(base.T, expected, inspectNerdctlLabels) +func createFileWithSize(base *testutil.Base, vol string, size int64) { + v := base.InspectVolume(vol) + token := make([]byte, size) + _, _ = rand.Read(token) + err := os.WriteFile(filepath.Join(v.Mountpoint, "test-file"), token, 0644) + assert.NilError(base.T, err) } -func TestVolumeInspectSize(t *testing.T) { - testutil.DockerIncompatible(t) +func TestVolumeInspect(t *testing.T) { t.Parallel() - testVolume := testutil.Identifier(t) + base := testutil.NewBase(t) - base.Cmd("volume", "create", testVolume).AssertOK() - defer base.Cmd("volume", "rm", "-f", testVolume).Run() + tID := testutil.Identifier(t) var size int64 = 1028 - createFileWithSize(t, testVolume, size) - volumeWithSize := base.InspectVolume(testVolume, []string{"--size"}...) - assert.Equal(t, volumeWithSize.Size, size) -} -func createFileWithSize(t *testing.T, volume string, bytes int64) { - base := testutil.NewBase(t) - v := base.InspectVolume(volume) - token := make([]byte, bytes) - rand.Read(token) - err := os.WriteFile(filepath.Join(v.Mountpoint, "test-file"), token, 0644) - assert.NilError(t, err) + malformed := errdefs.ErrInvalidArgument.Error() + notFound := errdefs.ErrNotFound.Error() + requireArg := "requires at least 1 arg" + if base.Target == testutil.Docker { + malformed = "no such volume" + notFound = "no such volume" + } + + tearUp := func(t *testing.T) { + base.Cmd("volume", "create", tID).AssertOK() + base.Cmd("volume", "create", "--label", "foo=fooval", "--label", "bar=barval", tID+"-second").AssertOK() + + // Obviously note here that if inspect code gets totally hosed, this entire suite will + // probably fail right here on the tearUp instead of actually testing something + createFileWithSize(base, tID, size) + } + + tearDown := func(t *testing.T) { + base.Cmd("volume", "rm", "-f", tID).Run() + base.Cmd("volume", "rm", "-f", tID+"-second").Run() + } + + tearDown(t) + t.Cleanup(func() { + tearDown(t) + }) + tearUp(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) + dockerIncompatible bool + }{ + { + description: "arg missing", + command: func(tID string) *testutil.Cmd { + return base.Cmd("volume", "inspect") + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 1, + Err: requireArg, + } + }, + }, + { + description: "invalid identifier", + command: func(tID string) *testutil.Cmd { + return base.Cmd("volume", "inspect", "∞") + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 1, + Err: malformed, + } + }, + }, + { + description: "non existent volume", + command: func(tID string) *testutil.Cmd { + return base.Cmd("volume", "inspect", "doesnotexist") + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 1, + Err: notFound, + } + }, + }, + { + description: "success", + command: func(tID string) *testutil.Cmd { + return base.Cmd("volume", "inspect", tID) + }, + tearDown: func(tID string) { + base.Cmd("volume", "rm", "-f", tID) + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 0, + Out: tID, + } + }, + inspect: func(t *testing.T, stdout string) { + var dc []native.Volume + if err := json.Unmarshal([]byte(stdout), &dc); err != nil { + t.Fatal(err) + } + assert.Assert(t, len(dc) == 1, fmt.Sprintf("one result, not %d", len(dc))) + assert.Assert(t, dc[0].Name == tID, fmt.Sprintf("expected name to be %q (was %q)", tID, dc[0].Name)) + assert.Assert(t, dc[0].Labels == nil, fmt.Sprintf("expected labels to be nil and were %v", dc[0].Labels)) + }, + }, + { + description: "multi success", + command: func(tID string) *testutil.Cmd { + return base.Cmd("volume", "inspect", tID, tID+"-second") + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 0, + } + }, + inspect: func(t *testing.T, stdout string) { + var dc []native.Volume + if err := json.Unmarshal([]byte(stdout), &dc); err != nil { + t.Fatal(err) + } + assert.Assert(t, len(dc) == 2, fmt.Sprintf("two results, not %d", len(dc))) + assert.Assert(t, dc[0].Name == tID, fmt.Sprintf("expected name to be %q (was %q)", tID, dc[0].Name)) + assert.Assert(t, dc[1].Name == tID+"-second", fmt.Sprintf("expected name to be %q (was %q)", tID+"-second", dc[1].Name)) + }, + }, + { + description: "inspect labels", + command: func(tID string) *testutil.Cmd { + return base.Cmd("volume", "inspect", tID+"-second") + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 0, + Out: tID, + } + }, + inspect: func(t *testing.T, stdout string) { + var dc []native.Volume + if err := json.Unmarshal([]byte(stdout), &dc); err != nil { + t.Fatal(err) + } + + labels := *dc[0].Labels + assert.Assert(t, len(labels) == 2, fmt.Sprintf("two results, not %d", len(labels))) + assert.Assert(t, labels["foo"] == "fooval", fmt.Sprintf("label foo should be fooval, not %s", labels["foo"])) + assert.Assert(t, labels["bar"] == "barval", fmt.Sprintf("label bar should be barval, not %s", labels["bar"])) + }, + }, + { + description: "inspect size", + command: func(tID string) *testutil.Cmd { + return base.Cmd("volume", "inspect", "--size", tID) + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 0, + Out: tID, + } + }, + inspect: func(t *testing.T, stdout string) { + var dc []native.Volume + if err := json.Unmarshal([]byte(stdout), &dc); err != nil { + t.Fatal(err) + } + assert.Assert(t, dc[0].Size == size, fmt.Sprintf("expected size to be %d (was %d)", size, dc[0].Size)) + }, + dockerIncompatible: true, + }, + { + description: "part success multi", + command: func(tID string) *testutil.Cmd { + return base.Cmd("volume", "inspect", "invalid∞", "nonexistent", tID) + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 1, + Out: tID, + Err: notFound, + } + }, + inspect: func(t *testing.T, stdout string) { + var dc []native.Volume + if err := json.Unmarshal([]byte(stdout), &dc); err != nil { + t.Fatal(err) + } + assert.Assert(t, len(dc) == 1, fmt.Sprintf("one result, not %d", len(dc))) + assert.Assert(t, dc[0].Name == tID, fmt.Sprintf("expected name to be %q (was %q)", tID, dc[0].Name)) + }, + }, + } + + for _, test := range testCases { + currentTest := test + t.Run(currentTest.description, func(tt *testing.T) { + if currentTest.dockerIncompatible { + testutil.DockerIncompatible(tt) + } + + tt.Parallel() + + // We use the main test tID here + if currentTest.tearDown != nil { + currentTest.tearDown(tID) + tt.Cleanup(func() { + currentTest.tearDown(tID) + }) + } + if currentTest.tearUp != nil { + currentTest.tearUp(tID) + } + + // See https://github.com/containerd/nerdctl/issues/3130 + // We run first to capture the underlying icmd command and output + cmd := currentTest.command(tID) + res := cmd.Run() + cmd.Assert(currentTest.expected(tID)) + if currentTest.inspect != nil { + currentTest.inspect(tt, res.Stdout()) + } + }) + } } diff --git a/cmd/nerdctl/volume_list_test.go b/cmd/nerdctl/volume_list_test.go index 9b88e13eac9..94a8522407a 100644 --- a/cmd/nerdctl/volume_list_test.go +++ b/cmd/nerdctl/volume_list_test.go @@ -42,8 +42,8 @@ func TestVolumeLs(t *testing.T) { base.Cmd("volume", "create", vol3).AssertOK() defer base.Cmd("volume", "rm", "-f", vol3).Run() - createFileWithSize(t, vol1, 102400) - createFileWithSize(t, vol2, 204800) + createFileWithSize(base, vol1, 102400) + createFileWithSize(base, vol2, 204800) base.Cmd("volume", "ls", "--size").AssertOutWithFunc(func(stdout string) error { var lines = strings.Split(strings.TrimSpace(stdout), "\n") @@ -279,10 +279,10 @@ func TestVolumeLsFilterSize(t *testing.T) { base.Cmd("volume", "create", vol4).AssertOK() defer base.Cmd("volume", "rm", "-f", vol4).Run() - createFileWithSize(t, vol1, 409600) - createFileWithSize(t, vol2, 1024000) - createFileWithSize(t, vol3, 409600) - createFileWithSize(t, vol4, 1024000) + createFileWithSize(base, vol1, 409600) + createFileWithSize(base, vol2, 1024000) + createFileWithSize(base, vol3, 409600) + createFileWithSize(base, vol4, 1024000) base.Cmd("volume", "ls", "--size", "--filter", "size=1024000").AssertOutWithFunc(func(stdout string) error { var lines = strings.Split(strings.TrimSpace(stdout), "\n") diff --git a/cmd/nerdctl/volume_namespace_test.go b/cmd/nerdctl/volume_namespace_test.go new file mode 100644 index 00000000000..71a94ed590f --- /dev/null +++ b/cmd/nerdctl/volume_namespace_test.go @@ -0,0 +1,139 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package main + +import ( + "fmt" + "testing" + + "github.com/containerd/containerd/errdefs" + "github.com/containerd/nerdctl/v2/pkg/testutil" + "gotest.tools/v3/icmd" +) + +func TestVolumeNamespace(t *testing.T) { + testutil.DockerIncompatible(t) + + t.Parallel() + + base := testutil.NewBase(t) + tID := testutil.Identifier(t) + otherBase := testutil.NewBaseWithNamespace(t, tID+"-1") + thirdBase := testutil.NewBaseWithNamespace(t, tID+"-2") + + tearUp := func(t *testing.T) { + base.Cmd("volume", "create", tID).AssertOK() + } + + tearDown := func(t *testing.T) { + base.Cmd("volume", "rm", "-f", tID).Run() + otherBase.Cmd("namespace", "rm", "-f", tID+"-1").Run() + thirdBase.Cmd("namespace", "rm", "-f", tID+"-2").Run() + } + + tearDown(t) + t.Cleanup(func() { + tearDown(t) + }) + tearUp(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 + }{ + { + description: "inspect", + command: func(tID string) *testutil.Cmd { + return otherBase.Cmd("volume", "inspect", tID) + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 1, + Err: fmt.Errorf("%s (%w)", tID, errdefs.ErrNotFound).Error(), + } + }, + }, + { + description: "remove", + command: func(tID string) *testutil.Cmd { + return otherBase.Cmd("volume", "remove", tID) + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 1, + Err: fmt.Errorf("%s (%w)", tID, errdefs.ErrNotFound).Error(), + } + }, + }, + { + description: "prune", + command: func(tID string) *testutil.Cmd { + return otherBase.Cmd("volume", "prune", "-a", "-f") + }, + tearDown: func(tID string) { + // Assert that the volume is here in the base namespace + // both before and after the prune command + base.Cmd("volume", "inspect", tID).AssertOK() + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 0, + } + }, + }, + { + description: "create", + command: func(tID string) *testutil.Cmd { + return thirdBase.Cmd("volume", "create", tID) + }, + tearDown: func(tID string) { + thirdBase.Cmd("volume", "remove", "-f", tID).Run() + }, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 0, + Out: tID, + } + }, + }, + } + + for _, test := range testCases { + currentTest := test + t.Run(currentTest.description, func(tt *testing.T) { + tt.Parallel() + + // Note that here we are using the main test tID + // since we are testing against the volume created in it + if currentTest.tearDown != nil { + currentTest.tearDown(tID) + tt.Cleanup(func() { + currentTest.tearDown(tID) + }) + } + if currentTest.tearUp != nil { + currentTest.tearUp(tID) + } + + cmd := currentTest.command(tID) + cmd.Assert(currentTest.expected(tID)) + }) + } +} diff --git a/cmd/nerdctl/volume_prune_linux_test.go b/cmd/nerdctl/volume_prune_linux_test.go index df610ca1a04..da6f833ac7a 100644 --- a/cmd/nerdctl/volume_prune_linux_test.go +++ b/cmd/nerdctl/volume_prune_linux_test.go @@ -17,30 +17,117 @@ package main import ( - "fmt" + "strings" "testing" + "gotest.tools/v3/assert" + "gotest.tools/v3/icmd" + "github.com/containerd/nerdctl/v2/pkg/testutil" ) +// Volume pruning cannot be parallelized for Docker, since we need namespaces to do that in a way that does interact with other tests func TestVolumePrune(t *testing.T) { - base := testutil.NewBase(t) - tID := testutil.Identifier(t) - base.Cmd("volume", "prune", "-a", "-f").Run() + if testutil.GetTarget() != testutil.Docker { + t.Parallel() + } + + subTearUp := func(tID string) { + base := testutil.NewBaseWithNamespace(t, tID) + res := base.Cmd("volume", "create").Run() + anonIDBusy := res.Stdout() + base.Cmd("volume", "create").Run() + base.Cmd("volume", "create", tID+"-busy").AssertOK() + base.Cmd("volume", "create", tID+"-free").AssertOK() + + base.Cmd("run", "--name", tID, "-v", tID+"-busy:/whatever", "-v", anonIDBusy, testutil.CommonImage).AssertOK() + } + + subTearDown := func(tID string) { + base := testutil.NewBaseWithNamespace(t, tID) + base.Cmd("rm", "-f", tID).Run() + //base.Cmd("volume", "rm", "-f", tID+"-busy").Run() + // base.Cmd("volume", "rm", "-f", tID+"-free").Run() + // base.Cmd("volume", "rm", "-f", anonIDFree).Run() + // base.Cmd("volume", "rm", "-f", anonIDBusy).Run() + base.Cmd("namespace", "remove", "-f", tID).Run() + } + + 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(tID string, stdout string) + }{ + { + description: "prune ano", + command: func(tID string) *testutil.Cmd { + base := testutil.NewBaseWithNamespace(t, tID) + return base.Cmd("volume", "prune", "-f") + }, + tearUp: subTearUp, + tearDown: subTearDown, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 0, + } + }, + inspect: func(tID string, stdout string) { + base := testutil.NewBaseWithNamespace(t, tID) + assert.Assert(base.T, !strings.Contains(stdout, tID+"-free")) + base.Cmd("inspect", tID+"-free").AssertOK() + assert.Assert(base.T, !strings.Contains(stdout, tID+"-busy")) + base.Cmd("inspect", tID+"-busy").AssertOK() + // TODO verify the anonymous volumes status + }, + }, + { + description: "prune all", + command: func(tID string) *testutil.Cmd { + base := testutil.NewBaseWithNamespace(t, tID) + return base.Cmd("volume", "prune", "-f", "--all") + }, + tearUp: subTearUp, + tearDown: subTearDown, + expected: func(tID string) icmd.Expected { + return icmd.Expected{ + ExitCode: 0, + } + }, + inspect: func(tID string, stdout string) { + base := testutil.NewBaseWithNamespace(t, tID) + assert.Assert(t, !strings.Contains(stdout, tID+"-busy")) + base.Cmd("inspect", tID+"-busy").AssertOK() + assert.Assert(t, strings.Contains(stdout, tID+"-free")) + base.Cmd("inspect", tID+"-free").AssertFail() + // TODO verify the anonymous volumes status + }, + }, + } - vID := base.Cmd("volume", "create").Out() - base.Cmd("volume", "create", tID+"-1").AssertOK() - base.Cmd("volume", "create", tID+"-2").AssertOK() + for _, test := range testCases { + currentTest := test + t.Run(currentTest.description, func(tt *testing.T) { + if testutil.GetTarget() != testutil.Docker { + tt.Parallel() + } - base.Cmd("run", "-v", fmt.Sprintf("%s:/volume", tID+"-1"), "--name", tID, testutil.CommonImage).AssertOK() - defer base.Cmd("rm", "-f", tID).Run() + subTID := testutil.Identifier(tt) - base.Cmd("volume", "prune", "-f").AssertOutContains(vID) - base.Cmd("volume", "prune", "-a", "-f").AssertOutContains(tID + "-2") - base.Cmd("volume", "ls").AssertOutContains(tID + "-1") - base.Cmd("volume", "ls").AssertOutNotContains(tID + "-2") + if currentTest.tearDown != nil { + currentTest.tearDown(subTID) + tt.Cleanup(func() { + currentTest.tearDown(subTID) + }) + } + if currentTest.tearUp != nil { + currentTest.tearUp(subTID) + } - base.Cmd("rm", "-f", tID).AssertOK() - base.Cmd("volume", "prune", "-a", "-f").AssertOK() - base.Cmd("volume", "ls").AssertOutNotContains(tID + "-1") + cmd := currentTest.command(subTID) + cmd.Assert(currentTest.expected(subTID)) + }) + } } diff --git a/cmd/nerdctl/volume_remove_linux_test.go b/cmd/nerdctl/volume_remove_linux_test.go index 6023381c782..fb6ea1f7f52 100644 --- a/cmd/nerdctl/volume_remove_linux_test.go +++ b/cmd/nerdctl/volume_remove_linux_test.go @@ -20,6 +20,7 @@ import ( "fmt" "testing" + "github.com/containerd/containerd/errdefs" "github.com/containerd/nerdctl/v2/pkg/testutil" "gotest.tools/v3/icmd" ) @@ -29,12 +30,14 @@ func TestVolumeRemove(t *testing.T) { base := testutil.NewBase(t) - malformed := "malformed volume name" - notFound := "no such volume" + malformed := errdefs.ErrInvalidArgument.Error() + notFound := errdefs.ErrNotFound.Error() requireArg := "requires at least 1 arg" - inUse := "is in use" + inUse := errdefs.ErrFailedPrecondition.Error() if base.Target == testutil.Docker { malformed = "no such volume" + notFound = "no such volume" + inUse = "volume is in use" } testCases := []struct { @@ -139,17 +142,17 @@ func TestVolumeRemove(t *testing.T) { }, }, { - "part success multi remove", - func(tID string) *testutil.Cmd { + description: "part success multi remove", + command: func(tID string) *testutil.Cmd { return base.Cmd("volume", "rm", "invalid∞", "nonexistent", tID) }, - func(tID string) { + tearUp: func(tID string) { base.Cmd("volume", "create", tID).AssertOK() }, - func(tID string) { + tearDown: func(tID string) { base.Cmd("volume", "rm", "-f", tID).Run() }, - func(tID string) icmd.Expected { + expected: func(tID string) icmd.Expected { return icmd.Expected{ ExitCode: 1, Out: tID, diff --git a/pkg/cmd/volume/create.go b/pkg/cmd/volume/create.go index c00fe00efa3..f8cdbbcd345 100644 --- a/pkg/cmd/volume/create.go +++ b/pkg/cmd/volume/create.go @@ -19,7 +19,6 @@ package volume import ( "fmt" - "github.com/containerd/containerd/identifiers" "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/inspecttypes/native" "github.com/containerd/nerdctl/v2/pkg/labels" @@ -32,9 +31,6 @@ func Create(name string, options types.VolumeCreateOptions) (*native.Volume, err name = stringid.GenerateRandomID() options.Labels = append(options.Labels, labels.AnonymousVolumes+"=") } - if err := identifiers.Validate(name); err != nil { - return nil, fmt.Errorf("malformed name %s: %w", name, err) - } volStore, err := Store(options.GOptions.Namespace, options.GOptions.DataRoot, options.GOptions.Address) if err != nil { return nil, err diff --git a/pkg/cmd/volume/inspect.go b/pkg/cmd/volume/inspect.go index 4c9442ed286..4a7ee1cd2e5 100644 --- a/pkg/cmd/volume/inspect.go +++ b/pkg/cmd/volume/inspect.go @@ -17,23 +17,40 @@ package volume import ( + "context" + "errors" + + "github.com/containerd/log" "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/formatter" ) -func Inspect(volumes []string, options types.VolumeInspectOptions) error { +func Inspect(ctx context.Context, volumes []string, options types.VolumeInspectOptions) error { volStore, err := Store(options.GOptions.Namespace, options.GOptions.DataRoot, options.GOptions.Address) if err != nil { return err } - result := make([]interface{}, len(volumes)) + result := []interface{}{} - for i, name := range volumes { + warns := []error{} + for _, name := range volumes { var vol, err = volStore.Get(name, options.Size) if err != nil { - return err + warns = append(warns, err) + continue } - result[i] = vol + result = append(result, vol) + } + err = formatter.FormatSlice(options.Format, options.Stdout, result) + if err != nil { + return err + } + for _, warn := range warns { + log.G(ctx).Warn(warn) + } + + if len(warns) != 0 { + return errors.New("some volumes could not be inspected") } - return formatter.FormatSlice(options.Format, options.Stdout, result) + return nil } diff --git a/pkg/cmd/volume/prune.go b/pkg/cmd/volume/prune.go index f9dff99f428..6f03b980460 100644 --- a/pkg/cmd/volume/prune.go +++ b/pkg/cmd/volume/prune.go @@ -49,10 +49,12 @@ func Prune(ctx context.Context, client *containerd.Client, options types.VolumeP continue } if !options.All { - val, ok := (*volume.Labels)[labels.AnonymousVolumes] - //skip the named volume and only remove the anonymous volume - if !ok || val != "" { - continue + if volume.Labels != nil { + val, ok := (*volume.Labels)[labels.AnonymousVolumes] + //skip the named volume and only remove the anonymous volume + if !ok || val != "" { + continue + } } } removeNames = append(removeNames, volume.Name) diff --git a/pkg/cmd/volume/rm.go b/pkg/cmd/volume/rm.go index 509957bb978..81d81213839 100644 --- a/pkg/cmd/volume/rm.go +++ b/pkg/cmd/volume/rm.go @@ -23,6 +23,7 @@ import ( "fmt" "github.com/containerd/containerd" + "github.com/containerd/containerd/errdefs" "github.com/containerd/log" "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/inspecttypes/dockercompat" @@ -47,7 +48,7 @@ func Remove(ctx context.Context, client *containerd.Client, volumes []string, op var volumenames []string // nolint: prealloc for _, name := range volumes { if _, ok := usedVolumes[name]; ok { - return fmt.Errorf("volume %q is in use", name) + return fmt.Errorf("volume %q is in use (%w)", name, errdefs.ErrFailedPrecondition) } volumenames = append(volumenames, name) } diff --git a/pkg/mountutil/mountutil.go b/pkg/mountutil/mountutil.go index f8e184033b0..6223a6d5501 100644 --- a/pkg/mountutil/mountutil.go +++ b/pkg/mountutil/mountutil.go @@ -17,14 +17,12 @@ package mountutil import ( - "errors" "fmt" "os" "path/filepath" "runtime" "strings" - "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/oci" "github.com/containerd/containerd/pkg/userns" @@ -203,16 +201,11 @@ func handleAnonymousVolumes(s string, volStore volumestore.VolumeStore) (volumeS func handleNamedVolumes(source string, volStore volumestore.VolumeStore) (volumeSpec, error) { var res volumeSpec res.Name = source - vol, err := volStore.Get(res.Name, false) + + // Create returns an existing volume or creates a new one if necessary. + vol, err := volStore.Create(res.Name, nil) if err != nil { - if errors.Is(err, errdefs.ErrNotFound) { - vol, err = volStore.Create(res.Name, nil) - if err != nil { - return res, fmt.Errorf("failed to create volume %q: %w", res.Name, err) - } - } else { - return res, fmt.Errorf("failed to get volume %q: %w", res.Name, err) - } + return res, fmt.Errorf("failed to get volume %q: %w", res.Name, err) } // src is now an absolute path res.Type = Volume diff --git a/pkg/mountutil/volumestore/volumestore.go b/pkg/mountutil/volumestore/volumestore.go index 8093212a7b7..010c5ce6ee3 100644 --- a/pkg/mountutil/volumestore/volumestore.go +++ b/pkg/mountutil/volumestore/volumestore.go @@ -81,7 +81,7 @@ func (vs *volumeStore) Dir() string { func (vs *volumeStore) Create(name string, labels []string) (*native.Volume, error) { if err := identifiers.Validate(name); err != nil { - return nil, fmt.Errorf("malformed volume name: %w", err) + return nil, fmt.Errorf("malformed volume name: %w (%w)", err, errdefs.ErrInvalidArgument) } volPath := filepath.Join(vs.dir, name) volDataPath := filepath.Join(volPath, DataDirName) @@ -108,7 +108,10 @@ func (vs *volumeStore) Create(name string, labels []string) (*native.Volume, err Labels map[string]string `json:"labels"` } - labelsMap := strutil.ConvertKVStringsToMap(labels) + var labelsMap map[string]string + if len(labels) > 0 { + labelsMap = strutil.ConvertKVStringsToMap(labels) + } volOpts := volumeOpts{ Labels: labelsMap, @@ -145,36 +148,45 @@ func (vs *volumeStore) Create(name string, labels []string) (*native.Volume, err } func (vs *volumeStore) Get(name string, size bool) (*native.Volume, error) { - if err := identifiers.Validate(name); err != nil { - return nil, fmt.Errorf("malformed name %s: %w", name, err) - } - dataPath := filepath.Join(vs.dir, name, DataDirName) - if _, err := os.Stat(dataPath); err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("volume %q not found: %w", name, errdefs.ErrNotFound) - } - return nil, err + entry := native.Volume{ + Name: name, } - volFilePath := filepath.Join(vs.dir, name, volumeJSONFileName) - volumeDataBytes, err := os.ReadFile(volFilePath) - if err != nil { - if !os.IsNotExist(err) { - return nil, err - } // on else, volume.json does not exists should not be blocking for inspect operation - } + fn := func() error { + if err := identifiers.Validate(name); err != nil { + return fmt.Errorf("malformed volume name: %w (%w)", err, errdefs.ErrInvalidArgument) + } + dataPath := filepath.Join(vs.dir, name, DataDirName) + if _, err := os.Stat(dataPath); err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("no such volume: %s (%w)", name, errdefs.ErrNotFound) + } + return err + } + entry.Mountpoint = dataPath - entry := native.Volume{ - Name: name, - Mountpoint: dataPath, - Labels: Labels(volumeDataBytes), - } - if size { - entry.Size, err = Size(&entry) + volFilePath := filepath.Join(vs.dir, name, volumeJSONFileName) + volumeDataBytes, err := os.ReadFile(volFilePath) if err != nil { - return nil, err + if !os.IsNotExist(err) { + return err + } // on else, volume.json does not exists should not be blocking for inspect operation + } + entry.Labels = Labels(volumeDataBytes) + + if size { + entry.Size, err = Size(&entry) + if err != nil { + return err + } } + return nil } + + if err := lockutil.WithDirLock(vs.dir, fn); err != nil { + return nil, err + } + return &entry, nil } @@ -200,12 +212,12 @@ func (vs *volumeStore) Remove(names []string) (removed []string, warns []error, fn := func() error { for _, name := range names { if err := identifiers.Validate(name); err != nil { - warns = append(warns, fmt.Errorf("malformed volume name: %w", err)) + warns = append(warns, fmt.Errorf("malformed volume name: %w (%w)", err, errdefs.ErrInvalidArgument)) continue } dir := filepath.Join(vs.dir, name) if _, err := os.Stat(dir); os.IsNotExist(err) { - warns = append(warns, fmt.Errorf("no such volume: %s", name)) + warns = append(warns, fmt.Errorf("no such volume: %s (%w)", name, errdefs.ErrNotFound)) continue } // This is a hard filesystem error. Exit on this.