Merge pull request #9021 from ywk253100/190909_scan_cron

Check the status behind error when trying to update the scan schedule

src/common/job/client.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"regexp"
 	"strings"
 
 	commonhttp "github.com/goharbor/harbor/src/common/http"
@@ -18,7 +19,9 @@ import (
 var (
 	// GlobalClient is an instance of the default client that can be used globally
 	// Notes: the client needs to be initialized before can be used
-	GlobalClient Client
+	GlobalClient             Client
+	statusBehindErrorPattern = "mismatch job status for stopping job: .*, job status (.*) is behind Running"
+	statusBehindErrorReg     = regexp.MustCompile(statusBehindErrorPattern)
 )
 
 // Client wraps interface to access jobservice.
@@ -30,6 +33,21 @@ type Client interface {
 	// TODO Redirect joblog when we see there's memory issue.
 }
 
+// StatusBehindError represents the error got when trying to stop a success/failed job
+type StatusBehindError struct {
+	status string
+}
+
+// Error returns the detail message about the error
+func (s *StatusBehindError) Error() string {
+	return "status behind error"
+}
+
+// Status returns the current status of the job
+func (s *StatusBehindError) Status() string {
+	return s.status
+}
+
 // DefaultClient is the default implementation of Client interface
 type DefaultClient struct {
 	endpoint string
@@ -156,5 +174,25 @@ func (d *DefaultClient) PostAction(uuid, action string) error {
 	}{
 		Action: action,
 	}
-	return d.client.Post(url, req)
+	if err := d.client.Post(url, req); err != nil {
+		status, flag := isStatusBehindError(err)
+		if flag {
+			return &StatusBehindError{
+				status: status,
+			}
+		}
+		return err
+	}
+	return nil
+}
+
+func isStatusBehindError(err error) (string, bool) {
+	if err == nil {
+		return "", false
+	}
+	strs := statusBehindErrorReg.FindStringSubmatch(err.Error())
+	if len(strs) != 2 {
+		return "", false
+	}
+	return strs[1], true
 }

src/common/job/client_test.go

@@ -1,11 +1,13 @@
 package job
 
 import (
+	"errors"
+	"os"
+	"testing"
+
 	"github.com/goharbor/harbor/src/common/job/models"
 	"github.com/goharbor/harbor/src/common/job/test"
 	"github.com/stretchr/testify/assert"
-	"os"
-	"testing"
 )
 
 var (
@@ -62,3 +64,20 @@ func TestPostAction(t *testing.T) {
 	err2 := testClient.PostAction(ID, "stop")
 	assert.Nil(err2)
 }
+
+func TestIsStatusBehindError(t *testing.T) {
+	// nil error
+	status, flag := isStatusBehindError(nil)
+	assert.False(t, flag)
+
+	// not status behind error
+	err := errors.New("not status behind error")
+	status, flag = isStatusBehindError(err)
+	assert.False(t, flag)
+
+	// status behind error
+	err = errors.New("mismatch job status for stopping job: 9feedf9933jffs, job status Error is behind Running")
+	status, flag = isStatusBehindError(err)
+	assert.True(t, flag)
+	assert.Equal(t, "Error", status)
+}

src/core/api/admin_job.go

@@ -62,9 +62,12 @@ func (aj *AJAPI) updateSchedule(ajr models.AdminJobReq) {
 
 	// stop the scheduled job and remove it.
 	if err = utils_core.GetJobServiceClient().PostAction(jobs[0].UUID, common_job.JobActionStop); err != nil {
-		if e, ok := err.(*common_http.Error); !ok || e.Code != http.StatusNotFound {
-			aj.SendInternalServerError(err)
-			return
+		_, ok := err.(*common_job.StatusBehindError)
+		if !ok {
+			if e, ok := err.(*common_http.Error); !ok || e.Code != http.StatusNotFound {
+				aj.SendInternalServerError(err)
+				return
+			}
 		}
 	}
 

src/replication/operation/controller.go

@@ -16,7 +16,6 @@ package operation
 
 import (
 	"fmt"
-	"regexp"
 	"strings"
 	"time"
 
@@ -49,9 +48,7 @@ const (
 )
 
 var (
-	statusBehindErrorPattern = "mismatch job status for stopping job: .*, job status (.*) is behind Running"
-	statusBehindErrorReg     = regexp.MustCompile(statusBehindErrorPattern)
-	jobNotFoundErrorMsg      = "object is not found"
+	jobNotFoundErrorMsg = "object is not found"
 )
 
 // NewController returns a controller implementation
@@ -163,8 +160,9 @@ func (c *controller) StopReplication(executionID int64) error {
 			continue
 		}
 		if err = c.scheduler.Stop(task.JobID); err != nil {
-			status, flag := isStatusBehindError(err)
-			if flag {
+			isStatusBehindError, ok := err.(*job.StatusBehindError)
+			if ok {
+				status := isStatusBehindError.Status()
 				switch hjob.Status(status) {
 				case hjob.ErrorStatus:
 					status = models.TaskStatusFailed
@@ -215,17 +213,6 @@ func isTaskInFinalStatus(task *models.Task) bool {
 	return false
 }
 
-func isStatusBehindError(err error) (string, bool) {
-	if err == nil {
-		return "", false
-	}
-	strs := statusBehindErrorReg.FindStringSubmatch(err.Error())
-	if len(strs) != 2 {
-		return "", false
-	}
-	return strs[1], true
-}
-
 func isJobNotFoundError(err error) bool {
 	if err == nil {
 		return false

src/replication/operation/controller_test.go

@@ -15,7 +15,6 @@
 package operation
 
 import (
-	"errors"
 	"io"
 	"os"
 	"testing"
@@ -382,20 +381,3 @@ func TestIsTaskRunning(t *testing.T) {
 		assert.Equal(t, c.isFinalStatus, isTaskInFinalStatus(c.task))
 	}
 }
-
-func TestIsStatusBehindError(t *testing.T) {
-	// nil error
-	status, flag := isStatusBehindError(nil)
-	assert.False(t, flag)
-
-	// not status behind error
-	err := errors.New("not status behind error")
-	status, flag = isStatusBehindError(err)
-	assert.False(t, flag)
-
-	// status behind error
-	err = errors.New("mismatch job status for stopping job: 9feedf9933jffs, job status Error is behind Running")
-	status, flag = isStatusBehindError(err)
-	assert.True(t, flag)
-	assert.Equal(t, "Error", status)
-}