From bf62f9a5cfb4319aa9b0f25b57b3f63e4b965672 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 12 Nov 2019 13:37:09 -0500 Subject: history: rewrite mappings Rewrite the backend for displaying the history of an image to simplify the code and be closer to docker's behaviour. Instead of driving index-based heuristics, create a reverse mapping from top-layers to the corresponding image IDs and lookup the layers on-demand. Also use the uncompressed layer size to be closer to Docker's behaviour. Note that intermediate images from local builds are not considered for the ID lookups anymore. Fixes: #3359 Signed-off-by: Valentin Rothberg --- libpod/image/image.go | 130 +++++++++++++++++--------------------------------- 1 file changed, 43 insertions(+), 87 deletions(-) (limited to 'libpod/image') diff --git a/libpod/image/image.go b/libpod/image/image.go index c912ac2ca..75ac85311 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -765,109 +765,65 @@ func (i *Image) History(ctx context.Context) ([]*History, error) { return nil, err } - // Use our layers list to find images that use any of them (or no - // layer, since every base layer is derived from an empty layer) as its - // topmost layer. - interestingLayers := make(map[string]bool) - var layer *storage.Layer - if i.TopLayer() != "" { - if layer, err = i.imageruntime.store.Layer(i.TopLayer()); err != nil { - return nil, err - } + // Build a mapping from top-layer to image ID. + images, err := i.imageruntime.GetImages() + if err != nil { + return nil, err } - interestingLayers[""] = true - for layer != nil { - interestingLayers[layer.ID] = true - if layer.Parent == "" { - break + topLayerMap := make(map[string]string) + for _, image := range images { + if _, exists := topLayerMap[image.TopLayer()]; !exists { + topLayerMap[image.TopLayer()] = image.ID() } - layer, err = i.imageruntime.store.Layer(layer.Parent) + } + + var allHistory []*History + var layer *storage.Layer + + // Check if we have an actual top layer to prevent lookup errors. + if i.TopLayer() != "" { + layer, err = i.imageruntime.store.Layer(i.TopLayer()) 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") - } - interestingImages := make([]*Image, 0, len(images)) - for i := range images { - if interestingLayers[images[i].TopLayer()] { - interestingImages = append(interestingImages, images[i]) - } - } + // Iterate in reverse order over the history entries, and lookup the + // corresponding image ID, size and get the next later if needed. + numHistories := len(oci.History) - 1 + for x := numHistories; x >= 0; x-- { + var size int64 - // 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()) + id := "" + if x == numHistories { + id = i.ID() + } else if layer != nil { + if !oci.History[x].EmptyLayer { + size = layer.UncompressedSize } - // 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] + if imageID, exists := topLayerMap[layer.ID]; exists { + id = imageID + // Delete the entry to avoid reusing it for following history items. + delete(topLayerMap, layer.ID) } } - } - var ( - size int64 - sizeCount = 1 - allHistory []*History - ) - - for i := len(oci.History) - 1; i >= 0; i-- { - imageID := "" - if historyImages[i] != nil { - imageID = historyImages[i].ID() - } - if !oci.History[i].EmptyLayer { - size = img.LayerInfos()[len(img.LayerInfos())-sizeCount].Size - sizeCount++ - } allHistory = append(allHistory, &History{ - ID: imageID, - Created: oci.History[i].Created, - CreatedBy: oci.History[i].CreatedBy, + ID: id, + Created: oci.History[x].Created, + CreatedBy: oci.History[x].CreatedBy, Size: size, - Comment: oci.History[i].Comment, + Comment: oci.History[x].Comment, }) + + if layer != nil && layer.Parent != "" && !oci.History[x].EmptyLayer { + layer, err = i.imageruntime.store.Layer(layer.Parent) + if err != nil { + return nil, err + } + } } + return allHistory, nil } -- cgit v1.2.3-54-g00ecf