Perform an O(1) check to find layers in an image

.travis.yml

@@ -1,7 +1,7 @@
 language: go
 
 go:
-  - 1.8
+  - "1.10"
 
 before_install:
   - go get github.com/alecthomas/gometalinter

hack/lib/constants.sh

@@ -4,7 +4,7 @@
 
 readonly OS_GO_PACKAGE=github.com/openshift/image-registry
 
-readonly OS_BUILD_ENV_GOLANG="${OS_BUILD_ENV_GOLANG:-1.8}"
+readonly OS_BUILD_ENV_GOLANG="${OS_BUILD_ENV_GOLANG:-1.10}"
 readonly OS_BUILD_ENV_IMAGE="${OS_BUILD_ENV_IMAGE:-openshift/origin-release:golang-${OS_BUILD_ENV_GOLANG}}"
 readonly OS_REQUIRED_GO_VERSION="go1.8"
 readonly OS_BUILD_ENV_WORKINGDIR="/go/src/${OS_GO_PACKAGE}"

hack/test-local-registry.sh

@@ -0,0 +1,26 @@
+#!/bin/bash
+
+#
+# This script launches the image registry pointing to the OpenShift master defined in
+# the KUBECONFIG environment variable (and uses the current credentials as the client
+# credentials). You may need to start your master with the internalRegistryHostname
+# configuration variable set for some image stream calls to function properly.
+#
+
+source "$(dirname "${BASH_SOURCE}")/lib/init.sh"
+
+os::build::setup_env
+
+os::util::ensure::built_binary_exists 'dockerregistry'
+
+url="${REGISTRY_OPENSHIFT_SERVER_ADDR:-localhost:5000}"
+# find the first builder service account token
+token="$(oc get $(oc get secrets -o name | grep builder-token | head -n 1) --template '{{ .data.token }}' | os::util::base64decode)"
+echo
+echo "Login with:"
+echo "  docker login -p \"${token}\" -u user ${url}"
+echo
+
+REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY="${REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY:-/tmp/registry}" \
+  REGISTRY_OPENSHIFT_SERVER_ADDR="${url}" \
+	dockerregistry images/dockerregistry/config.yml

pkg/dockerregistry/server/blobdescriptorservice.go

@@ -40,28 +40,40 @@ func (bs *blobDescriptorService) Stat(ctx context.Context, dgst digest.Digest) (
 	if err == nil {
 		return desc, nil
 	}
-
 	context.GetLogger(ctx).Debugf("(*blobDescriptorService).Stat: could not stat layer link %s in repository %s: %v", dgst.String(), bs.repo.Named().Name(), err)
 
+	// we couldn't find the layer link
 	desc, err = bs.repo.app.BlobStatter().Stat(ctx, dgst)
-	if err == nil {
-		context.GetLogger(ctx).Debugf("(*blobDescriptorService).Stat: blob %s exists in the global blob store", dgst.String())
-		// only non-empty layers is wise to check for existence in the image stream.
-		// schema v2 has no empty layers.
-		if !isEmptyDigest(dgst) {
-			// ensure it's referenced inside of corresponding image stream
-			if bs.repo.cache.ContainsRepository(dgst, bs.repo.imageStream.Reference()) {
-				context.GetLogger(ctx).Debugf("found cached blob %q in repository %s", dgst.String(), bs.repo.imageStream.Reference())
-			} else if image := bs.repo.imageStream.HasBlob(ctx, dgst); image != nil {
-				// remember all the layers of matching image
-				RememberLayersOfImage(ctx, bs.repo.cache, image, bs.repo.imageStream.Reference())
-			} else {
-				context.GetLogger(ctx).Debugf("(*blobDescriptorService).Stat: blob %s is neither empty nor referenced in image stream %s", dgst.String(), bs.repo.Named().Name())
-				return distribution.Descriptor{}, distribution.ErrBlobUnknown
-			}
-		}
+	if err != nil {
+		return desc, err
+	}
+	context.GetLogger(ctx).Debugf("(*blobDescriptorService).Stat: blob %s exists in the global blob store", dgst.String())
+
+	// Empty layers are considered to be "public" and we don't need to check whether they are referenced - schema v2
+	// has no empty layers.
+	if isEmptyDigest(dgst) {
 		return desc, nil
 	}
 
-	return desc, err
+	// ensure it's referenced inside of corresponding image stream
+	if bs.repo.cache.ContainsRepository(dgst, bs.repo.imageStream.Reference()) {
+		context.GetLogger(ctx).Debugf("(*blobDescriptorService).Stat: found cached blob %q in repository %s", dgst.String(), bs.repo.imageStream.Reference())
+		return desc, nil
+	}
+
+	found, layers, image := bs.repo.imageStream.HasBlob(ctx, dgst)
+	if !found {
+		context.GetLogger(ctx).Debugf("(*blobDescriptorService).Stat: blob %s is neither empty nor referenced in image stream %s", dgst.String(), bs.repo.Named().Name())
+		return distribution.Descriptor{}, distribution.ErrBlobUnknown
+	}
+
+	if layers != nil {
+		// remember all the layers of matching image
+		RememberLayersOfImageStream(ctx, bs.repo.cache, layers, bs.repo.imageStream.Reference())
+	}
+	if image != nil {
+		// remember all the layers of matching image
+		RememberLayersOfImage(ctx, bs.repo.cache, image, bs.repo.imageStream.Reference())
+	}
+	return desc, nil
 }

pkg/dockerregistry/server/client/interfaces.go

@@ -91,6 +91,7 @@ type ImageStreamInterface interface {
 	Get(name string, options metav1.GetOptions) (*imageapiv1.ImageStream, error)
 	Create(stream *imageapiv1.ImageStream) (*imageapiv1.ImageStream, error)
 	List(opts metav1.ListOptions) (*imageapiv1.ImageStreamList, error)
+	Layers(name string, options metav1.GetOptions) (*imageapiv1.ImageStreamLayers, error)
 }
 
 var _ ImageStreamMappingInterface = imageclientv1.ImageStreamMappingInterface(nil)

pkg/dockerregistry/server/repository_test.go

@@ -99,6 +99,7 @@ func TestRepositoryBlobStat(t *testing.T) {
 		stat               string
 		images             []imageapiv1.Image
 		imageStreams       []imageapiv1.ImageStream
+		imageStreamLayers  []imageapiv1.ImageStreamLayers
 		skipAuth           bool
 		deferredErrors     deferredErrors
 		expectedDescriptor distribution.Descriptor
@@ -137,11 +138,11 @@ func TestRepositoryBlobStat(t *testing.T) {
 				},
 			},
 			expectedDescriptor: testNewDescriptorForLayer(testImages["nm/repo:missing-layer-links"][0].DockerImageLayers[1]),
-			expectedActions:    []clientAction{{"get", "imagestreams"}, {"get", "images"}},
+			expectedActions:    []clientAction{{"get", "imagestreams/layers"}, {"get", "imagestreams"}, {"get", "images"}},
 		},
 
 		{
-			name:   "blob referenced only by not managed image with pullthrough on",
+			name:   "blob referenced only by unmanaged image with pullthrough on and server doesn't support layers subresource",
 			stat:   "nm/unmanaged@" + testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1].Name,
 			images: []imageapiv1.Image{*testImages["nm/unmanaged:missing-layer-links"][0]},
 			imageStreams: []imageapiv1.ImageStream{
@@ -165,7 +166,88 @@ func TestRepositoryBlobStat(t *testing.T) {
 				},
 			},
 			expectedDescriptor: testNewDescriptorForLayer(testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1]),
-			expectedActions:    []clientAction{{"get", "imagestreams"}, {"get", "images"}},
+			expectedActions:    []clientAction{{"get", "imagestreams/layers"}, {"get", "imagestreams"}, {"get", "images"}},
+		},
+
+		{
+			name: "blob not referenced and server supports layers subresource",
+			stat: "nm/unmanaged@" + testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1].Name,
+			imageStreams: []imageapiv1.ImageStream{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "nm",
+						Name:      "unmanaged",
+					},
+					Status: imageapiv1.ImageStreamStatus{
+						Tags: []imageapiv1.NamedTagEventList{
+							{
+								Tag: "latest",
+								Items: []imageapiv1.TagEvent{
+									{
+										Image: testImages["nm/unmanaged:missing-layer-links"][0].Name,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			imageStreamLayers: []imageapiv1.ImageStreamLayers{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "nm",
+						Name:      "unmanaged",
+					},
+					Blobs: map[string]imageapiv1.ImageLayerData{
+						testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[0].Name: {
+							LayerSize: &testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[0].LayerSize,
+							MediaType: testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[0].MediaType,
+						},
+					},
+					Images: map[string]imageapiv1.ImageBlobReferences{
+						testImages["nm/unmanaged:missing-layer-links"][0].Name: {
+							Layers: []string{
+								testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[0].Name,
+							},
+						},
+					},
+				},
+			},
+			expectedError:   distribution.ErrBlobUnknown,
+			expectedActions: []clientAction{{"get", "imagestreams/layers"}, {"get", "imagestreams"}, {"get", "imagestreams/secrets"}},
+		},
+
+		{
+			name: "blob referenced only by unmanaged image with pullthrough on and server supports layers subresource",
+			stat: "nm/unmanaged@" + testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1].Name,
+			imageStreamLayers: []imageapiv1.ImageStreamLayers{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "nm",
+						Name:      "unmanaged",
+					},
+					Blobs: map[string]imageapiv1.ImageLayerData{
+						testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[0].Name: {
+							LayerSize: &testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[0].LayerSize,
+							MediaType: testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[0].MediaType,
+						},
+						testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1].Name: {
+							LayerSize: &testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1].LayerSize,
+							MediaType: testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1].MediaType,
+						},
+					},
+					Images: map[string]imageapiv1.ImageBlobReferences{
+						testImages["nm/unmanaged:missing-layer-links"][0].Name: {
+							Layers: []string{
+								testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[0].Name,
+								testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1].Name,
+							},
+						},
+					},
+				},
+			},
+			expectedDescriptor: testNewDescriptorForLayer(testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1]),
+			expectedActions:    []clientAction{{"get", "imagestreams/layers"}},
 		},
 
 		{
@@ -179,7 +261,7 @@ func TestRepositoryBlobStat(t *testing.T) {
 		},
 
 		{
-			name:   "blob only tagged by not managed image with pullthrough off",
+			name:   "blob only tagged by unmanaged image with pullthrough off",
 			stat:   "nm/repo@" + testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1].Name,
 			images: []imageapiv1.Image{*testImages["nm/unmanaged:missing-layer-links"][0]},
 			imageStreams: []imageapiv1.ImageStream{
@@ -203,7 +285,7 @@ func TestRepositoryBlobStat(t *testing.T) {
 				},
 			},
 			expectedError:   distribution.ErrBlobUnknown,
-			expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "images"}, {"get", "imagestreams"}},
+			expectedActions: []clientAction{{"get", "imagestreams/layers"}, {"get", "imagestreams"}, {"get", "images"}, {"get", "imagestreams/secrets"}},
 		},
 
 		{
@@ -231,7 +313,7 @@ func TestRepositoryBlobStat(t *testing.T) {
 				},
 			},
 			expectedError:   distribution.ErrBlobUnknown,
-			expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "imagestreams"}},
+			expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "imagestreams/secrets"}},
 		},
 
 		{
@@ -259,7 +341,7 @@ func TestRepositoryBlobStat(t *testing.T) {
 				},
 			},
 			expectedError:   distribution.ErrBlobUnknown,
-			expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "imagestreams"}},
+			expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "imagestreams/secrets"}},
 		},
 
 		{
@@ -312,6 +394,13 @@ func TestRepositoryBlobStat(t *testing.T) {
 				}
 			}
 
+			for _, image := range tc.imageStreamLayers {
+				_, err = fos.CreateImageStreamLayers(image.Namespace, &image)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+
 			reg, err := newTestRegistry(ctx, registryclient.NewFakeRegistryAPIClient(nil, imageClient), driver, cfg.Cache.BlobRepositoryTTL, true)
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
@@ -419,7 +508,7 @@ func TestRepositoryBlobStatCacheEviction(t *testing.T) {
 		t.Fatalf("got unexpected descriptor: %#+v != %#+v", desc, blob1Desc)
 	}
 
-	expectedActions := []clientAction{{"get", "imagestreams"}, {"get", "images"}}
+	expectedActions := []clientAction{{"get", "imagestreams/layers"}, {"get", "imagestreams"}, {"get", "images"}}
 	compareActions(t, "1st roundtrip to etcd", imageClient.Actions(), expectedActions)
 
 	// remove the underlying blob
@@ -488,7 +577,7 @@ func TestRepositoryBlobStatCacheEviction(t *testing.T) {
 		t.Fatalf("got unexpected descriptor: %#+v != %#+v", desc, blob2Desc)
 	}
 
-	expectedActions = append(expectedActions, []clientAction{{"get", "imagestreams"}, {"get", "images"}}...)
+	expectedActions = append(expectedActions, []clientAction{{"get", "imagestreams/layers"}, {"get", "imagestreams"}, {"get", "images"}}...)
 	compareActions(t, "2nd roundtrip to etcd", imageClient.Actions(), expectedActions)
 
 	err = vacuum.RemoveBlob(blob2Dgst.String())
@@ -692,18 +781,23 @@ func testNewDescriptorForLayer(layer imageapiv1.ImageLayer) distribution.Descrip
 }
 
 func compareActions(t *testing.T, testCaseName string, actions []clientgotesting.Action, expectedActions []clientAction) {
+	t.Helper()
 	for i, action := range actions {
 		if i >= len(expectedActions) {
-			t.Errorf("[%s] got unexpected client action: %#+v", testCaseName, action)
+			t.Errorf("got unexpected client action: %#+v", action)
 			continue
 		}
 		expected := expectedActions[i]
-		if !action.Matches(expected.verb, expected.resource) {
-			t.Errorf("[%s] expected client action %s[%s], got instead: %#+v", testCaseName, expected.verb, expected.resource, action)
+		parts := strings.Split(expected.resource, "/")
+		if !action.Matches(expected.verb, parts[0]) {
+			t.Errorf("expected client action %s[%s] at index %d, got instead: %#+v", expected.verb, expected.resource, i, action)
+		}
+		if (len(parts) > 1 && action.GetSubresource() != parts[1]) || (len(parts) == 1 && len(action.GetSubresource()) > 0) {
+			t.Errorf("expected client action %s[%s] at index %d, got instead: %#+v", expected.verb, expected.resource, i, action)
 		}
 	}
 	for i := len(actions); i < len(expectedActions); i++ {
 		expected := expectedActions[i]
-		t.Errorf("[%s] expected action %s[%s] did not happen (%#v)", testCaseName, expected.verb, expected.resource, actions)
+		t.Errorf("expected action %s[%s] did not happen (%#v)", expected.verb, expected.resource, actions)
 	}
 }

pkg/dockerregistry/server/util.go

@@ -78,3 +78,10 @@ func RememberLayersOfImage(ctx context.Context, cache cache.RepositoryDigest, im
 		_ = cache.AddDigest(ref.Digest, cacheName)
 	}
 }
+
+// RememberLayersOfImageStream caches the layer digests of given image stream.
+func RememberLayersOfImageStream(ctx context.Context, cache cache.RepositoryDigest, layers *imageapiv1.ImageStreamLayers, cacheName string) {
+	for dgst := range layers.Blobs {
+		_ = cache.AddDigest(digest.Digest(dgst), cacheName)
+	}
+}

pkg/imagestream/cachedimagestreamgetter.go

@@ -22,10 +22,11 @@ const (
 
 // cachedImageStreamGetter wraps a master API client for getting image streams with a cache.
 type cachedImageStreamGetter struct {
-	namespace         string
-	name              string
-	isNamespacer      client.ImageStreamsNamespacer
-	cachedImageStream *imageapiv1.ImageStream
+	namespace               string
+	name                    string
+	isNamespacer            client.ImageStreamsNamespacer
+	cachedImageStream       *imageapiv1.ImageStream
+	cachedImageStreamLayers *imageapiv1.ImageStreamLayers
 }
 
 func (g *cachedImageStreamGetter) get() (*imageapiv1.ImageStream, *rerrors.Error) {
@@ -48,6 +49,26 @@ func (g *cachedImageStreamGetter) get() (*imageapiv1.ImageStream, *rerrors.Error
 	return is, nil
 }
 
+func (g *cachedImageStreamGetter) layers() (*imageapiv1.ImageStreamLayers, *rerrors.Error) {
+	if g.cachedImageStreamLayers != nil {
+		return g.cachedImageStreamLayers, nil
+	}
+	is, err := g.isNamespacer.ImageStreams(g.namespace).Layers(g.name, metav1.GetOptions{})
+	if err != nil {
+		switch {
+		case kerrors.IsNotFound(err):
+			return nil, rerrors.NewError(ErrImageStreamGetterNotFoundCode, fmt.Sprintf("%s/%s", g.namespace, g.name), err)
+		case kerrors.IsForbidden(err), kerrors.IsUnauthorized(err), quotautil.IsErrorQuotaExceeded(err):
+			return nil, rerrors.NewError(ErrImageStreamGetterForbiddenCode, fmt.Sprintf("%s/%s", g.namespace, g.name), err)
+		default:
+			return nil, rerrors.NewError(ErrImageStreamGetterUnknownCode, fmt.Sprintf("%s/%s", g.namespace, g.name), err)
+		}
+	}
+
+	g.cachedImageStreamLayers = is
+	return is, nil
+}
+
 func (g *cachedImageStreamGetter) cacheImageStream(is *imageapiv1.ImageStream) {
 	g.cachedImageStream = is
 }

pkg/imagestream/imagestream.go

@@ -53,7 +53,7 @@ type ImageStream interface {
 	ImageManifestBlobStored(ctx context.Context, image *imageapiv1.Image) *rerrors.Error
 	ResolveImageID(ctx context.Context, dgst digest.Digest) (*imageapiv1.TagEvent, *rerrors.Error)
 
-	HasBlob(ctx context.Context, dgst digest.Digest) *imageapiv1.Image
+	HasBlob(ctx context.Context, dgst digest.Digest) (bool, *imageapiv1.ImageStreamLayers, *imageapiv1.Image)
 	IdentifyCandidateRepositories(ctx context.Context, primary bool) ([]string, map[string]ImagePullthroughSpec, *rerrors.Error)
 	GetLimitRangeList(ctx context.Context, cache ProjectObjectListStore) (*corev1.LimitRangeList, *rerrors.Error)
 	GetSecrets() ([]corev1.Secret, *rerrors.Error)