Merge pull request #20381 from vrothberg/fix-20375

image history: fix walking layers

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/containernetworking/cni v1.1.2
 	github.com/containernetworking/plugins v1.3.0
 	github.com/containers/buildah v1.32.1-0.20231016164031-ade05159a485
-	github.com/containers/common v0.56.1-0.20231013064012-9f2f68b89872
+	github.com/containers/common v0.56.1-0.20231017183641-80fb777f79e5
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/gvisor-tap-vsock v0.7.1
 	github.com/containers/image/v5 v5.28.0

go.sum

@@ -251,8 +251,8 @@ github.com/containernetworking/plugins v1.3.0 h1:QVNXMT6XloyMUoO2wUOqWTC1hWFV62Q
 github.com/containernetworking/plugins v1.3.0/go.mod h1:Pc2wcedTQQCVuROOOaLBPPxrEXqqXBFt3cZ+/yVg6l0=
 github.com/containers/buildah v1.32.1-0.20231016164031-ade05159a485 h1:RqgxHW2iP5QJ3aRahT+KGI2aGXVZeZHTeulmeZQV0y0=
 github.com/containers/buildah v1.32.1-0.20231016164031-ade05159a485/go.mod h1:gOMfotERP5Gz2pN+AnuM3ephId/YL9DmbOtVck6fWfE=
-github.com/containers/common v0.56.1-0.20231013064012-9f2f68b89872 h1:8dydB0ivba6JVt04WuJmTlznJTGZNyQePU8aHqxLTFI=
+github.com/containers/common v0.56.1-0.20231017183641-80fb777f79e5 h1:1y1a9x5eG+7E2yzb/KMGLg44xJQoFQExfSfIHW63EZ0=
-github.com/containers/common v0.56.1-0.20231013064012-9f2f68b89872/go.mod h1:LM6Uyz5lq80P/DRnhs8NxvPIvBk2zmS2L/oednAGI/s=
+github.com/containers/common v0.56.1-0.20231017183641-80fb777f79e5/go.mod h1:LM6Uyz5lq80P/DRnhs8NxvPIvBk2zmS2L/oednAGI/s=
 github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
 github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
 github.com/containers/gvisor-tap-vsock v0.7.1 h1:+Rc+sOPplrkQb/BUXeN0ug8TxjgyrIqo/9P/eNS2A4c=

test/e2e/history_test.go

@@ -63,6 +63,11 @@ var _ = Describe("Podman history", func() {
 		Expect(session.OutputToString()).ToNot(ContainSubstring("..."))
 		// the second line in the alpine history contains a command longer than 45 chars
 		Expect(len(lines[1])).To(BeNumerically(">", 45))
+
+		session = podmanTest.Podman([]string{"history", "--no-trunc", "--format", "{{.Size}}", ALPINE})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+		Expect(session.OutputToStringArray()).To(Equal([]string{"0B", "5.84MB"}))
 	})
 
 	It("podman history with json flag", func() {

vendor/github.com/containers/common/libimage/history.go

@@ -2,9 +2,8 @@ package libimage
 
 import (
 	"context"
+	"fmt"
 	"time"
-
-	"github.com/containers/storage"
 )
 
 // ImageHistory contains the history information of an image.
@@ -29,17 +28,19 @@ func (i *Image) History(ctx context.Context) ([]ImageHistory, error) {
 		return nil, err
 	}
 
-	var allHistory []ImageHistory
+	var nextNode *layerNode
-	var layer *storage.Layer
 	if i.TopLayer() != "" {
-		layer, err = i.runtime.store.Layer(i.TopLayer())
+		layer, err := i.runtime.store.Layer(i.TopLayer())
 		if err != nil {
 			return nil, err
 		}
+		nextNode = layerTree.node(layer.ID)
 	}
 
 	// Iterate in reverse order over the history entries, and lookup the
-	// corresponding image ID, size and get the next later if needed.
+	// corresponding image ID, size.  If it's a non-empty history entry,
+	// pick the next "storage" layer by walking the layer tree.
+	var allHistory []ImageHistory
 	numHistories := len(ociImage.History) - 1
 	usedIDs := make(map[string]bool) // prevents assigning images IDs more than once
 	for x := numHistories; x >= 0; x-- {
@@ -50,31 +51,25 @@ func (i *Image) History(ctx context.Context) ([]ImageHistory, error) {
 			Comment:   ociImage.History[x].Comment,
 		}
 
-		if layer != nil {
+		if nextNode != nil && len(nextNode.images) > 0 {
-			if !ociImage.History[x].EmptyLayer {
+			id := nextNode.images[0].ID() // always use the first one
-				history.Size = layer.UncompressedSize
-			}
-			// Query the layer tree if it's the top layer of an
-			// image.
-			node := layerTree.node(layer.ID)
-			if len(node.images) > 0 {
-				id := node.images[0].ID() // always use the first one
 			if _, used := usedIDs[id]; !used {
 				history.ID = id
 				usedIDs[id] = true
 			}
-				for i := range node.images {
+			for i := range nextNode.images {
-					history.Tags = append(history.Tags, node.images[i].Names()...)
+				history.Tags = append(history.Tags, nextNode.images[i].Names()...)
 			}
 		}
-			if layer.Parent == "" {
+
-				layer = nil
+		if !ociImage.History[x].EmptyLayer {
-			} else if !ociImage.History[x].EmptyLayer {
+			if nextNode == nil { // If no layer's left, something's wrong.
-				layer, err = i.runtime.store.Layer(layer.Parent)
+				return nil, fmt.Errorf("no layer left for non-empty history entry: %v", history)
-				if err != nil {
-					return nil, err
-				}
 			}
+
+			history.Size = nextNode.layer.UncompressedSize
+
+			nextNode = nextNode.parent
 		}
 
 		allHistory = append(allHistory, history)

vendor/modules.txt

@@ -167,7 +167,7 @@ github.com/containers/buildah/pkg/sshagent
 github.com/containers/buildah/pkg/util
 github.com/containers/buildah/pkg/volumes
 github.com/containers/buildah/util
-# github.com/containers/common v0.56.1-0.20231013064012-9f2f68b89872
+# github.com/containers/common v0.56.1-0.20231017183641-80fb777f79e5
 ## explicit; go 1.18
 github.com/containers/common/libimage
 github.com/containers/common/libimage/define