Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set tag pull time for proxy cache #18731

Merged
merged 2 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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