Merge pull request #10954 from wy65701436/fix-gc-job

Fix gc issue on clean the artifact trash

make/photon/prepare/templates/jobservice/env.jinja

@@ -6,3 +6,5 @@ JOBSERVICE_WEBHOOK_JOB_MAX_RETRY={{notification_webhook_job_max_retry}}
 HTTP_PROXY={{jobservice_http_proxy}}
 HTTPS_PROXY={{jobservice_https_proxy}}
 NO_PROXY={{jobservice_no_proxy}}
+REGISTRY_CREDENTIAL_USERNAME={{registry_username}}
+REGISTRY_CREDENTIAL_PASSWORD={{registry_password}}

src/core/api/reg_gc.go

@@ -63,7 +63,10 @@ func (gc *GCAPI) Prepare() {
 //  }
 //	}
 func (gc *GCAPI) Post() {
-	ajr := models.AdminJobReq{}
+	parameters := make(map[string]interface{})
+	ajr := models.AdminJobReq{
+		Parameters: parameters,
+	}
 	isValid, err := gc.DecodeJSONReqAndValidate(&ajr)
 	if !isValid {
 		gc.SendBadRequestError(err)
@@ -87,7 +90,10 @@ func (gc *GCAPI) Post() {
 //  }
 //	}
 func (gc *GCAPI) Put() {
-	ajr := models.AdminJobReq{}
+	parameters := make(map[string]interface{})
+	ajr := models.AdminJobReq{
+		Parameters: parameters,
+	}
 	isValid, err := gc.DecodeJSONReqAndValidate(&ajr)
 	if !isValid {
 		gc.SendBadRequestError(err)

src/jobservice/job/impl/context.go

@@ -17,6 +17,8 @@ package impl
 import (
 	"context"
 	"fmt"
+	o "github.com/astaxie/beego/orm"
+	"github.com/goharbor/harbor/src/internal/orm"
 	"math"
 	"sync"
 	"time"
@@ -101,7 +103,7 @@ func (c *Context) Build(tracker job.Tracker) (job.Context, error) {
 	}
 
 	jContext := &Context{
-		sysContext: c.sysContext,
+		sysContext: orm.NewContext(c.sysContext, o.NewOrm()),
 		cfgMgr:     c.cfgMgr,
 		properties: make(map[string]interface{}),
 		tracker:    tracker,

src/jobservice/job/impl/context_test.go

@@ -16,6 +16,7 @@ package impl
 
 import (
 	"context"
+	common_dao "github.com/goharbor/harbor/src/common/dao"
 	"os"
 	"testing"
 
@@ -44,6 +45,7 @@ type ContextImplTestSuite struct {
 
 // TestContextImplTestSuite is entry of go test
 func TestContextImplTestSuite(t *testing.T) {
+	common_dao.PrepareTestForPostgresSQL()
 	suite.Run(t, new(ContextImplTestSuite))
 }
 

src/jobservice/job/impl/gc/garbage_collection.go

@@ -199,12 +199,12 @@ func (gc *GarbageCollector) cleanCache() error {
 // 1, required part, the artifacts were removed from Harbor.
 // 2, optional part, the untagged artifacts.
 func (gc *GarbageCollector) deleteCandidates(ctx job.Context) error {
-	// default is to clean trash
-	flushTrash := true
+	// default is not to clean trash
+	flushTrash := false
 	defer func() {
 		if flushTrash {
 			if err := gc.artrashMgr.Flush(ctx.SystemContext()); err != nil {
-				gc.logger.Errorf("failed to flush artifact trash")
+				gc.logger.Errorf("failed to flush artifact trash: %v", err)
 			}
 		}
 	}()
@@ -230,13 +230,13 @@ func (gc *GarbageCollector) deleteCandidates(ctx job.Context) error {
 	// handle the trash
 	required, err := gc.artrashMgr.Filter(ctx.SystemContext())
 	if err != nil {
-		return nil
+		return err
 	}
 	for _, art := range required {
 		if err := deleteManifest(art.RepositoryName, art.Digest); err != nil {
-			flushTrash = false
 			return fmt.Errorf("failed to delete manifest, %s:%s with error: %v", art.RepositoryName, art.Digest, err)
 		}
 	}
+	flushTrash = true
 	return nil
 }

src/pkg/artifactrash/dao/dao.go

@@ -73,7 +73,8 @@ func (d *dao) Filter(ctx context.Context) (arts []model.ArtifactTrash, err error
 
 	sql := `SELECT * FROM artifact_trash where artifact_trash.digest NOT IN (select digest from artifact)`
 
-	if err := ormer.Raw(sql).QueryRow(&deletedAfs); err != nil {
+	_, err = ormer.Raw(sql).QueryRows(&deletedAfs)
+	if err != nil {
 		return deletedAfs, err
 	}
 	return deletedAfs, nil
@@ -85,7 +86,7 @@ func (d *dao) Flush(ctx context.Context) (err error) {
 	if err != nil {
 		return err
 	}
-	sql := `DELETE * FROM artifact_trash`
+	sql := `DELETE FROM artifact_trash`
 	if err != nil {
 		return err
 	}

src/pkg/artifactrash/dao/dao_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/goharbor/harbor/src/pkg/artifactrash/model"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/suite"
+	"testing"
 )
 
 type daoTestSuite struct {
@@ -23,11 +24,10 @@ type daoTestSuite struct {
 
 func (d *daoTestSuite) SetupSuite() {
 	d.dao = New()
+	d.afDao = artdao.New()
 	common_dao.PrepareTestForPostgresSQL()
 	d.ctx = orm.NewContext(nil, beegoorm.NewOrm())
-}
 
-func (d *daoTestSuite) SetupTest() {
 	art1 := &artdao.Artifact{
 		Type:              "image",
 		ManifestMediaType: v1.MediaTypeImageManifest,
@@ -56,12 +56,11 @@ func (d *daoTestSuite) SetupTest() {
 	}
 	id, err = d.dao.Create(d.ctx, aft)
 	d.Require().Nil(err)
-	d.id = id
+	d.id = art2.ID
 }
 
-func (d *daoTestSuite) TearDownTest() {
-	err := d.dao.Delete(d.ctx, d.id)
-	d.Require().Nil(err)
+func (d *daoTestSuite) TearDownSuite() {
+	d.afDao.Delete(d.ctx, d.id)
 }
 
 func (d *daoTestSuite) TestCreate() {
@@ -69,6 +68,7 @@ func (d *daoTestSuite) TestCreate() {
 	aft := &model.ArtifactTrash{
 		ManifestMediaType: v1.MediaTypeImageManifest,
 		RepositoryName:    "test/hello-world",
+		Digest:            "1234",
 	}
 
 	_, err := d.dao.Create(d.ctx, aft)
@@ -86,7 +86,7 @@ func (d *daoTestSuite) TestDelete() {
 
 func (d *daoTestSuite) TestFilter() {
 	afs, err := d.dao.Filter(d.ctx)
-	d.Require().NotNil(err)
+	d.Require().Nil(err)
 	d.Require().Equal(afs[0].Digest, "1234")
 }
 
@@ -113,3 +113,7 @@ func (d *daoTestSuite) TestFlush() {
 	err = d.dao.Flush(d.ctx)
 	d.Require().Nil(err)
 }
+
+func TestDaoTestSuite(t *testing.T) {
+	suite.Run(t, &daoTestSuite{})
+}