feat: save summary for the scan all execution (#13931)

Compute the summary info for the scan all and save it to the extra attrs
of the execution.

Signed-off-by: He Weiwei <hweiwei@vmware.com>

src/controller/scan/base_controller.go

@@ -277,8 +277,7 @@ func (bc *basicController) Scan(ctx context.Context, artifact *ar.Artifact, opti
 }
 
 func (bc *basicController) ScanAll(ctx context.Context, trigger string, async bool) (int64, error) {
-	extraAttrs := map[string]interface{}{}
-	executionID, err := bc.execMgr.Create(ctx, job.ImageScanAllJob, 0, trigger, extraAttrs)
+	executionID, err := bc.execMgr.Create(ctx, job.ImageScanAllJob, 0, trigger)
 	if err != nil {
 		return 0, err
 	}
@@ -307,12 +306,19 @@ func (bc *basicController) ScanAll(ctx context.Context, trigger string, async bo
 }
 
 func (bc *basicController) startScanAll(ctx context.Context, executionID int64) error {
-	artifactCount := 0
-	artifactScannedCount := 0
-
 	batchSize := 50
+
+	summary := struct {
+		TotalCount        int `json:"total_count"`
+		SubmitCount       int `json:"submit_count"`
+		ConflictCount     int `json:"conflict_count"`
+		PreconditionCount int `json:"precondition_count"`
+		UnsupportCount    int `json:"unsupport_count"`
+		UnknowCount       int `json:"unknow_count"`
+	}{}
+
 	for artifact := range ar.Iterator(ctx, batchSize, nil, nil) {
-		artifactCount++
+		summary.TotalCount++
 
 		scan := func(ctx context.Context) error {
 			return bc.Scan(ctx, artifact, WithExecutionID(executionID))
@@ -321,23 +327,74 @@ func (bc *basicController) startScanAll(ctx context.Context, executionID int64)
 		if err := orm.WithTransaction(scan)(ctx); err != nil {
 			// Just logged
 			log.Errorf("failed to scan artifact %s, error %v", artifact, err)
-			continue
-		}
 
-		artifactScannedCount++
+			switch errors.ErrCode(err) {
+			case errors.ConflictCode:
+				// a previous scan process is ongoing for the artifact
+				summary.ConflictCount++
+			case errors.PreconditionCode:
+				// scanner not found or it's disabled
+				summary.PreconditionCount++
+			case errors.BadRequestCode:
+				// artifact is unsupport
+				summary.UnsupportCount++
+			default:
+				summary.UnknowCount++
+			}
+		} else {
+			summary.SubmitCount++
+		}
+	}
+
+	extraAttrs := map[string]interface{}{"summary": summary}
+	if err := bc.execMgr.UpdateExtraAttrs(ctx, executionID, extraAttrs); err != nil {
+		log.Errorf("failed to set the summary info for the scan all execution, error: %v", err)
+		return err
+	}
+
+	if summary.SubmitCount > 0 { // at least one artifact submitted to the job service
+		return nil
 	}
 
 	// not artifact found
-	if artifactCount == 0 || artifactScannedCount == 0 {
-		message := "no task found"
-		if artifactCount == 0 {
-			message = "no artifact found"
+	if summary.TotalCount == 0 {
+		if err := bc.execMgr.MarkDone(ctx, executionID, "no artifact found"); err != nil {
+			log.Errorf("failed to mark the execution %d to be done, error: %v", executionID, err)
+			return err
 		}
+	} else if summary.PreconditionCount+summary.UnknowCount == 0 { // not scan job submitted and no failed
+		message := fmt.Sprintf("%d artifact(s) found", summary.TotalCount)
+
+		if summary.UnsupportCount > 0 {
+			message = fmt.Sprintf("%s, %d artifact(s) not scannable", message, summary.UnsupportCount)
+		}
+
+		if summary.ConflictCount > 0 {
+			message = fmt.Sprintf("%s, %d artifact(s) have a previous ongoing scan process", message, summary.ConflictCount)
+		}
+
+		message = fmt.Sprintf("%s, but no scan job submitted to the job service", message)
 
 		if err := bc.execMgr.MarkDone(ctx, executionID, message); err != nil {
 			log.Errorf("failed to mark the execution %d to be done, error: %v", executionID, err)
 			return err
 		}
+	} else { // not scan job submitted and failed
+		message := fmt.Sprintf("%d artifact(s) found", summary.TotalCount)
+
+		if summary.PreconditionCount > 0 {
+			message = fmt.Sprintf("%s, scanner not found or disabled for %d of them", message, summary.PreconditionCount)
+		}
+
+		if summary.UnknowCount > 0 {
+			message = fmt.Sprintf("%s, internal error happened for %d of them", message, summary.UnknowCount)
+		}
+
+		message = fmt.Sprintf("%s, but no scan job submitted to the job service", message)
+		if err := bc.execMgr.MarkError(ctx, executionID, message); err != nil {
+			log.Errorf("failed to mark the execution %d to be error, error: %v", executionID, err)
+			return err
+		}
 	}
 
 	return nil

src/controller/scan/base_controller_test.go

@@ -479,14 +479,16 @@ func (suite *ControllerTestSuite) TestScanAll() {
 		executionID := int64(1)
 
 		suite.execMgr.On(
-			"Create", ctx, "IMAGE_SCAN_ALL", int64(0), "SCHEDULE", map[string]interface{}{},
+			"Create", ctx, "IMAGE_SCAN_ALL", int64(0), "SCHEDULE",
 		).Return(executionID, nil).Once()
 
 		mock.OnAnything(suite.artifactCtl, "List").Return([]*artifact.Artifact{}, nil).Once()
 
 		suite.taskMgr.On("Count", ctx, q.New(q.KeyWords{"execution_id": executionID})).Return(int64(0), nil).Once()
 
-		suite.execMgr.On("MarkDone", ctx, executionID, "no artifact found").Return(nil).Once()
+		mock.OnAnything(suite.execMgr, "UpdateExtraAttrs").Return(nil).Once()
+
+		suite.execMgr.On("MarkDone", ctx, executionID, mock.Anything).Return(nil).Once()
 
 		_, err := suite.c.ScanAll(ctx, "SCHEDULE", false)
 		suite.NoError(err)
@@ -499,7 +501,7 @@ func (suite *ControllerTestSuite) TestScanAll() {
 		executionID := int64(1)
 
 		suite.execMgr.On(
-			"Create", ctx, "IMAGE_SCAN_ALL", int64(0), "SCHEDULE", map[string]interface{}{},
+			"Create", ctx, "IMAGE_SCAN_ALL", int64(0), "SCHEDULE",
 		).Return(executionID, nil).Once()
 
 		mock.OnAnything(suite.artifactCtl, "List").Return([]*artifact.Artifact{suite.artifact}, nil).Once()
@@ -513,8 +515,8 @@ func (suite *ControllerTestSuite) TestScanAll() {
 		mock.OnAnything(suite.reportMgr, "Delete").Return(nil).Once()
 		mock.OnAnything(suite.reportMgr, "Create").Return("uuid", nil).Once()
 		mock.OnAnything(suite.taskMgr, "Create").Return(int64(0), fmt.Errorf("failed")).Once()
-
-		suite.execMgr.On("MarkDone", ctx, executionID, "no task found").Return(nil).Once()
+		mock.OnAnything(suite.execMgr, "UpdateExtraAttrs").Return(nil).Once()
+		suite.execMgr.On("MarkError", ctx, executionID, mock.Anything).Return(nil).Once()
 
 		_, err := suite.c.ScanAll(ctx, "SCHEDULE", false)
 		suite.NoError(err)

src/controller/scan/callback_test.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/goharbor/harbor/src/controller/artifact"
 	"github.com/goharbor/harbor/src/jobservice/job"
-	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/scan"
 	dscan "github.com/goharbor/harbor/src/pkg/scan/dao/scan"
 	"github.com/goharbor/harbor/src/pkg/task"
@@ -165,7 +164,7 @@ func (suite *CallbackTestSuite) TestScanAllCallback() {
 	{
 		// create execution failed
 		suite.execMgr.On(
-			"Create", context.TODO(), "IMAGE_SCAN_ALL", int64(0), "SCHEDULE", map[string]interface{}{},
+			"Create", context.TODO(), "IMAGE_SCAN_ALL", int64(0), "SCHEDULE",
 		).Return(int64(0), fmt.Errorf("failed")).Once()
 
 		suite.Error(scanAllCallback(context.TODO(), ""))
@@ -175,7 +174,7 @@ func (suite *CallbackTestSuite) TestScanAllCallback() {
 		executionID := int64(1)
 
 		suite.execMgr.On(
-			"Create", context.TODO(), "IMAGE_SCAN_ALL", int64(0), "SCHEDULE", map[string]interface{}{},
+			"Create", context.TODO(), "IMAGE_SCAN_ALL", int64(0), "SCHEDULE",
 		).Return(executionID, nil).Once()
 
 		suite.execMgr.On(
@@ -184,9 +183,9 @@ func (suite *CallbackTestSuite) TestScanAllCallback() {
 
 		mock.OnAnything(suite.artifactCtl, "List").Return([]*artifact.Artifact{}, nil).Once()
 
-		suite.taskMgr.On("Count", context.TODO(), q.New(q.KeyWords{"execution_id": executionID})).Return(int64(0), nil).Once()
+		mock.OnAnything(suite.execMgr, "UpdateExtraAttrs").Return(nil).Once()
 
-		suite.execMgr.On("MarkDone", context.TODO(), executionID, "no artifact found").Return(nil).Once()
+		suite.execMgr.On("MarkDone", context.TODO(), executionID, mock.Anything).Return(nil).Once()
 
 		suite.NoError(scanAllCallback(context.TODO(), ""))
 	}

src/pkg/task/execution.go

@@ -45,6 +45,8 @@ type ExecutionManager interface {
 	// The "extraAttrs" can be used to set the customized attributes
 	Create(ctx context.Context, vendorType string, vendorID int64, trigger string,
 		extraAttrs ...map[string]interface{}) (id int64, err error)
+	// Update the extra attributes of the specified execution
+	UpdateExtraAttrs(ctx context.Context, id int64, extraAttrs map[string]interface{}) (err error)
 	// MarkDone marks the status of the specified execution as success.
 	// It must be called to update the execution status if the created execution contains no tasks.
 	// In other cases, the execution status can be calculated from the referenced tasks automatically
@@ -168,6 +170,21 @@ func (e *executionManager) sweep(ctx context.Context, vendorType string) error {
 	}
 }
 
+func (e *executionManager) UpdateExtraAttrs(ctx context.Context, id int64, extraAttrs map[string]interface{}) error {
+	data, err := json.Marshal(extraAttrs)
+	if err != nil {
+		return err
+	}
+
+	execution := &dao.Execution{
+		ID:         id,
+		ExtraAttrs: string(data),
+		UpdateTime: time.Now(),
+	}
+
+	return e.executionDAO.Update(ctx, execution, "ExtraAttrs", "UpdateTime")
+}
+
 func (e *executionManager) MarkDone(ctx context.Context, id int64, message string) error {
 	now := time.Now()
 	return e.executionDAO.Update(ctx, &dao.Execution{

src/pkg/task/execution_test.go

@@ -71,6 +71,13 @@ func (e *executionManagerTestSuite) TestCreate() {
 	e.ormCreator.AssertExpectations(e.T())
 }
 
+func (e *executionManagerTestSuite) TestUpdateExtraAttrs() {
+	e.execDAO.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
+	err := e.execMgr.UpdateExtraAttrs(nil, 1, map[string]interface{}{"key": "value"})
+	e.Require().Nil(err)
+	e.execDAO.AssertExpectations(e.T())
+}
+
 func (e *executionManagerTestSuite) TestMarkDone() {
 	e.execDAO.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	err := e.execMgr.MarkDone(nil, 1, "success")

src/testing/pkg/task/execution_manager.go

@@ -182,3 +182,17 @@ func (_m *ExecutionManager) StopAndWait(ctx context.Context, id int64, timeout t
 
 	return r0
 }
+
+// UpdateExtraAttrs provides a mock function with given fields: ctx, id, extraAttrs
+func (_m *ExecutionManager) UpdateExtraAttrs(ctx context.Context, id int64, extraAttrs map[string]interface{}) error {
+	ret := _m.Called(ctx, id, extraAttrs)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, int64, map[string]interface{}) error); ok {
+		r0 = rf(ctx, id, extraAttrs)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}