From 967ac1c35e304eacc226c5e48d3a4aa9857d07a7 Mon Sep 17 00:00:00 2001 From: Gianluca Zuccarelli Date: Fri, 22 Jul 2022 13:18:47 +0100 Subject: [PATCH] worker/server: job status struct The number of return values from the `jobStatus` function was growing and getting out of hand. Not all return values were being used in all cases and so returning a single struct with the information and status of a job makes more sense. Then in each case the resulting fields can be used as needed. --- internal/cloudapi/v2/handler.go | 58 +++++------ internal/cloudapi/v2/server.go | 2 +- internal/cloudapi/v2/v2_koji_test.go | 4 +- internal/weldr/api.go | 16 +-- internal/weldr/compose_test.go | 8 +- internal/worker/server.go | 149 +++++++++++++++------------ internal/worker/server_test.go | 6 +- 7 files changed, 128 insertions(+), 115 deletions(-) diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go index 383ef34cf..301270e50 100644 --- a/internal/cloudapi/v2/handler.go +++ b/internal/cloudapi/v2/handler.go @@ -576,7 +576,7 @@ func (h *apiHandlers) getComposeStatusImpl(ctx echo.Context, id string) error { if jobType == worker.JobTypeOSBuild { var result worker.OSBuildJobResult - status, _, err := h.server.workers.OSBuildJobStatus(jobId, &result) + jobInfo, err := h.server.workers.OSBuildJobStatus(jobId, &result) if err != nil { return HTTPError(ErrorMalformedOSBuildJobResult) } @@ -607,36 +607,36 @@ func (h *apiHandlers) getComposeStatusImpl(ctx echo.Context, id string) error { Id: jobId.String(), Kind: "ComposeStatus", }, - Status: composeStatusFromOSBuildJobStatus(status, &result), + Status: composeStatusFromOSBuildJobStatus(jobInfo.JobStatus, &result), ImageStatus: ImageStatus{ - Status: imageStatusFromOSBuildJobStatus(status, &result), + Status: imageStatusFromOSBuildJobStatus(jobInfo.JobStatus, &result), Error: composeStatusErrorFromJobError(jobError), UploadStatus: us, }, }) } else if jobType == worker.JobTypeKojiFinalize { var result worker.KojiFinalizeJobResult - finalizeStatus, deps, err := h.server.workers.KojiFinalizeJobStatus(jobId, &result) + finalizeInfo, err := h.server.workers.KojiFinalizeJobStatus(jobId, &result) if err != nil { return HTTPError(ErrorMalformedOSBuildJobResult) } - if len(deps) < 2 { + if len(finalizeInfo.Deps) < 2 { return HTTPError(ErrorUnexpectedNumberOfImageBuilds) } var initResult worker.KojiInitJobResult - _, _, err = h.server.workers.KojiInitJobStatus(deps[0], &initResult) + _, err = h.server.workers.KojiInitJobStatus(finalizeInfo.Deps[0], &initResult) if err != nil { return HTTPError(ErrorMalformedOSBuildJobResult) } var buildJobResults []worker.OSBuildJobResult var buildJobStatuses []ImageStatus - for i := 1; i < len(deps); i++ { + for i := 1; i < len(finalizeInfo.Deps); i++ { var buildJobResult worker.OSBuildJobResult - buildJobStatus, _, err := h.server.workers.OSBuildJobStatus(deps[i], &buildJobResult) + buildInfo, err := h.server.workers.OSBuildJobStatus(finalizeInfo.Deps[i], &buildJobResult) if err != nil { return HTTPError(ErrorMalformedOSBuildJobResult) } - buildJobError, err := h.server.workers.JobDependencyChainErrors(deps[i]) + buildJobError, err := h.server.workers.JobDependencyChainErrors(finalizeInfo.Deps[i]) if err != nil { return HTTPError(ErrorGettingBuildDependencyStatus) } @@ -661,7 +661,7 @@ func (h *apiHandlers) getComposeStatusImpl(ctx echo.Context, id string) error { buildJobResults = append(buildJobResults, buildJobResult) buildJobStatuses = append(buildJobStatuses, ImageStatus{ - Status: imageStatusFromKojiJobStatus(buildJobStatus, &initResult, &buildJobResult), + Status: imageStatusFromKojiJobStatus(buildInfo.JobStatus, &initResult, &buildJobResult), Error: composeStatusErrorFromJobError(buildJobError), UploadStatus: us, }) @@ -672,7 +672,7 @@ func (h *apiHandlers) getComposeStatusImpl(ctx echo.Context, id string) error { Id: jobId.String(), Kind: "ComposeStatus", }, - Status: composeStatusFromKojiJobStatus(finalizeStatus, &initResult, buildJobResults, &result), + Status: composeStatusFromKojiJobStatus(finalizeInfo.JobStatus, &initResult, buildJobResults, &result), ImageStatus: buildJobStatuses[0], // backwards compatibility ImageStatuses: &buildJobStatuses, KojiStatus: &KojiStatus{}, @@ -816,7 +816,7 @@ func (h *apiHandlers) getComposeMetadataImpl(ctx echo.Context, id string) error } var result worker.OSBuildJobResult - status, _, err := h.server.workers.OSBuildJobStatus(jobId, &result) + buildInfo, err := h.server.workers.OSBuildJobStatus(jobId, &result) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } @@ -826,7 +826,7 @@ func (h *apiHandlers) getComposeMetadataImpl(ctx echo.Context, id string) error return HTTPErrorWithInternal(ErrorComposeNotFound, err) } - if status.Finished.IsZero() { + if buildInfo.JobStatus.Finished.IsZero() { // job still running: empty response return ctx.JSON(200, ComposeMetadata{ ObjectReference: ObjectReference{ @@ -837,7 +837,7 @@ func (h *apiHandlers) getComposeMetadataImpl(ctx echo.Context, id string) error }) } - if status.Canceled || !result.Success { + if buildInfo.JobStatus.Canceled || !result.Success { // job canceled or failed, empty response return ctx.JSON(200, ComposeMetadata{ ObjectReference: ObjectReference{ @@ -939,19 +939,19 @@ func (h *apiHandlers) getComposeLogsImpl(ctx echo.Context, id string) error { switch jobType { case worker.JobTypeKojiFinalize: var finalizeResult worker.KojiFinalizeJobResult - _, deps, err := h.server.workers.KojiFinalizeJobStatus(jobId, &finalizeResult) + finalizeInfo, err := h.server.workers.KojiFinalizeJobStatus(jobId, &finalizeResult) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } var initResult worker.KojiInitJobResult - _, _, err = h.server.workers.KojiInitJobStatus(deps[0], &initResult) + _, err = h.server.workers.KojiInitJobStatus(finalizeInfo.Deps[0], &initResult) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } - for i := 1; i < len(deps); i++ { - buildJobType, err := h.server.workers.JobType(deps[i]) + for i := 1; i < len(finalizeInfo.Deps); i++ { + buildJobType, err := h.server.workers.JobType(finalizeInfo.Deps[i]) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } @@ -959,7 +959,7 @@ func (h *apiHandlers) getComposeLogsImpl(ctx echo.Context, id string) error { switch buildJobType { case worker.JobTypeOSBuild: var buildResult worker.OSBuildJobResult - _, _, err = h.server.workers.OSBuildJobStatus(deps[i], &buildResult) + _, err = h.server.workers.OSBuildJobStatus(finalizeInfo.Deps[i], &buildResult) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } @@ -978,7 +978,7 @@ func (h *apiHandlers) getComposeLogsImpl(ctx echo.Context, id string) error { case worker.JobTypeOSBuild: var buildResult worker.OSBuildJobResult - _, _, err = h.server.workers.OSBuildJobStatus(jobId, &buildResult) + _, err = h.server.workers.OSBuildJobStatus(jobId, &buildResult) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } @@ -1004,7 +1004,7 @@ func manifestJobResultsFromJobDeps(w *worker.Server, deps []uuid.UUID) (*worker. return nil, err } if depType == worker.JobTypeManifestIDOnly { - _, _, err = w.ManifestJobStatus(deps[i], &manifestResult) + _, err = w.ManifestJobStatus(deps[i], &manifestResult) if err != nil { return nil, err } @@ -1036,13 +1036,13 @@ func (h *apiHandlers) getComposeManifestsImpl(ctx echo.Context, id string) error switch jobType { case worker.JobTypeKojiFinalize: var finalizeResult worker.KojiFinalizeJobResult - _, deps, err := h.server.workers.KojiFinalizeJobStatus(jobId, &finalizeResult) + finalizeInfo, err := h.server.workers.KojiFinalizeJobStatus(jobId, &finalizeResult) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } - for i := 1; i < len(deps); i++ { - buildJobType, err := h.server.workers.JobType(deps[i]) + for i := 1; i < len(finalizeInfo.Deps); i++ { + buildJobType, err := h.server.workers.JobType(finalizeInfo.Deps[i]) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } @@ -1052,7 +1052,7 @@ func (h *apiHandlers) getComposeManifestsImpl(ctx echo.Context, id string) error switch buildJobType { case worker.JobTypeOSBuild: var buildJob worker.OSBuildJob - err = h.server.workers.OSBuildJob(deps[i], &buildJob) + err = h.server.workers.OSBuildJob(finalizeInfo.Deps[i], &buildJob) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } @@ -1060,11 +1060,11 @@ func (h *apiHandlers) getComposeManifestsImpl(ctx echo.Context, id string) error if len(buildJob.Manifest) != 0 { manifest = buildJob.Manifest } else { - _, buildDeps, err := h.server.workers.OSBuildJobStatus(deps[i], &worker.OSBuildJobResult{}) + buildInfo, err := h.server.workers.OSBuildJobStatus(finalizeInfo.Deps[i], &worker.OSBuildJobResult{}) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } - manifestResult, err := manifestJobResultsFromJobDeps(h.server.workers, buildDeps) + manifestResult, err := manifestJobResultsFromJobDeps(h.server.workers, buildInfo.Deps) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, fmt.Errorf("job %q: %v", jobId, err)) } @@ -1089,11 +1089,11 @@ func (h *apiHandlers) getComposeManifestsImpl(ctx echo.Context, id string) error if len(buildJob.Manifest) != 0 { manifest = buildJob.Manifest } else { - _, deps, err := h.server.workers.OSBuildJobStatus(jobId, &worker.OSBuildJobResult{}) + buildInfo, err := h.server.workers.OSBuildJobStatus(jobId, &worker.OSBuildJobResult{}) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, err) } - manifestResult, err := manifestJobResultsFromJobDeps(h.server.workers, deps) + manifestResult, err := manifestJobResultsFromJobDeps(h.server.workers, buildInfo.Deps) if err != nil { return HTTPErrorWithInternal(ErrorComposeNotFound, fmt.Errorf("job %q: %v", jobId, err)) } diff --git a/internal/cloudapi/v2/server.go b/internal/cloudapi/v2/server.go index 4d7a6671e..4ca96b7e0 100644 --- a/internal/cloudapi/v2/server.go +++ b/internal/cloudapi/v2/server.go @@ -292,7 +292,7 @@ func generateManifest(ctx context.Context, workers *worker.Server, depsolveJobID return } - _, _, err = workers.DepsolveJobStatus(depsolveJobID, &depsolveResults) + _, err = workers.DepsolveJobStatus(depsolveJobID, &depsolveResults) if err != nil { reason := "Error reading depsolve status" jobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorReadingJobStatus, reason) diff --git a/internal/cloudapi/v2/v2_koji_test.go b/internal/cloudapi/v2/v2_koji_test.go index 0a9f54997..fdc69d0cb 100644 --- a/internal/cloudapi/v2/v2_koji_test.go +++ b/internal/cloudapi/v2/v2_koji_test.go @@ -458,9 +458,9 @@ func TestKojiCompose(t *testing.T) { // Finishing of the goroutine handling the manifest job is not deterministic and as a result, we may get // the second osbuild job first. // The build jobs ID is determined from the dependencies of the koji-finalize job dependencies. - _, finalizeJobDeps, err := workerServer.KojiFinalizeJobStatus(composeId, &worker.KojiFinalizeJobResult{}) + finalizeInfo, err := workerServer.KojiFinalizeJobStatus(composeId, &worker.KojiFinalizeJobResult{}) require.NoError(t, err) - buildJobIDs := finalizeJobDeps[1:] + buildJobIDs := finalizeInfo.Deps[1:] require.Len(t, buildJobIDs, 2) // handle build jobs diff --git a/internal/weldr/api.go b/internal/weldr/api.go index 4e23f9db1..402012419 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -359,16 +359,16 @@ func (api *API) getComposeStatus(compose store.Compose) *composeStatus { // All jobs are "osbuild" jobs. var result worker.OSBuildJobResult - jobStatus, _, err := api.workers.OSBuildJobStatus(jobId, &result) + jobInfo, err := api.workers.OSBuildJobStatus(jobId, &result) if err != nil { panic(err) } return &composeStatus{ - State: composeStateFromJobStatus(jobStatus, &result), - Queued: jobStatus.Queued, - Started: jobStatus.Started, - Finished: jobStatus.Finished, + State: composeStateFromJobStatus(jobInfo.JobStatus, &result), + Queued: jobInfo.JobStatus.Queued, + Started: jobInfo.JobStatus.Started, + Finished: jobInfo.JobStatus.Finished, Result: result.OSBuildOutput, } } @@ -2196,7 +2196,7 @@ func (api *API) resolveContainersForImageType(bp blueprint.Blueprint, imageType var result worker.ContainerResolveJobResult for { - status, _, err := api.workers.ContainerResolveJobStatus(jobId, &result) + jobInfo, err := api.workers.ContainerResolveJobStatus(jobId, &result) if err != nil { return specs, err @@ -2204,9 +2204,9 @@ func (api *API) resolveContainersForImageType(bp blueprint.Blueprint, imageType if result.JobError != nil { return specs, errors.New(result.JobError.Reason) - } else if status.Canceled { + } else if jobInfo.JobStatus.Canceled { return specs, fmt.Errorf("Failed to resolve containers: job cancelled") - } else if !status.Finished.IsZero() { + } else if !jobInfo.JobStatus.Finished.IsZero() { break } diff --git a/internal/weldr/compose_test.go b/internal/weldr/compose_test.go index 8f74031fb..525b87b1a 100644 --- a/internal/weldr/compose_test.go +++ b/internal/weldr/compose_test.go @@ -57,10 +57,10 @@ func TestComposeStatusFromLegacyError(t *testing.T) { err = api.workers.FinishJob(token, rawResult) require.NoError(t, err) - jobStatus, _, err := api.workers.OSBuildJobStatus(jobId, &jobResult) + jobInfo, err := api.workers.OSBuildJobStatus(jobId, &jobResult) require.NoError(t, err) - state := composeStateFromJobStatus(jobStatus, &jobResult) + state := composeStateFromJobStatus(jobInfo.JobStatus, &jobResult) require.Equal(t, "FAILED", state.ToString()) } @@ -100,9 +100,9 @@ func TestComposeStatusFromJobError(t *testing.T) { err = api.workers.FinishJob(token, rawResult) require.NoError(t, err) - jobStatus, _, err := api.workers.OSBuildJobStatus(jobId, &jobResult) + jobInfo, err := api.workers.OSBuildJobStatus(jobId, &jobResult) require.NoError(t, err) - state := composeStateFromJobStatus(jobStatus, &jobResult) + state := composeStateFromJobStatus(jobInfo.JobStatus, &jobResult) require.Equal(t, "FAILED", state.ToString()) } diff --git a/internal/worker/server.go b/internal/worker/server.go index 973986e4a..376f7ecc1 100644 --- a/internal/worker/server.go +++ b/internal/worker/server.go @@ -50,6 +50,13 @@ type JobStatus struct { Canceled bool } +type JobInfo struct { + JobType string + Channel string + JobStatus *JobStatus + Deps []uuid.UUID +} + var ErrInvalidToken = errors.New("token does not exist") var ErrJobNotRunning = errors.New("job isn't running") var ErrInvalidJobType = errors.New("job has invalid type") @@ -161,11 +168,11 @@ func (s *Server) JobDependencyChainErrors(id uuid.UUID) (*clienterrors.Error, er } var jobResult *JobResult - var jobDeps []uuid.UUID + var jobInfo *JobInfo switch jobType { case JobTypeOSBuild: var osbuildJR OSBuildJobResult - _, jobDeps, err = s.OSBuildJobStatus(id, &osbuildJR) + jobInfo, err = s.OSBuildJobStatus(id, &osbuildJR) if err != nil { return nil, err } @@ -173,7 +180,7 @@ func (s *Server) JobDependencyChainErrors(id uuid.UUID) (*clienterrors.Error, er case JobTypeDepsolve: var depsolveJR DepsolveJobResult - _, jobDeps, err = s.DepsolveJobStatus(id, &depsolveJR) + jobInfo, err = s.DepsolveJobStatus(id, &depsolveJR) if err != nil { return nil, err } @@ -181,7 +188,7 @@ func (s *Server) JobDependencyChainErrors(id uuid.UUID) (*clienterrors.Error, er case JobTypeManifestIDOnly: var manifestJR ManifestJobByIDResult - _, jobDeps, err = s.ManifestJobStatus(id, &manifestJR) + jobInfo, err = s.ManifestJobStatus(id, &manifestJR) if err != nil { return nil, err } @@ -189,7 +196,7 @@ func (s *Server) JobDependencyChainErrors(id uuid.UUID) (*clienterrors.Error, er case JobTypeKojiInit: var kojiInitJR KojiInitJobResult - _, jobDeps, err = s.KojiInitJobStatus(id, &kojiInitJR) + jobInfo, err = s.KojiInitJobStatus(id, &kojiInitJR) if err != nil { return nil, err } @@ -197,7 +204,7 @@ func (s *Server) JobDependencyChainErrors(id uuid.UUID) (*clienterrors.Error, er case JobTypeKojiFinalize: var kojiFinalizeJR KojiFinalizeJobResult - _, jobDeps, err = s.KojiFinalizeJobStatus(id, &kojiFinalizeJR) + jobInfo, err = s.KojiFinalizeJobStatus(id, &kojiFinalizeJR) if err != nil { return nil, err } @@ -205,7 +212,7 @@ func (s *Server) JobDependencyChainErrors(id uuid.UUID) (*clienterrors.Error, er case JobTypeContainerResolve: var containerResolveJR ContainerResolveJobResult - _, jobDeps, err = s.ContainerResolveJobStatus(id, &containerResolveJR) + jobInfo, err = s.ContainerResolveJobStatus(id, &containerResolveJR) if err != nil { return nil, err } @@ -219,7 +226,7 @@ func (s *Server) JobDependencyChainErrors(id uuid.UUID) (*clienterrors.Error, er depErrors := []*clienterrors.Error{} if jobError.IsDependencyError() { // check job's dependencies - for _, dep := range jobDeps { + for _, dep := range jobInfo.Deps { depError, err := s.JobDependencyChainErrors(dep) if err != nil { return nil, err @@ -240,17 +247,17 @@ func (s *Server) JobDependencyChainErrors(id uuid.UUID) (*clienterrors.Error, er return nil, nil } -func (s *Server) OSBuildJobStatus(id uuid.UUID, result *OSBuildJobResult) (*JobStatus, []uuid.UUID, error) { - jobType, _, status, deps, err := s.jobStatus(id, result) +func (s *Server) OSBuildJobStatus(id uuid.UUID, result *OSBuildJobResult) (*JobInfo, error) { + jobInfo, err := s.jobStatus(id, result) if err != nil { - return nil, nil, err + return nil, err } - if !strings.HasPrefix(jobType, JobTypeOSBuild+":") { // Build jobs get automatic arch suffix: Check prefix - return nil, nil, fmt.Errorf("expected \"%s:*\", found %q job instead", JobTypeOSBuild, jobType) + if !strings.HasPrefix(jobInfo.JobType, JobTypeOSBuild+":") { // Build jobs get automatic arch suffix: Check prefix + return nil, fmt.Errorf("expected \"%s:*\", found %q job instead", JobTypeOSBuild, jobInfo.JobType) } - if result.JobError == nil && !status.Finished.IsZero() { + if result.JobError == nil && !jobInfo.JobStatus.Finished.IsZero() { if result.OSBuildOutput == nil { result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, "osbuild build failed") } else if len(result.OSBuildOutput.Error) > 0 { @@ -265,51 +272,51 @@ func (s *Server) OSBuildJobStatus(id uuid.UUID, result *OSBuildJobResult) (*JobS result.Success = result.OSBuildOutput.Success && result.JobError == nil } - return status, deps, nil + return jobInfo, nil } -func (s *Server) KojiInitJobStatus(id uuid.UUID, result *KojiInitJobResult) (*JobStatus, []uuid.UUID, error) { - jobType, _, status, deps, err := s.jobStatus(id, result) +func (s *Server) KojiInitJobStatus(id uuid.UUID, result *KojiInitJobResult) (*JobInfo, error) { + jobInfo, err := s.jobStatus(id, result) if err != nil { - return nil, nil, err + return nil, err } - if jobType != JobTypeKojiInit { - return nil, nil, fmt.Errorf("expected %q, found %q job instead", JobTypeKojiInit, jobType) + if jobInfo.JobType != JobTypeKojiInit { + return nil, fmt.Errorf("expected %q, found %q job instead", JobTypeKojiInit, jobInfo.JobType) } if result.JobError == nil && result.KojiError != "" { result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOldResultCompatible, result.KojiError) } - return status, deps, nil + return jobInfo, nil } -func (s *Server) KojiFinalizeJobStatus(id uuid.UUID, result *KojiFinalizeJobResult) (*JobStatus, []uuid.UUID, error) { - jobType, _, status, deps, err := s.jobStatus(id, result) +func (s *Server) KojiFinalizeJobStatus(id uuid.UUID, result *KojiFinalizeJobResult) (*JobInfo, error) { + jobInfo, err := s.jobStatus(id, result) if err != nil { - return nil, nil, err + return nil, err } - if jobType != JobTypeKojiFinalize { - return nil, nil, fmt.Errorf("expected %q, found %q job instead", JobTypeKojiFinalize, jobType) + if jobInfo.JobType != JobTypeKojiFinalize { + return nil, fmt.Errorf("expected %q, found %q job instead", JobTypeKojiFinalize, jobInfo.JobType) } if result.JobError == nil && result.KojiError != "" { result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorOldResultCompatible, result.KojiError) } - return status, deps, nil + return jobInfo, nil } -func (s *Server) DepsolveJobStatus(id uuid.UUID, result *DepsolveJobResult) (*JobStatus, []uuid.UUID, error) { - jobType, _, status, deps, err := s.jobStatus(id, result) +func (s *Server) DepsolveJobStatus(id uuid.UUID, result *DepsolveJobResult) (*JobInfo, error) { + jobInfo, err := s.jobStatus(id, result) if err != nil { - return nil, nil, err + return nil, err } - if jobType != JobTypeDepsolve { - return nil, nil, fmt.Errorf("expected %q, found %q job instead", JobTypeDepsolve, jobType) + if jobInfo.JobType != JobTypeDepsolve { + return nil, fmt.Errorf("expected %q, found %q job instead", JobTypeDepsolve, jobInfo.JobType) } if result.JobError == nil && result.Error != "" { @@ -320,55 +327,60 @@ func (s *Server) DepsolveJobStatus(id uuid.UUID, result *DepsolveJobResult) (*Jo } } - return status, deps, nil + return jobInfo, nil } -func (s *Server) ManifestJobStatus(id uuid.UUID, result *ManifestJobByIDResult) (*JobStatus, []uuid.UUID, error) { - jobType, _, status, deps, err := s.jobStatus(id, result) +func (s *Server) ManifestJobStatus(id uuid.UUID, result *ManifestJobByIDResult) (*JobInfo, error) { + jobInfo, err := s.jobStatus(id, result) if err != nil { - return nil, nil, err + return nil, err } - if jobType != JobTypeManifestIDOnly { - return nil, nil, fmt.Errorf("expected %q, found %q job instead", JobTypeManifestIDOnly, jobType) + if jobInfo.JobType != JobTypeManifestIDOnly { + return nil, fmt.Errorf("expected %q, found %q job instead", JobTypeManifestIDOnly, jobInfo.JobType) } - return status, deps, nil + return jobInfo, nil } -func (s *Server) ContainerResolveJobStatus(id uuid.UUID, result *ContainerResolveJobResult) (*JobStatus, []uuid.UUID, error) { - jobType, _, status, deps, err := s.jobStatus(id, result) +func (s *Server) ContainerResolveJobStatus(id uuid.UUID, result *ContainerResolveJobResult) (*JobInfo, error) { + jobInfo, err := s.jobStatus(id, result) if err != nil { - return nil, nil, err + return nil, err } - if jobType != JobTypeContainerResolve { - return nil, nil, fmt.Errorf("expected %q, found %q job instead", JobTypeDepsolve, jobType) + if jobInfo.JobType != JobTypeContainerResolve { + return nil, fmt.Errorf("expected %q, found %q job instead", JobTypeDepsolve, jobInfo.JobType) } - return status, deps, nil + return jobInfo, nil } -func (s *Server) jobStatus(id uuid.UUID, result interface{}) (string, string, *JobStatus, []uuid.UUID, error) { +func (s *Server) jobStatus(id uuid.UUID, result interface{}) (*JobInfo, error) { jobType, channel, rawResult, queued, started, finished, canceled, deps, err := s.jobs.JobStatus(id) if err != nil { - return "", "", nil, nil, err + return nil, err } if result != nil && !finished.IsZero() && !canceled { err = json.Unmarshal(rawResult, result) if err != nil { - return "", "", nil, nil, fmt.Errorf("error unmarshaling result for job '%s': %v", id, err) + return nil, fmt.Errorf("error unmarshaling result for job '%s': %v", id, err) } } - return jobType, channel, &JobStatus{ - Queued: queued, - Started: started, - Finished: finished, - Canceled: canceled, - }, deps, nil + return &JobInfo{ + JobType: strings.Split(jobType, ":")[0], + Channel: channel, + JobStatus: &JobStatus{ + Queued: queued, + Started: started, + Finished: finished, + Canceled: canceled, + }, + Deps: deps, + }, nil } // OSBuildJob returns the parameters of an OSBuildJob @@ -403,11 +415,11 @@ func (s *Server) JobType(id uuid.UUID) (string, error) { } func (s *Server) Cancel(id uuid.UUID) error { - jobType, channel, status, _, err := s.jobStatus(id, nil) + jobInfo, err := s.jobStatus(id, nil) if err != nil { logrus.Errorf("error getting job status: %v", err) } else { - prometheus.CancelJobMetrics(status.Started, jobType, channel) + prometheus.CancelJobMetrics(jobInfo.JobStatus.Started, jobInfo.JobType, jobInfo.Channel) } return s.jobs.CancelJob(id) } @@ -419,12 +431,12 @@ func (s *Server) JobArtifact(id uuid.UUID, name string) (io.Reader, int64, error return nil, 0, errors.New("Artifacts not enabled") } - _, _, status, _, err := s.jobStatus(id, nil) + jobInfo, err := s.jobStatus(id, nil) if err != nil { return nil, 0, err } - if status.Finished.IsZero() { + if jobInfo.JobStatus.Finished.IsZero() { return nil, 0, fmt.Errorf("Cannot access artifacts before job is finished: %s", id) } @@ -448,12 +460,12 @@ func (s *Server) DeleteArtifacts(id uuid.UUID) error { return errors.New("Artifacts not enabled") } - _, _, status, _, err := s.jobStatus(id, nil) + jobInfo, err := s.jobStatus(id, nil) if err != nil { return err } - if status.Finished.IsZero() { + if jobInfo.JobStatus.Finished.IsZero() { return fmt.Errorf("Cannot delete artifacts before job is finished: %s", id) } @@ -502,7 +514,7 @@ func (s *Server) requestJob(ctx context.Context, arch string, jobTypes []string, return } - jobType, channel, status, _, err := s.jobStatus(jobId, nil) + jobInfo, err := s.jobStatus(jobId, nil) if err != nil { logrus.Errorf("error retrieving job status: %v", err) } @@ -511,7 +523,8 @@ func (s *Server) requestJob(ctx context.Context, arch string, jobTypes []string, // long it has been queued for, in case it has no dependencies, or // how long it has been since all its dependencies finished, if it // has any. - pending := status.Queued + pending := jobInfo.JobStatus.Queued + jobType = jobInfo.JobType for _, depID := range depIDs { // TODO: include type of arguments @@ -536,7 +549,7 @@ func (s *Server) requestJob(ctx context.Context, arch string, jobTypes []string, // TODO: Drop the ':$architecture' for metrics too, first prometheus queries for alerts and // dashboards need to be adjusted. - prometheus.DequeueJobMetrics(pending, status.Started, jobType, channel) + prometheus.DequeueJobMetrics(pending, jobInfo.JobStatus.Started, jobType, jobInfo.Channel) if jobType == JobTypeOSBuild+":"+arch { jobType = JobTypeOSBuild } @@ -566,13 +579,13 @@ func (s *Server) FinishJob(token uuid.UUID, result json.RawMessage) error { } var jobResult JobResult - jobType, channel, status, _, err := s.jobStatus(jobId, &jobResult) + jobInfo, err := s.jobStatus(jobId, &jobResult) if err != nil { logrus.Errorf("error finding job status: %v", err) } else { statusCode := clienterrors.GetStatusCode(jobResult.JobError) - arch := getBuildJobArch(jobType, result) - prometheus.FinishJobMetrics(status.Started, status.Finished, status.Canceled, jobType, channel, arch, statusCode) + arch := getBuildJobArch(jobInfo.JobType, result) + prometheus.FinishJobMetrics(jobInfo.JobStatus.Started, jobInfo.JobStatus.Finished, jobInfo.JobStatus.Canceled, jobInfo.JobType, jobInfo.Channel, arch, statusCode) } // Move artifacts from the temporary location to the final job @@ -723,7 +736,7 @@ func (h *apiHandlers) GetJob(ctx echo.Context, tokenstr string) error { h.server.jobs.RefreshHeartbeat(token) - _, _, status, _, err := h.server.jobStatus(jobId, nil) + jobInfo, err := h.server.jobStatus(jobId, nil) if err != nil { return api.HTTPErrorWithInternal(api.ErrorRetrievingJobStatus, err) } @@ -734,7 +747,7 @@ func (h *apiHandlers) GetJob(ctx echo.Context, tokenstr string) error { Id: token.String(), Kind: "JobStatus", }, - Canceled: status.Canceled, + Canceled: jobInfo.JobStatus.Canceled, }) } diff --git a/internal/worker/server_test.go b/internal/worker/server_test.go index c4ff8b584..181ac1cf4 100644 --- a/internal/worker/server_test.go +++ b/internal/worker/server_test.go @@ -516,7 +516,7 @@ func TestMixedOSBuildJob(t *testing.T) { require.NoError(err) oldJobResultRead := new(worker.OSBuildJobResult) - _, _, err = server.OSBuildJobStatus(oldJobID, oldJobResultRead) + _, err = server.OSBuildJobStatus(oldJobID, oldJobResultRead) require.NoError(err) // oldJobResultRead should have PipelineNames now @@ -554,7 +554,7 @@ func TestMixedOSBuildJob(t *testing.T) { require.NoError(err) newJobResultRead := new(worker.OSBuildJobResult) - _, _, err = server.OSBuildJobStatus(newJobID, newJobResultRead) + _, err = server.OSBuildJobStatus(newJobID, newJobResultRead) require.NoError(err) require.Equal(newJobResult, newJobResultRead) } @@ -589,7 +589,7 @@ func TestDepsolveLegacyErrorConversion(t *testing.T) { ErrorType: errType, } - _, _, err = server.DepsolveJobStatus(depsolveJobId, &depsolveJobResult) + _, err = server.DepsolveJobStatus(depsolveJobId, &depsolveJobResult) require.NoError(t, err) require.Equal(t, expectedResult, depsolveJobResult) }