diff --git a/internal/jobqueue/fsjobqueue/fsjobqueue.go b/internal/jobqueue/fsjobqueue/fsjobqueue.go index d384ce84c..6f01e2913 100644 --- a/internal/jobqueue/fsjobqueue/fsjobqueue.go +++ b/internal/jobqueue/fsjobqueue/fsjobqueue.go @@ -263,20 +263,13 @@ func (q *fsJobQueue) CancelJob(id uuid.UUID) error { return nil } -func (q *fsJobQueue) JobStatus(id uuid.UUID, result interface{}) (queued, started, finished time.Time, canceled bool, err error) { +func (q *fsJobQueue) JobStatus(id uuid.UUID) (result json.RawMessage, queued, started, finished time.Time, canceled bool, err error) { j, err := q.readJob(id) if err != nil { return } - if !j.FinishedAt.IsZero() && !j.Canceled { - err = json.Unmarshal(j.Result, result) - if err != nil { - err = fmt.Errorf("error unmarshaling result for job '%s': %v", id, err) - return - } - } - + result = j.Result queued = j.QueuedAt started = j.StartedAt finished = j.FinishedAt diff --git a/internal/jobqueue/fsjobqueue/fsjobqueue_test.go b/internal/jobqueue/fsjobqueue/fsjobqueue_test.go index a49deb182..9ceab3eeb 100644 --- a/internal/jobqueue/fsjobqueue/fsjobqueue_test.go +++ b/internal/jobqueue/fsjobqueue/fsjobqueue_test.go @@ -147,7 +147,7 @@ func TestDependencies(t *testing.T) { require.ElementsMatch(t, []uuid.UUID{one, two}, r) j := pushTestJob(t, q, "test", nil, []uuid.UUID{one, two}) - queued, started, finished, canceled, err := q.JobStatus(j, nil) + _, queued, started, finished, canceled, err := q.JobStatus(j) require.NoError(t, err) require.True(t, !queued.IsZero()) require.True(t, started.IsZero()) @@ -156,12 +156,15 @@ func TestDependencies(t *testing.T) { require.Equal(t, j, finishNextTestJob(t, q, "test", testResult{}, []uuid.UUID{one, two})) - queued, started, finished, canceled, err = q.JobStatus(j, &testResult{}) + result, queued, started, finished, canceled, err := q.JobStatus(j) require.NoError(t, err) require.True(t, !queued.IsZero()) require.True(t, !started.IsZero()) require.True(t, !finished.IsZero()) require.False(t, canceled) + + err = json.Unmarshal(result, &testResult{}) + require.NoError(t, err) }) t.Run("done-after-pushing-dependant", func(t *testing.T) { @@ -169,7 +172,7 @@ func TestDependencies(t *testing.T) { two := pushTestJob(t, q, "test", nil, nil) j := pushTestJob(t, q, "test", nil, []uuid.UUID{one, two}) - queued, started, finished, canceled, err := q.JobStatus(j, nil) + _, queued, started, finished, canceled, err := q.JobStatus(j) require.NoError(t, err) require.True(t, !queued.IsZero()) require.True(t, started.IsZero()) @@ -183,12 +186,15 @@ func TestDependencies(t *testing.T) { require.Equal(t, j, finishNextTestJob(t, q, "test", testResult{}, []uuid.UUID{one, two})) - queued, started, finished, canceled, err = q.JobStatus(j, &testResult{}) + result, queued, started, finished, canceled, err := q.JobStatus(j) require.NoError(t, err) require.True(t, !queued.IsZero()) require.True(t, !started.IsZero()) require.True(t, !finished.IsZero()) require.False(t, canceled) + + err = json.Unmarshal(result, &testResult{}) + require.NoError(t, err) }) } @@ -242,9 +248,10 @@ func TestCancel(t *testing.T) { require.NotEmpty(t, id) err = q.CancelJob(id) require.NoError(t, err) - _, _, _, canceled, err := q.JobStatus(id, &testResult{}) + result, _, _, _, canceled, err := q.JobStatus(id) require.NoError(t, err) require.True(t, canceled) + require.Nil(t, result) err = q.FinishJob(id, &testResult{}) require.Error(t, err) @@ -259,9 +266,10 @@ func TestCancel(t *testing.T) { require.Equal(t, json.RawMessage("null"), args) err = q.CancelJob(id) require.NoError(t, err) - _, _, _, canceled, err = q.JobStatus(id, &testResult{}) + result, _, _, _, canceled, err = q.JobStatus(id) require.NoError(t, err) require.True(t, canceled) + require.Nil(t, result) err = q.FinishJob(id, &testResult{}) require.Error(t, err) @@ -278,7 +286,9 @@ func TestCancel(t *testing.T) { require.NoError(t, err) err = q.CancelJob(id) require.NoError(t, err) - _, _, _, canceled, err = q.JobStatus(id, &testResult{}) + result, _, _, _, canceled, err = q.JobStatus(id) require.NoError(t, err) require.False(t, canceled) + err = json.Unmarshal(result, &testResult{}) + require.NoError(t, err) } diff --git a/internal/jobqueue/jobqueue.go b/internal/jobqueue/jobqueue.go index b4e36266a..0f90d91c4 100644 --- a/internal/jobqueue/jobqueue.go +++ b/internal/jobqueue/jobqueue.go @@ -56,7 +56,7 @@ type JobQueue interface { // finished, respectively. // // If the job is finished, its result will be returned in `result`. - JobStatus(id uuid.UUID, result interface{}) (queued, started, finished time.Time, canceled bool, err error) + JobStatus(id uuid.UUID) (result json.RawMessage, queued, started, finished time.Time, canceled bool, err error) } var ( diff --git a/internal/jobqueue/testjobqueue/testjobqueue.go b/internal/jobqueue/testjobqueue/testjobqueue.go index 0ff89fc97..afee7fa12 100644 --- a/internal/jobqueue/testjobqueue/testjobqueue.go +++ b/internal/jobqueue/testjobqueue/testjobqueue.go @@ -141,20 +141,14 @@ func (q *testJobQueue) CancelJob(id uuid.UUID) error { return nil } -func (q *testJobQueue) JobStatus(id uuid.UUID, result interface{}) (queued, started, finished time.Time, canceled bool, err error) { +func (q *testJobQueue) JobStatus(id uuid.UUID) (result json.RawMessage, queued, started, finished time.Time, canceled bool, err error) { j, exists := q.jobs[id] if !exists { err = jobqueue.ErrNotExist return } - if !j.FinishedAt.IsZero() { - err = json.Unmarshal(j.Result, result) - if err != nil { - return - } - } - + result = j.Result queued = j.QueuedAt started = j.StartedAt finished = j.FinishedAt diff --git a/internal/weldr/api.go b/internal/weldr/api.go index 6359ef474..b7e469ea4 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -247,8 +247,10 @@ func (api *API) getComposeStatus(compose store.Compose) *composeStatus { // All jobs are "osbuild" jobs. var result worker.OSBuildJobResult - // is it ok to ignore this error? - jobStatus, _ := api.workers.JobStatus(jobId, &result) + jobStatus, _, err := api.workers.JobStatus(jobId, &result) + if err != nil { + panic(err) + } return &composeStatus{ State: composeStateFromJobStatus(jobStatus, &result), Queued: jobStatus.Queued, diff --git a/internal/worker/server.go b/internal/worker/server.go index 47ade4d94..04367ef35 100644 --- a/internal/worker/server.go +++ b/internal/worker/server.go @@ -88,11 +88,18 @@ func (s *Server) Enqueue(arch string, job *OSBuildJob) (uuid.UUID, error) { } func (s *Server) JobStatus(id uuid.UUID, result interface{}) (*JobStatus, error) { - queued, started, finished, canceled, err := s.jobs.JobStatus(id, result) + rawResult, queued, started, finished, canceled, err := s.jobs.JobStatus(id) if err != nil { return nil, err } + if !finished.IsZero() && !canceled { + err = json.Unmarshal(rawResult, result) + if err != nil { + return nil, fmt.Errorf("error unmarshaling result for job '%s': %v", id, err) + } + } + // For backwards compatibility: OSBuildJobResult didn't use to have a // top-level `Success` flag. Override it here by looking into the job. if r, ok := result.(*OSBuildJobResult); ok {