From d10c03bc3d6b58a3d37b4e99a462705a80cdbcbb Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Mon, 10 Apr 2023 09:47:20 -0400 Subject: [PATCH] Extend driver.ListLayers() Implement ListLayers() for the aufs, btrfs, and devicemapper drivers, along with a unit test for them. Stop filtering out directories with names that aren't 64-hex chars in vfs and overlay ListLayers() implementations, which is more a convention than a hard rule. Have layerStore.Wipe() try to remove remaining listed layers after it removes the layers that the layerStore knew of. Close() a dangling ReadCloser in NaiveCreateFromTemplate. Switch from using plain defer to using t.Cleanup() to handle deleting layers that tests create, have the addManyLayers() test function do so as well. Remove vfs.CopyDir, which near as I can tell isn't referenced anywhere. Signed-off-by: Nalin Dahyabhai --- Makefile | 2 +- drivers/aufs/aufs.go | 16 +++- drivers/aufs/aufs_test.go | 4 + drivers/btrfs/btrfs.go | 26 ++++-- drivers/btrfs/btrfs_test.go | 4 + drivers/devmapper/deviceset.go | 31 ++++++- drivers/devmapper/devmapper_test.go | 14 ++-- drivers/devmapper/driver.go | 6 -- drivers/graphtest/graphbench_unix.go | 4 +- drivers/graphtest/graphtest_unix.go | 120 +++++++++++++++++++-------- drivers/graphtest/testutil.go | 11 ++- drivers/graphtest/testutil_unix.go | 11 +-- drivers/overlay/overlay.go | 36 ++++---- drivers/overlay/overlay_test.go | 12 ++- drivers/quota/projectquota.go | 6 +- drivers/template.go | 1 + drivers/vfs/driver.go | 13 +-- drivers/vfs/vfs_test.go | 12 ++- drivers/zfs/zfs.go | 5 +- layers.go | 12 +++ 20 files changed, 242 insertions(+), 104 deletions(-) diff --git a/Makefile b/Makefile index 52266da0f..42b3aceb4 100644 --- a/Makefile +++ b/Makefile @@ -66,7 +66,7 @@ docs: install.tools ## build the docs on the host local-test: local-binary local-test-unit local-test-integration ## build the binaries and run the tests local-test-unit test-unit: local-binary ## run the unit tests on the host (requires\nsuperuser privileges) - @$(GO) test $(MOD_VENDOR) $(BUILDFLAGS) $(TESTFLAGS) $(shell $(GO) list ./... | grep -v ^$(PACKAGE)/vendor) + @$(GO) test -count 1 $(MOD_VENDOR) $(BUILDFLAGS) $(TESTFLAGS) $(shell $(GO) list ./... | grep -v ^$(PACKAGE)/vendor) local-test-integration test-integration: local-binary ## run the integration tests on the host (requires\nsuperuser privileges) @cd tests; ./test_runner.bash diff --git a/drivers/aufs/aufs.go b/drivers/aufs/aufs.go index 301ee24d2..4b0133a05 100644 --- a/drivers/aufs/aufs.go +++ b/drivers/aufs/aufs.go @@ -251,9 +251,21 @@ func (a *Driver) Exists(id string) bool { return true } -// List layers (not including additional image stores) +// ListLayers() returns all of the layers known to the driver. func (a *Driver) ListLayers() ([]string, error) { - return nil, graphdriver.ErrNotSupported + diffsDir := filepath.Join(a.rootPath(), "diff") + entries, err := os.ReadDir(diffsDir) + if err != nil { + return nil, err + } + results := make([]string, 0, len(entries)) + for _, entry := range entries { + if !entry.IsDir() { + continue + } + results = append(results, entry.Name()) + } + return results, nil } // AdditionalImageStores returns additional image stores supported by the driver diff --git a/drivers/aufs/aufs_test.go b/drivers/aufs/aufs_test.go index 415d07021..432df7ff2 100644 --- a/drivers/aufs/aufs_test.go +++ b/drivers/aufs/aufs_test.go @@ -824,3 +824,7 @@ func TestAufsCreateFromTemplate(t *testing.T) { func TestAufsEcho(t *testing.T) { graphtest.DriverTestEcho(t, "aufs") } + +func TestAufsListLayers(t *testing.T) { + graphtest.DriverTestListLayers(t, "aufs") +} diff --git a/drivers/btrfs/btrfs.go b/drivers/btrfs/btrfs.go index 8452fa189..e68804416 100644 --- a/drivers/btrfs/btrfs.go +++ b/drivers/btrfs/btrfs.go @@ -70,7 +70,7 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) if err != nil { return nil, err } - if err := idtools.MkdirAllAs(home, 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAs(filepath.Join(home, "subvolumes"), 0700, rootUID, rootGID); err != nil { return nil, err } @@ -119,7 +119,7 @@ func parseOptions(opt []string) (btrfsOptions, bool, error) { case "btrfs.mountopt": return options, userDiskQuota, fmt.Errorf("btrfs driver does not support mount options") default: - return options, userDiskQuota, fmt.Errorf("unknown option %s", key) + return options, userDiskQuota, fmt.Errorf("unknown option %s (%q)", key, option) } } return options, userDiskQuota, nil @@ -479,8 +479,8 @@ func (d *Driver) CreateReadWrite(id, parent string, opts *graphdriver.CreateOpts // Create the filesystem with given id. func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error { - quotas := path.Join(d.home, "quotas") - subvolumes := path.Join(d.home, "subvolumes") + quotas := d.quotasDir() + subvolumes := d.subvolumesDir() rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) if err != nil { return err @@ -560,7 +560,7 @@ func (d *Driver) parseStorageOpt(storageOpt map[string]string, driver *Driver) e } driver.options.size = uint64(size) default: - return fmt.Errorf("unknown option %s", key) + return fmt.Errorf("unknown option %s (%q)", key, storageOpt) } } @@ -679,9 +679,21 @@ func (d *Driver) Exists(id string) bool { return err == nil } -// List layers (not including additional image stores) +// List all of the layers known to the driver. func (d *Driver) ListLayers() ([]string, error) { - return nil, graphdriver.ErrNotSupported + subvolumesDir := filepath.Join(d.home, "subvolumes") + entries, err := os.ReadDir(subvolumesDir) + if err != nil { + return nil, err + } + results := make([]string, 0, len(entries)) + for _, entry := range entries { + if !entry.IsDir() { + continue + } + results = append(results, entry.Name()) + } + return results, nil } // AdditionalImageStores returns additional image stores supported by the driver diff --git a/drivers/btrfs/btrfs_test.go b/drivers/btrfs/btrfs_test.go index 005bfd17b..c876ebaf9 100644 --- a/drivers/btrfs/btrfs_test.go +++ b/drivers/btrfs/btrfs_test.go @@ -68,6 +68,10 @@ func TestBtrfsEcho(t *testing.T) { graphtest.DriverTestEcho(t, "btrfs") } +func TestBtrfsListLayers(t *testing.T) { + graphtest.DriverTestListLayers(t, "btrfs") +} + func TestBtrfsTeardown(t *testing.T) { graphtest.PutDriver(t) } diff --git a/drivers/devmapper/deviceset.go b/drivers/devmapper/deviceset.go index d2d0effc3..8603a0516 100644 --- a/drivers/devmapper/deviceset.go +++ b/drivers/devmapper/deviceset.go @@ -1001,6 +1001,10 @@ func (devices *DeviceSet) verifyBaseDeviceUUIDFS(baseInfo *devInfo) error { devices.Lock() defer devices.Unlock() + if devices.filesystem == "" { + devices.filesystem = determineDefaultFS() + } + if err := devices.activateDeviceIfNeeded(baseInfo, false); err != nil { return err } @@ -1707,6 +1711,9 @@ func (devices *DeviceSet) initDevmapper(doInit bool) (retErr error) { if err := os.MkdirAll(devices.metadataDir(), 0700); err != nil { return err } + if err := idtools.MkdirAs(filepath.Join(devices.root, "mnt"), 0700, uid, gid); err != nil && !errors.Is(err, os.ErrExist) { + return err + } prevSetupConfig, err := readLVMConfig(devices.root) if err != nil { @@ -2317,7 +2324,7 @@ func (devices *DeviceSet) Shutdown(home string) error { info.lock.Lock() devices.Lock() if err := devices.deactivateDevice(info); err != nil { - logrus.Debugf("devmapper: Shutdown deactivate base , error: %s", err) + logrus.Debugf("devmapper: Shutdown deactivate base, error: %s", err) } devices.Unlock() info.lock.Unlock() @@ -2326,7 +2333,7 @@ func (devices *DeviceSet) Shutdown(home string) error { devices.Lock() if devices.thinPoolDevice == "" { if err := devices.deactivatePool(); err != nil { - logrus.Debugf("devmapper: Shutdown deactivate pool , error: %s", err) + logrus.Debugf("devmapper: Shutdown deactivate pool, error: %s", err) } } devices.Unlock() @@ -2483,6 +2490,26 @@ func (devices *DeviceSet) List() []string { return ids } +// ListLayers returns a list of device IDs, omitting the ""/"base" device and +// any which have been marked as deleted. +func (devices *DeviceSet) ListLayers() ([]string, error) { + if err := devices.cleanupDeletedDevices(); err != nil { + return nil, err + } + + devices.Lock() + defer devices.Unlock() + + ids := make([]string, 0, len(devices.Devices)) + for k, d := range devices.Devices { + if k == "" || d.Deleted { + continue + } + ids = append(ids, k) + } + return ids, nil +} + func (devices *DeviceSet) deviceStatus(devName string) (sizeInSectors, mappedSectors, highestMappedSector uint64, err error) { var params string _, sizeInSectors, _, params, err = devicemapper.GetStatus(devName) diff --git a/drivers/devmapper/devmapper_test.go b/drivers/devmapper/devmapper_test.go index 85c51b7ff..89a74f3c8 100644 --- a/drivers/devmapper/devmapper_test.go +++ b/drivers/devmapper/devmapper_test.go @@ -91,14 +91,18 @@ func TestDevmapperCreateFromTemplate(t *testing.T) { graphtest.DriverTestCreateFromTemplate(t, "devicemapper", "test=1") } -func TestDevmapperTeardown(t *testing.T) { - graphtest.PutDriver(t) -} - func TestDevmapperEcho(t *testing.T) { graphtest.DriverTestEcho(t, "devicemapper", "test=1") } +func TestDevmapperListLayers(t *testing.T) { + graphtest.DriverTestListLayers(t, "devicemapper", "test=1") +} + +func TestDevmapperTeardown(t *testing.T) { + graphtest.PutDriver(t) +} + func TestDevmapperReduceLoopBackSize(t *testing.T) { tenMB := int64(10 * 1024 * 1024) testChangeLoopBackSize(t, -tenMB, defaultDataLoopbackSize, defaultMetaDataLoopbackSize) @@ -130,7 +134,7 @@ func testChangeLoopBackSize(t *testing.T, delta, expectDataSize, expectMetaDataS } driver = d.(*graphdriver.NaiveDiffDriver).ProtoDriver.(*Driver) if s := driver.DeviceSet.Status(); s.Data.Total != uint64(expectDataSize) || s.Metadata.Total != uint64(expectMetaDataSize) { - t.Fatal("data or metadata loop back size is incorrect") + t.Fatalf("data or metadata loop back size is incorrect (data actual/expected=%d/%d, metadata actual/expected = %d/%d)", s.Data.Total, expectDataSize, s.Metadata.Total, expectMetaDataSize) } if err := driver.Cleanup(); err != nil { t.Fatal(err) diff --git a/drivers/devmapper/driver.go b/drivers/devmapper/driver.go index 8b3ee51df..f53f0625f 100644 --- a/drivers/devmapper/driver.go +++ b/drivers/devmapper/driver.go @@ -55,7 +55,6 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) ctr: graphdriver.NewRefCounter(graphdriver.NewDefaultChecker()), locker: locker.New(), } - return graphdriver.NewNaiveDiffDriver(d, graphdriver.NewNaiveLayerIDMapUpdater(d)), nil } @@ -267,11 +266,6 @@ func (d *Driver) Exists(id string) bool { return d.DeviceSet.HasDevice(id) } -// List layers (not including additional image stores) -func (d *Driver) ListLayers() ([]string, error) { - return nil, graphdriver.ErrNotSupported -} - // AdditionalImageStores returns additional image stores supported by the driver func (d *Driver) AdditionalImageStores() []string { return nil diff --git a/drivers/graphtest/graphbench_unix.go b/drivers/graphtest/graphbench_unix.go index f67ceb09b..e3e6f2f28 100644 --- a/drivers/graphtest/graphbench_unix.go +++ b/drivers/graphtest/graphbench_unix.go @@ -198,7 +198,7 @@ func DriverBenchDeepLayerDiff(b *testing.B, layerCount int, drivername string, d b.Fatal(err) } - topLayer, err := addManyLayers(driver, base, layerCount) + topLayer, err := addManyLayers(b, driver, base, layerCount) if err != nil { b.Fatal(err) } @@ -232,7 +232,7 @@ func DriverBenchDeepLayerRead(b *testing.B, layerCount int, drivername string, d b.Fatal(err) } - topLayer, err := addManyLayers(driver, base, layerCount) + topLayer, err := addManyLayers(b, driver, base, layerCount) if err != nil { b.Fatal(err) } diff --git a/drivers/graphtest/graphtest_unix.go b/drivers/graphtest/graphtest_unix.go index bc0739143..3001a8933 100644 --- a/drivers/graphtest/graphtest_unix.go +++ b/drivers/graphtest/graphtest_unix.go @@ -11,6 +11,7 @@ import ( "os" "path" "path/filepath" + "sort" "testing" graphdriver "github.com/containers/storage/drivers" @@ -27,8 +28,12 @@ var ( ) const ( - defaultPerms = os.FileMode(0555) - modifiedPerms = os.FileMode(0711) + defaultPerms = os.FileMode(0o555) + modifiedPerms = os.FileMode(0o711) + defaultSubdirPerms = os.FileMode(0o705) + defaultSubdirOwner = 1 + defaultSubdirGroup = 2 + defaultFilePerms = os.FileMode(0o222) ) // Driver conforms to graphdriver.Driver interface and @@ -92,14 +97,12 @@ func PutDriver(t testing.TB) { // DriverTestCreateEmpty creates a new image and verifies it is empty and the right metadata func DriverTestCreateEmpty(t testing.TB, drivername string, driverOptions ...string) { driver := GetDriver(t, drivername, driverOptions...) + require.NotNil(t, drv.Driver, "initializing driver") defer PutDriver(t) err := driver.Create("empty", "", nil) require.NoError(t, err) - - defer func() { - require.NoError(t, driver.Remove("empty")) - }() + t.Cleanup(func() { removeLayer(t, driver, "empty") }) if !driver.Exists("empty") { t.Fatal("Newly created image doesn't exist") @@ -121,30 +124,24 @@ func DriverTestCreateEmpty(t testing.TB, drivername string, driverOptions ...str // DriverTestCreateBase create a base driver and verify. func DriverTestCreateBase(t testing.TB, drivername string, driverOptions ...string) { driver := GetDriver(t, drivername, driverOptions...) + require.NotNil(t, drv.Driver, "initializing driver") defer PutDriver(t) createBase(t, driver, "Base1") - defer func() { - require.NoError(t, driver.Remove("Base1")) - }() verifyBase(t, driver, "Base1", defaultPerms) } // DriverTestCreateSnap Create a driver and snap and verify. func DriverTestCreateSnap(t testing.TB, drivername string, driverOptions ...string) { driver := GetDriver(t, drivername, driverOptions...) + require.NotNil(t, drv.Driver, "initializing driver") defer PutDriver(t) createBase(t, driver, "Base2") - defer func() { - require.NoError(t, driver.Remove("Base2")) - }() err := driver.Create("Snap2", "Base2", nil) require.NoError(t, err) - defer func() { - require.NoError(t, driver.Remove("Snap2")) - }() + t.Cleanup(func() { removeLayer(t, driver, "Snap2") }) verifyBase(t, driver, "Snap2", defaultPerms) @@ -156,9 +153,7 @@ func DriverTestCreateSnap(t testing.TB, drivername string, driverOptions ...stri err = driver.Create("SecondSnap", "Snap2", nil) require.NoError(t, err) - defer func() { - require.NoError(t, driver.Remove("SecondSnap")) - }() + t.Cleanup(func() { removeLayer(t, driver, "SecondSnap") }) verifyBase(t, driver, "SecondSnap", modifiedPerms) } @@ -167,18 +162,15 @@ func DriverTestCreateSnap(t testing.TB, drivername string, driverOptions ...stri // contents. func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions ...string) { driver := GetDriver(t, drivername, driverOptions...) + require.NotNil(t, drv.Driver, "initializing driver") defer PutDriver(t) createBase(t, driver, "Base3") - defer func() { - require.NoError(t, driver.Remove("Base3")) - }() + verifyBase(t, driver, "Base3", defaultPerms) err := driver.Create("Snap3", "Base3", nil) require.NoError(t, err) - defer func() { - require.NoError(t, driver.Remove("Snap3")) - }() + t.Cleanup(func() { removeLayer(t, driver, "Snap3") }) content := []byte("test content") if err := addFile(driver, "Snap3", "testfile.txt", content); err != nil { @@ -187,15 +179,10 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions err = driver.CreateFromTemplate("FromTemplate", "Snap3", nil, "Base3", nil, nil, true) require.NoError(t, err) - defer func() { - require.NoError(t, driver.Remove("FromTemplate")) - }() - + t.Cleanup(func() { removeLayer(t, driver, "FromTemplate") }) err = driver.CreateFromTemplate("ROFromTemplate", "Snap3", nil, "Base3", nil, nil, false) require.NoError(t, err) - defer func() { - require.NoError(t, driver.Remove("ROFromTemplate")) - }() + t.Cleanup(func() { removeLayer(t, driver, "ROFromTemplate") }) noChanges := []archive.Change{} @@ -260,19 +247,21 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions // DriverTestDeepLayerRead reads a file from a lower layer under a given number of layers func DriverTestDeepLayerRead(t testing.TB, layerCount int, drivername string, driverOptions ...string) { driver := GetDriver(t, drivername, driverOptions...) + require.NotNil(t, drv.Driver, "initializing driver") defer PutDriver(t) base := stringid.GenerateRandomID() if err := driver.Create(base, "", nil); err != nil { t.Fatal(err) } + t.Cleanup(func() { removeLayer(t, driver, base) }) content := []byte("test content") if err := addFile(driver, base, "testfile.txt", content); err != nil { t.Fatal(err) } - topLayer, err := addManyLayers(driver, base, layerCount) + topLayer, err := addManyLayers(t, driver, base, layerCount) if err != nil { t.Fatal(err) } @@ -290,6 +279,7 @@ func DriverTestDeepLayerRead(t testing.TB, layerCount int, drivername string, dr // DriverTestDiffApply tests diffing and applying produces the same layer func DriverTestDiffApply(t testing.TB, fileCount int, drivername string, driverOptions ...string) { driver := GetDriver(t, drivername, driverOptions...) + require.NotNil(t, drv.Driver, "initializing driver") defer PutDriver(t) base := stringid.GenerateRandomID() upper := stringid.GenerateRandomID() @@ -300,6 +290,7 @@ func DriverTestDiffApply(t testing.TB, fileCount int, drivername string, driverO if err := driver.Create(base, "", nil); err != nil { t.Fatal(err) } + t.Cleanup(func() { removeLayer(t, driver, base) }) if err := addManyFiles(driver, base, fileCount, 3); err != nil { t.Fatal(err) @@ -316,6 +307,7 @@ func DriverTestDiffApply(t testing.TB, fileCount int, drivername string, driverO if err := driver.Create(upper, base, nil); err != nil { t.Fatal(err) } + t.Cleanup(func() { removeLayer(t, driver, upper) }) if err := addManyFiles(driver, upper, fileCount, 6); err != nil { t.Fatal(err) @@ -334,6 +326,7 @@ func DriverTestDiffApply(t testing.TB, fileCount int, drivername string, driverO if err := driver.Create(diff, base, nil); err != nil { t.Fatal(err) } + t.Cleanup(func() { removeLayer(t, driver, diff) }) if err := checkManyFiles(driver, diff, fileCount, 3); err != nil { t.Fatal(err) @@ -381,12 +374,15 @@ func DriverTestDiffApply(t testing.TB, fileCount int, drivername string, driverO // DriverTestChanges tests computed changes on a layer matches changes made func DriverTestChanges(t testing.TB, drivername string, driverOptions ...string) { driver := GetDriver(t, drivername, driverOptions...) + require.NotNil(t, drv.Driver, "initializing driver") defer PutDriver(t) base := stringid.GenerateRandomID() upper := stringid.GenerateRandomID() + if err := driver.Create(base, "", nil); err != nil { t.Fatal(err) } + t.Cleanup(func() { removeLayer(t, driver, base) }) if err := addManyFiles(driver, base, 20, 3); err != nil { t.Fatal(err) @@ -395,6 +391,7 @@ func DriverTestChanges(t testing.TB, drivername string, driverOptions ...string) if err := driver.Create(upper, base, nil); err != nil { t.Fatal(err) } + t.Cleanup(func() { removeLayer(t, driver, upper) }) expectedChanges, err := changeManyFiles(driver, upper, 20, 6) if err != nil { @@ -425,15 +422,19 @@ func writeRandomFile(path string, size uint64) error { // DriverTestSetQuota Create a driver and test setting quota. func DriverTestSetQuota(t *testing.T, drivername string) { driver := GetDriver(t, drivername) + require.NotNil(t, drv.Driver, "initializing driver") defer PutDriver(t) createBase(t, driver, "Base4") + verifyBase(t, driver, "Base4", defaultPerms) + createOpts := &graphdriver.CreateOpts{} createOpts.StorageOpt = make(map[string]string, 1) createOpts.StorageOpt["size"] = "50M" if err := driver.Create("quotaTest", "Base4", createOpts); err != nil { t.Fatal(err) } + t.Cleanup(func() { removeLayer(t, driver, "quotaTest") }) mountPath, err := driver.Get("quotaTest", graphdriver.MountOpts{}) if err != nil { @@ -454,6 +455,7 @@ func DriverTestSetQuota(t *testing.T, drivername string) { // DriverTestEcho tests that we can diff a layer correctly, focusing on trouble spots that NaiveDiff doesn't have func DriverTestEcho(t testing.TB, drivername string, driverOptions ...string) { driver := GetDriver(t, drivername, driverOptions...) + require.NotNil(t, drv.Driver, "initializing driver") defer PutDriver(t) var err error var root string @@ -464,17 +466,19 @@ func DriverTestEcho(t testing.TB, drivername string, driverOptions ...string) { second := stringid.GenerateRandomID() third := stringid.GenerateRandomID() - if err := driver.Create(base, "", nil); err != nil { - t.Fatal(err) - } + createBase(t, driver, base) + verifyBase(t, driver, base, defaultPerms) if root, err = driver.Get(base, graphdriver.MountOpts{}); err != nil { t.Fatal(err) } + expectedChanges := []archive.Change{ + {Kind: archive.ChangeAdd, Path: "/a file"}, + {Kind: archive.ChangeAdd, Path: "/a subdir"}, + } paths := []string{} path := "/" - expectedChanges := []archive.Change{} for i := 0; i < components-1; i++ { path = filepath.Join(path, fmt.Sprintf("subdir%d", i+1)) paths = append(paths, path) @@ -502,6 +506,7 @@ func DriverTestEcho(t testing.TB, drivername string, driverOptions ...string) { if err := driver.Create(second, base, nil); err != nil { t.Fatal(err) } + t.Cleanup(func() { removeLayer(t, driver, second) }) if root, err = driver.Get(second, graphdriver.MountOpts{}); err != nil { t.Fatal(err) @@ -528,6 +533,7 @@ func DriverTestEcho(t testing.TB, drivername string, driverOptions ...string) { if err = driver.Create(third, second, nil); err != nil { t.Fatal(err) } + t.Cleanup(func() { removeLayer(t, driver, third) }) if root, err = driver.Get(third, graphdriver.MountOpts{}); err != nil { t.Fatal(err) @@ -571,3 +577,45 @@ func DriverTestEcho(t testing.TB, drivername string, driverOptions ...string) { } } } + +// DriverTestListLayers makes sure ListLayers() returns what we expected, nothing more, nothing less +func DriverTestListLayers(t testing.TB, drivername string, driverOptions ...string) { + driver := GetDriver(t, drivername, driverOptions...) + require.NotNil(t, drv.Driver, "initializing driver") + defer PutDriver(t) + base := stringid.GenerateRandomID() + mid := stringid.GenerateRandomID() + upper := stringid.GenerateRandomID() + + createBase(t, driver, base) + verifyBase(t, driver, base, defaultPerms) + + if err := addManyFiles(driver, base, 20, 3); err != nil { + t.Fatal(err) + } + + if err := driver.Create(mid, base, nil); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { removeLayer(t, driver, mid) }) + + if err := addManyFiles(driver, mid, 20, 3); err != nil { + t.Fatal(err) + } + + if err := driver.Create(upper, mid, nil); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { removeLayer(t, driver, upper) }) + + list, err := driver.ListLayers() + if err != nil { + t.Fatal(err) + } + sort.Strings(list) + + expected := []string{base, mid, upper} + sort.Strings(expected) + + assert.Equal(t, expected, list, "listed layers were not exactly what we created") +} diff --git a/drivers/graphtest/testutil.go b/drivers/graphtest/testutil.go index 7dd4a6012..473dd17ee 100644 --- a/drivers/graphtest/testutil.go +++ b/drivers/graphtest/testutil.go @@ -13,10 +13,12 @@ import ( "os" "path" "sort" + "testing" graphdriver "github.com/containers/storage/drivers" "github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/stringid" + "github.com/stretchr/testify/require" ) func randomContent(size int, seed int64) []byte { @@ -282,13 +284,14 @@ func addLayerFiles(drv graphdriver.Driver, layer, parent string, i int) error { return nil } -func addManyLayers(drv graphdriver.Driver, baseLayer string, count int) (string, error) { +func addManyLayers(t testing.TB, drv graphdriver.Driver, baseLayer string, count int) (string, error) { lastLayer := baseLayer for i := 1; i <= count; i++ { nextLayer := stringid.GenerateRandomID() if err := drv.Create(nextLayer, lastLayer, nil); err != nil { return "", err } + t.Cleanup(func() { removeLayer(t, drv, nextLayer) }) if err := addLayerFiles(drv, nextLayer, lastLayer, i); err != nil { return "", err } @@ -351,3 +354,9 @@ func readDir(dir string) ([]os.DirEntry, error) { return b, nil } + +// removeLayer tries to remove the layer +func removeLayer(t testing.TB, driver graphdriver.Driver, name string) { + err := driver.Remove(name) + require.NoError(t, err) +} diff --git a/drivers/graphtest/testutil_unix.go b/drivers/graphtest/testutil_unix.go index f461da764..b3a684245 100644 --- a/drivers/graphtest/testutil_unix.go +++ b/drivers/graphtest/testutil_unix.go @@ -39,17 +39,18 @@ func createBase(t testing.TB, driver graphdriver.Driver, name string) { err := driver.CreateReadWrite(name, "", nil) require.NoError(t, err) + t.Cleanup(func() { removeLayer(t, driver, name) }) dir, err := driver.Get(name, graphdriver.MountOpts{}) require.NoError(t, err) defer driver.Put(name) subdir := path.Join(dir, "a subdir") - require.NoError(t, os.Mkdir(subdir, 0705|os.ModeSticky)) - require.NoError(t, os.Chown(subdir, 1, 2)) + require.NoError(t, os.Mkdir(subdir, defaultSubdirPerms|os.ModeSticky)) + require.NoError(t, os.Chown(subdir, defaultSubdirOwner, defaultSubdirGroup)) file := path.Join(dir, "a file") - err = os.WriteFile(file, []byte("Some data"), 0222|os.ModeSetuid) + err = os.WriteFile(file, []byte("Some data"), defaultFilePerms|os.ModeSetuid) require.NoError(t, err) } @@ -61,10 +62,10 @@ func verifyBase(t testing.TB, driver graphdriver.Driver, name string, defaultPer verifyFile(t, dir, defaultPerm|os.ModeDir, 0, 0) subdir := path.Join(dir, "a subdir") - verifyFile(t, subdir, 0705|os.ModeDir|os.ModeSticky, 1, 2) + verifyFile(t, subdir, defaultSubdirPerms|os.ModeDir|os.ModeSticky, defaultSubdirOwner, defaultSubdirGroup) file := path.Join(dir, "a file") - verifyFile(t, file, 0222|os.ModeSetuid, 0, 0) + verifyFile(t, file, defaultFilePerms|os.ModeSetuid, 0, 0) files, err := readDir(dir) require.NoError(t, err) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 8216d617a..218b7c405 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -29,7 +29,6 @@ import ( "github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/mount" "github.com/containers/storage/pkg/parsers" - "github.com/containers/storage/pkg/stringid" "github.com/containers/storage/pkg/system" "github.com/containers/storage/pkg/unshare" units "github.com/docker/go-units" @@ -47,7 +46,9 @@ var ( ) const ( - defaultPerms = os.FileMode(0555) + defaultPerms = os.FileMode(0555) + selinuxLabelTest = "system_u:object_r:container_file_t:s0" + mountProgramFlagFile = ".has-mount-program" ) // This backend uses the overlay union filesystem for containers @@ -78,9 +79,10 @@ const ( // that mounts do not fail due to length. const ( - linkDir = "l" - lowerFile = "lower" - maxDepth = 500 + linkDir = "l" + stagingDir = "staging" + lowerFile = "lower" + maxDepth = 500 // idLength represents the number of random characters // which can be used to create the unique link identifier @@ -124,7 +126,6 @@ type Driver struct { } type additionalLayerStore struct { - // path is the directory where this store is available on the host. path string @@ -175,7 +176,7 @@ func hasVolatileOption(opts []string) bool { } func getMountProgramFlagFile(path string) string { - return filepath.Join(path, ".has-mount-program") + return filepath.Join(path, mountProgramFlagFile) } func checkSupportVolatile(home, runhome string) (bool, error) { @@ -958,7 +959,7 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, disable return err } if parent != "" { - st, err := system.Stat(d.dir(parent)) + st, err := system.Stat(filepath.Join(d.dir(parent), "diff")) if err != nil { return err } @@ -1725,20 +1726,23 @@ func (d *Driver) ListLayers() ([]string, error) { if err != nil { return nil, err } - layers := make([]string, 0) for _, entry := range entries { id := entry.Name() - // Does it look like a datadir directory? - if !entry.IsDir() || stringid.ValidateID(id) != nil { + switch id { + case linkDir, stagingDir, quota.BackingFsBlockDeviceLink, mountProgramFlagFile: + // expected, but not a layer. skip it continue + default: + // Does it look like a datadir directory? + if !entry.IsDir() { + continue + } + layers = append(layers, id) } - - layers = append(layers, id) } - - return layers, err + return layers, nil } // isParent returns if the passed in parent is the direct parent of the passed in layer @@ -1795,7 +1799,7 @@ func (g *overlayFileGetter) Close() error { } func (d *Driver) getStagingDir() string { - return filepath.Join(d.home, "staging") + return filepath.Join(d.home, stagingDir) } // DiffGetter returns a FileGetCloser that can read files from the directory that diff --git a/drivers/overlay/overlay_test.go b/drivers/overlay/overlay_test.go index 4027c61e4..439ac264a 100644 --- a/drivers/overlay/overlay_test.go +++ b/drivers/overlay/overlay_test.go @@ -67,14 +67,18 @@ func TestOverlayChanges(t *testing.T) { graphtest.DriverTestChanges(t, driverName) } -func TestOverlayTeardown(t *testing.T) { - graphtest.PutDriver(t) -} - func TestOverlayEcho(t *testing.T) { graphtest.DriverTestEcho(t, driverName) } +func TestOverlayListLayers(t *testing.T) { + graphtest.DriverTestListLayers(t, driverName) +} + +func TestOverlayTeardown(t *testing.T) { + graphtest.PutDriver(t) +} + // Benchmarks should always setup new driver func BenchmarkExists(b *testing.B) { diff --git a/drivers/quota/projectquota.go b/drivers/quota/projectquota.go index f5484dee7..96c37b231 100644 --- a/drivers/quota/projectquota.go +++ b/drivers/quota/projectquota.go @@ -67,6 +67,10 @@ import ( const projectIDsAllocatedPerQuotaHome = 10000 +// BackingFsBlockDeviceLink is the name of a file that we place in +// the home directory of a driver that uses this package. +const BackingFsBlockDeviceLink = "backingFsBlockDev" + // Quota limit params - currently we only control blocks hard limit and inodes type Quota struct { Size uint64 @@ -421,7 +425,7 @@ func makeBackingFsDev(home string) (string, error) { return "", err } - backingFsBlockDev := path.Join(home, "backingFsBlockDev") + backingFsBlockDev := path.Join(home, BackingFsBlockDeviceLink) backingFsBlockDevTmp := backingFsBlockDev + ".tmp" // Re-create just in case someone copied the home directory over to a new device if err := unix.Mknod(backingFsBlockDevTmp, unix.S_IFBLK|0600, int(stat.Dev)); err != nil { diff --git a/drivers/template.go b/drivers/template.go index 7b96c082d..66ab89f7f 100644 --- a/drivers/template.go +++ b/drivers/template.go @@ -34,6 +34,7 @@ func NaiveCreateFromTemplate(d TemplateDriver, id, template string, templateIDMa } return err } + defer diff.Close() applyOptions := ApplyDiffOpts{ Diff: diff, diff --git a/drivers/vfs/driver.go b/drivers/vfs/driver.go index 2c6a63d6e..443cd3da2 100644 --- a/drivers/vfs/driver.go +++ b/drivers/vfs/driver.go @@ -14,18 +14,12 @@ import ( "github.com/containers/storage/pkg/directory" "github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/parsers" - "github.com/containers/storage/pkg/stringid" "github.com/containers/storage/pkg/system" "github.com/opencontainers/selinux/go-selinux/label" "github.com/sirupsen/logrus" "github.com/vbatts/tar-split/tar/storage" ) -var ( - // CopyDir defines the copy method to use. - CopyDir = dirCopy -) - const defaultPerms = os.FileMode(0555) func init() { @@ -42,11 +36,10 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) } rootIDs := d.idMappings.RootPair() - if err := idtools.MkdirAllAndChown(home, 0700, rootIDs); err != nil { + if err := idtools.MkdirAllAndChown(filepath.Join(home, "dir"), 0700, rootIDs); err != nil { return nil, err } for _, option := range options.DriverOptions { - key, val, err := parsers.ParseKeyValueOpt(option) if err != nil { return nil, err @@ -268,7 +261,7 @@ func (d *Driver) Exists(id string) bool { // List layers (not including additional image stores) func (d *Driver) ListLayers() ([]string, error) { - entries, err := os.ReadDir(d.homes[0]) + entries, err := os.ReadDir(filepath.Join(d.homes[0], "dir")) if err != nil { return nil, err } @@ -278,7 +271,7 @@ func (d *Driver) ListLayers() ([]string, error) { for _, entry := range entries { id := entry.Name() // Does it look like a datadir directory? - if !entry.IsDir() || stringid.ValidateID(id) != nil { + if !entry.IsDir() { continue } diff --git a/drivers/vfs/vfs_test.go b/drivers/vfs/vfs_test.go index b982ab1fb..fb60f8f14 100644 --- a/drivers/vfs/vfs_test.go +++ b/drivers/vfs/vfs_test.go @@ -37,10 +37,6 @@ func TestVfsCreateFromTemplate(t *testing.T) { graphtest.DriverTestCreateFromTemplate(t, "vfs") } -func TestVfsTeardown(t *testing.T) { - graphtest.PutDriver(t) -} - func TestVfsDiffApply100Files(t *testing.T) { graphtest.DriverTestDiffApply(t, 100, "vfs") } @@ -52,3 +48,11 @@ func TestVfsChanges(t *testing.T) { func TestVfsEcho(t *testing.T) { graphtest.DriverTestEcho(t, "vfs") } + +func TestVfsListLayers(t *testing.T) { + graphtest.DriverTestListLayers(t, "vfs") +} + +func TestVfsTeardown(t *testing.T) { + graphtest.PutDriver(t) +} diff --git a/drivers/zfs/zfs.go b/drivers/zfs/zfs.go index aeef64103..703b660d2 100644 --- a/drivers/zfs/zfs.go +++ b/drivers/zfs/zfs.go @@ -409,7 +409,6 @@ func (d *Driver) Remove(id string) error { // Get returns the mountpoint for the given id after creating the target directories if necessary. func (d *Driver) Get(id string, options graphdriver.MountOpts) (_ string, retErr error) { - mountpoint := d.mountPath(id) if count := d.ctr.Increment(mountpoint); count > 1 { return mountpoint, nil @@ -506,7 +505,9 @@ func (d *Driver) Exists(id string) bool { return d.filesystemsCache[d.zfsPath(id)] } -// List layers (not including additional image stores) +// List layers (not including additional image stores). Our layers aren't all +// dependent on a single well-known dataset, so we can't reliably tell which +// datasets are ours and which ones just look like they could be ours. func (d *Driver) ListLayers() ([]string, error) { return nil, graphdriver.ErrNotSupported } diff --git a/layers.go b/layers.go index 3f37405b0..51cca6a63 100644 --- a/layers.go +++ b/layers.go @@ -1922,6 +1922,18 @@ func (r *layerStore) Wipe() error { return err } } + ids, err := r.driver.ListLayers() + if err != nil { + if !errors.Is(err, drivers.ErrNotSupported) { + return err + } + ids = nil + } + for _, id := range ids { + if err := r.driver.Remove(id); err != nil { + return err + } + } return nil }