diff --git a/cmd/osbuild-worker/jobimpl-koji-finalize.go b/cmd/osbuild-worker/jobimpl-koji-finalize.go index ae7099975..7479aabc9 100644 --- a/cmd/osbuild-worker/jobimpl-koji-finalize.go +++ b/cmd/osbuild-worker/jobimpl-koji-finalize.go @@ -105,7 +105,7 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error { return err } - if initArgs.KojiError != nil { + if initArgs.KojiError != "" { failure = true } @@ -127,7 +127,7 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error { if err != nil { return err } - if !buildArgs.OSBuildOutput.Success || buildArgs.KojiError != nil { + if !buildArgs.OSBuildOutput.Success || buildArgs.KojiError != "" { failure = true break } @@ -167,9 +167,15 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error { var result worker.KojiFinalizeJobResult if failure { - result.KojiError = impl.kojiFail(args.Server, int(initArgs.BuildID), initArgs.Token) + err = impl.kojiFail(args.Server, int(initArgs.BuildID), initArgs.Token) + if err != nil { + result.KojiError = err.Error() + } } else { - result.KojiError = impl.kojiImport(args.Server, build, buildRoots, images, args.KojiDirectory, initArgs.Token) + err = impl.kojiImport(args.Server, build, buildRoots, images, args.KojiDirectory, initArgs.Token) + if err != nil { + result.KojiError = err.Error() + } } err = job.Update(&result) diff --git a/cmd/osbuild-worker/jobimpl-koji-init.go b/cmd/osbuild-worker/jobimpl-koji-init.go index ca1970db1..21d54b506 100644 --- a/cmd/osbuild-worker/jobimpl-koji-init.go +++ b/cmd/osbuild-worker/jobimpl-koji-init.go @@ -60,7 +60,10 @@ func (impl *KojiInitJobImpl) Run(job worker.Job) error { } var result worker.KojiInitJobResult - result.Token, result.BuildID, result.KojiError = impl.kojiInit(args.Server, args.Name, args.Version, args.Release) + result.Token, result.BuildID, err = impl.kojiInit(args.Server, args.Name, args.Version, args.Release) + if err != nil { + result.KojiError = err.Error() + } err = job.Update(&result) if err != nil { diff --git a/cmd/osbuild-worker/jobimpl-osbuild-koji.go b/cmd/osbuild-worker/jobimpl-osbuild-koji.go index 0087a2ddc..f7d2d26fb 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild-koji.go +++ b/cmd/osbuild-worker/jobimpl-osbuild-koji.go @@ -84,7 +84,7 @@ func (impl *OSBuildKojiJobImpl) Run(job worker.Job) error { return err } - if initArgs.KojiError == nil { + if initArgs.KojiError == "" { result.OSBuildOutput, err = RunOSBuild(args.Manifest, impl.Store, outputDirectory, os.Stderr) if err != nil { return err @@ -95,7 +95,10 @@ func (impl *OSBuildKojiJobImpl) Run(job worker.Job) error { if err != nil { return err } - result.ImageHash, result.ImageSize, result.KojiError = impl.kojiUpload(f, args.KojiServer, args.KojiDirectory, args.KojiFilename) + result.ImageHash, result.ImageSize, err = impl.kojiUpload(f, args.KojiServer, args.KojiDirectory, args.KojiFilename) + if err != nil { + result.KojiError = err.Error() + } } } diff --git a/internal/kojiapi/server.go b/internal/kojiapi/server.go index 09e928bd1..76b6eb336 100644 --- a/internal/kojiapi/server.go +++ b/internal/kojiapi/server.go @@ -190,7 +190,7 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { } time.Sleep(500 * time.Millisecond) } - if initResult.KojiError != nil { + if initResult.KojiError != "" { return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Could not initialize build with koji: %v", initResult.KojiError)) } @@ -224,7 +224,7 @@ func composeStatusFromJobStatus(js *worker.JobStatus, initResult *worker.KojiIni return "failure" } - if initResult.KojiError != nil { + if initResult.KojiError != "" { return "failure" } @@ -232,7 +232,7 @@ func composeStatusFromJobStatus(js *worker.JobStatus, initResult *worker.KojiIni if buildResult.OSBuildOutput != nil && !buildResult.OSBuildOutput.Success { return "failure" } - if buildResult.KojiError != nil { + if buildResult.KojiError != "" { return "failure" } } @@ -241,7 +241,7 @@ func composeStatusFromJobStatus(js *worker.JobStatus, initResult *worker.KojiIni return "pending" } - if result.KojiError == nil { + if result.KojiError == "" { return "success" } @@ -253,7 +253,7 @@ func imageStatusFromJobStatus(js *worker.JobStatus, initResult *worker.KojiInitJ return "failure" } - if initResult.KojiError != nil { + if initResult.KojiError != "" { return "failure" } @@ -265,7 +265,7 @@ func imageStatusFromJobStatus(js *worker.JobStatus, initResult *worker.KojiInitJ return "building" } - if buildResult.OSBuildOutput != nil && buildResult.OSBuildOutput.Success && buildResult.KojiError == nil { + if buildResult.OSBuildOutput != nil && buildResult.OSBuildOutput.Success && buildResult.KojiError == "" { return "success" } diff --git a/internal/kojiapi/server_test.go b/internal/kojiapi/server_test.go index f240e9507..c57b44c01 100644 --- a/internal/kojiapi/server_test.go +++ b/internal/kojiapi/server_test.go @@ -17,6 +17,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/kojiapi/api" distro_mock "github.com/osbuild/osbuild-composer/internal/mocks/distro" rpmmd_mock "github.com/osbuild/osbuild-composer/internal/mocks/rpmmd" + "github.com/osbuild/osbuild-composer/internal/osbuild" "github.com/osbuild/osbuild-composer/internal/test" "github.com/osbuild/osbuild-composer/internal/worker" "github.com/stretchr/testify/require" @@ -55,6 +56,10 @@ func TestStatus(t *testing.T) { test.TestRoute(t, handler, false, "GET", "/api/composer-koji/v1/status", ``, http.StatusOK, `{"status":"OK"}`, "message") } +type jobResult struct { + Result interface{} `json:"result"` +} + func TestCompose(t *testing.T) { dir, err := ioutil.TempDir("", "osbuild-composer-test-kojiapi-") if err != nil { @@ -64,139 +69,282 @@ func TestCompose(t *testing.T) { kojiServer, workerServer := newTestKojiServer(t, dir) handler := kojiServer.Handler("/api/composer-koji/v1") - var wg sync.WaitGroup - wg.Add(1) - go func() { - token, _, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), "x86_64", []string{"koji-init"}) - require.NoError(t, err) - require.Equal(t, "koji-init", jobType) + type kojiCase struct { + initResult worker.KojiInitJobResult + buildResult worker.OSBuildKojiJobResult + finalizeResult worker.KojiFinalizeJobResult + composeReplyCode int + composeReply string + composeStatus string + } - var initJob worker.KojiInitJob - err = json.Unmarshal(rawJob, &initJob) + var cases = []kojiCase{ + { + initResult: worker.KojiInitJobResult{ + BuildID: 42, + Token: `"foobar"`, + }, + buildResult: worker.OSBuildKojiJobResult{ + Arch: "x86_64", + HostOS: "fedora-30", + ImageHash: "browns", + ImageSize: 42, + OSBuildOutput: &osbuild.Result{ + Success: true, + }, + }, + composeReplyCode: http.StatusCreated, + composeReply: `{"koji_build_id":42}`, + composeStatus: `{ + "image_statuses": [ + { + "status": "success" + }, + { + "status": "success" + } + ], + "koji_build_id": 42, + "koji_task_id": 0, + "status": "success" + }`, + }, + { + initResult: worker.KojiInitJobResult{ + KojiError: "failure", + }, + buildResult: worker.OSBuildKojiJobResult{ + Arch: "x86_64", + HostOS: "fedora-30", + ImageHash: "browns", + ImageSize: 42, + OSBuildOutput: &osbuild.Result{ + Success: true, + }, + }, + composeReplyCode: http.StatusBadRequest, + composeReply: `{"message":"Could not initialize build with koji: failure"}`, + composeStatus: `{ + "image_statuses": [ + { + "status": "failure" + }, + { + "status": "failure" + } + ], + "koji_task_id": 0, + "status": "failure" + }`, + }, + { + initResult: worker.KojiInitJobResult{ + BuildID: 42, + Token: `"foobar"`, + }, + buildResult: worker.OSBuildKojiJobResult{ + Arch: "x86_64", + HostOS: "fedora-30", + ImageHash: "browns", + ImageSize: 42, + OSBuildOutput: &osbuild.Result{ + Success: false, + }, + }, + composeReplyCode: http.StatusCreated, + composeReply: `{"koji_build_id":42}`, + composeStatus: `{ + "image_statuses": [ + { + "status": "failure" + }, + { + "status": "success" + } + ], + "koji_build_id": 42, + "koji_task_id": 0, + "status": "failure" + }`, + }, + { + initResult: worker.KojiInitJobResult{ + BuildID: 42, + Token: `"foobar"`, + }, + buildResult: worker.OSBuildKojiJobResult{ + Arch: "x86_64", + HostOS: "fedora-30", + ImageHash: "browns", + ImageSize: 42, + OSBuildOutput: &osbuild.Result{ + Success: true, + }, + KojiError: "failure", + }, + composeReplyCode: http.StatusCreated, + composeReply: `{"koji_build_id":42}`, + composeStatus: `{ + "image_statuses": [ + { + "status": "failure" + }, + { + "status": "success" + } + ], + "koji_build_id": 42, + "koji_task_id": 0, + "status": "failure" + }`, + }, + { + initResult: worker.KojiInitJobResult{ + BuildID: 42, + Token: `"foobar"`, + }, + buildResult: worker.OSBuildKojiJobResult{ + Arch: "x86_64", + HostOS: "fedora-30", + ImageHash: "browns", + ImageSize: 42, + OSBuildOutput: &osbuild.Result{ + Success: true, + }, + }, + finalizeResult: worker.KojiFinalizeJobResult{ + KojiError: "failure", + }, + composeReplyCode: http.StatusCreated, + composeReply: `{"koji_build_id":42}`, + composeStatus: `{ + "image_statuses": [ + { + "status": "success" + }, + { + "status": "success" + } + ], + "koji_build_id": 42, + "koji_task_id": 0, + "status": "failure" + }`, + }, + } + for _, c := range cases { + var wg sync.WaitGroup + wg.Add(1) + + go func(t *testing.T, result worker.KojiInitJobResult) { + token, _, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), "x86_64", []string{"koji-init"}) + require.NoError(t, err) + require.Equal(t, "koji-init", jobType) + + var initJob worker.KojiInitJob + err = json.Unmarshal(rawJob, &initJob) + require.NoError(t, err) + require.Equal(t, "koji.example.com", initJob.Server) + require.Equal(t, "foo", initJob.Name) + require.Equal(t, "1", initJob.Version) + require.Equal(t, "2", initJob.Release) + + initJobResult, err := json.Marshal(&jobResult{Result: result}) + require.NoError(t, err) + test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(initJobResult), http.StatusOK, `{}`) + + wg.Done() + }(t, c.initResult) + + test.TestRoute(t, handler, false, "POST", "/api/composer-koji/v1/compose", ` + { + "name":"foo", + "version":"1", + "release":"2", + "distribution":"fedora-30", + "image_requests": [ + { + "architecture": "x86_64", + "image_type": "qcow2", + "repositories": [ + { + "baseurl": "https://repo.example.com/" + } + ] + }, + { + "architecture": "x86_64", + "image_type": "qcow2", + "repositories": [ + { + "baseurl": "https://repo.example.com/" + } + ] + } + ], + "koji": { + "server": "koji.example.com" + } + }`, c.composeReplyCode, c.composeReply, "id") + wg.Wait() + + token, _, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), "x86_64", []string{"osbuild-koji"}) require.NoError(t, err) - require.Equal(t, "koji.example.com", initJob.Server) - require.Equal(t, "foo", initJob.Name) - require.Equal(t, "1", initJob.Version) - require.Equal(t, "2", initJob.Release) + require.Equal(t, "osbuild-koji", jobType) + + var osbuildJob worker.OSBuildKojiJob + err = json.Unmarshal(rawJob, &osbuildJob) + require.NoError(t, err) + require.Equal(t, "koji.example.com", osbuildJob.KojiServer) + require.Equal(t, "test.img", osbuildJob.ImageName) + require.NotEmpty(t, osbuildJob.KojiDirectory) + + buildJobResult, err := json.Marshal(&jobResult{Result: c.buildResult}) + require.NoError(t, err) + test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(buildJobResult), http.StatusOK, `{}`) + + token, _, jobType, rawJob, _, err = workerServer.RequestJob(context.Background(), "x86_64", []string{"osbuild-koji"}) + require.NoError(t, err) + require.Equal(t, "osbuild-koji", jobType) + + err = json.Unmarshal(rawJob, &osbuildJob) + require.NoError(t, err) + require.Equal(t, "koji.example.com", osbuildJob.KojiServer) + require.Equal(t, "test.img", osbuildJob.ImageName) + require.NotEmpty(t, osbuildJob.KojiDirectory) test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), `{ - "result": { - "build_id": 42, - "token": "foobar" - } - }`, http.StatusOK, `{}`) - wg.Done() - }() - - test.TestRoute(t, handler, false, "POST", "/api/composer-koji/v1/compose", ` - { - "name":"foo", - "version":"1", - "release":"2", - "distribution":"fedora-30", - "image_requests": [ - { - "architecture": "x86_64", - "image_type": "qcow2", - "repositories": [ - { - "baseurl": "https://repo.example.com/" - } - ] - }, - { - "architecture": "x86_64", - "image_type": "qcow2", - "repositories": [ - { - "baseurl": "https://repo.example.com/" - } - ] + "result": { + "arch": "x86_64", + "host_os": "fedora-30", + "image_hash": "browns", + "image_size": 42, + "osbuild_output": { + "success": true + } } - ], - "koji": { - "server": "koji.example.com" - } - }`, http.StatusCreated, `{"koji_build_id":42}`, "id") - wg.Wait() + }`, http.StatusOK, `{}`) - token, _, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), "x86_64", []string{"osbuild-koji"}) - require.NoError(t, err) - require.Equal(t, "osbuild-koji", jobType) + token, finalizeID, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), "x86_64", []string{"koji-finalize"}) + require.NoError(t, err) + require.Equal(t, "koji-finalize", jobType) - var osbuildJob worker.OSBuildKojiJob - err = json.Unmarshal(rawJob, &osbuildJob) - require.NoError(t, err) - require.Equal(t, "koji.example.com", osbuildJob.KojiServer) - require.Equal(t, "test.img", osbuildJob.ImageName) - require.NotEmpty(t, osbuildJob.KojiDirectory) + var kojiFinalizeJob worker.KojiFinalizeJob + err = json.Unmarshal(rawJob, &kojiFinalizeJob) + require.NoError(t, err) + require.Equal(t, "koji.example.com", kojiFinalizeJob.Server) + require.Equal(t, "1", kojiFinalizeJob.Version) + require.Equal(t, "2", kojiFinalizeJob.Release) + require.ElementsMatch(t, []string{"foo-1-2.x86_64.img", "foo-1-2.x86_64.img"}, kojiFinalizeJob.KojiFilenames) + require.NotEmpty(t, kojiFinalizeJob.KojiDirectory) - test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), `{ - "result": { - "arch": "x86_64", - "host_os": "fedora-30", - "image_hash": "browns", - "image_size": 42, - "osbuild_output": { - "success": true - } - } - }`, http.StatusOK, `{}`) + finalizeResult, err := json.Marshal(&jobResult{Result: c.finalizeResult}) + require.NoError(t, err) + test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(finalizeResult), http.StatusOK, `{}`) - token, _, jobType, rawJob, _, err = workerServer.RequestJob(context.Background(), "x86_64", []string{"osbuild-koji"}) - require.NoError(t, err) - require.Equal(t, "osbuild-koji", jobType) - - err = json.Unmarshal(rawJob, &osbuildJob) - require.NoError(t, err) - require.Equal(t, "koji.example.com", osbuildJob.KojiServer) - require.Equal(t, "test.img", osbuildJob.ImageName) - require.NotEmpty(t, osbuildJob.KojiDirectory) - - test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), `{ - "result": { - "arch": "x86_64", - "host_os": "fedora-30", - "image_hash": "browns", - "image_size": 42, - "osbuild_output": { - "success": true - } - } - }`, http.StatusOK, `{}`) - - token, finalizeID, jobType, rawJob, _, err := workerServer.RequestJob(context.Background(), "x86_64", []string{"koji-finalize"}) - require.NoError(t, err) - require.Equal(t, "koji-finalize", jobType) - - var kojiFinalizeJob worker.KojiFinalizeJob - err = json.Unmarshal(rawJob, &kojiFinalizeJob) - require.NoError(t, err) - require.Equal(t, "koji.example.com", kojiFinalizeJob.Server) - require.Equal(t, "1", kojiFinalizeJob.Version) - require.Equal(t, "2", kojiFinalizeJob.Release) - require.ElementsMatch(t, []string{"foo-1-2.x86_64.img", "foo-1-2.x86_64.img"}, kojiFinalizeJob.KojiFilenames) - require.NotEmpty(t, kojiFinalizeJob.KojiDirectory) - - test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), `{ - "result": { - } - }`, http.StatusOK, `{}`) - - test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/composer-koji/v1/compose/%v", finalizeID), ``, http.StatusOK, `{ - "image_statuses": [ - { - "status": "success" - }, - { - "status": "success" - } - ], - "koji_build_id": 42, - "koji_task_id": 0, - "status": "success" - }`) + test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/composer-koji/v1/compose/%v", finalizeID), ``, http.StatusOK, c.composeStatus) + } } func TestRequest(t *testing.T) { diff --git a/internal/worker/json.go b/internal/worker/json.go index 8828f316b..708a26fcb 100644 --- a/internal/worker/json.go +++ b/internal/worker/json.go @@ -36,7 +36,7 @@ type KojiInitJob struct { type KojiInitJobResult struct { BuildID uint64 `json:"build_id"` Token string `json:"token"` - KojiError error `json:"koji_error"` + KojiError string `json:"koji_error"` } type OSBuildKojiJob struct { @@ -53,7 +53,7 @@ type OSBuildKojiJobResult struct { OSBuildOutput *osbuild.Result `json:"osbuild_output"` ImageHash string `json:"image_hash"` ImageSize uint64 `json:"image_size"` - KojiError error `json:"koji_error"` + KojiError string `json:"koji_error"` } type KojiFinalizeJob struct { @@ -68,7 +68,7 @@ type KojiFinalizeJob struct { } type KojiFinalizeJobResult struct { - KojiError error `json:"koji_error"` + KojiError string `json:"koji_error"` } //