WIP - Skip empty layers during image download [specific ci=Group1-Docker-Commands]

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.
 

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?

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

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
+}

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

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
-
 }

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"`
 }