From a4e653156588b7a8bd53e83678873dd7df51b0e0 Mon Sep 17 00:00:00 2001 From: Tomas Hozza Date: Wed, 18 May 2022 13:05:33 +0200 Subject: [PATCH] worker: define job types as constants Define supported job type names as constants and use them in all places, instead of string literals. There are multiple benefits of this approach. Using constants removed the room for typos in the string literals. One can use autocompletion in IDE for job types. Using constant makes it easier to find all references where it is used and thus all places that are handling a specific job type. --- cmd/osbuild-service-maintenance/db.go | 3 +- cmd/osbuild-worker/main.go | 12 ++-- internal/cloudapi/v2/handler.go | 10 +-- internal/cloudapi/v2/v2_koji_test.go | 16 ++--- internal/cloudapi/v2/v2_multi_tenancy_test.go | 21 ++++-- internal/cloudapi/v2/v2_test.go | 22 +++--- internal/kojiapi/server_test.go | 16 ++--- internal/weldr/compose_test.go | 4 +- internal/worker/client_test.go | 4 +- internal/worker/server.go | 69 +++++++++++-------- internal/worker/server_test.go | 44 ++++++------ 11 files changed, 119 insertions(+), 102 deletions(-) diff --git a/cmd/osbuild-service-maintenance/db.go b/cmd/osbuild-service-maintenance/db.go index 5a56e05fb..5f8f8ebbc 100644 --- a/cmd/osbuild-service-maintenance/db.go +++ b/cmd/osbuild-service-maintenance/db.go @@ -7,6 +7,7 @@ import ( "github.com/sirupsen/logrus" "github.com/osbuild/osbuild-composer/internal/jobqueue/dbjobqueue" + "github.com/osbuild/osbuild-composer/internal/worker" ) func DBCleanup(dbURL string, dryRun bool, cutoff time.Time) error { @@ -17,7 +18,7 @@ func DBCleanup(dbURL string, dryRun bool, cutoff time.Time) error { // The results of these jobs take up the most space and can contain sensitive data. Delete // them after a while. - jobsByType, err := jobs.JobsUptoByType([]string{"manifest-id-only", "depsolve"}, cutoff) + jobsByType, err := jobs.JobsUptoByType([]string{worker.JobTypeManifestIDOnly, worker.JobTypeDepsolve}, cutoff) if err != nil { return fmt.Errorf("Error querying jobs: %v", err) } diff --git a/cmd/osbuild-worker/main.go b/cmd/osbuild-worker/main.go index 4f266447e..c4153ec6d 100644 --- a/cmd/osbuild-worker/main.go +++ b/cmd/osbuild-worker/main.go @@ -170,7 +170,7 @@ func RequestAndRunJob(client *worker.Client, acceptedJobTypes []string, jobImpls // Depsolve requests needs reactivity, since setting the protection can take up to 6s to timeout if the worker isn't // in an AWS env, disable this setting for them. - if job.Type() != "depsolve" { + if job.Type() != worker.JobTypeDepsolve { setProtection(true) defer setProtection(false) } @@ -420,7 +420,7 @@ func main() { defer depsolveCtxCancel() go func() { jobImpls := map[string]JobImplementation{ - "depsolve": &DepsolveJobImpl{ + worker.JobTypeDepsolve: &DepsolveJobImpl{ Solver: solver, }, } @@ -448,7 +448,7 @@ func main() { // non-depsolve job jobImpls := map[string]JobImplementation{ - "osbuild": &OSBuildJobImpl{ + worker.JobTypeOSBuild: &OSBuildJobImpl{ Store: store, Output: output, KojiServers: kojiServers, @@ -466,17 +466,17 @@ func main() { SkipSSLVerification: genericS3SkipSSLVerification, }, }, - "osbuild-koji": &OSBuildKojiJobImpl{ + worker.JobTypeOSBuildKoji: &OSBuildKojiJobImpl{ Store: store, Output: output, KojiServers: kojiServers, relaxTimeoutFactor: config.RelaxTimeoutFactor, }, - "koji-init": &KojiInitJobImpl{ + worker.JobTypeKojiInit: &KojiInitJobImpl{ KojiServers: kojiServers, relaxTimeoutFactor: config.RelaxTimeoutFactor, }, - "koji-finalize": &KojiFinalizeJobImpl{ + worker.JobTypeKojiFinalize: &KojiFinalizeJobImpl{ KojiServers: kojiServers, relaxTimeoutFactor: config.RelaxTimeoutFactor, }, diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go index a636c0f0c..7140162a5 100644 --- a/internal/cloudapi/v2/handler.go +++ b/internal/cloudapi/v2/handler.go @@ -508,7 +508,7 @@ func (h *apiHandlers) GetComposeStatus(ctx echo.Context, id string) error { return HTTPError(ErrorComposeNotFound) } - if jobType == "osbuild" { + if jobType == worker.JobTypeOSBuild { var result worker.OSBuildJobResult status, deps, err := h.server.workers.OSBuildJobStatus(jobId, &result) if err != nil { @@ -584,7 +584,7 @@ func (h *apiHandlers) GetComposeStatus(ctx echo.Context, id string) error { UploadStatus: us, }, }) - } else if jobType == "koji-finalize" { + } else if jobType == worker.JobTypeKojiFinalize { var result worker.KojiFinalizeJobResult finalizeStatus, deps, err := h.server.workers.KojiFinalizeJobStatus(jobId, &result) if err != nil { @@ -751,7 +751,7 @@ func (h *apiHandlers) GetComposeMetadata(ctx echo.Context, id string) error { } // TODO: support koji builds - if jobType != "osbuild" { + if jobType != worker.JobTypeOSBuild { return HTTPError(ErrorInvalidJobType) } @@ -862,7 +862,7 @@ func (h *apiHandlers) GetComposeLogs(ctx echo.Context, id string) error { } // TODO: support non-koji builds - if jobType != "koji-finalize" { + if jobType != worker.JobTypeKojiFinalize { return HTTPError(ErrorInvalidJobType) } @@ -920,7 +920,7 @@ func (h *apiHandlers) GetComposeManifests(ctx echo.Context, id string) error { } // TODO: support non-koji builds - if jobType != "koji-finalize" { + if jobType != worker.JobTypeKojiFinalize { return HTTPError(ErrorInvalidJobType) } diff --git a/internal/cloudapi/v2/v2_koji_test.go b/internal/cloudapi/v2/v2_koji_test.go index 5f3a494b0..330c7d13d 100644 --- a/internal/cloudapi/v2/v2_koji_test.go +++ b/internal/cloudapi/v2/v2_koji_test.go @@ -426,9 +426,9 @@ func TestKojiCompose(t *testing.T) { c.composeReplyCode, c.composeReply, "id", "operation_id") // handle koji-init - _, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{"koji-init"}, []string{""}) + _, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeKojiInit}, []string{""}) require.NoError(t, err) - require.Equal(t, "koji-init", jobType) + require.Equal(t, worker.JobTypeKojiInit, jobType) var initJob worker.KojiInitJob err = json.Unmarshal(rawJob, &initJob) @@ -444,9 +444,9 @@ func TestKojiCompose(t *testing.T) { fmt.Sprintf(`{"href":"/api/worker/v1/jobs/%v","id":"%v","kind":"UpdateJobResponse"}`, token, token)) // handle osbuild-koji #1 - _, token, jobType, rawJob, _, err = workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{"osbuild-koji"}, []string{""}) + _, token, jobType, rawJob, _, err = workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeOSBuildKoji}, []string{""}) require.NoError(t, err) - require.Equal(t, "osbuild-koji", jobType) + require.Equal(t, worker.JobTypeOSBuildKoji, jobType) var osbuildJob worker.OSBuildKojiJob err = json.Unmarshal(rawJob, &osbuildJob) @@ -461,9 +461,9 @@ func TestKojiCompose(t *testing.T) { fmt.Sprintf(`{"href":"/api/worker/v1/jobs/%v","id":"%v","kind":"UpdateJobResponse"}`, token, token)) // handle osbuild-koji #2 - _, token, jobType, rawJob, _, err = workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{"osbuild-koji"}, []string{""}) + _, token, jobType, rawJob, _, err = workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeOSBuildKoji}, []string{""}) require.NoError(t, err) - require.Equal(t, "osbuild-koji", jobType) + require.Equal(t, worker.JobTypeOSBuildKoji, jobType) err = json.Unmarshal(rawJob, &osbuildJob) require.NoError(t, err) @@ -485,9 +485,9 @@ func TestKojiCompose(t *testing.T) { fmt.Sprintf(`{"href":"/api/worker/v1/jobs/%v","id":"%v","kind":"UpdateJobResponse"}`, token, token)) // handle koji-finalize - finalizeID, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{"koji-finalize"}, []string{""}) + finalizeID, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeKojiFinalize}, []string{""}) require.NoError(t, err) - require.Equal(t, "koji-finalize", jobType) + require.Equal(t, worker.JobTypeKojiFinalize, jobType) var kojiFinalizeJob worker.KojiFinalizeJob err = json.Unmarshal(rawJob, &kojiFinalizeJob) diff --git a/internal/cloudapi/v2/v2_multi_tenancy_test.go b/internal/cloudapi/v2/v2_multi_tenancy_test.go index 8f7f44c86..3a191f52e 100644 --- a/internal/cloudapi/v2/v2_multi_tenancy_test.go +++ b/internal/cloudapi/v2/v2_multi_tenancy_test.go @@ -17,6 +17,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/distro/test_distro" "github.com/osbuild/osbuild-composer/internal/jobqueue" "github.com/osbuild/osbuild-composer/internal/test" + "github.com/osbuild/osbuild-composer/internal/worker" ) func kojiRequest() string { @@ -121,14 +122,20 @@ func jobRequest() string { return fmt.Sprintf(` { "types": [ - "koji-init", - "osbuild", - "osbuild-koji", - "koji-finalize", - "depsolve" + %q, + %q, + %q, + %q, + %q ], - "arch": "%s" - }`, test_distro.TestArch3Name) + "arch": %q + }`, + worker.JobTypeKojiInit, + worker.JobTypeOSBuild, + worker.JobTypeOSBuildKoji, + worker.JobTypeKojiFinalize, + worker.JobTypeDepsolve, + test_distro.TestArch3Name) } func runNextJob(t *testing.T, jobs []uuid.UUID, workerHandler http.Handler, orgID string) { diff --git a/internal/cloudapi/v2/v2_test.go b/internal/cloudapi/v2/v2_test.go index e67a02a20..e6e5a443b 100644 --- a/internal/cloudapi/v2/v2_test.go +++ b/internal/cloudapi/v2/v2_test.go @@ -47,7 +47,7 @@ func newV2Server(t *testing.T, dir string, depsolveChannels []string, enableJWT go func() { defer wg.Done() for { - _, token, _, _, _, err := workerServer.RequestJob(depsolveContext, test_distro.TestDistroName, []string{"depsolve"}, depsolveChannels) + _, token, _, _, _, err := workerServer.RequestJob(depsolveContext, test_distro.TestDistroName, []string{worker.JobTypeDepsolve}, depsolveChannels) select { case <-depsolveContext.Done(): return @@ -572,9 +572,9 @@ func TestComposeStatusSuccess(t *testing.T) { "kind": "ComposeId" }`, "id") - jobId, token, jobType, args, dynArgs, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{"osbuild"}, []string{""}) + jobId, token, jobType, args, dynArgs, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) - require.Equal(t, "osbuild", jobType) + require.Equal(t, worker.JobTypeOSBuild, jobType) var osbuildJob worker.OSBuildJob err = json.Unmarshal(args, &osbuildJob) @@ -642,9 +642,9 @@ func TestComposeStatusFailure(t *testing.T) { "kind": "ComposeId" }`, "id") - jobId, token, jobType, _, _, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{"osbuild"}, []string{""}) + jobId, token, jobType, _, _, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) - require.Equal(t, "osbuild", jobType) + require.Equal(t, worker.JobTypeOSBuild, jobType) test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%v", jobId), ``, http.StatusOK, fmt.Sprintf(` { @@ -698,9 +698,9 @@ func TestComposeLegacyError(t *testing.T) { "kind": "ComposeId" }`, "id") - jobId, token, jobType, _, _, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{"osbuild"}, []string{""}) + jobId, token, jobType, _, _, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) - require.Equal(t, "osbuild", jobType) + require.Equal(t, worker.JobTypeOSBuild, jobType) test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%v", jobId), ``, http.StatusOK, fmt.Sprintf(` { @@ -757,9 +757,9 @@ func TestComposeJobError(t *testing.T) { "kind": "ComposeId" }`, "id") - jobId, token, jobType, _, _, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{"osbuild"}, []string{""}) + jobId, token, jobType, _, _, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) - require.Equal(t, "osbuild", jobType) + require.Equal(t, worker.JobTypeOSBuild, jobType) test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%v", jobId), ``, http.StatusOK, fmt.Sprintf(` { @@ -819,9 +819,9 @@ func TestComposeDependencyError(t *testing.T) { "kind": "ComposeId" }`, "id") - jobId, token, jobType, _, _, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{"osbuild"}, []string{""}) + jobId, token, jobType, _, _, err := wrksrv.RequestJob(context.Background(), test_distro.TestArch3Name, []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) - require.Equal(t, "osbuild", jobType) + require.Equal(t, worker.JobTypeOSBuild, jobType) test.TestRoute(t, srv.Handler("/api/image-builder-composer/v2"), false, "GET", fmt.Sprintf("/api/image-builder-composer/v2/composes/%v", jobId), ``, http.StatusOK, fmt.Sprintf(` { diff --git a/internal/kojiapi/server_test.go b/internal/kojiapi/server_test.go index 75c98c615..2dda92012 100644 --- a/internal/kojiapi/server_test.go +++ b/internal/kojiapi/server_test.go @@ -352,9 +352,9 @@ func TestCompose(t *testing.T) { wg.Add(1) go func(t *testing.T, result worker.KojiInitJobResult) { - _, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArchName, []string{"koji-init"}, []string{""}) + _, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArchName, []string{worker.JobTypeKojiInit}, []string{""}) require.NoError(t, err) - require.Equal(t, "koji-init", jobType) + require.Equal(t, worker.JobTypeKojiInit, jobType) var initJob worker.KojiInitJob err = json.Unmarshal(rawJob, &initJob) @@ -405,9 +405,9 @@ func TestCompose(t *testing.T) { c.composeReplyCode, c.composeReply, "id") wg.Wait() - _, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArchName, []string{"osbuild-koji"}, []string{""}) + _, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArchName, []string{worker.JobTypeOSBuildKoji}, []string{""}) require.NoError(t, err) - require.Equal(t, "osbuild-koji", jobType) + require.Equal(t, worker.JobTypeOSBuildKoji, jobType) var osbuildJob worker.OSBuildKojiJob err = json.Unmarshal(rawJob, &osbuildJob) @@ -421,9 +421,9 @@ func TestCompose(t *testing.T) { test.TestRoute(t, workerHandler, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(buildJobResult), http.StatusOK, fmt.Sprintf(`{"href":"/api/worker/v1/jobs/%v","id":"%v","kind":"UpdateJobResponse"}`, token, token)) - _, token, jobType, rawJob, _, err = workerServer.RequestJob(context.Background(), test_distro.TestArchName, []string{"osbuild-koji"}, []string{""}) + _, token, jobType, rawJob, _, err = workerServer.RequestJob(context.Background(), test_distro.TestArchName, []string{worker.JobTypeOSBuildKoji}, []string{""}) require.NoError(t, err) - require.Equal(t, "osbuild-koji", jobType) + require.Equal(t, worker.JobTypeOSBuildKoji, jobType) err = json.Unmarshal(rawJob, &osbuildJob) require.NoError(t, err) @@ -444,9 +444,9 @@ func TestCompose(t *testing.T) { }`, test_distro.TestArchName, test_distro.TestDistroName), http.StatusOK, fmt.Sprintf(`{"href":"/api/worker/v1/jobs/%v","id":"%v","kind":"UpdateJobResponse"}`, token, token)) - finalizeID, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArchName, []string{"koji-finalize"}, []string{""}) + finalizeID, token, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), test_distro.TestArchName, []string{worker.JobTypeKojiFinalize}, []string{""}) require.NoError(t, err) - require.Equal(t, "koji-finalize", jobType) + require.Equal(t, worker.JobTypeKojiFinalize, jobType) var kojiFinalizeJob worker.KojiFinalizeJob err = json.Unmarshal(rawJob, &kojiFinalizeJob) diff --git a/internal/weldr/compose_test.go b/internal/weldr/compose_test.go index a04f27039..a2c2bf5ce 100644 --- a/internal/weldr/compose_test.go +++ b/internal/weldr/compose_test.go @@ -40,7 +40,7 @@ func TestComposeStatusFromLegacyError(t *testing.T) { jobId, err := api.workers.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}, "") require.NoError(t, err) - j, token, _, _, _, err := api.workers.RequestJob(context.Background(), arch.Name(), []string{"osbuild"}, []string{""}) + j, token, _, _, _, err := api.workers.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) require.Equal(t, jobId, j) @@ -82,7 +82,7 @@ func TestComposeStatusFromJobError(t *testing.T) { jobId, err := api.workers.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}, "") require.NoError(t, err) - j, token, _, _, _, err := api.workers.RequestJob(context.Background(), arch.Name(), []string{"osbuild"}, []string{""}) + j, token, _, _, _, err := api.workers.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) require.Equal(t, jobId, j) diff --git a/internal/worker/client_test.go b/internal/worker/client_test.go index 0f6e20c05..93a9b6b8f 100644 --- a/internal/worker/client_test.go +++ b/internal/worker/client_test.go @@ -92,7 +92,7 @@ func TestOAuth(t *testing.T) { BasePath: "/api/image-builder-worker/v1", }) require.NoError(t, err) - job, err := client.RequestJob([]string{"osbuild"}, "arch") + job, err := client.RequestJob([]string{worker.JobTypeOSBuild}, "arch") require.NoError(t, err) r := strings.NewReader("artifact contents") require.NoError(t, job.UploadArtifact("some-artifact", r)) @@ -120,7 +120,7 @@ func TestProxy(t *testing.T) { }) require.NoError(t, err) - job, err := client.RequestJob([]string{"osbuild"}, "arch") + job, err := client.RequestJob([]string{worker.JobTypeOSBuild}, "arch") require.NoError(t, err) r := strings.NewReader("artifact contents") require.NoError(t, job.UploadArtifact("some-artifact", r)) diff --git a/internal/worker/server.go b/internal/worker/server.go index 857db3b1c..0c560d51d 100644 --- a/internal/worker/server.go +++ b/internal/worker/server.go @@ -28,6 +28,15 @@ import ( "github.com/osbuild/osbuild-composer/internal/worker/clienterrors" ) +const ( + JobTypeOSBuild string = "osbuild" + JobTypeOSBuildKoji string = "osbuild-koji" + JobTypeKojiInit string = "koji-init" + JobTypeKojiFinalize string = "koji-finalize" + JobTypeDepsolve string = "depsolve" + JobTypeManifestIDOnly string = "manifest-id-only" +) + type Server struct { jobs jobqueue.JobQueue logger *log.Logger @@ -101,35 +110,35 @@ func (s *Server) WatchHeartbeats() { } func (s *Server) EnqueueOSBuild(arch string, job *OSBuildJob, channel string) (uuid.UUID, error) { - return s.enqueue("osbuild:"+arch, job, nil, channel) + return s.enqueue(JobTypeOSBuild+":"+arch, job, nil, channel) } func (s *Server) EnqueueOSBuildAsDependency(arch string, job *OSBuildJob, dependencies []uuid.UUID, channel string) (uuid.UUID, error) { - return s.enqueue("osbuild:"+arch, job, dependencies, channel) + return s.enqueue(JobTypeOSBuild+":"+arch, job, dependencies, channel) } func (s *Server) EnqueueOSBuildKoji(arch string, job *OSBuildKojiJob, initID uuid.UUID, channel string) (uuid.UUID, error) { - return s.enqueue("osbuild-koji:"+arch, job, []uuid.UUID{initID}, channel) + return s.enqueue(JobTypeOSBuildKoji+":"+arch, job, []uuid.UUID{initID}, channel) } func (s *Server) EnqueueOSBuildKojiAsDependency(arch string, job *OSBuildKojiJob, manifestID, initID uuid.UUID, channel string) (uuid.UUID, error) { - return s.enqueue("osbuild-koji:"+arch, job, []uuid.UUID{initID, manifestID}, channel) + return s.enqueue(JobTypeOSBuildKoji+":"+arch, job, []uuid.UUID{initID, manifestID}, channel) } func (s *Server) EnqueueKojiInit(job *KojiInitJob, channel string) (uuid.UUID, error) { - return s.enqueue("koji-init", job, nil, channel) + return s.enqueue(JobTypeKojiInit, job, nil, channel) } func (s *Server) EnqueueKojiFinalize(job *KojiFinalizeJob, initID uuid.UUID, buildIDs []uuid.UUID, channel string) (uuid.UUID, error) { - return s.enqueue("koji-finalize", job, append([]uuid.UUID{initID}, buildIDs...), channel) + return s.enqueue(JobTypeKojiFinalize, job, append([]uuid.UUID{initID}, buildIDs...), channel) } func (s *Server) EnqueueDepsolve(job *DepsolveJob, channel string) (uuid.UUID, error) { - return s.enqueue("depsolve", job, nil, channel) + return s.enqueue(JobTypeDepsolve, job, nil, channel) } func (s *Server) EnqueueManifestJobByID(job *ManifestJobByID, parent uuid.UUID, channel string) (uuid.UUID, error) { - return s.enqueue("manifest-id-only", job, []uuid.UUID{parent}, channel) + return s.enqueue(JobTypeManifestIDOnly, job, []uuid.UUID{parent}, channel) } func (s *Server) enqueue(jobType string, job interface{}, dependencies []uuid.UUID, channel string) (uuid.UUID, error) { @@ -171,8 +180,8 @@ func (s *Server) OSBuildJobStatus(id uuid.UUID, result *OSBuildJobResult) (*JobS return nil, nil, err } - if !strings.HasPrefix(jobType, "osbuild:") { // Build jobs get automatic arch suffix: Check prefix - return nil, nil, fmt.Errorf("expected osbuild:*, found %q job instead", jobType) + 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 result.JobError == nil && !status.Finished.IsZero() { @@ -199,8 +208,8 @@ func (s *Server) OSBuildKojiJobStatus(id uuid.UUID, result *OSBuildKojiJobResult return nil, nil, err } - if !strings.HasPrefix(jobType, "osbuild-koji:") { // Build jobs get automatic arch suffix: Check prefix - return nil, nil, fmt.Errorf("expected \"osbuild-koji:*\", found %q job instead", jobType) + if !strings.HasPrefix(jobType, JobTypeOSBuildKoji+":") { // Build jobs get automatic arch suffix: Check prefix + return nil, nil, fmt.Errorf("expected \"%s:*\", found %q job instead", JobTypeOSBuildKoji, jobType) } if result.JobError == nil && !status.Finished.IsZero() { @@ -222,8 +231,8 @@ func (s *Server) KojiInitJobStatus(id uuid.UUID, result *KojiInitJobResult) (*Jo return nil, nil, err } - if jobType != "koji-init" { - return nil, nil, fmt.Errorf("expected \"koji-init\", found %q job instead", jobType) + if jobType != JobTypeKojiInit { + return nil, nil, fmt.Errorf("expected %q, found %q job instead", JobTypeKojiInit, jobType) } if result.JobError == nil && result.KojiError != "" { @@ -239,8 +248,8 @@ func (s *Server) KojiFinalizeJobStatus(id uuid.UUID, result *KojiFinalizeJobResu return nil, nil, err } - if jobType != "koji-finalize" { - return nil, nil, fmt.Errorf("expected \"koji-finalize\", found %q job instead", jobType) + if jobType != JobTypeKojiFinalize { + return nil, nil, fmt.Errorf("expected %q, found %q job instead", JobTypeKojiFinalize, jobType) } if result.JobError == nil && result.KojiError != "" { @@ -256,8 +265,8 @@ func (s *Server) DepsolveJobStatus(id uuid.UUID, result *DepsolveJobResult) (*Jo return nil, nil, err } - if jobType != "depsolve" { - return nil, nil, fmt.Errorf("expected \"depsolve\", found %q job instead", jobType) + if jobType != JobTypeDepsolve { + return nil, nil, fmt.Errorf("expected %q, found %q job instead", JobTypeDepsolve, jobType) } if result.JobError == nil && result.Error != "" { @@ -277,8 +286,8 @@ func (s *Server) ManifestJobStatus(id uuid.UUID, result *ManifestJobByIDResult) return nil, nil, err } - if jobType != "manifest-id-only" { - return nil, nil, fmt.Errorf("expected \"manifest-id-only\", found %q job instead", jobType) + if jobType != JobTypeManifestIDOnly { + return nil, nil, fmt.Errorf("expected %q, found %q job instead", JobTypeManifestIDOnly, jobType) } return status, deps, nil @@ -312,8 +321,8 @@ func (s *Server) OSBuildJob(id uuid.UUID, job *OSBuildJob) error { return err } - if !strings.HasPrefix(jobType, "osbuild:") { // Build jobs get automatic arch suffix: Check prefix - return fmt.Errorf("expected osbuild:*, found %q job instead for job '%s'", jobType, id) + if !strings.HasPrefix(jobType, JobTypeOSBuild+":") { // Build jobs get automatic arch suffix: Check prefix + return fmt.Errorf("expected %s:*, found %q job instead for job '%s'", JobTypeOSBuild, jobType, id) } if err := json.Unmarshal(rawArgs, job); err != nil { @@ -330,8 +339,8 @@ func (s *Server) OSBuildKojiJob(id uuid.UUID, job *OSBuildKojiJob) error { return err } - if !strings.HasPrefix(jobType, "osbuild-koji:") { // Build jobs get automatic arch suffix: Check prefix - return fmt.Errorf("expected osbuild-koji:*, found %q job instead for job '%s'", jobType, id) + if !strings.HasPrefix(jobType, JobTypeOSBuildKoji+":") { // Build jobs get automatic arch suffix: Check prefix + return fmt.Errorf("expected %s:*, found %q job instead for job '%s'", JobTypeOSBuildKoji, jobType, id) } if err := json.Unmarshal(rawArgs, job); err != nil { @@ -422,10 +431,10 @@ func (s *Server) requestJob(ctx context.Context, arch string, jobTypes []string, // restriction: arch for osbuild jobs. jts := []string{} for _, t := range jobTypes { - if t == "osbuild" || t == "osbuild-koji" { + if t == JobTypeOSBuild || t == JobTypeOSBuildKoji { t = t + ":" + arch } - if t == "manifest-id-only" { + if t == JobTypeManifestIDOnly { return uuid.Nil, uuid.Nil, "", nil, nil, ErrInvalidJobType } jts = append(jts, t) @@ -486,10 +495,10 @@ 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) - if jobType == "osbuild:"+arch { - jobType = "osbuild" - } else if jobType == "osbuild-koji:"+arch { - jobType = "osbuild-koji" + if jobType == JobTypeOSBuild+":"+arch { + jobType = JobTypeOSBuild + } else if jobType == JobTypeOSBuildKoji+":"+arch { + jobType = JobTypeOSBuildKoji } return diff --git a/internal/worker/server_test.go b/internal/worker/server_test.go index f707f0b8d..e3ee14a11 100644 --- a/internal/worker/server_test.go +++ b/internal/worker/server_test.go @@ -124,8 +124,8 @@ func TestCreate(t *testing.T) { require.NoError(t, err) test.TestRoute(t, handler, false, "POST", "/api/worker/v1/jobs", - fmt.Sprintf(`{"types":["osbuild"],"arch":"%s"}`, test_distro.TestArchName), http.StatusCreated, - `{"kind":"RequestJob","href":"/api/worker/v1/jobs","type":"osbuild","args":{"manifest":{"pipeline":{},"sources":{}}}}`, "id", "location", "artifact_location") + fmt.Sprintf(`{"types":["%s"],"arch":"%s"}`, worker.JobTypeOSBuild, test_distro.TestArchName), http.StatusCreated, + fmt.Sprintf(`{"kind":"RequestJob","href":"/api/worker/v1/jobs","type":"%s","args":{"manifest":{"pipeline":{},"sources":{}}}}`, worker.JobTypeOSBuild), "id", "location", "artifact_location") } func TestCancel(t *testing.T) { @@ -148,10 +148,10 @@ func TestCancel(t *testing.T) { jobId, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}, "") require.NoError(t, err) - j, token, typ, args, dynamicArgs, err := server.RequestJob(context.Background(), arch.Name(), []string{"osbuild"}, []string{""}) + j, token, typ, args, dynamicArgs, err := server.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) require.Equal(t, jobId, j) - require.Equal(t, "osbuild", typ) + require.Equal(t, worker.JobTypeOSBuild, typ) require.NotNil(t, args) require.Nil(t, dynamicArgs) @@ -185,10 +185,10 @@ func TestUpdate(t *testing.T) { jobId, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}, "") require.NoError(t, err) - j, token, typ, args, dynamicArgs, err := server.RequestJob(context.Background(), arch.Name(), []string{"osbuild"}, []string{""}) + j, token, typ, args, dynamicArgs, err := server.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) require.Equal(t, jobId, j) - require.Equal(t, "osbuild", typ) + require.Equal(t, worker.JobTypeOSBuild, typ) require.NotNil(t, args) require.Nil(t, dynamicArgs) @@ -221,7 +221,7 @@ func TestArgs(t *testing.T) { jobId, err := server.EnqueueOSBuild(arch.Name(), &job, "") require.NoError(t, err) - _, _, _, args, _, err := server.RequestJob(context.Background(), arch.Name(), []string{"osbuild"}, []string{""}) + _, _, _, args, _, err := server.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) require.NotNil(t, args) @@ -251,10 +251,10 @@ func TestUpload(t *testing.T) { jobID, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}, "") require.NoError(t, err) - j, token, typ, args, dynamicArgs, err := server.RequestJob(context.Background(), arch.Name(), []string{"osbuild"}, []string{""}) + j, token, typ, args, dynamicArgs, err := server.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) require.Equal(t, jobID, j) - require.Equal(t, "osbuild", typ) + require.Equal(t, worker.JobTypeOSBuild, typ) require.NotNil(t, args) require.Nil(t, dynamicArgs) @@ -281,10 +281,10 @@ func TestUploadAlteredBasePath(t *testing.T) { jobID, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}, "") require.NoError(t, err) - j, token, typ, args, dynamicArgs, err := server.RequestJob(context.Background(), arch.Name(), []string{"osbuild"}, []string{""}) + j, token, typ, args, dynamicArgs, err := server.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(t, err) require.Equal(t, jobID, j) - require.Equal(t, "osbuild", typ) + require.Equal(t, worker.JobTypeOSBuild, typ) require.NotNil(t, args) require.Nil(t, dynamicArgs) @@ -299,7 +299,7 @@ func TestTimeout(t *testing.T) { } server := newTestServer(t, t.TempDir(), time.Millisecond*10, "/api/image-builder-worker/v1") - _, _, _, _, _, err = server.RequestJob(context.Background(), arch.Name(), []string{"osbuild"}, []string{""}) + _, _, _, _, _, err = server.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeOSBuild}, []string{""}) require.Equal(t, jobqueue.ErrDequeueTimeout, err) test.TestRoute(t, server.Handler(), false, "POST", "/api/image-builder-worker/v1/jobs", `{"arch":"arch","types":["types"]}`, http.StatusNoContent, @@ -321,13 +321,13 @@ func TestRequestJobById(t *testing.T) { jobId, err := server.EnqueueManifestJobByID(&worker.ManifestJobByID{}, depsolveJobId, "") require.NoError(t, err) - test.TestRoute(t, server.Handler(), false, "POST", "/api/worker/v1/jobs", `{"arch":"arch","types":["manifest-id-only"]}`, http.StatusBadRequest, + test.TestRoute(t, server.Handler(), false, "POST", "/api/worker/v1/jobs", fmt.Sprintf(`{"arch":"arch","types":["%s"]}`, worker.JobTypeManifestIDOnly), http.StatusBadRequest, `{"href":"/api/worker/v1/errors/15","kind":"Error","id": "15","code":"IMAGE-BUILDER-WORKER-15"}`, "operation_id", "reason", "message") _, _, _, _, _, err = server.RequestJobById(context.Background(), arch.Name(), jobId) require.Error(t, jobqueue.ErrNotPending, err) - _, token, _, _, _, err := server.RequestJob(context.Background(), arch.Name(), []string{"depsolve"}, []string{""}) + _, token, _, _, _, err := server.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeDepsolve}, []string{""}) require.NoError(t, err) depsolveJR, err := json.Marshal(worker.DepsolveJobResult{}) @@ -338,7 +338,7 @@ func TestRequestJobById(t *testing.T) { j, token, typ, args, dynamicArgs, err := server.RequestJobById(context.Background(), arch.Name(), jobId) require.NoError(t, err) require.Equal(t, jobId, j) - require.Equal(t, "manifest-id-only", typ) + require.Equal(t, worker.JobTypeManifestIDOnly, typ) require.NotNil(t, args) require.NotNil(t, dynamicArgs) @@ -399,7 +399,7 @@ func TestMixedOSBuildJob(t *testing.T) { // don't block forever if the jobs weren't added or can't be retrieved ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() - id, token, _, _, _, err := server.RequestJob(ctx, "x", []string{"osbuild"}, []string{""}) + id, token, _, _, _, err := server.RequestJob(ctx, "x", []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(err) return id, token } @@ -544,7 +544,7 @@ func TestMixedOSBuildKojiJob(t *testing.T) { for idx := uint(0); idx < 2; idx++ { ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() - _, token, _, _, _, err := server.RequestJob(ctx, "k", []string{"koji-init"}, []string{""}) + _, token, _, _, _, err := server.RequestJob(ctx, "k", []string{worker.JobTypeKojiInit}, []string{""}) require.NoError(err) require.NoError(server.FinishJob(token, nil)) } @@ -553,7 +553,7 @@ func TestMixedOSBuildKojiJob(t *testing.T) { // don't block forever if the jobs weren't added or can't be retrieved ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() - id, token, _, _, _, err := server.RequestJob(ctx, "k", []string{"osbuild-koji"}, []string{""}) + id, token, _, _, _, err := server.RequestJob(ctx, "k", []string{worker.JobTypeOSBuildKoji}, []string{""}) require.NoError(err) return id, token } @@ -653,7 +653,7 @@ func TestDepsolveLegacyErrorConversion(t *testing.T) { depsolveJobId, err := server.EnqueueDepsolve(&worker.DepsolveJob{}, "") require.NoError(t, err) - _, _, _, _, _, err = server.RequestJob(context.Background(), arch.Name(), []string{"depsolve"}, []string{""}) + _, _, _, _, _, err = server.RequestJob(context.Background(), arch.Name(), []string{worker.JobTypeDepsolve}, []string{""}) require.NoError(t, err) reason := "Depsolve failed" @@ -722,7 +722,7 @@ func TestMixedOSBuildJobErrors(t *testing.T) { // don't block forever if the jobs weren't added or can't be retrieved ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() - id, token, _, _, _, err := server.RequestJob(ctx, "x", []string{"osbuild"}, []string{""}) + id, token, _, _, _, err := server.RequestJob(ctx, "x", []string{worker.JobTypeOSBuild}, []string{""}) require.NoError(err) return id, token } @@ -832,7 +832,7 @@ func TestMixedOSBuildKojiJobErrors(t *testing.T) { for idx := uint(0); idx < 2; idx++ { ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() - _, token, _, _, _, err := server.RequestJob(ctx, "k", []string{"koji-init"}, []string{""}) + _, token, _, _, _, err := server.RequestJob(ctx, "k", []string{worker.JobTypeKojiInit}, []string{""}) require.NoError(err) require.NoError(server.FinishJob(token, nil)) } @@ -841,7 +841,7 @@ func TestMixedOSBuildKojiJobErrors(t *testing.T) { // don't block forever if the jobs weren't added or can't be retrieved ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() - id, token, _, _, _, err := server.RequestJob(ctx, "k", []string{"osbuild-koji"}, []string{""}) + id, token, _, _, _, err := server.RequestJob(ctx, "k", []string{worker.JobTypeOSBuildKoji}, []string{""}) require.NoError(err) return id, token }