From 3987a15b3861026efc3f36bd359a519b50290571 Mon Sep 17 00:00:00 2001 From: Zach Tucker Date: Mon, 4 Jun 2018 20:08:08 +0000 Subject: [PATCH] Skip empty layers during image download --- lib/apiservers/engine/backends/container.go | 4 +- lib/apiservers/engine/backends/image.go | 2 +- lib/imagec/docker.go | 8 +++- lib/imagec/download.go | 52 +++++++++++++++++++-- lib/imagec/imagec.go | 20 ++++++-- lib/imagec/storage.go | 3 +- lib/metadata/image_config.go | 10 +++- 7 files changed, 84 insertions(+), 15 deletions(-) diff --git a/lib/apiservers/engine/backends/container.go b/lib/apiservers/engine/backends/container.go index a950cc4d56..d721364482 100644 --- a/lib/apiservers/engine/backends/container.go +++ b/lib/apiservers/engine/backends/container.go @@ -663,7 +663,7 @@ func (c *ContainerBackend) ContainerCreate(config types.ContainerCreateConfig) ( // Reserve the container name to prevent duplicates during a parallel operation. if config.Name != "" { - err := cache.ContainerCache().ReserveName(container, config.Name) + err = cache.ContainerCache().ReserveName(container, config.Name) if err != nil { return containertypes.ContainerCreateCreatedBody{}, derr.NewRequestConflictError(err) } @@ -1757,7 +1757,7 @@ func clientFriendlyContainerName(name string) string { func createInternalVicContainer(image *metadata.ImageConfig, config *types.ContainerCreateConfig) (*viccontainer.VicContainer, error) { // provide basic container config via the image container := viccontainer.NewVicContainer() - container.LayerID = image.V1Image.ID // store childmost layer ID to map to the proper vmdk + container.LayerID = image.VMDK // store childmost layer ID to map to the proper vmdk container.ImageID = image.ImageID container.Config = image.Config //Set defaults. Overrides will get copied below. diff --git a/lib/apiservers/engine/backends/image.go b/lib/apiservers/engine/backends/image.go index 666e5577de..099364bb6d 100644 --- a/lib/apiservers/engine/backends/image.go +++ b/lib/apiservers/engine/backends/image.go @@ -123,7 +123,7 @@ func (i *ImageBackend) ImageDelete(imageRef string, force, prune bool) ([]types. keepNodes[idx] = imgURL.String() } - params := storage.NewDeleteImageParamsWithContext(ctx).WithStoreName(storeName).WithID(img.ID).WithKeepNodes(keepNodes) + params := storage.NewDeleteImageParamsWithContext(ctx).WithStoreName(storeName).WithID(img.VMDK).WithKeepNodes(keepNodes) // TODO: This will fail if any containerVMs are referencing the vmdk - vanilla docker // allows the removal of an image (via force flag) even if a container is referencing it // should vic? diff --git a/lib/imagec/docker.go b/lib/imagec/docker.go index 92d43b3a2b..36695b9ce3 100644 --- a/lib/imagec/docker.go +++ b/lib/imagec/docker.go @@ -45,8 +45,14 @@ import ( const ( // DigestSHA256EmptyTar is the canonical sha256 digest of empty tar file - - // (1024 NULL bytes) + // (1024 NULL bytes) - aka, the "empty" layer diffID. DigestSHA256EmptyTar = string(dlayer.DigestSHA256EmptyTar) + + // DigestSHA256EmptyBlobSum is the canonical sha256 digest of a gzipped empty tar file - + // (1024 NULL bytes, gzipped) - aka, the "empty" blobsum. This is used to identify empty + // layers using the schema v1 manifest without having to download, unzip and sha256 the + // 1024-byte empty layer tar. + DigestSHA256EmptyBlobSum = "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4" ) // FSLayer is a container struct for BlobSums defined in an image manifest diff --git a/lib/imagec/download.go b/lib/imagec/download.go index 40057627b6..4e361f2c15 100644 --- a/lib/imagec/download.go +++ b/lib/imagec/download.go @@ -33,6 +33,11 @@ const ( maxConcurrentDownloads = 3 ) +var ( + vmdkMap map[string]string + leafLayer *ImageWithMeta +) + type downloadTransfer struct { xfer.Transfer @@ -127,12 +132,19 @@ func (ldm *LayerDownloader) DownloadLayers(ctx context.Context, ic *ImageC) erro // Grab the imageLayers layers := ic.ImageLayers + vmdkMap = getVMDKMap(layers) + // iterate backwards through layers to download for i := len(layers) - 1; i >= 0; i-- { layer := layers[i] - id := layer.ID + if layer.EmptyLayer { + // skip attempting to download empty layers + continue + } + + id := layer.ID layerConfig, err := LayerCache().Get(id) if err != nil { @@ -154,6 +166,10 @@ func (ldm *LayerDownloader) DownloadLayers(ctx context.Context, ic *ImageC) erro layer.Downloading = true LayerCache().Add(layer) + // the child-most non-empty layer should be used as the leaf layer, and will + // be the parent of the container R/W layer VMDK + leafLayer = layer + continue default: return err @@ -282,20 +298,27 @@ func (ldm *LayerDownloader) makeDownloadFunc(layer *ImageWithMeta, ic *ImageC, p } } + // set this layer's parent to the appropriate (non-empty) VMDK + layer.DiskParent = vmdkMap[layer.ID] + // is this the leaf layer? - imageLayer := layer.ID == layers[0].ID + isLeaf := layer.ID == leafLayer.ID if !ic.Standalone { // if this is the leaf layer, we are done and can now create the image config - if imageLayer { + if isLeaf { imageConfig, err := ic.CreateImageConfig(layers) if err != nil { d.err = err return } + + // set the image VMDK so container creation uses the correct disk as the R/W's parent + imageConfig.VMDK = layer.ID + // cache and persist the image cache.ImageCache().Add(&imageConfig) - if err := cache.ImageCache().Save(); err != nil { + if err = cache.ImageCache().Save(); err != nil { d.err = fmt.Errorf("error saving image cache: %s", err) return } @@ -315,7 +338,7 @@ func (ldm *LayerDownloader) makeDownloadFunc(layer *ImageWithMeta, ic *ImageC, p defer ldm.m.Unlock() // Write blob to the storage layer - if err := ic.WriteImageBlob(layer, progressOutput, imageLayer); err != nil { + if err := ic.WriteImageBlob(layer, progressOutput, isLeaf); err != nil { d.err = err return } @@ -386,3 +409,22 @@ func (ldm *LayerDownloader) makeDownloadFuncFromDownload(layer *ImageWithMeta, s } } + +func getVMDKMap(layers []*ImageWithMeta) map[string]string { + result := make(map[string]string) + diskParent := "scratch" + + for i := len(layers) - 1; i > 0; i-- { + layer := layers[i] + if layer.EmptyLayer { + // skip empty layers + continue + } + // set layer's parent to last known non-empty layer in the chain + result[layer.ID] = diskParent + // this layer isn't empty, so it will be the next parent layer in the chain + diskParent = layer.ID + } + + return result +} diff --git a/lib/imagec/imagec.go b/lib/imagec/imagec.go index b9b47b00a2..da67499b0d 100644 --- a/lib/imagec/imagec.go +++ b/lib/imagec/imagec.go @@ -125,6 +125,13 @@ type ImageWithMeta struct { Meta string Size int64 + // DiskParent is the ID of the most recent non-empty parent layer that will serve + // as this layer's parent in the disk chain + DiskParent string + + // EmptyLayer is true if this layer contains no filesystem data, false otherwise + EmptyLayer bool + Downloading bool } @@ -242,6 +249,12 @@ func (ic *ImageC) LayersToDownload() ([]*ImageWithMeta, error) { return nil, fmt.Errorf("Failed to unmarshall image history: %s", err) } + log.Infof(">>>>> Layer %s empty? %t - blobsum: %s", v1.ID, layer.BlobSum == DigestSHA256EmptyBlobSum, layer.BlobSum) + emptyLayer := false + if layer.BlobSum == DigestSHA256EmptyBlobSum { + emptyLayer = true + } + // if parent is empty set it to scratch parent := constants.ScratchLayerID if v1.Parent != "" { @@ -255,8 +268,9 @@ func (ic *ImageC) LayersToDownload() ([]*ImageWithMeta, error) { Parent: parent, Store: ic.Storename, }, - Meta: history.V1Compatibility, - Layer: layer, + Meta: history.V1Compatibility, + Layer: layer, + EmptyLayer: emptyLayer, } // populate manifest layer with existing cached data @@ -399,7 +413,7 @@ func (ic *ImageC) CreateImageConfig(images []*ImageWithMeta) (metadata.ImageConf } // is this an empty layer? - if layer.DiffID == dockerLayer.DigestSHA256EmptyTar.String() { + if layer.Layer.BlobSum == DigestSHA256EmptyBlobSum { h.EmptyLayer = true } else { // if not empty, add diffID to rootFS diff --git a/lib/imagec/storage.go b/lib/imagec/storage.go index 97de109659..e5a2261a35 100644 --- a/lib/imagec/storage.go +++ b/lib/imagec/storage.go @@ -98,7 +98,7 @@ func WriteImage(host string, image *ImageWithMeta, data io.ReadCloser) error { r, err := client.Storage.WriteImage( storage.NewWriteImageParamsWithContext(ctx). WithImageID(image.ID). - WithParentID(image.Parent). + WithParentID(image.DiskParent). WithStoreName(image.Store). WithMetadatakey(key). WithMetadataval(blob). @@ -112,5 +112,4 @@ func WriteImage(host string, image *ImageWithMeta, data io.ReadCloser) error { log.Printf("Created an image %#v", r.Payload) return nil - } diff --git a/lib/metadata/image_config.go b/lib/metadata/image_config.go index 8030bf3f0e..59eef1c3ec 100644 --- a/lib/metadata/image_config.go +++ b/lib/metadata/image_config.go @@ -18,7 +18,8 @@ import ( docker "github.com/docker/docker/image" ) -// ImageConfig contains configuration data describing images and their layers +// ImageConfig defines the docker format for representing an image. When marshaled to JSON, the sha256 sum +// of the resulting bytes is the image ID. type ImageConfig struct { docker.V1Image @@ -30,4 +31,11 @@ type ImageConfig struct { DiffIDs map[string]string `json:"diff_ids,omitempty"` History []docker.History `json:"history,omitempty"` Reference string `json:"registry"` + + // VMDK is the ID of the VMDK to be used as the R/W layer's parent disk when + // creating a container from the image. This is the type that is stored in our + // image cache. + // + // This field is ignored when marshalling into JSON to preserve the image ID. + VMDK string `json:"vmdk,omitempty"` }