Merge pull request #13709 from stonezdj/201209_dockerhub_limit2

Cache manifest list for proxy cache

src/controller/proxy/controller.go

@@ -29,6 +29,7 @@ import (
 	"github.com/goharbor/harbor/src/controller/blob"
 	"github.com/goharbor/harbor/src/controller/event/operator"
 	"github.com/goharbor/harbor/src/lib"
+	"github.com/goharbor/harbor/src/lib/cache"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/orm"
@@ -41,6 +42,8 @@ const (
 	maxManifestListWait = 20
 	maxManifestWait     = 10
 	sleepIntervalSec    = 20
+	// keep manifest list in cache for one week
+	manifestListCacheIntervalSec = 7 * 24 * 60 * 60
 )
 
 var (
@@ -54,7 +57,7 @@ type Controller interface {
 	// UseLocalBlob check if the blob should use local copy
 	UseLocalBlob(ctx context.Context, art lib.ArtifactInfo) bool
 	// UseLocalManifest check manifest should use local copy
-	UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, error)
+	UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, *ManifestList, error)
 	// ProxyBlob proxy the blob request to the remote server, p is the proxy project
 	// art is the ArtifactInfo which includes the digest of the blob
 	ProxyBlob(ctx context.Context, p *models.Project, art lib.ArtifactInfo) (int64, io.ReadCloser, error)
@@ -68,6 +71,7 @@ type controller struct {
 	blobCtl     blob.Controller
 	artifactCtl artifact.Controller
 	local       localInterface
+	cache       cache.Cache
 }
 
 // ControllerInstance -- Get the proxy controller instance
@@ -79,6 +83,7 @@ func ControllerInstance() Controller {
 			blobCtl:     blob.Ctl,
 			artifactCtl: artifact.Ctl,
 			local:       newLocalHelper(),
+			cache:       cache.Default(),
 		}
 	})
 
@@ -96,33 +101,55 @@ func (c *controller) UseLocalBlob(ctx context.Context, art lib.ArtifactInfo) boo
 	return exist
 }
 
-func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, error) {
+// ManifestList ...
+type ManifestList struct {
+	Content     []byte
+	Digest      string
+	ContentType string
+}
+
+func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, *ManifestList, error) {
 	a, err := c.local.GetManifest(ctx, art)
 	if err != nil {
-		return false, err
+		return false, nil, err
 	}
-	if a == nil {
+	// Pull by digest when artifact exist in local
-		return false, nil
+	if a != nil && len(art.Digest) > 0 {
+		return true, nil, nil
 	}
-	// Pull by digest
+
-	if len(art.Digest) > 0 {
-		return true, nil
-	}
-	// Pull by tag
 	remoteRepo := getRemoteRepo(art)
-	exist, dig, err := remote.ManifestExist(remoteRepo, art.Tag) // HEAD
+	exist, dig, err := remote.ManifestExist(remoteRepo, getReference(art)) // HEAD
 	if err != nil {
-		return false, err
+		return false, nil, err
 	}
 	if !exist {
 		go func() {
 			c.local.DeleteManifest(remoteRepo, art.Tag)
 		}()
-		return false, errors.NotFoundError(fmt.Errorf("repo %v, tag %v not found", art.Repository, art.Tag))
+		return false, nil, errors.NotFoundError(fmt.Errorf("repo %v, tag %v not found", art.Repository, art.Tag))
 	}
-	return dig == a.Digest, nil // digest matches
+
+	var content []byte
+	if c.cache != nil {
+		err = c.cache.Fetch(getManifestListKey(art.Repository, dig), &content)
+		if err == nil {
+			log.Debugf("Get the manifest list with key=cache:%v", getManifestListKey(art.Repository, dig))
+			return true, &ManifestList{content, dig, manifestlist.MediaTypeManifestList}, nil
+		}
+		if err == cache.ErrNotFound {
+			log.Debugf("Digest is not found in manifest list cache, key=cache:%v", getManifestListKey(art.Repository, dig))
+		} else {
+			log.Errorf("Failed to get manifest list from cache, error: %v", err)
+		}
+	}
+	return a != nil && dig == a.Digest, nil, nil // digest matches
 }
 
+func getManifestListKey(repo, dig string) string {
+	// actual redis key format is cache:manifestlist:<repo name>:sha256:xxxx
+	return "manifestlist:" + repo + ":" + dig
+}
 func (c *controller) ProxyManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (distribution.Manifest, error) {
 	var man distribution.Manifest
 	remoteRepo := getRemoteRepo(art)
@@ -150,8 +177,11 @@ func (c *controller) ProxyManifest(ctx context.Context, art lib.ArtifactInfo, re
 		}
 		// Push manifest to local when pull with digest, or artifact not found, or digest mismatch
 		if len(art.Tag) == 0 || a == nil || a.Digest != dig {
-			// pull with digest
+			artInfo := art
-			c.waitAndPushManifest(ctx, remoteRepo, man, art, ct, remote)
+			if len(artInfo.Digest) == 0 {
+				artInfo.Digest = dig
+			}
+			c.waitAndPushManifest(ctx, remoteRepo, man, artInfo, ct, remote)
 		}
 
 		// Query artifact after push
@@ -209,6 +239,19 @@ func (c *controller) putBlobToLocal(remoteRepo string, localRepo string, desc di
 }
 
 func (c *controller) waitAndPushManifest(ctx context.Context, remoteRepo string, man distribution.Manifest, art lib.ArtifactInfo, contType string, r RemoteInterface) {
+	if contType == manifestlist.MediaTypeManifestList {
+		_, payload, err := man.Payload()
+		if err != nil {
+			log.Errorf("failed to get payload, error %v", err)
+			return
+		}
+		key := getManifestListKey(art.Repository, art.Digest)
+		log.Debugf("Cache manifest list with key=cache:%v", key)
+		err = c.cache.Save(key, payload, manifestListCacheIntervalSec)
+		if err != nil {
+			log.Errorf("failed to cache payload, error %v", err)
+		}
+	}
 	if contType == manifestlist.MediaTypeManifestList || contType == v1.MediaTypeImageIndex {
 		err := c.local.PushManifestList(ctx, art.Repository, getReference(art), man)
 		if err != nil {

src/controller/proxy/controller_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/goharbor/harbor/src/controller/artifact"
 	"github.com/goharbor/harbor/src/controller/blob"
 	"github.com/goharbor/harbor/src/lib"
+	_ "github.com/goharbor/harbor/src/lib/cache"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/suite"
@@ -116,7 +117,7 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_True() {
 	art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
 	p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
 
-	result, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
+	result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
 	p.Assert().Nil(err)
 	p.Assert().True(result)
 }
@@ -125,8 +126,9 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_False() {
 	ctx := context.Background()
 	dig := "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b"
 	art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
+	p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(true, dig, nil)
 	p.local.On("GetManifest", mock.Anything, mock.Anything).Return(nil, nil)
-	result, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
+	result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
 	p.Assert().Nil(err)
 	p.Assert().False(result)
 }
@@ -136,7 +138,7 @@ func (p *proxyControllerTestSuite) TestUseLocalManifestWithTag_False() {
 	art := lib.ArtifactInfo{Repository: "library/hello-world", Tag: "latest"}
 	p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
 	p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, "", nil)
-	result, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
+	result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
 	p.Assert().True(errors.IsNotFoundErr(err))
 	p.Assert().False(result)
 }

src/controller/proxy/local.go

@@ -127,7 +127,7 @@ func (l *localHelper) PushManifest(repo string, ref string, manifest distributio
 
 // DeleteManifest cleanup delete tag from local repo
 func (l *localHelper) DeleteManifest(repo, ref string) {
-	log.Debug("Remove tag from repo if it is exist")
+	log.Debugf("Remove tag from repo if it is exist, repo: %v ref: %v", repo, ref)
 	if err := l.registry.DeleteManifest(repo, ref); err != nil {
 		// sometimes user pull a non-exist image
 		log.Warningf("failed to remove artifact, error %v", err)

src/server/middleware/repoproxy/proxy.go

@@ -36,6 +36,13 @@ import (
 
 var registryMgr = registry.NewDefaultManager()
 
+const (
+	contentLength       = "Content-Length"
+	contentType         = "Content-Type"
+	dockerContentDigest = "Docker-Content-Digest"
+	etag                = "Etag"
+)
+
 // BlobGetMiddleware handle get blob request
 func BlobGetMiddleware() func(http.Handler) http.Handler {
 	return middleware.New(func(w http.ResponseWriter, r *http.Request, next http.Handler) {
@@ -106,14 +113,28 @@ func handleManifest(w http.ResponseWriter, r *http.Request, next http.Handler) e
 	if err != nil {
 		return err
 	}
-	useLocal, err := proxyCtl.UseLocalManifest(ctx, art, remote)
+	useLocal, man, err := proxyCtl.UseLocalManifest(ctx, art, remote)
+
 	if err != nil {
 		return err
 	}
 	if useLocal {
+		if man != nil {
+			w.Header().Set(contentLength, fmt.Sprintf("%v", len(man.Content)))
+			w.Header().Set(contentType, man.ContentType)
+			w.Header().Set(dockerContentDigest, man.Digest)
+			w.Header().Set(etag, man.Digest)
+			if r.Method == http.MethodGet {
+				w.Write(man.Content)
+			}
+			return nil
+		}
 		next.ServeHTTP(w, r)
 		return nil
 	}
+
+	log.Warningf("Artifact: %v:%v, digest:%v is not found in proxy cache, fetch it from remote repo", art.Repository, art.Tag, art.Digest)
+
 	log.Debugf("the tag is %v, digest is %v", art.Tag, art.Digest)
 	if r.Method == http.MethodHead {
 		err = proxyManifestHead(ctx, w, proxyCtl, p, art, remote)
@@ -163,12 +184,12 @@ func canProxy(p *models.Project) bool {
 
 func setHeaders(w http.ResponseWriter, size int64, mediaType string, dig string) {
 	h := w.Header()
-	h.Set("Content-Length", fmt.Sprintf("%v", size))
+	h.Set(contentLength, fmt.Sprintf("%v", size))
 	if len(mediaType) > 0 {
-		h.Set("Content-Type", mediaType)
+		h.Set(contentType, mediaType)
 	}
-	h.Set("Docker-Content-Digest", dig)
+	h.Set(dockerContentDigest, dig)
-	h.Set("Etag", dig)
+	h.Set(etag, dig)
 }
 
 // isProxySession check if current security context is proxy session
@@ -213,7 +234,7 @@ func proxyManifestHead(ctx context.Context, w http.ResponseWriter, ctl proxy.Con
 	if !exist {
 		return errors.NotFoundError(fmt.Errorf("The tag %v:%v is not found", art.Repository, art.Tag))
 	}
-	w.Header().Set("Docker-Content-Digest", dig)
+	w.Header().Set(dockerContentDigest, dig)
-	w.Header().Set("Etag", dig)
+	w.Header().Set(etag, dig)
 	return nil
 }