Initial implementation of consistency checks

Add initial Check() and Repair() methods to Stores.

Check() checks for inconsistencies between the layers which the
lower-level storage driver claims to know about and the ones which we
know we're managing.  It checks that layers referenced by layers,
images, and containers are known to us and that images referenced by
containers are known to us.  It checks that data which we store
alongside layers, images, and containers is still present, and to the
extent which we store other information about that data (frequenly just
the size of the data), verifies that it matches recorded expectations.
Lastly, it checks that layers which are part of images (and which we
therefore know what they should have in them) have the expected content,
and nothing else.

Repair() removes any containers, images, and layers which have any
errors associated with them.  This is destructive, so its use should be
considered and deliberate.

Signed-off-by: Nalin Dahyabhai <[email protected]>

check_test.go

@@ -0,0 +1,101 @@
+package storage
+
+import (
+	"archive/tar"
+	"sort"
+	"testing"
+
+	"github.com/containers/storage/pkg/archive"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCheckDirectory(t *testing.T) {
+	vectors := []struct {
+		description string
+		headers     []tar.Header
+		expected    []string
+	}{
+		{
+			description: "basic",
+			headers: []tar.Header{
+				{Name: "a", Typeflag: tar.TypeDir},
+			},
+			expected: []string{
+				"a/",
+			},
+		},
+		{
+			description: "whiteout",
+			headers: []tar.Header{
+				{Name: "a", Typeflag: tar.TypeDir},
+				{Name: "a/b", Typeflag: tar.TypeDir},
+				{Name: "a/b/c", Typeflag: tar.TypeReg},
+				{Name: "a/b/d", Typeflag: tar.TypeReg},
+				{Name: "a/b/" + archive.WhiteoutPrefix + "c", Typeflag: tar.TypeReg},
+			},
+			expected: []string{
+				"a/",
+				"a/b/",
+				"a/b/d",
+			},
+		},
+		{
+			description: "opaque",
+			headers: []tar.Header{
+				{Name: "a", Typeflag: tar.TypeDir},
+				{Name: "a/b", Typeflag: tar.TypeDir},
+				{Name: "a/b/c", Typeflag: tar.TypeReg},
+				{Name: "a/b/d", Typeflag: tar.TypeReg},
+				{Name: "a/b/" + archive.WhiteoutOpaqueDir, Typeflag: tar.TypeReg},
+			},
+			expected: []string{
+				"a/",
+				"a/b/",
+			},
+		},
+	}
+	for i := range vectors {
+		t.Run(vectors[i].description, func(t *testing.T) {
+			cd := newCheckDirectoryDefaults()
+			for _, hdr := range vectors[i].headers {
+				cd.header(&hdr)
+			}
+			actual := cd.names()
+			sort.Strings(actual)
+			expected := append([]string{}, vectors[i].expected...)
+			sort.Strings(expected)
+			assert.Equal(t, expected, actual)
+		})
+	}
+}
+
+func TestCheckDetectWriteable(t *testing.T) {
+	var sawRWlayers, sawRWimages bool
+	stoar, err := GetStore(StoreOptions{
+		RunRoot:         t.TempDir(),
+		GraphRoot:       t.TempDir(),
+		GraphDriverName: "vfs",
+	})
+	require.NoError(t, err, "unexpected error initializing test store")
+	s, ok := stoar.(*store)
+	require.True(t, ok, "unexpected error making type assertion")
+	done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) {
+		if roLayerStoreIsReallyReadWrite(store) { // implicitly checking that the type assertion in this function doesn't panic
+			sawRWlayers = true
+		}
+		return false, nil
+	})
+	assert.False(t, done, "unexpected error from readAllLayerStores")
+	assert.NoError(t, err, "unexpected error from readAllLayerStores")
+	assert.True(t, sawRWlayers, "unexpected error detecting which layer store is writeable")
+	done, err = s.readAllImageStores(func(store roImageStore) (bool, error) {
+		if roImageStoreIsReallyReadWrite(store) { // implicitly checking that the type assertion in this function doesn't panic
+			sawRWimages = true
+		}
+		return false, nil
+	})
+	assert.False(t, done, "unexpected error from readAllImageStores")
+	assert.NoError(t, err, "unexpected error from readAllImageStores")
+	assert.True(t, sawRWimages, "unexpected error detecting which image store is writeable")
+}

cmd/containers-storage/check.go

@@ -0,0 +1,141 @@
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"time"
+
+	"github.com/containers/storage"
+	"github.com/containers/storage/pkg/mflag"
+)
+
+var (
+	quickCheck, repair, forceRepair bool
+	maximumUnreferencedLayerAge     string
+)
+
+func check(flags *mflag.FlagSet, action string, m storage.Store, args []string) (int, error) {
+	if forceRepair {
+		repair = true
+	}
+	defer func() {
+		if _, err := m.Shutdown(true); err != nil {
+			fmt.Fprintf(os.Stderr, "shutdown: %v\n", err)
+		}
+	}()
+	checkOptions := storage.CheckEverything()
+	if quickCheck {
+		checkOptions = storage.CheckMost()
+	}
+	if maximumUnreferencedLayerAge != "" {
+		age, err := time.ParseDuration(maximumUnreferencedLayerAge)
+		if err != nil {
+			return 1, err
+		}
+		checkOptions.LayerUnreferencedMaximumAge = &age
+	}
+	report, err := m.Check(checkOptions)
+	if err != nil {
+		return 1, err
+	}
+	outputNonJSON := func(report storage.CheckReport) {
+		for id, errs := range report.Layers {
+			if len(errs) > 0 {
+				fmt.Fprintf(os.Stdout, "layer %s:\n", id)
+			}
+			for _, err := range errs {
+				fmt.Fprintf(os.Stdout, " %v\n", err)
+			}
+		}
+		for id, errs := range report.ROLayers {
+			if len(errs) > 0 {
+				fmt.Fprintf(os.Stdout, "read-only layer %s:\n", id)
+			}
+			for _, err := range errs {
+				fmt.Fprintf(os.Stdout, " %v\n", err)
+			}
+		}
+		for id, errs := range report.Images {
+			if len(errs) > 0 {
+				fmt.Fprintf(os.Stdout, "image %s:\n", id)
+			}
+			for _, err := range errs {
+				fmt.Fprintf(os.Stdout, " %v\n", err)
+			}
+		}
+		for id, errs := range report.ROImages {
+			if len(errs) > 0 {
+				fmt.Fprintf(os.Stdout, "read-only image %s:\n", id)
+			}
+			for _, err := range errs {
+				fmt.Fprintf(os.Stdout, " %v\n", err)
+			}
+		}
+		for id, errs := range report.Containers {
+			if len(errs) > 0 {
+				fmt.Fprintf(os.Stdout, "container %s:\n", id)
+			}
+			for _, err := range errs {
+				fmt.Fprintf(os.Stdout, " %v\n", err)
+			}
+		}
+	}
+
+	if jsonOutput {
+		if err := json.NewEncoder(os.Stdout).Encode(report); err != nil {
+			return 1, err
+		}
+	} else {
+		outputNonJSON(report)
+	}
+
+	if !repair {
+		if len(report.Layers) > 0 || len(report.ROLayers) > 0 || len(report.Images) > 0 || len(report.ROImages) > 0 || len(report.Containers) > 0 {
+			return 1, fmt.Errorf("%d layer errors, %d read-only layer errors, %d image errors, %d read-only image errors, %d container errors", len(report.Layers), len(report.ROLayers), len(report.Images), len(report.ROImages), len(report.Containers))
+		}
+	} else {
+		options := storage.RepairOptions{
+			RemoveContainers: forceRepair,
+		}
+		if errs := m.Repair(report, &options); len(errs) != 0 {
+			if jsonOutput {
+				if err := json.NewEncoder(os.Stdout).Encode(errs); err != nil {
+					return 1, err
+				}
+			} else {
+				for _, err := range errs {
+					fmt.Fprintf(os.Stderr, "%v\n", err)
+				}
+			}
+			return 1, errs[0]
+		}
+		if len(report.ROLayers) > 0 || len(report.ROImages) > 0 || (!options.RemoveContainers && len(report.Containers) > 0) {
+			var err error
+			if options.RemoveContainers {
+				err = fmt.Errorf("%d read-only layer errors, %d read-only image errors", len(report.ROLayers), len(report.ROImages))
+			} else {
+				err = fmt.Errorf("%d read-only layer errors, %d read-only image errors, %d container errors", len(report.ROLayers), len(report.ROImages), len(report.Containers))
+			}
+			return 1, err
+		}
+	}
+	return 0, nil
+}
+
+func init() {
+	commands = append(commands, command{
+		names:   []string{"check"},
+		usage:   "Check storage consistency",
+		minArgs: 0,
+		maxArgs: 0,
+		action:  check,
+		addFlags: func(flags *mflag.FlagSet, cmd *command) {
+			flags.BoolVar(&jsonOutput, []string{"-json", "j"}, jsonOutput, "Prefer JSON output")
+			flags.StringVar(&maximumUnreferencedLayerAge, []string{"-max", "m"}, "24h", "Maximum allowed age for unreferenced layers")
+			flags.BoolVar(&repair, []string{"-repair", "r"}, repair, "Remove damaged images and layers")
+			flags.BoolVar(&forceRepair, []string{"-force", "f"}, forceRepair, "Remove damaged containers")
+			flags.BoolVar(&quickCheck, []string{"-quick", "q"}, quickCheck, "Perform only quick checks")
+		},
+	})
+}

docs/containers-storage-check.md

@@ -0,0 +1,34 @@
+## containers-storage-check 1 "September 2022"
+
+## NAME
+containers-storage check - Check for and remove damaged layers/images/containers
+
+## SYNOPSIS
+**containers-storage** **check** [-q] [-r [-f]]
+
+## DESCRIPTION
+Checks layers, images, and containers for identifiable damage.
+
+## OPTIONS
+
+**-f**
+
+When repairing damage, also remove damaged containers.  No effect unless *-r*
+is used.
+
+**-r**
+
+Attempt to repair damage by removing damaged images and layers.  If not
+specified, damage is reported but not acted upon.
+
+**-q**
+
+Perform only checks which are not expected to be time-consuming.  This
+currently skips verifying that a layer which was initialized using a diff can
+reproduce that diff if asked to.
+
+## EXAMPLE
+**containers-storage check -r -f
+
+## SEE ALSO
+containers-storage(1)

docs/containers-storage.md

@@ -58,6 +58,8 @@ The *containers-storage* command's features are broken down into several subcomm
 
  **containers-storage changes(1)**             Compare two layers
 
+ **containers-storage check(1)**               Check for and possibly remove damaged layers/images/containers
+
  **containers-storage container(1)**           Examine a container
 
  **containers-storage containers(1)**          List containers

drivers/driver.go

@@ -111,6 +111,10 @@ type ProtoDriver interface {
 	Exists(id string) bool
 	// Returns a list of layer ids that exist on this driver (does not include
 	// additional storage layers). Not supported by all backends.
+	// If the driver requires that layers be removed in a particular order,
+	// usually due to parent-child relationships that it cares about, The
+	// list should be sorted well enough so that if all layers need to be
+	// removed, they can be removed in the order in which they're returned.
 	ListLayers() ([]string, error)
 	// Status returns a set of key-value pairs which give low
 	// level diagnostic status about this driver.

store.go

@@ -520,6 +520,13 @@ type Store interface {
 	// references in the json files. These can happen in the case of unclean
 	// shutdowns or regular restarts in transient store mode.
 	GarbageCollect() error
+
+	// Check returns a report of things that look wrong in the store.
+	Check(options *CheckOptions) (CheckReport, error)
+	// Repair attempts to remediate problems mentioned in the CheckReport,
+	// usually by deleting layers and images which are damaged.  If the
+	// right options are set, it will remove containers as well.
+	Repair(report CheckReport, options *RepairOptions) []error
 }
 
 // AdditionalLayer represents a layer that is contained in the additional layer store
@@ -1081,7 +1088,7 @@ func (s *store) bothLayerStoreKindsLocked() (rwLayerStore, []roLayerStore, error
 }
 
 // bothLayerStoreKinds returns the primary, and additional read-only, layer store objects used by the store.
-// It must be called with s.graphLock held.
+// It must be called WITHOUT s.graphLock held.
 func (s *store) bothLayerStoreKinds() (rwLayerStore, []roLayerStore, error) {
 	if err := s.startUsingGraphDriver(); err != nil {
 		return nil, nil, err
@@ -1204,7 +1211,7 @@ func (s *store) readAllImageStores(fn func(store roImageStore) (bool, error)) (b
 	return false, nil
 }
 
-// writeToImageStore is a convenience helper for working with store.getImageStore():
+// writeToImageStore is a convenience helper for working with store.imageStore:
 // It locks the store for writing, checks for updates, and calls fn(), which can then access store.imageStore.
 // It returns the return value of fn, or its own error initializing the store.
 func (s *store) writeToImageStore(fn func() error) error {
@@ -1215,7 +1222,18 @@ func (s *store) writeToImageStore(fn func() error) error {
 	return fn()
 }
 
-// writeToContainerStore is a convenience helper for working with store.getContainerStore():
+// readContainerStore is a convenience helper for working with store.containerStore:
+// It locks the store for reading, checks for updates, and calls fn(), which can then access store.containerStore.
+// It returns the return value of fn, or its own error initializing the store.
+func (s *store) readContainerStore(fn func() (bool, error)) (bool, error) {
+	if err := s.containerStore.startReading(); err != nil {
+		return true, err
+	}
+	defer s.containerStore.stopReading()
+	return fn()
+}
+
+// writeToContainerStore is a convenience helper for working with store.containerStore:
 // It locks the store for writing, checks for updates, and calls fn(), which can then access store.containerStore.
 // It returns the return value of fn, or its own error initializing the store.
 func (s *store) writeToContainerStore(fn func() error) error {
@@ -1809,13 +1827,17 @@ func (s *store) Metadata(id string) (string, error) {
 		return res, err
 	}
 
-	if err := s.containerStore.startReading(); err != nil {
-		return "", err
-	}
-	defer s.containerStore.stopReading()
-	if s.containerStore.Exists(id) {
-		return s.containerStore.Metadata(id)
+	if done, err := s.readContainerStore(func() (bool, error) {
+		if s.containerStore.Exists(id) {
+			var err error
+			res, err = s.containerStore.Metadata(id)
+			return true, err
+		}
+		return false, nil
+	}); done {
+		return res, err
 	}
+
 	return "", ErrNotAnID
 }