Merge pull request #10805 from mfojtik/take-care-of-manifest-config

Merged by openshift-bot

hack/util.sh

@@ -660,7 +660,9 @@ function install_registry() {
 readonly -f install_registry
 
 function wait_for_registry() {
-	wait_for_command "oc get endpoints docker-registry --template='{{ len .subsets }}' --config='${ADMIN_KUBECONFIG}' | grep -q '[1-9][0-9]*'" $((5*TIME_MIN))
+	local generation=$(oc get dc/docker-registry -o jsonpath='{.metadata.generation}')
+	local onereplicajs='{.status.observedGeneration},{.status.replicas},{.status.updatedReplicas},{.status.availableReplicas}'
+	wait_for_command "oc get dc/docker-registry -o jsonpath='$onereplicajs' --config='${ADMIN_KUBECONFIG}' | grep '^$generation,1,1,1$'"  $((5*TIME_MIN))
 	local readyjs='{.items[*].status.conditions[?(@.type=="Ready")].status}'
 	wait_for_command "oc get pod -l deploymentconfig=docker-registry -o jsonpath='$readyjs' --config='${ADMIN_KUBECONFIG}' | grep -qi true" $TIME_MIN
 }

pkg/cmd/admin/top/images.go

@@ -175,14 +175,18 @@ func (o TopImagesOptions) imagesTop() []Info {
 
 func getStorage(image *imageapi.Image) int64 {
 	storage := int64(0)
-	layerSet := sets.NewString()
+	blobSet := sets.NewString()
 	for _, layer := range image.DockerImageLayers {
-		if layerSet.Has(layer.Name) {
+		if blobSet.Has(layer.Name) {
 			continue
 		}
-		layerSet.Insert(layer.Name)
+		blobSet.Insert(layer.Name)
 		storage += layer.LayerSize
 	}
+	if len(image.DockerImageConfig) > 0 && !blobSet.Has(image.DockerImageMetadata.ID) {
+		blobSet.Insert(image.DockerImageMetadata.ID)
+		storage += int64(len(image.DockerImageConfig))
+	}
 	return storage
 }
 

pkg/cmd/admin/top/images_test.go

@@ -89,6 +89,65 @@ func TestImagesTop(t *testing.T) {
 				},
 			},
 		},
+		"with metadata and image config": {
+			images: &imageapi.ImageList{
+				Items: []imageapi.Image{
+					{
+						ObjectMeta: kapi.ObjectMeta{Name: "image1"},
+						DockerImageLayers: []imageapi.ImageLayer{
+							{Name: "layer1", LayerSize: int64(512)},
+							{Name: "layer2", LayerSize: int64(512)},
+						},
+						DockerImageManifest: "non empty metadata",
+						DockerImageConfig:   "raw image config",
+						DockerImageMetadata: imageapi.DockerImage{
+							ID: "manifestConfigID",
+						},
+					},
+				},
+			},
+			streams: &imageapi.ImageStreamList{},
+			pods:    &kapi.PodList{},
+			expected: []Info{
+				imageInfo{
+					Image:    "image1",
+					Metadata: true,
+					Parents:  []string{},
+					Usage:    []string{},
+					Storage:  int64(1024 + len("raw image config")),
+				},
+			},
+		},
+		"with metadata and image config and some layers duplicated": {
+			images: &imageapi.ImageList{
+				Items: []imageapi.Image{
+					{
+						ObjectMeta: kapi.ObjectMeta{Name: "image1"},
+						DockerImageLayers: []imageapi.ImageLayer{
+							{Name: "layer1", LayerSize: int64(512)},
+							{Name: "layer2", LayerSize: int64(256)},
+							{Name: "layer1", LayerSize: int64(512)},
+						},
+						DockerImageManifest: "non empty metadata",
+						DockerImageConfig:   "raw image config",
+						DockerImageMetadata: imageapi.DockerImage{
+							ID: "layer2",
+						},
+					},
+				},
+			},
+			streams: &imageapi.ImageStreamList{},
+			pods:    &kapi.PodList{},
+			expected: []Info{
+				imageInfo{
+					Image:    "image1",
+					Metadata: true,
+					Parents:  []string{},
+					Usage:    []string{},
+					Storage:  int64(512 + 256),
+				},
+			},
+		},
 		"multiple tags": {
 			images: &imageapi.ImageList{
 				Items: []imageapi.Image{

pkg/cmd/admin/top/imagestreams.go

@@ -144,7 +144,7 @@ func getImageStreamSize(g graph.Graph, node *imagegraph.ImageStreamNode) (int64,
 	storage := int64(0)
 	images := len(imageEdges)
 	layers := 0
-	layerSet := sets.NewString()
+	blobSet := sets.NewString()
 	for _, e := range imageEdges {
 		imageNode, ok := e.To().(*imagegraph.ImageNode)
 		if !ok {
@@ -154,12 +154,16 @@ func getImageStreamSize(g graph.Graph, node *imagegraph.ImageStreamNode) (int64,
 		layers += len(image.DockerImageLayers)
 		// we're counting only unique layers per the entire stream
 		for _, layer := range image.DockerImageLayers {
-			if layerSet.Has(layer.Name) {
+			if blobSet.Has(layer.Name) {
 				continue
 			}
-			layerSet.Insert(layer.Name)
+			blobSet.Insert(layer.Name)
 			storage += layer.LayerSize
 		}
+		if len(image.DockerImageConfig) > 0 && !blobSet.Has(image.DockerImageMetadata.ID) {
+			blobSet.Insert(image.DockerImageMetadata.ID)
+			storage += int64(len(image.DockerImageConfig))
+		}
 	}
 
 	return storage, images, layers

pkg/cmd/admin/top/imagestreams_test.go

@@ -178,6 +178,56 @@ func TestImageStreamsTop(t *testing.T) {
 				},
 			},
 		},
+		"multiple images with manifest config": {
+			images: &imageapi.ImageList{
+				Items: []imageapi.Image{
+					{
+						ObjectMeta:        kapi.ObjectMeta{Name: "image1"},
+						DockerImageLayers: []imageapi.ImageLayer{{Name: "layer1", LayerSize: int64(1024)}},
+						DockerImageConfig: "raw image config",
+						DockerImageMetadata: imageapi.DockerImage{
+							ID: "manifestConfigID",
+						},
+					},
+					{
+						ObjectMeta: kapi.ObjectMeta{Name: "image2"},
+						DockerImageLayers: []imageapi.ImageLayer{
+							{Name: "layer1", LayerSize: int64(1024)},
+							{Name: "layer2", LayerSize: int64(128)},
+						},
+						DockerImageConfig: "raw image config",
+						DockerImageMetadata: imageapi.DockerImage{
+							ID: "manifestConfigID",
+						},
+					},
+				},
+			},
+			streams: &imageapi.ImageStreamList{
+				Items: []imageapi.ImageStream{
+					{
+						ObjectMeta: kapi.ObjectMeta{Name: "stream1", Namespace: "ns1"},
+						Status: imageapi.ImageStreamStatus{
+							Tags: map[string]imageapi.TagEventList{
+								"tag1": {
+									Items: []imageapi.TagEvent{{Image: "image1"}},
+								},
+								"tag2": {
+									Items: []imageapi.TagEvent{{Image: "image2"}},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: []Info{
+				imageStreamInfo{
+					ImageStream: "ns1/stream1",
+					Storage:     int64(1152 + len("raw image config")),
+					Images:      2,
+					Layers:      3,
+				},
+			},
+		},
 		"multiple unreferenced images": {
 			images: &imageapi.ImageList{
 				Items: []imageapi.Image{

pkg/dockerregistry/server/blobdescriptorservice.go

@@ -203,5 +203,12 @@ func imageHasBlob(
 		}
 	}
 
+	// only manifest V2 schema2 has docker image config filled where dockerImage.Metadata.id is its digest
+	if len(image.DockerImageConfig) > 0 && image.DockerImageMetadata.ID == blobDigest {
+		// remember manifest config reference of schema 2 as well
+		r.rememberLayersOfImage(image, cacheName)
+		return true
+	}
+
 	return false
 }

pkg/dockerregistry/server/pullthroughblobstore.go

@@ -43,6 +43,12 @@ func (r *pullthroughBlobStore) Stat(ctx context.Context, dgst digest.Digest) (di
 		return desc, err
 	}
 
+	return r.remoteStat(ctx, dgst)
+}
+
+// remoteStat attempts to find requested blob in candidate remote repositories and if found, it updates
+// digestToRepository store. ErrBlobUnknown will be returned if not found.
+func (r *pullthroughBlobStore) remoteStat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) {
 	// look up the potential remote repositories that this blob could be part of (at this time,
 	// we don't know which image in the image stream surfaced the content).
 	is, err := r.repo.getImageStream()
@@ -112,13 +118,13 @@ func (r *pullthroughBlobStore) ServeBlob(ctx context.Context, w http.ResponseWri
 
 	desc, err := store.Stat(ctx, dgst)
 	if err != nil {
-		context.GetLogger(ctx).Errorf("Failed to stat digest %q: %v", dgst.String(), err)
+		context.GetLogger(ctx).Errorf("failed to stat digest %q: %v", dgst.String(), err)
 		return err
 	}
 
 	remoteReader, err := store.Open(ctx, dgst)
 	if err != nil {
-		context.GetLogger(ctx).Errorf("Failure to open remote store for digest %q: %v", dgst.String(), err)
+		context.GetLogger(ctx).Errorf("failure to open remote store for digest %q: %v", dgst.String(), err)
 		return err
 	}
 	defer remoteReader.Close()
@@ -130,6 +136,30 @@ func (r *pullthroughBlobStore) ServeBlob(ctx context.Context, w http.ResponseWri
 	return nil
 }
 
+// Get attempts to fetch the requested blob by digest using a remote proxy store if necessary.
+func (r *pullthroughBlobStore) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) {
+	store, ok := r.digestToStore[dgst.String()]
+	if ok {
+		return store.Get(ctx, dgst)
+	}
+
+	data, originalErr := r.BlobStore.Get(ctx, dgst)
+	if originalErr == nil {
+		return data, nil
+	}
+
+	desc, err := r.remoteStat(ctx, dgst)
+	if err != nil {
+		context.GetLogger(ctx).Errorf("failed to stat blob %q in remote repositories: %v", dgst.String(), err)
+		return nil, originalErr
+	}
+	store, ok = r.digestToStore[desc.Digest.String()]
+	if !ok {
+		return nil, originalErr
+	}
+	return store.Get(ctx, desc.Digest)
+}
+
 // findCandidateRepository looks in search for a particular blob, referring to previously cached items
 func (r *pullthroughBlobStore) findCandidateRepository(ctx context.Context, search map[string]*imageapi.DockerImageReference, cachedLayers []string, dgst digest.Digest, retriever importer.RepositoryRetriever) (distribution.Descriptor, error) {
 	// no possible remote locations to search, exit early

pkg/dockerregistry/server/repositorymiddleware.go

@@ -452,7 +452,7 @@ func (r *repository) signedManifestFillImageMetadata(manifest *schema1.SignedMan
 
 	refs := manifest.References()
 
-	layerSet := sets.NewString()
+	blobSet := sets.NewString()
 	image.DockerImageMetadata.Size = int64(0)
 
 	blobs := r.Blobs(r.ctx)
@@ -473,11 +473,15 @@ func (r *repository) signedManifestFillImageMetadata(manifest *schema1.SignedMan
 		}
 		layer.LayerSize = desc.Size
 		// count empty layer just once (empty layer may actually have non-zero size)
-		if !layerSet.Has(layer.Name) {
+		if !blobSet.Has(layer.Name) {
 			image.DockerImageMetadata.Size += desc.Size
-			layerSet.Insert(layer.Name)
+			blobSet.Insert(layer.Name)
 		}
 	}
+	if len(image.DockerImageConfig) > 0 && !blobSet.Has(image.DockerImageMetadata.ID) {
+		blobSet.Insert(image.DockerImageMetadata.ID)
+		image.DockerImageMetadata.Size += int64(len(image.DockerImageConfig))
+	}
 
 	return nil
 }
@@ -544,7 +548,7 @@ func (r *repository) getImageStreamImage(dgst digest.Digest) (*imageapi.ImageStr
 
 // rememberLayersOfImage caches the layer digests of given image
 func (r *repository) rememberLayersOfImage(image *imageapi.Image, cacheName string) {
-	if len(image.DockerImageLayers) == 0 && len(image.DockerImageManifestMediaType) > 0 {
+	if len(image.DockerImageLayers) == 0 && len(image.DockerImageManifestMediaType) > 0 && len(image.DockerImageConfig) == 0 {
 		// image has no layers
 		return
 	}
@@ -553,6 +557,10 @@ func (r *repository) rememberLayersOfImage(image *imageapi.Image, cacheName stri
 		for _, layer := range image.DockerImageLayers {
 			r.cachedLayers.RememberDigest(digest.Digest(layer.Name), r.blobrepositorycachettl, cacheName)
 		}
+		// remember reference to manifest config as well for schema 2
+		if image.DockerImageManifestMediaType == schema2.MediaTypeManifest && len(image.DockerImageMetadata.ID) > 0 {
+			r.cachedLayers.RememberDigest(digest.Digest(image.DockerImageMetadata.ID), r.blobrepositorycachettl, cacheName)
+		}
 		return
 	}
 

pkg/image/api/helper.go

@@ -552,17 +552,15 @@ func ImageWithMetadata(image *Image) error {
 		image.DockerImageMetadata.Architecture = config.Architecture
 		image.DockerImageMetadata.Size = int64(len(image.DockerImageConfig))
 
+		layerSet := sets.NewString(image.DockerImageMetadata.ID)
 		if len(image.DockerImageLayers) > 0 {
-			layerSet := sets.NewString()
 			for _, layer := range image.DockerImageLayers {
 				if layerSet.Has(layer.Name) {
 					continue
 				}
 				layerSet.Insert(layer.Name)
 				image.DockerImageMetadata.Size += layer.LayerSize
 			}
-		} else {
-			image.DockerImageMetadata.Size += config.Size
 		}
 	default:
 		return fmt.Errorf("unrecognized Docker image manifest schema %d for %q (%s)", manifest.SchemaVersion, image.Name, image.DockerImageReference)

pkg/image/importer/client_test.go

@@ -41,7 +41,7 @@ type mockRepository struct {
 
 	blobs *mockBlobStore
 
-	manifest *schema1.SignedManifest
+	manifest distribution.Manifest
 	tags     map[string]string
 }
 
@@ -79,6 +79,8 @@ func (r *mockRepository) Tags(ctx context.Context) distribution.TagService {
 type mockBlobStore struct {
 	distribution.BlobStore
 
+	blobs map[digest.Digest][]byte
+
 	statErr, serveErr, openErr error
 }
 
@@ -94,6 +96,14 @@ func (r *mockBlobStore) Open(ctx context.Context, dgst digest.Digest) (distribut
 	return nil, r.openErr
 }
 
+func (r *mockBlobStore) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) {
+	b, exists := r.blobs[dgst]
+	if !exists {
+		return nil, distribution.ErrBlobUnknown
+	}
+	return b, nil
+}
+
 type mockTagService struct {
 	distribution.TagService
 

pkg/image/importer/importer.go

@@ -310,15 +310,15 @@ func formatRepositoryError(repository *importRepository, refName string, refID s
 func (isi *ImageStreamImporter) calculateImageSize(ctx gocontext.Context, repo distribution.Repository, image *api.Image) error {
 	bs := repo.Blobs(ctx)
 
-	layerSet := sets.NewString()
+	blobSet := sets.NewString()
 	size := int64(0)
 	for i := range image.DockerImageLayers {
 		layer := &image.DockerImageLayers[i]
 
-		if layerSet.Has(layer.Name) {
+		if blobSet.Has(layer.Name) {
 			continue
 		}
-		layerSet.Insert(layer.Name)
+		blobSet.Insert(layer.Name)
 
 		if layerSize, ok := isi.digestToLayerSizeCache[layer.Name]; ok {
 			size += layerSize
@@ -335,6 +335,11 @@ func (isi *ImageStreamImporter) calculateImageSize(ctx gocontext.Context, repo d
 		size += desc.Size
 	}
 
+	if len(image.DockerImageConfig) > 0 && !blobSet.Has(image.DockerImageMetadata.ID) {
+		blobSet.Insert(image.DockerImageMetadata.ID)
+		size += int64(len(image.DockerImageConfig))
+	}
+
 	image.DockerImageMetadata.Size = size
 	return nil
 }