From e550d31bd172335e9f927df1844684dbdb232f41 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 13 Jun 2024 15:42:53 +0200 Subject: [PATCH 1/3] osbuild-worker-executor: `job-id` in control.json as hostname This commit adds support to set the hostname to the job-id that is part of the control.json. --- cmd/osbuild-worker-executor/export_test.go | 8 ++++++ cmd/osbuild-worker-executor/handler_build.go | 16 +++++++++++ .../handler_build_test.go | 27 +++++++++++++++++++ cmd/osbuild-worker-executor/main_test.go | 5 ++++ 4 files changed, 56 insertions(+) diff --git a/cmd/osbuild-worker-executor/export_test.go b/cmd/osbuild-worker-executor/export_test.go index 9698623c89..b479766336 100644 --- a/cmd/osbuild-worker-executor/export_test.go +++ b/cmd/osbuild-worker-executor/export_test.go @@ -12,6 +12,14 @@ var ( HandleIncludedSources = handleIncludedSources ) +func MockUnixSethostname(new func([]byte) error) (restore func()) { + saved := unixSethostname + unixSethostname = new + return func() { + unixSethostname = saved + } +} + func MockOsbuildBinary(t *testing.T, new string) (restore func()) { t.Helper() diff --git a/cmd/osbuild-worker-executor/handler_build.go b/cmd/osbuild-worker-executor/handler_build.go index eed9b740d6..13edd9ab75 100644 --- a/cmd/osbuild-worker-executor/handler_build.go +++ b/cmd/osbuild-worker-executor/handler_build.go @@ -13,6 +13,7 @@ import ( "strings" "golang.org/x/exp/slices" + "golang.org/x/sys/unix" "github.com/sirupsen/logrus" ) @@ -108,6 +109,7 @@ func runOsbuild(logger *logrus.Logger, buildDir string, control *controlJSON, ou type controlJSON struct { Environments []string `json:"environments"` Exports []string `json:"exports"` + JobID string `json:"job-id"` } func mustRead(atar *tar.Reader, name string) error { @@ -235,6 +237,15 @@ func handleIncludedSources(atar *tar.Reader, buildDir string) error { } } +var unixSethostname = unix.Sethostname + +func setHostname(name string) error { + if name == "" { + return nil + } + return unixSethostname([]byte(name)) +} + // test for real via: // curl -o - --data-binary "@./test.tar" -H "Content-Type: application/x-tar" -X POST http://localhost:8001/api/v1/build func handleBuild(logger *logrus.Logger, config *Config) http.Handler { @@ -262,6 +273,11 @@ func handleBuild(logger *logrus.Logger, config *Config) http.Handler { http.Error(w, "cannot decode control.json", http.StatusBadRequest) return } + if err := setHostname(control.JobID); err != nil { + logger.Error(err) + http.Error(w, "cannot set hostname", http.StatusBadRequest) + return + } buildDir, err := createBuildDir(config) if err != nil { diff --git a/cmd/osbuild-worker-executor/handler_build_test.go b/cmd/osbuild-worker-executor/handler_build_test.go index 57decab580..d70f1c19ff 100644 --- a/cmd/osbuild-worker-executor/handler_build_test.go +++ b/cmd/osbuild-worker-executor/handler_build_test.go @@ -324,3 +324,30 @@ func TestBuildErrorHandlingTar(t *testing.T) { assert.Contains(t, string(body), "cannot tar output directory:") assert.Contains(t, loggerHook.LastEntry().Message, "cannot tar output directory:") } + +func TestBuildSethostname(t *testing.T) { + baseURL, baseBuildDir, _ := runTestServer(t) + endpoint := baseURL + "api/v1/build" + + restore := main.MockOsbuildBinary(t, fmt.Sprintf(`#!/bin/sh -e +# simulate output +mkdir -p %[1]s/build/output/image +echo "fake-build-result" > %[1]s/build/output/image/disk.img +`, baseBuildDir)) + t.Cleanup(restore) + + var hostnameCalls []string + restore = main.MockUnixSethostname(func(hn []byte) error { + hostnameCalls = append(hostnameCalls, string(hn)) + return nil + }) + t.Cleanup(restore) + + buf := makeTestPost(t, `{"exports": ["tree"], "job-id": "1234-56"}`, `{"fake": "manifest"}`) + rsp, err := http.Post(endpoint, "application/x-tar", buf) + assert.NoError(t, err) + defer func() { _, _ = io.ReadAll(rsp.Body) }() + defer rsp.Body.Close() + + assert.Equal(t, []string{"1234-56"}, hostnameCalls) +} diff --git a/cmd/osbuild-worker-executor/main_test.go b/cmd/osbuild-worker-executor/main_test.go index 97aca273f6..f989d24f82 100644 --- a/cmd/osbuild-worker-executor/main_test.go +++ b/cmd/osbuild-worker-executor/main_test.go @@ -55,6 +55,11 @@ func runTestServer(t *testing.T) (baseURL, buildBaseDir string, loggerHook *logr buildBaseDir = t.TempDir() baseURL = fmt.Sprintf("http://%s:%s/", host, port) + restore := main.MockUnixSethostname(func([]byte) error { + return nil + }) + t.Cleanup(restore) + ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) From 446a628246ace6cf121ed9da815a15125fa41033 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 13 Jun 2024 16:56:12 +0200 Subject: [PATCH 2/3] osbuildexecutor: tweak `RunOSBuild()` signature and use opts Introduce a new OsbuildOpts struct to make the API a bit easier to extend and use in the packages. Also add a new `JobID` field in the `OsbuildOpts`. --- cmd/osbuild-worker/jobimpl-osbuild.go | 11 ++++++++++- internal/osbuildexecutor/osbuild-executor.go | 15 ++++++++++++++- .../osbuildexecutor/runner-impl-aws-ec2.go | 19 +++++++++++++------ internal/osbuildexecutor/runner-impl-host.go | 9 ++++++--- 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index e066eb0cb1..98945de0b0 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/cmd/osbuild-worker/jobimpl-osbuild.go @@ -526,7 +526,16 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { exportPaths = append(exportPaths, path.Join(jobTarget.OsbuildArtifact.ExportName, jobTarget.OsbuildArtifact.ExportFilename)) } - osbuildJobResult.OSBuildOutput, err = executor.RunOSBuild(jobArgs.Manifest, impl.Store, outputDirectory, exports, exportPaths, nil, extraEnv, true, os.Stderr) + opts := &osbuildexecutor.OsbuildOpts{ + StoreDir: impl.Store, + OutputDir: outputDirectory, + Exports: exports, + ExportPaths: exportPaths, + ExtraEnv: extraEnv, + Result: true, + JobID: job.Id().String(), + } + osbuildJobResult.OSBuildOutput, err = executor.RunOSBuild(jobArgs.Manifest, opts, os.Stderr) // First handle the case when "running" osbuild failed if err != nil { osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, "osbuild build failed", err.Error()) diff --git a/internal/osbuildexecutor/osbuild-executor.go b/internal/osbuildexecutor/osbuild-executor.go index 136b514259..9748d1e425 100644 --- a/internal/osbuildexecutor/osbuild-executor.go +++ b/internal/osbuildexecutor/osbuild-executor.go @@ -6,6 +6,19 @@ import ( "github.com/osbuild/images/pkg/osbuild" ) +type OsbuildOpts struct { + StoreDir string + OutputDir string + Exports []string + ExportPaths []string + Checkpoints []string + ExtraEnv []string + Result bool + + // not strict a osbuild opt + JobID string +} + type Executor interface { - RunOSBuild(manifest []byte, store, outputDirectory string, exports, exportPaths, checkpoints, extraEnv []string, result bool, errorWriter io.Writer) (*osbuild.Result, error) + RunOSBuild(manifest []byte, opts *OsbuildOpts, errorWriter io.Writer) (*osbuild.Result, error) } diff --git a/internal/osbuildexecutor/runner-impl-aws-ec2.go b/internal/osbuildexecutor/runner-impl-aws-ec2.go index a68d8ea545..be23de914a 100644 --- a/internal/osbuildexecutor/runner-impl-aws-ec2.go +++ b/internal/osbuildexecutor/runner-impl-aws-ec2.go @@ -30,7 +30,12 @@ type awsEC2Executor struct { func prepareSources(manifest []byte, store string, extraEnv []string, result bool, errorWriter io.Writer) error { hostExecutor := NewHostExecutor() - _, err := hostExecutor.RunOSBuild(manifest, store, "", nil, nil, nil, extraEnv, result, errorWriter) + opts := &OsbuildOpts{ + StoreDir: store, + ExtraEnv: extraEnv, + Result: result, + } + _, err := hostExecutor.RunOSBuild(manifest, opts, errorWriter) return err } @@ -243,10 +248,12 @@ func extractOutputArchive(outputDirectory, outputTar string) error { } -func (ec2e *awsEC2Executor) RunOSBuild(manifest []byte, store, outputDirectory string, exports, exportPaths, checkpoints, - extraEnv []string, result bool, errorWriter io.Writer) (*osbuild.Result, error) { +func (ec2e *awsEC2Executor) RunOSBuild(manifest []byte, opts *OsbuildOpts, errorWriter io.Writer) (*osbuild.Result, error) { + if opts == nil { + opts = &OsbuildOpts{} + } - err := prepareSources(manifest, store, extraEnv, result, errorWriter) + err := prepareSources(manifest, opts.StoreDir, opts.ExtraEnv, opts.Result, errorWriter) if err != nil { return nil, fmt.Errorf("Failed to prepare sources: %w", err) } @@ -280,7 +287,7 @@ func (ec2e *awsEC2Executor) RunOSBuild(manifest []byte, store, outputDirectory s return nil, fmt.Errorf("Timeout waiting for executor to come online") } - inputArchive, err := writeInputArchive(ec2e.tmpDir, store, exports, manifest) + inputArchive, err := writeInputArchive(ec2e.tmpDir, opts.StoreDir, opts.Exports, manifest) if err != nil { logrus.Errorf("Unable to write input archive: %v", err) return nil, err @@ -301,7 +308,7 @@ func (ec2e *awsEC2Executor) RunOSBuild(manifest []byte, store, outputDirectory s return nil, err } - err = extractOutputArchive(outputDirectory, outputArchive) + err = extractOutputArchive(opts.OutputDir, outputArchive) if err != nil { logrus.Errorf("Unable to extract executor output: %v", err) return nil, err diff --git a/internal/osbuildexecutor/runner-impl-host.go b/internal/osbuildexecutor/runner-impl-host.go index 8b6d839a86..5fa26cbfb6 100644 --- a/internal/osbuildexecutor/runner-impl-host.go +++ b/internal/osbuildexecutor/runner-impl-host.go @@ -8,9 +8,12 @@ import ( type hostExecutor struct{} -func (he *hostExecutor) RunOSBuild(manifest []byte, store, outputDirectory string, exports, exportPaths, checkpoints, - extraEnv []string, result bool, errorWriter io.Writer) (*osbuild.Result, error) { - return osbuild.RunOSBuild(manifest, store, outputDirectory, exports, checkpoints, extraEnv, result, errorWriter) +func (he *hostExecutor) RunOSBuild(manifest []byte, opts *OsbuildOpts, errorWriter io.Writer) (*osbuild.Result, error) { + if opts == nil { + opts = &OsbuildOpts{} + } + + return osbuild.RunOSBuild(manifest, opts.StoreDir, opts.OutputDir, opts.Exports, opts.Checkpoints, opts.ExtraEnv, opts.Result, errorWriter) } func NewHostExecutor() Executor { From 63567c339cd7d3950e78f462163a5a5cde5e4d58 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 13 Jun 2024 17:12:36 +0200 Subject: [PATCH 3/3] osbuildexecutor: write the job-id intro control.json Set the jobID into control.json so that the osbuild-worker-executor can set the hostname based on the jobID. --- internal/osbuildexecutor/runner-impl-aws-ec2.go | 6 ++++-- internal/osbuildexecutor/runner-impl-aws-ec2_test.go | 6 +++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/osbuildexecutor/runner-impl-aws-ec2.go b/internal/osbuildexecutor/runner-impl-aws-ec2.go index be23de914a..d49beb7a9a 100644 --- a/internal/osbuildexecutor/runner-impl-aws-ec2.go +++ b/internal/osbuildexecutor/runner-impl-aws-ec2.go @@ -73,15 +73,17 @@ func waitForSI(ctx context.Context, host string) bool { } } -func writeInputArchive(cacheDir, store string, exports []string, manifestData []byte) (string, error) { +func writeInputArchive(cacheDir, store, jobID string, exports []string, manifestData []byte) (string, error) { archive := filepath.Join(cacheDir, "input.tar") control := filepath.Join(cacheDir, "control.json") manifest := filepath.Join(cacheDir, "manifest.json") controlData := struct { Exports []string `json:"exports"` + JobID string `json:"job-id"` }{ Exports: exports, + JobID: jobID, } controlDataBytes, err := json.Marshal(controlData) if err != nil { @@ -287,7 +289,7 @@ func (ec2e *awsEC2Executor) RunOSBuild(manifest []byte, opts *OsbuildOpts, error return nil, fmt.Errorf("Timeout waiting for executor to come online") } - inputArchive, err := writeInputArchive(ec2e.tmpDir, opts.StoreDir, opts.Exports, manifest) + inputArchive, err := writeInputArchive(ec2e.tmpDir, opts.StoreDir, opts.JobID, opts.Exports, manifest) if err != nil { logrus.Errorf("Unable to write input archive: %v", err) return nil, err diff --git a/internal/osbuildexecutor/runner-impl-aws-ec2_test.go b/internal/osbuildexecutor/runner-impl-aws-ec2_test.go index ed36d6b893..4261f2160f 100644 --- a/internal/osbuildexecutor/runner-impl-aws-ec2_test.go +++ b/internal/osbuildexecutor/runner-impl-aws-ec2_test.go @@ -46,7 +46,7 @@ func TestWriteInputArchive(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(storeDir, "contents"), []byte("storedata"), 0600)) require.NoError(t, os.WriteFile(filepath.Join(storeSubDir, "contents"), []byte("storedata"), 0600)) - archive, err := osbuildexecutor.WriteInputArchive(cacheDir, storeDir, []string{"image"}, []byte("{\"version\": 2}")) + archive, err := osbuildexecutor.WriteInputArchive(cacheDir, storeDir, "some-job-id", []string{"image"}, []byte("{\"version\": 2}")) require.NoError(t, err) cmd := exec.Command("tar", @@ -64,6 +64,10 @@ func TestWriteInputArchive(t *testing.T) { "store/contents", "", }, strings.Split(string(out), "\n")) + + output, err := exec.Command("tar", "xOf", archive, "control.json").CombinedOutput() + require.NoError(t, err) + require.Equal(t, `{"exports":["image"],"job-id":"some-job-id"}`, string(output)) } func TestHandleBuild(t *testing.T) {