diff --git a/src/common/job/client.go b/src/common/job/client.go index 01f3c18e2..3c5e060ac 100644 --- a/src/common/job/client.go +++ b/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 } diff --git a/src/common/job/client_test.go b/src/common/job/client_test.go index 8dd208841..53dfa5fe8 100644 --- a/src/common/job/client_test.go +++ b/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) +} diff --git a/src/core/api/admin_job.go b/src/core/api/admin_job.go index 14bdbcae6..71a486c50 100644 --- a/src/core/api/admin_job.go +++ b/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 + } } } diff --git a/src/replication/operation/controller.go b/src/replication/operation/controller.go index ab419f581..11f77acec 100644 --- a/src/replication/operation/controller.go +++ b/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 diff --git a/src/replication/operation/controller_test.go b/src/replication/operation/controller_test.go index 778e9f5ea..9da380c6a 100644 --- a/src/replication/operation/controller_test.go +++ b/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) -}