Merge pull request #8771 from wy65701436/fix-manifest-dup

fix quota count size for same manifest in different repo
This commit is contained in:
Wang Yan 2019-08-23 08:37:03 +08:00 committed by GitHub
commit 299032d602
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 35 additions and 53 deletions

View File

@ -78,10 +78,15 @@ func GetBlobsByArtifact(artifactDigest string) ([]*models.Blob, error) {
// GetExclusiveBlobs returns layers of repository:tag which are not shared with other repositories in the project
func GetExclusiveBlobs(projectID int64, repository, digest string) ([]*models.Blob, error) {
var exclusive []*models.Blob
blobs, err := GetBlobsByArtifact(digest)
if err != nil {
return nil, err
}
if len(blobs) == 0 {
return exclusive, nil
}
sql := fmt.Sprintf(`
SELECT
@ -103,13 +108,11 @@ FROM
)
) AS a
LEFT JOIN artifact_blob b ON a.digest = b.digest_af
AND b.digest_blob IN (%s)`, ParamPlaceholderForIn(len(blobs)-1))
AND b.digest_blob IN (%s)`, ParamPlaceholderForIn(len(blobs)))
params := []interface{}{projectID, repository, projectID, digest}
for _, blob := range blobs {
if blob.Digest != digest {
params = append(params, blob.Digest)
}
params = append(params, blob.Digest)
}
var rows []struct {
@ -125,9 +128,8 @@ FROM
shared[row.Digest] = true
}
var exclusive []*models.Blob
for _, blob := range blobs {
if blob.Digest != digest && !shared[blob.Digest] {
if !shared[blob.Digest] {
exclusive = append(exclusive, blob)
}
}

View File

@ -133,30 +133,32 @@ func (suite *GetExclusiveBlobsSuite) mustPrepareImage(projectID int64, projectNa
func (suite *GetExclusiveBlobsSuite) TestInSameRepository() {
withProject(func(projectID int64, projectName string) {
digest1 := digest.FromString(utils.GenerateRandomString()).String()
digest2 := digest.FromString(utils.GenerateRandomString()).String()
digest3 := digest.FromString(utils.GenerateRandomString()).String()
manifest1 := suite.mustPrepareImage(projectID, projectName, "mysql", "latest", digest1, digest2)
if blobs, err := GetExclusiveBlobs(projectID, projectName+"/mysql", manifest1); suite.Nil(err) {
suite.Len(blobs, 2)
suite.Len(blobs, 3)
}
manifest2 := suite.mustPrepareImage(projectID, projectName, "mysql", "8.0", digest1, digest2)
if blobs, err := GetExclusiveBlobs(projectID, projectName+"/mysql", manifest2); suite.Nil(err) {
suite.Len(blobs, 2)
suite.Len(blobs, 3)
}
manifest3 := suite.mustPrepareImage(projectID, projectName, "mysql", "dev", digest1, digest2, digest3)
if blobs, err := GetExclusiveBlobs(projectID, projectName+"/mysql", manifest1); suite.Nil(err) {
suite.Len(blobs, 0)
suite.Len(blobs, 1)
suite.Equal(manifest1, blobs[0].Digest)
}
if blobs, err := GetExclusiveBlobs(projectID, projectName+"/mysql", manifest2); suite.Nil(err) {
suite.Len(blobs, 0)
suite.Len(blobs, 1)
suite.Equal(manifest2, blobs[0].Digest)
}
if blobs, err := GetExclusiveBlobs(projectID, projectName+"/mysql", manifest3); suite.Nil(err) {
suite.Len(blobs, 1)
suite.Equal(digest3, blobs[0].Digest)
suite.Len(blobs, 2)
}
})
}
@ -169,7 +171,7 @@ func (suite *GetExclusiveBlobsSuite) TestInDifferentRepositories() {
manifest1 := suite.mustPrepareImage(projectID, projectName, "mysql", "latest", digest1, digest2)
if blobs, err := GetExclusiveBlobs(projectID, projectName+"/mysql", manifest1); suite.Nil(err) {
suite.Len(blobs, 2)
suite.Len(blobs, 3)
}
manifest2 := suite.mustPrepareImage(projectID, projectName, "mariadb", "latest", digest1, digest2)
@ -188,8 +190,7 @@ func (suite *GetExclusiveBlobsSuite) TestInDifferentRepositories() {
suite.Len(blobs, 0)
}
if blobs, err := GetExclusiveBlobs(projectID, projectName+"/mysql", manifest3); suite.Nil(err) {
suite.Len(blobs, 1)
suite.Equal(digest3, blobs[0].Digest)
suite.Len(blobs, 2)
}
})
}
@ -201,16 +202,16 @@ func (suite *GetExclusiveBlobsSuite) TestInDifferentProjects() {
manifest1 := suite.mustPrepareImage(projectID, projectName, "mysql", "latest", digest1, digest2)
if blobs, err := GetExclusiveBlobs(projectID, projectName+"/mysql", manifest1); suite.Nil(err) {
suite.Len(blobs, 2)
suite.Len(blobs, 3)
}
withProject(func(id int64, name string) {
manifest2 := suite.mustPrepareImage(id, name, "mysql", "latest", digest1, digest2)
if blobs, err := GetExclusiveBlobs(projectID, projectName+"/mysql", manifest1); suite.Nil(err) {
suite.Len(blobs, 2)
suite.Len(blobs, 3)
}
if blobs, err := GetExclusiveBlobs(id, name+"/mysql", manifest2); suite.Nil(err) {
suite.Len(blobs, 2)
suite.Len(blobs, 3)
}
})

View File

@ -77,7 +77,7 @@ func (*manifestCreationBuilder) Build(req *http.Request) (interceptor.Intercepto
info, ok := util.ManifestInfoFromContext(req.Context())
if !ok {
var err error
info, err = util.ParseManifestInfo(req)
info, err = util.ParseManifestInfoFromReq(req)
if err != nil {
return nil, fmt.Errorf("failed to parse manifest, error %v", err)
}

View File

@ -20,7 +20,6 @@ import (
"strconv"
"github.com/goharbor/harbor/src/common/dao"
"github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/utils/log"
"github.com/goharbor/harbor/src/core/config"
"github.com/goharbor/harbor/src/core/middlewares/interceptor"
@ -112,7 +111,7 @@ func (*manifestCreationBuilder) Build(req *http.Request) (interceptor.Intercepto
return nil, nil
}
info, err := util.ParseManifestInfo(req)
info, err := util.ParseManifestInfoFromReq(req)
if err != nil {
return nil, err
}
@ -192,18 +191,6 @@ func (*manifestDeletionBuilder) Build(req *http.Request) (interceptor.Intercepto
quota.MutexKeys(mutexKeys...),
quota.OnFulfilled(func(http.ResponseWriter, *http.Request) error {
blobs := info.ExclusiveBlobs
total, err := dao.GetTotalOfArtifacts(&models.ArtifactQuery{
PID: info.ProjectID,
Digest: info.Digest,
})
if err == nil && total > 0 {
blob, err := dao.GetBlob(info.Digest)
if err == nil {
blobs = append(blobs, blob)
}
}
return dao.RemoveBlobsFromProject(info.ProjectID, blobs...)
}),
}

View File

@ -453,10 +453,10 @@ func (suite *HandlerSuite) TestPushImageToDifferentRepositories() {
suite.checkStorageUsage(size, projectID)
pushImage(projectName, "redis", "latest", manifest)
suite.checkStorageUsage(size+sizeOfManifest(manifest), projectID)
suite.checkStorageUsage(size, projectID)
pushImage(projectName, "postgres", "latest", manifest)
suite.checkStorageUsage(size+2*sizeOfManifest(manifest), projectID)
suite.checkStorageUsage(size, projectID)
})
}
@ -562,7 +562,7 @@ func (suite *HandlerSuite) TestDeleteManifestInDifferentRepositories() {
pushImage(projectName, "redis", "latest", manifest)
suite.checkCountUsage(3, projectID)
suite.checkStorageUsage(size+sizeOfManifest(manifest), projectID)
suite.checkStorageUsage(size, projectID)
deleteManifest(projectName, "redis", digestOfManifest(manifest))
suite.checkCountUsage(2, projectID)
@ -570,7 +570,7 @@ func (suite *HandlerSuite) TestDeleteManifestInDifferentRepositories() {
pushImage(projectName, "redis", "latest", manifest)
suite.checkCountUsage(3, projectID)
suite.checkStorageUsage(size+sizeOfManifest(manifest), projectID)
suite.checkStorageUsage(size, projectID)
})
}

View File

@ -146,7 +146,7 @@ func parseBlobInfoFromComplete(req *http.Request) (*util.BlobInfo, error) {
func parseBlobInfoFromManifest(req *http.Request) (*util.BlobInfo, error) {
info, ok := util.ManifestInfoFromContext(req.Context())
if !ok {
manifest, err := util.ParseManifestInfo(req)
manifest, err := util.ParseManifestInfoFromReq(req)
if err != nil {
return nil, err
}
@ -295,14 +295,7 @@ func computeResourcesForManifestDeletion(req *http.Request) (types.ResourceList,
info.ExclusiveBlobs = blobs
blob, err := dao.GetBlob(info.Digest)
if err != nil {
return nil, err
}
// manifest size will always be released
size := blob.Size
var size int64
for _, blob := range blobs {
size = size + blob.Size
}

View File

@ -228,7 +228,6 @@ func (info *ManifestInfo) ManifestExists() (bool, error) {
info.manifestExistOnce.Do(func() {
total, err := dao.GetTotalOfArtifacts(&models.ArtifactQuery{
PID: info.ProjectID,
Repo: info.Repository,
Digest: info.Digest,
})
@ -441,8 +440,8 @@ func NewManifestInfoContext(ctx context.Context, info *ManifestInfo) context.Con
return context.WithValue(ctx, manifestInfoKey, info)
}
// ParseManifestInfo prase manifest from request
func ParseManifestInfo(req *http.Request) (*ManifestInfo, error) {
// ParseManifestInfoFromReq parse manifest from request
func ParseManifestInfoFromReq(req *http.Request) (*ManifestInfo, error) {
match, repository, reference := MatchManifestURL(req)
if !match {
return nil, fmt.Errorf("not match url %s for manifest", req.URL.Path)
@ -496,7 +495,7 @@ func ParseManifestInfo(req *http.Request) (*ManifestInfo, error) {
}, nil
}
// ParseManifestInfoFromPath prase manifest from request path
// ParseManifestInfoFromPath parse manifest from request path
func ParseManifestInfoFromPath(req *http.Request) (*ManifestInfo, error) {
match, repository, reference := MatchManifestURL(req)
if !match {

View File

@ -326,13 +326,13 @@ func TestParseManifestInfo(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ParseManifestInfo(tt.req())
got, err := ParseManifestInfoFromReq(tt.req())
if (err != nil) != tt.wantErr {
t.Errorf("ParseManifestInfo() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("ParseManifestInfoFromReq() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("ParseManifestInfo() = %v, want %v", got, tt.want)
t.Errorf("ParseManifestInfoFromReq() = %v, want %v", got, tt.want)
}
})
}