Skip to content

Commit

Permalink
set tag pull time for proxy cache (#18731)
Browse files Browse the repository at this point in the history
fixes #18708

to set the pull time of tag for the first time cache the artifact.

Signed-off-by: Wang Yan <wangyan@vmware.com>
  • Loading branch information
wy65701436 authored May 26, 2023
1 parent 135ca37 commit 06aa87a
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/controller/artifact/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/controller/artifact/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 12 additions & 1 deletion src/controller/proxy/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 10 additions & 9 deletions src/controller/tag/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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 ...
Expand Down
6 changes: 3 additions & 3 deletions src/controller/tag/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand All @@ -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())

Expand All @@ -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())
}
Expand Down
4 changes: 2 additions & 2 deletions src/testing/controller/tag/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ...
Expand Down

0 comments on commit 06aa87a

Please sign in to comment.