set tag pull time for proxy cache (goharbor#18731)

fixes goharbor#18708 to set the pull time of tag for the first time cache the artifact. Signed-off-by: Wang Yan <wangyan@vmware.com>
WilfredAlmeida · Jul 8, 2023 · eee4d9a · eee4d9a
1 parent fab3be6
commit eee4d9a
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 18 deletions.
diff --git a/src/controller/artifact/controller.go b/src/controller/artifact/controller.go
@@ -154,7 +154,7 @@ func (c *controller) Ensure(ctx context.Context, repository, digest string, opti
 	}
 	if option != nil {
 		for _, tag := range option.Tags {
-			if err = c.tagCtl.Ensure(ctx, artifact.RepositoryID, artifact.ID, tag); err != nil {
+			if _, err = c.tagCtl.Ensure(ctx, artifact.RepositoryID, artifact.ID, tag); err != nil {
 				return false, 0, err
 			}
 		}

diff --git a/src/controller/artifact/controller_test.go b/src/controller/artifact/controller_test.go
@@ -250,7 +250,7 @@ func (c *controllerTestSuite) TestEnsure() {
 	c.artMgr.On("GetByDigest", mock.Anything, mock.Anything, mock.Anything).Return(nil, errors.NotFoundError(nil))
 	c.artMgr.On("Create", mock.Anything, mock.Anything).Return(int64(1), nil)
 	c.abstractor.On("AbstractMetadata").Return(nil)
-	c.tagCtl.On("Ensure").Return(nil)
+	c.tagCtl.On("Ensure").Return(int64(1), nil)
 	c.accMgr.On("Ensure").Return(nil)
 	_, id, err := c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), "library/hello-world", digest, &ArtOption{
 		Tags: []string{"latest"},
@@ -563,7 +563,7 @@ func (c *controllerTestSuite) TestCopy() {
 	c.abstractor.On("AbstractMetadata").Return(nil)
 	c.artMgr.On("Create", mock.Anything, mock.Anything).Return(int64(1), nil)
 	c.regCli.On("Copy", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
-	c.tagCtl.On("Ensure").Return(nil)
+	c.tagCtl.On("Ensure").Return(int64(1), nil)
 	c.accMgr.On("Ensure", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	_, err := c.ctl.Copy(orm.NewContext(nil, &ormtesting.FakeOrmer{}), "library/hello-world", "latest", "library/hello-world2")
 	c.Require().Nil(err)

diff --git a/src/controller/proxy/controller.go b/src/controller/proxy/controller.go
@@ -35,6 +35,7 @@ import (
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/orm"
 	proModels "github.com/goharbor/harbor/src/pkg/project/models"
+	model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag"
 )
 
 const (
@@ -117,7 +118,17 @@ func (c *controller) EnsureTag(ctx context.Context, art lib.ArtifactInfo, tagNam
 	if a == nil {
 		return fmt.Errorf("the artifact is not ready yet, failed to tag it to %v", tagName)
 	}
-	return tag.Ctl.Ensure(ctx, a.RepositoryID, a.Artifact.ID, tagName)
+	tagID, err := tag.Ctl.Ensure(ctx, a.RepositoryID, a.Artifact.ID, tagName)
+	if err != nil {
+		return err
+	}
+	// update the pull time of tag for the first time cache
+	return tag.Ctl.Update(ctx, &tag.Tag{
+		Tag: model_tag.Tag{
+			ID:       tagID,
+			PullTime: time.Now(),
+		},
+	}, "PullTime")
 }
 
 func (c *controller) UseLocalBlob(ctx context.Context, art lib.ArtifactInfo) bool {

diff --git a/src/controller/tag/controller.go b/src/controller/tag/controller.go
@@ -41,7 +41,7 @@ var (
 // Controller manages the tags
 type Controller interface {
 	// Ensure
-	Ensure(ctx context.Context, repositoryID, artifactID int64, name string) error
+	Ensure(ctx context.Context, repositoryID, artifactID int64, name string) (int64, error)
 	// Count returns the total count of tags according to the query.
 	Count(ctx context.Context, query *q.Query) (total int64, err error)
 	// List tags according to the query
@@ -74,7 +74,7 @@ type controller struct {
 }
 
 // Ensure ...
-func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) error {
+func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) (int64, error) {
 	query := &q.Query{
 		Keywords: map[string]interface{}{
 			"repository_id": repositoryID,
@@ -85,43 +85,44 @@ func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64,
 		WithImmutableStatus: true,
 	})
 	if err != nil {
-		return err
+		return 0, err
 	}
 	// the tag already exists under the repository
 	if len(tags) > 0 {
 		tag := tags[0]
 		// the tag already exists under the repository and is attached to the artifact, return directly
 		if tag.ArtifactID == artifactID {
-			return nil
+			return tag.ID, nil
 		}
 		// existing tag must check the immutable status and signature
 		if tag.Immutable {
-			return errors.New(nil).WithCode(errors.PreconditionCode).
+			return 0, errors.New(nil).WithCode(errors.PreconditionCode).
 				WithMessage("the tag %s configured as immutable, cannot be updated", tag.Name)
 		}
 		// the tag exists under the repository, but it is attached to other artifact
 		// update it to point to the provided artifact
 		tag.ArtifactID = artifactID
 		tag.PushTime = time.Now()
-		return c.Update(ctx, tag, "ArtifactID", "PushTime")
+		return tag.ID, c.Update(ctx, tag, "ArtifactID", "PushTime")
 	}
 
 	// the tag doesn't exist under the repository, create it
 	// use orm.WithTransaction here to avoid the issue:
 	// https://www.postgresql.org/message-id/002e01c04da9%24a8f95c20%2425efe6c1%40lasting.ro
+	tagID := int64(0)
 	if err = orm.WithTransaction(func(ctx context.Context) error {
 		tag := &Tag{}
 		tag.RepositoryID = repositoryID
 		tag.ArtifactID = artifactID
 		tag.Name = name
 		tag.PushTime = time.Now()
-		_, err = c.Create(ctx, tag)
+		tagID, err = c.Create(ctx, tag)
 		return err
 	})(orm.SetTransactionOpNameToContext(ctx, "tx-tag-ensure")); err != nil && !errors.IsConflictErr(err) {
-		return err
+		return 0, err
 	}
 
-	return nil
+	return tagID, nil
 }
 
 // Count ...

diff --git a/src/controller/tag/controller_test.go b/src/controller/tag/controller_test.go
@@ -68,7 +68,7 @@ func (c *controllerTestSuite) TestEnsureTag() {
 		ID: 1,
 	}, nil)
 	c.immutableMtr.On("Match").Return(false, nil)
-	err := c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest")
+	_, err := c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest")
 	c.Require().Nil(err)
 	c.tagMgr.AssertExpectations(c.T())
 
@@ -89,7 +89,7 @@ func (c *controllerTestSuite) TestEnsureTag() {
 		ID: 1,
 	}, nil)
 	c.immutableMtr.On("Match").Return(false, nil)
-	err = c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest")
+	_, err = c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest")
 	c.Require().Nil(err)
 	c.tagMgr.AssertExpectations(c.T())
 
@@ -103,7 +103,7 @@ func (c *controllerTestSuite) TestEnsureTag() {
 		ID: 1,
 	}, nil)
 	c.immutableMtr.On("Match").Return(false, nil)
-	err = c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest")
+	_, err = c.ctl.Ensure(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, 1, "latest")
 	c.Require().Nil(err)
 	c.tagMgr.AssertExpectations(c.T())
 }

diff --git a/src/testing/controller/tag/controller.go b/src/testing/controller/tag/controller.go
@@ -15,9 +15,9 @@ type FakeController struct {
 }
 
 // Ensure ...
-func (f *FakeController) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) error {
+func (f *FakeController) Ensure(ctx context.Context, repositoryID, artifactID int64, name string) (int64, error) {
 	args := f.Called()
-	return args.Error(0)
+	return int64(0), args.Error(1)
 }
 
 // Count ...