From baebe0edc671bd3f2e8e18513551df126d025186 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 18 Oct 2022 11:11:26 +0200 Subject: [PATCH] DiskUsage: return total images size It turns out only counting the layers size is not sufficient for `podman systemd df` as it excludes the size of the manifests, configs and potentially attached meta data in the storage. Instead, sum the image sizes but distract redundant layers. That indeed gives the expected result and does not yield negative results for reclaimable space. Remove the unrelease LayersDiskUsage function again to avoid expensive recalculation of the layer tree. We are still below 1.0, and I am convinced the total image size belongs into DiskUsage. NOTE: The DiskUsage function does not have test coverage in libimage. This should be addressed at some point but in the interest of time I leverage podman's system tests. Signed-off-by: Valentin Rothberg --- libimage/corrupted_test.go | 6 ++--- libimage/disk_usage.go | 50 +++++++++++++++++++++++--------------- libimage/image.go | 1 + libimage/layer_tree.go | 11 +++++++++ 4 files changed, 44 insertions(+), 24 deletions(-) diff --git a/libimage/corrupted_test.go b/libimage/corrupted_test.go index ed631017c..eb7291d37 100644 --- a/libimage/corrupted_test.go +++ b/libimage/corrupted_test.go @@ -40,10 +40,8 @@ func TestCorruptedLayers(t *testing.T) { require.True(t, exists, "healthy image exists") // Disk usage works. - _, err = runtime.DiskUsage(ctx) + _, _, err = runtime.DiskUsage(ctx) require.NoError(t, err, "disk usage works on healthy image") - _, err = runtime.LayersDiskUsage(ctx) - require.NoError(t, err, "layers 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 @@ -77,7 +75,7 @@ func TestCorruptedLayers(t *testing.T) { require.False(t, exists, "corrupted image should not be marked to exist") // Disk usage does not work. - _, err = runtime.DiskUsage(ctx) + _, _, 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") diff --git a/libimage/disk_usage.go b/libimage/disk_usage.go index 0bff29c0c..431642f5d 100644 --- a/libimage/disk_usage.go +++ b/libimage/disk_usage.go @@ -5,21 +5,6 @@ import ( "time" ) -// LayersDiskUsage returns the sum of the size of all layers in the current store. -func (r *Runtime) LayersDiskUsage(ctx context.Context) (int64, error) { - layers, err := r.store.Layers() - if err != nil { - return -1, err - } - - var size int64 - for _, l := range layers { - size += l.UncompressedSize - } - - return size, nil -} - // ImageDiskUsage reports the total size of an image. That is the size type ImageDiskUsage struct { // Number of containers using the image. @@ -43,26 +28,51 @@ type ImageDiskUsage struct { // DiskUsage calculates the disk usage for each image in the local containers // storage. Note that a single image may yield multiple usage reports, one for // each repository tag. -func (r *Runtime) DiskUsage(ctx context.Context) ([]ImageDiskUsage, error) { +func (r *Runtime) DiskUsage(ctx context.Context) ([]ImageDiskUsage, int64, error) { layerTree, err := r.layerTree() if err != nil { - return nil, err + return nil, -1, err } images, err := r.ListImages(ctx, nil, nil) if err != nil { - return nil, err + return nil, -1, err } + var totalSize int64 + visitedImages := make(map[string]bool) + visistedLayers := make(map[string]bool) + var allUsages []ImageDiskUsage for _, image := range images { usages, err := diskUsageForImage(ctx, image, layerTree) if err != nil { - return nil, err + return nil, -1, err } allUsages = append(allUsages, usages...) + + if _, ok := visitedImages[image.ID()]; ok { + // Do not count an image twice + continue + } + visitedImages[image.ID()] = true + + size, err := image.Size() + if err != nil { + return nil, -1, err + } + for _, layer := range layerTree.layersOf(image) { + if _, ok := visistedLayers[layer.ID]; ok { + // Do not count a layer twice, so remove its + // size from the image size. + size -= layer.UncompressedSize + continue + } + visistedLayers[layer.ID] = true + } + totalSize += size } - return allUsages, err + return allUsages, totalSize, err } // diskUsageForImage returns the disk-usage baseistics for the specified image. diff --git a/libimage/image.go b/libimage/image.go index 0130532c2..505266073 100644 --- a/libimage/image.go +++ b/libimage/image.go @@ -775,6 +775,7 @@ func (i *Image) Unmount(force bool) error { // Size computes the size of the image layers and associated data. func (i *Image) Size() (int64, error) { + // TODO: cache the result to optimize performance of subsequent calls return i.runtime.store.ImageSize(i.ID()) } diff --git a/libimage/layer_tree.go b/libimage/layer_tree.go index 05f21531b..8c84dc41f 100644 --- a/libimage/layer_tree.go +++ b/libimage/layer_tree.go @@ -126,6 +126,17 @@ func (r *Runtime) layerTree() (*layerTree, error) { return &tree, nil } +// layersOf returns all storage layers of the specified image. +func (t *layerTree) layersOf(image *Image) []*storage.Layer { + var layers []*storage.Layer + node := t.node(image.TopLayer()) + for node != nil { + layers = append(layers, node.layer) + node = node.parent + } + return layers +} + // children returns the child images of parent. Child images are images with // either the same top layer as parent or parent being the true parent layer. // Furthermore, the history of the parent and child images must match with the