image: rework parent/child/history matching

Rework our expectations of how images that are derived from each other
look, so that we don't assume that an image that's derived from a base
image always adds layers relative to that base image.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

2 files changed, 197 insertions, 34 deletions

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 588809bbc..0b0002d42 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -679,7 +679,8 @@ type History struct {
 	Comment   string     `json:"comment"`
 }
 
-// History gets the history of an image and information about its layers
+// History gets the history of an image and the IDs of images that are part of
+// its history
 func (i *Image) History(ctx context.Context) ([]*History, error) {
 	img, err := i.toImageRef(ctx)
 	if err != nil {
@@ -690,31 +691,92 @@ func (i *Image) History(ctx context.Context) ([]*History, error) {
 		return nil, err
 	}
 
-	// Get the IDs of the images making up the history layers
-	// if the images exist locally in the store
+	// Use our layers list to find images that use one of them as its
+	// topmost layer.
+	interestingLayers := make(map[string]bool)
+	layer, err := i.imageruntime.store.Layer(i.TopLayer())
+	if err != nil {
+		return nil, err
+	}
+	for layer != nil {
+		interestingLayers[layer.ID] = true
+		if layer.Parent == "" {
+			break
+		}
+		layer, err = i.imageruntime.store.Layer(layer.Parent)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	// Get the IDs of the images that share some of our layers.  Hopefully
+	// this step means that we'll be able to avoid reading the
+	// configuration of every single image in local storage later on.
 	images, err := i.imageruntime.GetImages()
 	if err != nil {
 		return nil, errors.Wrapf(err, "error getting images from store")
 	}
-	imageIDs := []string{i.ID()}
-	if err := i.historyLayerIDs(i.TopLayer(), images, &imageIDs); err != nil {
-		return nil, errors.Wrap(err, "error getting image IDs for layers in history")
+	interestingImages := make([]*Image, 0, len(images))
+	for i := range images {
+		if interestingLayers[images[i].TopLayer()] {
+			interestingImages = append(interestingImages, images[i])
+		}
+	}
+
+	// Build a list of image IDs that correspond to our history entries.
+	historyImages := make([]*Image, len(oci.History))
+	if len(oci.History) > 0 {
+		// The starting image shares its whole history with itself.
+		historyImages[len(historyImages)-1] = i
+		for i := range interestingImages {
+			image, err := images[i].ociv1Image(ctx)
+			if err != nil {
+				return nil, errors.Wrapf(err, "error getting image configuration for image %q", images[i].ID())
+			}
+			// If the candidate has a longer history or no history
+			// at all, then it doesn't share the portion of our
+			// history that we're interested in matching with other
+			// images.
+			if len(image.History) == 0 || len(image.History) > len(historyImages) {
+				continue
+			}
+			// If we don't include all of the layers that the
+			// candidate image does (i.e., our rootfs didn't look
+			// like its rootfs at any point), then it can't be part
+			// of our history.
+			if len(image.RootFS.DiffIDs) > len(oci.RootFS.DiffIDs) {
+				continue
+			}
+			candidateLayersAreUsed := true
+			for i := range image.RootFS.DiffIDs {
+				if image.RootFS.DiffIDs[i] != oci.RootFS.DiffIDs[i] {
+					candidateLayersAreUsed = false
+					break
+				}
+			}
+			if !candidateLayersAreUsed {
+				continue
+			}
+			// If the candidate's entire history is an initial
+			// portion of our history, then we're based on it,
+			// either directly or indirectly.
+			sharedHistory := historiesMatch(oci.History, image.History)
+			if sharedHistory == len(image.History) {
+				historyImages[sharedHistory-1] = images[i]
+			}
+		}
 	}
 
 	var (
-		imageID    string
-		imgIDCount = 0
 		size       int64
 		sizeCount  = 1
 		allHistory []*History
 	)
 
 	for i := len(oci.History) - 1; i >= 0; i-- {
-		if imgIDCount < len(imageIDs) {
-			imageID = imageIDs[imgIDCount]
-			imgIDCount++
-		} else {
-			imageID = "<missing>"
+		imageID := "<missing>"
+		if historyImages[i] != nil {
+			imageID = historyImages[i].ID()
 		}
 		if !oci.History[i].EmptyLayer {
 			size = img.LayerInfos()[len(img.LayerInfos())-sizeCount].Size
@@ -1007,25 +1069,109 @@ func splitString(input string) string {
 // the parent of any other layer in store. Double check that image with that
 // layer exists as well.
 func (i *Image) IsParent(ctx context.Context) (bool, error) {
-	children, err := i.GetChildren(ctx)
+	children, err := i.getChildren(ctx, 1)
 	if err != nil {
 		return false, err
 	}
 	return len(children) > 0, nil
 }
 
+// historiesMatch returns the number of entries in the histories which have the
+// same contents
+func historiesMatch(a, b []imgspecv1.History) int {
+	i := 0
+	for i < len(a) && i < len(b) {
+		if a[i].Created != nil && b[i].Created == nil {
+			return i
+		}
+		if a[i].Created == nil && b[i].Created != nil {
+			return i
+		}
+		if a[i].Created != nil && b[i].Created != nil {
+			if !a[i].Created.Equal(*(b[i].Created)) {
+				return i
+			}
+		}
+		if a[i].CreatedBy != b[i].CreatedBy {
+			return i
+		}
+		if a[i].Author != b[i].Author {
+			return i
+		}
+		if a[i].Comment != b[i].Comment {
+			return i
+		}
+		if a[i].EmptyLayer != b[i].EmptyLayer {
+			return i
+		}
+		i++
+	}
+	return i
+}
+
+// areParentAndChild checks diff ID and history in the two images and return
+// true if the second should be considered to be directly based on the first
+func areParentAndChild(parent, child *imgspecv1.Image) bool {
+	// the child and candidate parent should share all of the
+	// candidate parent's diff IDs, which together would have
+	// controlled which layers were used
+	if len(parent.RootFS.DiffIDs) > len(child.RootFS.DiffIDs) {
+		return false
+	}
+	childUsesCandidateDiffs := true
+	for i := range parent.RootFS.DiffIDs {
+		if child.RootFS.DiffIDs[i] != parent.RootFS.DiffIDs[i] {
+			childUsesCandidateDiffs = false
+			break
+		}
+	}
+	if !childUsesCandidateDiffs {
+		return false
+	}
+	// the child should have the same history as the parent, plus
+	// one more entry
+	if len(parent.History)+1 != len(child.History) {
+		return false
+	}
+	if historiesMatch(parent.History, child.History) != len(parent.History) {
+		return false
+	}
+	return true
+}
+
 // GetParent returns the image ID of the parent. Return nil if a parent is not found.
 func (i *Image) GetParent(ctx context.Context) (*Image, error) {
 	images, err := i.imageruntime.GetImages()
 	if err != nil {
 		return nil, err
 	}
-	layer, err := i.imageruntime.store.Layer(i.TopLayer())
+	childLayer, err := i.imageruntime.store.Layer(i.TopLayer())
+	if err != nil {
+		return nil, err
+	}
+	// fetch the configuration for the child image
+	child, err := i.ociv1Image(ctx)
 	if err != nil {
 		return nil, err
 	}
 	for _, img := range images {
-		if img.TopLayer() == layer.Parent {
+		if img.ID() == i.ID() {
+			continue
+		}
+		candidateLayer := img.TopLayer()
+		// as a child, our top layer is either the candidate parent's
+		// layer, or one that's derived from it, so skip over any
+		// candidate image where we know that isn't the case
+		if candidateLayer != childLayer.Parent && candidateLayer != childLayer.ID {
+			continue
+		}
+		// fetch the configuration for the candidate image
+		candidate, err := img.ociv1Image(ctx)
+		if err != nil {
+			return nil, err
+		}
+		// compare them
+		if areParentAndChild(candidate, child) {
 			return img, nil
 		}
 	}
@@ -1034,35 +1180,52 @@ func (i *Image) GetParent(ctx context.Context) (*Image, error) {
 
 // GetChildren returns a list of the imageIDs that depend on the image
 func (i *Image) GetChildren(ctx context.Context) ([]string, error) {
+	return i.getChildren(ctx, 0)
+}
+
+// getChildren returns a list of at most "max" imageIDs that depend on the image
+func (i *Image) getChildren(ctx context.Context, max int) ([]string, error) {
 	var children []string
 	images, err := i.imageruntime.GetImages()
 	if err != nil {
 		return nil, err
 	}
-	layers, err := i.imageruntime.store.Layers()
+
+	// fetch the configuration for the parent image
+	parent, err := i.ociv1Image(ctx)
 	if err != nil {
 		return nil, err
 	}
+	parentLayer := i.TopLayer()
 
-	for _, layer := range layers {
-		if layer.Parent == i.TopLayer() {
-			if imageID := getImageOfTopLayer(images, layer.ID); len(imageID) > 0 {
-				children = append(children, imageID...)
-			}
-		}
-	}
-	return children, nil
-}
-
-// getImageOfTopLayer returns the image ID where layer is the top layer of the image
-func getImageOfTopLayer(images []*Image, layer string) []string {
-	var matches []string
 	for _, img := range images {
-		if img.TopLayer() == layer {
-			matches = append(matches, img.ID())
+		if img.ID() == i.ID() {
+			continue
+		}
+		candidateLayer, err := img.Layer()
+		if err != nil {
+			return nil, err
+		}
+		// if this image's top layer is not our top layer, and is not
+		// based on our top layer, we can skip it
+		if candidateLayer.Parent != parentLayer && candidateLayer.ID != parentLayer {
+			continue
+		}
+		// fetch the configuration for the candidate image
+		candidate, err := img.ociv1Image(ctx)
+		if err != nil {
+			return nil, err
+		}
+		// compare them
+		if areParentAndChild(parent, candidate) {
+			children = append(children, img.ID())
+		}
+		// if we're not building an exhaustive list, maybe we're done?
+		if max > 0 && len(children) >= max {
+			break
 		}
 	}
-	return matches
+	return children, nil
 }
 
 // InputIsID returns a bool if the user input for an image
diff --git a/libpod/image/image_test.go b/libpod/image/image_test.go
index 075ba119d..d917e3205 100644
--- a/libpod/image/image_test.go
+++ b/libpod/image/image_test.go
@@ -139,7 +139,7 @@ func TestImage_New(t *testing.T) {
 		newImage, err := ir.New(context.Background(), img, "", "", writer, nil, SigningOptions{}, false, nil)
 		assert.NoError(t, err)
 		assert.NotEqual(t, newImage.ID(), "")
-		err = newImage.Remove(false)
+		err = newImage.Remove(context.Background(), false)
 		assert.NoError(t, err)
 	}