From 9d26abc8889d930113e443c5076dc81495c3e9d1 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 31 Aug 2021 10:57:11 +0200 Subject: [PATCH 1/3] libimage: disk usage: catch corrupted images Make sure to check an image for corruption before running disk usage on it. Such checks are already applied on various execution paths but not yet on disk usage. Further update the corrupted-image error to include that the image should be removed to resolve the error. This should ultimately guide users to resolve the issue. Fixes: #751 Signed-off-by: Valentin Rothberg --- libimage/corrupted_test.go | 11 ++++++++++- libimage/disk_usage.go | 4 ++++ libimage/image.go | 5 ++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/libimage/corrupted_test.go b/libimage/corrupted_test.go index e683c1fa3..614072837 100644 --- a/libimage/corrupted_test.go +++ b/libimage/corrupted_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestCorruptedImage(t *testing.T) { +func TestCorruptedLayers(t *testing.T) { // Regression tests for https://bugzilla.redhat.com/show_bug.cgi?id=1966872. runtime, cleanup := testNewRuntime(t) defer cleanup() @@ -41,6 +41,10 @@ func TestCorruptedImage(t *testing.T) { require.NoError(t, err, "healthy image exists") require.True(t, exists, "healthy image exists") + // Disk usage works. + _, err = runtime.DiskUsage(ctx) + require.NoError(t, err, "disk usage works on healthy image") + // Now remove one layer from the layers.json index in the storage. The // image will still be listed in the container storage but attempting // to use it will yield "layer not known" errors. @@ -71,6 +75,11 @@ func TestCorruptedImage(t *testing.T) { require.NoError(t, err, "corrupted image exists should not fail") require.False(t, exists, "corrupted image should not be marked to exist") + // Disk usage does not work. + _, err = runtime.DiskUsage(ctx) + require.Error(t, err, "disk usage does not work on corrupted image") + require.Contains(t, err.Error(), "exists in local storage but may be corrupted", "disk usage reports corrupted image") + // Now make sure that pull will detect the corrupted image and repulls // if needed which will repair the data corruption. pulledImages, err = runtime.Pull(ctx, imageName, config.PullPolicyNewer, pullOptions) diff --git a/libimage/disk_usage.go b/libimage/disk_usage.go index edfd095a0..2cde09846 100644 --- a/libimage/disk_usage.go +++ b/libimage/disk_usage.go @@ -52,6 +52,10 @@ func (r *Runtime) DiskUsage(ctx context.Context) ([]ImageDiskUsage, error) { // diskUsageForImage returns the disk-usage baseistics for the specified image. func diskUsageForImage(ctx context.Context, image *Image, tree *layerTree) ([]ImageDiskUsage, error) { + if err := image.isCorrupted(""); err != nil { + return nil, err + } + base := ImageDiskUsage{ ID: image.ID(), Created: image.Created(), diff --git a/libimage/image.go b/libimage/image.go index c47e63339..81442e5b8 100644 --- a/libimage/image.go +++ b/libimage/image.go @@ -74,7 +74,10 @@ func (i *Image) isCorrupted(name string) error { } if _, err := ref.NewImage(context.Background(), nil); err != nil { - return errors.Errorf("Image %s exists in local storage but may be corrupted: %v", name, err) + if name == "" { + name = i.ID()[:12] + } + return errors.Errorf("Image %s exists in local storage but may be corrupted (remove the image to resolve the issue): %v", name, err) } return nil } From ad7d21a165c221cff28367575ffc7b9e0e8d12dc Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 1 Sep 2021 15:54:03 -0400 Subject: [PATCH 2/3] Switch default Rootless Networking to "CNI" for OSX This should better support rootless CNI usescases. Fixes https://github.com/containers/podman/issues/11396 Signed-off-by: Matthew Heon --- pkg/config/config.go | 2 +- pkg/config/default.go | 6 +----- pkg/config/default_linux.go | 6 ++++++ pkg/config/default_unsupported.go | 6 ++++++ 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 008cfb642..38743aad0 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -167,7 +167,7 @@ type ContainersConfig struct { // RootlessNetworking depicts the "kind" of networking for rootless // containers. Valid options are `slirp4netns` and `cni`. Default is - // `slirp4netns` + // `slirp4netns` on Linux, and `cni` on non-Linux OSes. RootlessNetworking string `toml:"rootless_networking,omitempty"` // SeccompProfile is the seccomp.json profile path which is used as the diff --git a/pkg/config/default.go b/pkg/config/default.go index a16dd0e02..304a99207 100644 --- a/pkg/config/default.go +++ b/pkg/config/default.go @@ -82,10 +82,6 @@ var ( "/usr/local/lib/cni", "/opt/cni/bin", } - - // DefaultRootlessNetwork is the kind of of rootless networking - // for containers - DefaultRootlessNetwork = "slirp4netns" ) const ( @@ -197,7 +193,7 @@ func DefaultConfig() (*Config, error) { NoHosts: false, PidsLimit: DefaultPidsLimit, PidNS: "private", - RootlessNetworking: DefaultRootlessNetwork, + RootlessNetworking: getDefaultRootlessNetwork(), ShmSize: DefaultShmSize, TZ: "", Umask: "0022", diff --git a/pkg/config/default_linux.go b/pkg/config/default_linux.go index f61d9ba54..c68c0b130 100644 --- a/pkg/config/default_linux.go +++ b/pkg/config/default_linux.go @@ -13,6 +13,12 @@ const ( oldMaxSize = uint64(1048576) ) +// getDefaultRootlessNetwork returns the default rootless network configuration. +// It is "slirp4netns" for Linux. +func getDefaultRootlessNetwork() string { + return "slirp4netns" +} + // getDefaultProcessLimits returns the nproc for the current process in ulimits format // Note that nfile sometimes cannot be set to unlimited, and the limit is hardcoded // to (oldMaxSize) 1048576 (2^20), see: http://stackoverflow.com/a/1213069/1811501 diff --git a/pkg/config/default_unsupported.go b/pkg/config/default_unsupported.go index 1ae1dd12c..e38fb810d 100644 --- a/pkg/config/default_unsupported.go +++ b/pkg/config/default_unsupported.go @@ -2,6 +2,12 @@ package config +// getDefaultRootlessNetwork returns the default rootless network configuration. +// It is "cni" for non-Linux OSes (to better support `podman-machine` usecases). +func getDefaultRootlessNetwork() string { + return "cni" +} + // isCgroup2UnifiedMode returns whether we are running in cgroup2 mode. func isCgroup2UnifiedMode() (isUnified bool, isUnifiedErr error) { return false, nil From 211dda5054a514c1ff05dd491266ab9d61a46f5a Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 8 Sep 2021 14:22:05 +0200 Subject: [PATCH 3/3] Fix the fallback runtime path Podman should not use `/tmp/run-...`. The Podman PR#8241 changed the path to `/tmp/podman-run-...` and added systemd tmpfile config to make sure the path is not removed. However the tmpDir is set in c/common and was never changed. Fixes containers/podman#11478 Signed-off-by: Paul Holzinger --- pkg/config/util_supported.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/util_supported.go b/pkg/config/util_supported.go index 417e3a375..33e4a9e8f 100644 --- a/pkg/config/util_supported.go +++ b/pkg/config/util_supported.go @@ -48,7 +48,7 @@ func getRuntimeDir() (string, error) { } } if runtimeDir == "" { - tmpDir := filepath.Join(os.TempDir(), fmt.Sprintf("run-%s", uid)) + tmpDir := filepath.Join(os.TempDir(), fmt.Sprintf("podman-run-%s", uid)) if err := os.MkdirAll(tmpDir, 0700); err != nil { logrus.Debugf("unable to make temp dir %v", err) }