add time windows support in artifact trash (#12400)

support with time window to filter and delete item in artifact trash

Signed-off-by: wang yan <wangyan@vmware.com>

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
 	}

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 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) (arts []model.ArtifactTrash, err error)
+	Filter(ctx context.Context, cutOff time.Time) (arts []model.ArtifactTrash, err error)
-	// Flush clean the trash table
+	// Flush cleans the trash table record, which creation_time must be less than or equal to the cut-off.
-	Flush(ctx context.Context) (err error)
+	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 ...
+// Flush delete all of items beside the one in the time window.
-func (d *dao) Flush(ctx context.Context) (err error) {
+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
 	}

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
+	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)
 }
 

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 lists the artifact that needs to be cleaned, which creation_time is not in the time window.
-	Filter(ctx context.Context) (arts []model.ArtifactTrash, err error)
+	// The unit of timeWindow is hour, the represent cut-off is time.now() - timeWindow * time.Hours
-	// Flush clean the trash table
+	Filter(ctx context.Context, timeWindow int64) (arts []model.ArtifactTrash, err error)
-	Flush(ctx context.Context) (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) {
+func (m *manager) Filter(ctx context.Context, timeWindow int64) (arts []model.ArtifactTrash, err error) {
-	return m.dao.Filter(ctx)
+	return m.dao.Filter(ctx, time.Now().Add(-time.Duration(timeWindow)*time.Hour))
 }
 
-func (m *manager) Flush(ctx context.Context) (err error) {
+func (m *manager) Flush(ctx context.Context, timeWindow int64) (err error) {
-	return m.dao.Flush(ctx)
+	return m.dao.Flush(ctx, time.Now().Add(-time.Duration(timeWindow)*time.Hour))
 }

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())
 }

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)
 }