diff --git a/src/jobservice/job/impl/gc/garbage_collection.go b/src/jobservice/job/impl/gc/garbage_collection.go index 29388ed48..945461db4 100644 --- a/src/jobservice/job/impl/gc/garbage_collection.go +++ b/src/jobservice/job/impl/gc/garbage_collection.go @@ -234,7 +234,7 @@ func (gc *GarbageCollector) deleteCandidates(ctx job.Context) error { defer func() { if flushTrash { gc.logger.Info("flush artifact trash") - if err := gc.artrashMgr.Flush(ctx.SystemContext()); err != nil { + if err := gc.artrashMgr.Flush(ctx.SystemContext(), 0); err != nil { gc.logger.Errorf("failed to flush artifact trash: %v", err) } } @@ -264,7 +264,7 @@ func (gc *GarbageCollector) deleteCandidates(ctx job.Context) error { } // handle the trash - required, err := gc.artrashMgr.Filter(ctx.SystemContext()) + required, err := gc.artrashMgr.Filter(ctx.SystemContext(), 0) if err != nil { return err } diff --git a/src/pkg/artifactrash/dao/dao.go b/src/pkg/artifactrash/dao/dao.go index 5ae057f34..cdda22a89 100644 --- a/src/pkg/artifactrash/dao/dao.go +++ b/src/pkg/artifactrash/dao/dao.go @@ -15,10 +15,10 @@ type DAO interface { Create(ctx context.Context, artifactrsh *model.ArtifactTrash) (id int64, err error) // Delete the artifact trash specified by ID Delete(ctx context.Context, id int64) (err error) - // Filter lists the artifact that needs to be cleaned - Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) - // Flush clean the trash table - Flush(ctx context.Context) (err error) + // Filter lists the artifact that needs to be cleaned, which creation_time must be less than or equal to the cut-off. + Filter(ctx context.Context, cutOff time.Time) (arts []model.ArtifactTrash, err error) + // Flush cleans the trash table record, which creation_time must be less than or equal to the cut-off. + Flush(ctx context.Context, cutOff time.Time) (err error) } // New returns an instance of the default DAO @@ -64,16 +64,16 @@ func (d *dao) Delete(ctx context.Context, id int64) (err error) { } // Filter the results are: all of records in artifact_trash excludes the records in artifact with same repo and digest. -func (d *dao) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) { +func (d *dao) Filter(ctx context.Context, cutOff time.Time) (arts []model.ArtifactTrash, err error) { var deletedAfs []model.ArtifactTrash ormer, err := orm.FromContext(ctx) if err != nil { return deletedAfs, err } - sql := `SELECT aft.* FROM artifact_trash AS aft LEFT JOIN artifact af ON (aft.repository_name=af.repository_name AND aft.digest=af.digest) WHERE (af.digest IS NULL AND af.repository_name IS NULL)` + sql := `SELECT aft.* FROM artifact_trash AS aft LEFT JOIN artifact af ON (aft.repository_name=af.repository_name AND aft.digest=af.digest) WHERE (af.digest IS NULL AND af.repository_name IS NULL) AND aft.creation_time <= ?` - _, err = ormer.Raw(sql).QueryRows(&deletedAfs) + _, err = ormer.Raw(sql, cutOff).QueryRows(&deletedAfs) if err != nil { return deletedAfs, err } @@ -81,17 +81,17 @@ func (d *dao) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error return deletedAfs, nil } -// Flush ... -func (d *dao) Flush(ctx context.Context) (err error) { +// Flush delete all of items beside the one in the time window. +func (d *dao) Flush(ctx context.Context, cutOff time.Time) (err error) { ormer, err := orm.FromContext(ctx) if err != nil { return err } - sql := `DELETE FROM artifact_trash` + sql := `DELETE FROM artifact_trash where creation_time <= ?` if err != nil { return err } - _, err = ormer.Raw(sql).Exec() + _, err = ormer.Raw(sql, cutOff).Exec() if err != nil { return err } diff --git a/src/pkg/artifactrash/dao/dao_test.go b/src/pkg/artifactrash/dao/dao_test.go index c94b73faa..383a6e8b2 100644 --- a/src/pkg/artifactrash/dao/dao_test.go +++ b/src/pkg/artifactrash/dao/dao_test.go @@ -3,36 +3,40 @@ package dao import ( "context" beegoorm "github.com/astaxie/beego/orm" - common_dao "github.com/goharbor/harbor/src/common/dao" errors "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/orm" artdao "github.com/goharbor/harbor/src/pkg/artifact/dao" "github.com/goharbor/harbor/src/pkg/artifactrash/model" + htesting "github.com/goharbor/harbor/src/testing" v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/suite" "testing" + "time" ) type daoTestSuite struct { - suite.Suite - dao DAO - afDao artdao.DAO - id int64 - ctx context.Context + dao DAO + afDao artdao.DAO + id int64 + ctx context.Context + digest string + htesting.Suite } func (d *daoTestSuite) SetupSuite() { + d.Suite.SetupSuite() + d.Suite.ClearTables = []string{"artifact", "artifact_trash"} d.dao = New() d.afDao = artdao.New() - common_dao.PrepareTestForPostgresSQL() d.ctx = orm.NewContext(nil, beegoorm.NewOrm()) + d.digest = d.Suite.DigestString() art1 := &artdao.Artifact{ Type: "image", ManifestMediaType: v1.MediaTypeImageManifest, ProjectID: 10, RepositoryID: 10, - Digest: "1234", + Digest: d.digest, } id, err := d.afDao.Create(d.ctx, art1) d.Require().Nil(err) @@ -43,7 +47,7 @@ func (d *daoTestSuite) SetupSuite() { ManifestMediaType: v1.MediaTypeImageManifest, ProjectID: 10, RepositoryID: 10, - Digest: "5678", + Digest: d.Suite.DigestString(), } _, err = d.afDao.Create(d.ctx, art2) d.Require().Nil(err) @@ -51,7 +55,7 @@ func (d *daoTestSuite) SetupSuite() { aft := &model.ArtifactTrash{ ManifestMediaType: v1.MediaTypeImageManifest, RepositoryName: "test/hello-world", - Digest: "1234", + Digest: d.digest, } id, err = d.dao.Create(d.ctx, aft) d.Require().Nil(err) @@ -67,7 +71,7 @@ func (d *daoTestSuite) TestCreate() { aft := &model.ArtifactTrash{ ManifestMediaType: v1.MediaTypeImageManifest, RepositoryName: "test/hello-world", - Digest: "1234", + Digest: d.digest, } _, err := d.dao.Create(d.ctx, aft) @@ -84,22 +88,23 @@ func (d *daoTestSuite) TestDelete() { } func (d *daoTestSuite) TestFilter() { - afs, err := d.dao.Filter(d.ctx) + afs, err := d.dao.Filter(d.ctx, time.Now().Add(time.Second*10)) d.Require().Nil(err) - d.Require().Equal(afs[0].Digest, "1234") + d.Require().Equal(afs[0].Digest, d.digest) // clean it in GC - err = d.dao.Flush(d.ctx) + err = d.dao.Flush(d.ctx, time.Now().Add(time.Second*10)) d.Require().Nil(err) // push hello-world to projecta + digest := d.Suite.DigestString() art1 := &artdao.Artifact{ Type: "image", ManifestMediaType: v1.MediaTypeImageManifest, ProjectID: 11, RepositoryID: 11, RepositoryName: "projectA/hello-world", - Digest: "sha256:hello-world", + Digest: digest, } _, err = d.afDao.Create(d.ctx, art1) d.Require().Nil(err) @@ -111,7 +116,7 @@ func (d *daoTestSuite) TestFilter() { ProjectID: 12, RepositoryID: 12, RepositoryName: "projectB/hello-world", - Digest: "sha256:hello-world", + Digest: digest, } _, err = d.afDao.Create(d.ctx, art2) d.Require().Nil(err) @@ -123,17 +128,21 @@ func (d *daoTestSuite) TestFilter() { aft2 := &model.ArtifactTrash{ ManifestMediaType: v1.MediaTypeImageManifest, RepositoryName: "projectA/hello-world", - Digest: "sha256:hello-world", + Digest: digest, } _, err = d.dao.Create(d.ctx, aft2) d.Require().Nil(err) // filter results should contain projectA hello-world - afs1, err := d.dao.Filter(d.ctx) + afs1, err := d.dao.Filter(d.ctx, time.Now().Add(time.Second*10)) d.Require().Nil(err) - d.Require().Equal(afs1[0].Digest, "sha256:hello-world") + d.Require().Equal(afs1[0].Digest, digest) d.Require().Equal(afs1[0].RepositoryName, "projectA/hello-world") + afs1, err = d.dao.Filter(d.ctx, time.Now().Add(-1*time.Hour)) + d.Require().Nil(err) + d.Require().Equal(0, len(afs1)) + // push hello-world again to projecta art3 := &artdao.Artifact{ Type: "image", @@ -141,13 +150,13 @@ func (d *daoTestSuite) TestFilter() { ProjectID: 11, RepositoryID: 13, RepositoryName: "projectA/hello-world", - Digest: "sha256:hello-world", + Digest: digest, } _, err = d.afDao.Create(d.ctx, art3) d.Require().Nil(err) // filter results should contain nothing - afs2, err := d.dao.Filter(d.ctx) + afs2, err := d.dao.Filter(d.ctx, time.Now()) d.Require().Nil(err) d.Require().Equal(0, len(afs2)) @@ -157,23 +166,23 @@ func (d *daoTestSuite) TestFlush() { _, err := d.dao.Create(d.ctx, &model.ArtifactTrash{ ManifestMediaType: v1.MediaTypeImageManifest, RepositoryName: "hello-world", - Digest: "abcd", + Digest: d.Suite.DigestString(), }) d.Require().Nil(err) _, err = d.dao.Create(d.ctx, &model.ArtifactTrash{ ManifestMediaType: v1.MediaTypeImageManifest, RepositoryName: "hello-world2", - Digest: "efgh", + Digest: d.Suite.DigestString(), }) d.Require().Nil(err) _, err = d.dao.Create(d.ctx, &model.ArtifactTrash{ ManifestMediaType: v1.MediaTypeImageManifest, RepositoryName: "hello-world3", - Digest: "ijkl", + Digest: d.Suite.DigestString(), }) d.Require().Nil(err) - err = d.dao.Flush(d.ctx) + err = d.dao.Flush(d.ctx, time.Now()) d.Require().Nil(err) } diff --git a/src/pkg/artifactrash/manager.go b/src/pkg/artifactrash/manager.go index 07ab58ad7..cd302bca3 100644 --- a/src/pkg/artifactrash/manager.go +++ b/src/pkg/artifactrash/manager.go @@ -18,6 +18,7 @@ import ( "context" "github.com/goharbor/harbor/src/pkg/artifactrash/dao" "github.com/goharbor/harbor/src/pkg/artifactrash/model" + "time" ) var ( @@ -31,10 +32,12 @@ type Manager interface { Create(ctx context.Context, artifactrsh *model.ArtifactTrash) (id int64, err error) // Delete ... Delete(ctx context.Context, id int64) (err error) - // Filter ... - Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) - // Flush clean the trash table - Flush(ctx context.Context) (err error) + // Filter lists the artifact that needs to be cleaned, which creation_time is not in the time window. + // The unit of timeWindow is hour, the represent cut-off is time.now() - timeWindow * time.Hours + Filter(ctx context.Context, timeWindow int64) (arts []model.ArtifactTrash, err error) + // Flush cleans the trash table record, which creation_time is not in the time window. + // The unit of timeWindow is hour, the represent cut-off is time.now() - timeWindow * time.Hours + Flush(ctx context.Context, timeWindow int64) (err error) } // NewManager returns an instance of the default manager @@ -56,10 +59,10 @@ func (m *manager) Create(ctx context.Context, artifactrsh *model.ArtifactTrash) func (m *manager) Delete(ctx context.Context, id int64) error { return m.dao.Delete(ctx, id) } -func (m *manager) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) { - return m.dao.Filter(ctx) +func (m *manager) Filter(ctx context.Context, timeWindow int64) (arts []model.ArtifactTrash, err error) { + return m.dao.Filter(ctx, time.Now().Add(-time.Duration(timeWindow)*time.Hour)) } -func (m *manager) Flush(ctx context.Context) (err error) { - return m.dao.Flush(ctx) +func (m *manager) Flush(ctx context.Context, timeWindow int64) (err error) { + return m.dao.Flush(ctx, time.Now().Add(-time.Duration(timeWindow)*time.Hour)) } diff --git a/src/pkg/artifactrash/manager_test.go b/src/pkg/artifactrash/manager_test.go index 1b1e01bec..c7eecff87 100644 --- a/src/pkg/artifactrash/manager_test.go +++ b/src/pkg/artifactrash/manager_test.go @@ -6,6 +6,7 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" + "time" ) type fakeDao struct { @@ -20,11 +21,11 @@ func (f *fakeDao) Delete(ctx context.Context, id int64) (err error) { args := f.Called() return args.Error(0) } -func (f *fakeDao) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) { +func (f *fakeDao) Filter(ctx context.Context, timeWindow time.Time) (arts []model.ArtifactTrash, err error) { args := f.Called() return args.Get(0).([]model.ArtifactTrash), args.Error(1) } -func (f *fakeDao) Flush(ctx context.Context) (err error) { +func (f *fakeDao) Flush(ctx context.Context, timeWindow time.Time) (err error) { args := f.Called() return args.Error(0) } @@ -69,14 +70,14 @@ func (m *managerTestSuite) TestFilter() { Digest: "5678", }, }, nil) - arts, err := m.mgr.Filter(nil) + arts, err := m.mgr.Filter(nil, 0) m.Require().Nil(err) m.Equal(len(arts), 1) } func (m *managerTestSuite) TestFlush() { m.dao.On("Flush", mock.Anything).Return(nil) - err := m.mgr.Flush(nil) + err := m.mgr.Flush(nil, 0) m.Require().Nil(err) m.dao.AssertExpectations(m.T()) } diff --git a/src/testing/pkg/artifactrash/manager.go b/src/testing/pkg/artifactrash/manager.go index 0a52f0329..30e73842c 100644 --- a/src/testing/pkg/artifactrash/manager.go +++ b/src/testing/pkg/artifactrash/manager.go @@ -41,13 +41,13 @@ func (f *FakeManager) Delete(ctx context.Context, id int64) error { } // Filter ... -func (f *FakeManager) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error) { +func (f *FakeManager) Filter(ctx context.Context, timeWindow int64) (arts []model.ArtifactTrash, err error) { args := f.Called() return args.Get(0).([]model.ArtifactTrash), args.Error(1) } // Flush ... -func (f *FakeManager) Flush(ctx context.Context) (err error) { +func (f *FakeManager) Flush(ctx context.Context, timeWindow int64) (err error) { args := f.Called() return args.Error(0) }