-
Notifications
You must be signed in to change notification settings - Fork 234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial implementation of consistency checks #1569
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If all the report entries have this regular structure, a helper vaguely like |
||
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") | ||
Comment on lines
+134
to
+135
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These flags are not documented; should they be? At least the latter one; |
||
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") | ||
}, | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The pattern of returning Let me dust off a Go 1.18 generics-based update to these helpers, that should make things nicer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
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 | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I’m not sure what’s the purpose of this nested function — it seems to me it could be just open-coded inside that
if
below, or an external function. But this works just fine.)