Merge pull request #1569 from nalind/check

Initial implementation of consistency checks

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
 }