From 43e87632fbaf4aca2bef6c2d42360cf265cc593d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Hozza?= Date: Fri, 19 Jan 2024 21:05:46 +0100 Subject: [PATCH] Drop `common.CurrentArch()` in favor of osbuild/images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop `common.CurrentArch()` implementation and use `arch.Current().String()` from the osbuild/images instead. Signed-off-by: Tomáš Hozza --- cmd/osbuild-image-tests/main_test.go | 7 +++--- cmd/osbuild-worker/jobimpl-osbuild.go | 6 ++--- cmd/osbuild-worker/main.go | 4 +-- internal/boot/aws.go | 4 +-- internal/boot/context-managers.go | 5 ++-- internal/common/helpers.go | 17 ------------- internal/common/helpers_test.go | 35 --------------------------- internal/test/helpers.go | 4 +-- internal/weldr/api.go | 3 ++- internal/worker/client.go | 3 ++- internal/worker/server_test.go | 5 ++-- 11 files changed, 20 insertions(+), 73 deletions(-) diff --git a/cmd/osbuild-image-tests/main_test.go b/cmd/osbuild-image-tests/main_test.go index 5b5dcbed8..90470b637 100644 --- a/cmd/osbuild-image-tests/main_test.go +++ b/cmd/osbuild-image-tests/main_test.go @@ -29,7 +29,6 @@ import ( "github.com/osbuild/osbuild-composer/internal/boot/azuretest" "github.com/osbuild/osbuild-composer/internal/boot/openstacktest" "github.com/osbuild/osbuild-composer/internal/boot/vmwaretest" - "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/test" ) @@ -353,7 +352,7 @@ func testBootUsingAWS(t *testing.T, imagePath string) { "aarch64": "t4g.micro", } - instanceType, exists := instanceTypeForArch[common.CurrentArch()] + instanceType, exists := instanceTypeForArch[arch.Current().String()] if !exists { panic("unsupported AWS arch") } @@ -407,7 +406,7 @@ func testBootUsingAzure(t *testing.T, imagePath string) { func testBootUsingOpenStack(t *testing.T, imagePath string) { creds, err := openstack.AuthOptionsFromEnv() - currentArch := common.CurrentArch() + currentArch := arch.Current().String() // skip on aarch64 because we don't have aarch64 openstack or kvm machines if currentArch == arch.ARCH_AARCH64.String() { @@ -632,7 +631,7 @@ func runTests(t *testing.T, cases []string) { err = json.NewDecoder(f).Decode(&testcase) require.NoErrorf(t, err, "%s: cannot decode test case", p) - currentArch := common.CurrentArch() + currentArch := arch.Current().String() if testcase.ComposeRequest.Arch != currentArch { t.Skipf("the required arch is %s, the current arch is %s", testcase.ComposeRequest.Arch, currentArch) } diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index 2255564c4..2a64d9dc6 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/cmd/osbuild-worker/jobimpl-osbuild.go @@ -15,6 +15,7 @@ import ( "path/filepath" "strings" + "github.com/osbuild/images/pkg/arch" "github.com/osbuild/images/pkg/container" "github.com/osbuild/images/pkg/distro" "github.com/osbuild/images/pkg/osbuild" @@ -27,7 +28,6 @@ import ( "github.com/osbuild/osbuild-composer/internal/cloud/awscloud" "github.com/osbuild/osbuild-composer/internal/cloud/gcp" - "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/target" "github.com/osbuild/osbuild-composer/internal/upload/azure" "github.com/osbuild/osbuild-composer/internal/upload/koji" @@ -349,7 +349,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { Success: false, }, UploadStatus: "failure", - Arch: common.CurrentArch(), + Arch: arch.Current().String(), } hostOS, err := distro.GetHostDistroName() @@ -627,7 +627,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { break } - ami, err := a.Register(jobTarget.ImageName, bucket, targetOptions.Key, targetOptions.ShareWithAccounts, common.CurrentArch(), targetOptions.BootMode) + ami, err := a.Register(jobTarget.ImageName, bucket, targetOptions.Key, targetOptions.ShareWithAccounts, arch.Current().String(), targetOptions.BootMode) if err != nil { targetResult.TargetError = clienterrors.WorkerClientError(clienterrors.ErrorImportingImage, err.Error(), nil) break diff --git a/cmd/osbuild-worker/main.go b/cmd/osbuild-worker/main.go index b4409e83f..2ce068cde 100644 --- a/cmd/osbuild-worker/main.go +++ b/cmd/osbuild-worker/main.go @@ -20,7 +20,7 @@ import ( "github.com/aws/aws-sdk-go/service/autoscaling" "github.com/sirupsen/logrus" - "github.com/osbuild/osbuild-composer/internal/common" + "github.com/osbuild/images/pkg/arch" "github.com/osbuild/osbuild-composer/internal/dnfjson" "github.com/osbuild/osbuild-composer/internal/upload/azure" "github.com/osbuild/osbuild-composer/internal/upload/koji" @@ -161,7 +161,7 @@ func setProtection(protected bool) { // Returning an error here will result in the worker backing off for a while and retrying func RequestAndRunJob(client *worker.Client, acceptedJobTypes []string, jobImpls map[string]JobImplementation) error { logrus.Debug("Waiting for a new job...") - job, err := client.RequestJob(acceptedJobTypes, common.CurrentArch()) + job, err := client.RequestJob(acceptedJobTypes, arch.Current().String()) if err == worker.ErrClientRequestJobTimeout { logrus.Debugf("Requesting job timed out: %v", err) return nil diff --git a/internal/boot/aws.go b/internal/boot/aws.go index 037103f7b..4b155510e 100644 --- a/internal/boot/aws.go +++ b/internal/boot/aws.go @@ -13,8 +13,8 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/osbuild/images/pkg/arch" "github.com/osbuild/osbuild-composer/internal/cloud/awscloud" - "github.com/osbuild/osbuild-composer/internal/common" ) type awsCredentials struct { @@ -99,7 +99,7 @@ func UploadImageToAWS(c *awsCredentials, imagePath string, imageName string) err if err != nil { return fmt.Errorf("cannot upload the image: %v", err) } - _, err = uploader.Register(imageName, c.Bucket, imageName, nil, common.CurrentArch(), nil) + _, err = uploader.Register(imageName, c.Bucket, imageName, nil, arch.Current().String(), nil) if err != nil { return fmt.Errorf("cannot register the image: %v", err) } diff --git a/internal/boot/context-managers.go b/internal/boot/context-managers.go index 298a25ec2..30d2f452b 100644 --- a/internal/boot/context-managers.go +++ b/internal/boot/context-managers.go @@ -16,7 +16,6 @@ import ( "github.com/osbuild/images/pkg/arch" "github.com/osbuild/images/pkg/distro" "github.com/osbuild/osbuild-composer/cmd/osbuild-image-tests/constants" - "github.com/osbuild/osbuild-composer/internal/common" ) // WithNetworkNamespace provides the function f with a new network namespace @@ -114,7 +113,7 @@ func WithBootedQemuImage(image string, ns NetNS, f func() error) error { } var qemuCmd *exec.Cmd - if common.CurrentArch() == "x86_64" { + if arch.Current() == arch.ARCH_X86_64 { hostDistroName, err := distro.GetHostDistroName() if err != nil { return fmt.Errorf("cannot determing the current distro: %v", err) @@ -139,7 +138,7 @@ func WithBootedQemuImage(image string, ns NetNS, f func() error) error { "-nographic", image, ) - } else if common.CurrentArch() == arch.ARCH_AARCH64.String() { + } else if arch.Current() == arch.ARCH_AARCH64 { // This command does not use KVM as I was unable to make it work in Beaker, // once we have machines that can use KVM, enable it to make it faster qemuCmd = ns.NamespacedCommand( diff --git a/internal/common/helpers.go b/internal/common/helpers.go index 729e05189..865b88c36 100644 --- a/internal/common/helpers.go +++ b/internal/common/helpers.go @@ -4,28 +4,11 @@ import ( "fmt" "io" "regexp" - "runtime" "sort" "strconv" "strings" ) -var RuntimeGOARCH = runtime.GOARCH - -func CurrentArch() string { - if RuntimeGOARCH == "amd64" { - return "x86_64" - } else if RuntimeGOARCH == "arm64" { - return "aarch64" - } else if RuntimeGOARCH == "ppc64le" { - return "ppc64le" - } else if RuntimeGOARCH == "s390x" { - return "s390x" - } else { - panic("unsupported architecture") - } -} - func PanicOnError(err error) { if err != nil { panic(err) diff --git a/internal/common/helpers_test.go b/internal/common/helpers_test.go index e8ecaf67e..d703790d7 100644 --- a/internal/common/helpers_test.go +++ b/internal/common/helpers_test.go @@ -8,41 +8,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestCurrentArchAMD64(t *testing.T) { - origRuntimeGOARCH := RuntimeGOARCH - defer func() { RuntimeGOARCH = origRuntimeGOARCH }() - RuntimeGOARCH = "amd64" - assert.Equal(t, "x86_64", CurrentArch()) -} - -func TestCurrentArchARM64(t *testing.T) { - origRuntimeGOARCH := RuntimeGOARCH - defer func() { RuntimeGOARCH = origRuntimeGOARCH }() - RuntimeGOARCH = "arm64" - assert.Equal(t, "aarch64", CurrentArch()) -} - -func TestCurrentArchPPC64LE(t *testing.T) { - origRuntimeGOARCH := RuntimeGOARCH - defer func() { RuntimeGOARCH = origRuntimeGOARCH }() - RuntimeGOARCH = "ppc64le" - assert.Equal(t, "ppc64le", CurrentArch()) -} - -func TestCurrentArchS390X(t *testing.T) { - origRuntimeGOARCH := RuntimeGOARCH - defer func() { RuntimeGOARCH = origRuntimeGOARCH }() - RuntimeGOARCH = "s390x" - assert.Equal(t, "s390x", CurrentArch()) -} - -func TestCurrentArchUnsupported(t *testing.T) { - origRuntimeGOARCH := RuntimeGOARCH - defer func() { RuntimeGOARCH = origRuntimeGOARCH }() - RuntimeGOARCH = "UKNOWN" - assert.PanicsWithValue(t, "unsupported architecture", func() { CurrentArch() }) -} - func TestPanicOnError(t *testing.T) { err := errors.New("Error message") assert.PanicsWithValue(t, err, func() { PanicOnError(err) }) diff --git a/internal/test/helpers.go b/internal/test/helpers.go index 1b845c586..fa6adf73a 100644 --- a/internal/test/helpers.go +++ b/internal/test/helpers.go @@ -18,8 +18,8 @@ import ( "github.com/BurntSushi/toml" "github.com/google/go-cmp/cmp" "github.com/google/uuid" + "github.com/osbuild/images/pkg/arch" "github.com/osbuild/images/pkg/distro" - "github.com/osbuild/osbuild-composer/internal/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -245,7 +245,7 @@ func GenerateCIArtifactName(prefix string) (string, error) { return "", fmt.Errorf("The environment variables must specify BRANCH_NAME, BUILD_ID, and DISTRO_CODE") } - arch := common.CurrentArch() + arch := arch.Current().String() return fmt.Sprintf("%s%s-%s-%s-%s", prefix, distroCode, arch, branchName, buildId), nil } diff --git a/internal/weldr/api.go b/internal/weldr/api.go index 3093ff22d..3e5fbcd12 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -31,6 +31,7 @@ import ( "github.com/julienschmidt/httprouter" "github.com/osbuild/osbuild-composer/pkg/jobqueue" + "github.com/osbuild/images/pkg/arch" "github.com/osbuild/images/pkg/container" "github.com/osbuild/images/pkg/distro" "github.com/osbuild/images/pkg/distrofactory" @@ -165,7 +166,7 @@ func New(repoPaths []string, stateDir string, solver *dnfjson.BaseSolver, df *di if err != nil { return nil, fmt.Errorf("failed to read host distro information") } - hostArch := common.CurrentArch() + hostArch := arch.Current().String() rr, err := reporegistry.New(repoPaths) if err != nil { diff --git a/internal/worker/client.go b/internal/worker/client.go index 32bc93224..871769cc9 100644 --- a/internal/worker/client.go +++ b/internal/worker/client.go @@ -17,6 +17,7 @@ import ( "github.com/google/uuid" "github.com/sirupsen/logrus" + "github.com/osbuild/images/pkg/arch" "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/worker/api" ) @@ -172,7 +173,7 @@ func (c *Client) registerWorker() error { var buf bytes.Buffer err = json.NewEncoder(&buf).Encode(api.PostWorkersRequest{ - Arch: common.CurrentArch(), + Arch: arch.Current().String(), }) if err != nil { logrus.Errorf("Unable create worker request: %v", err) diff --git a/internal/worker/server_test.go b/internal/worker/server_test.go index d45aa3457..1ec1f92ee 100644 --- a/internal/worker/server_test.go +++ b/internal/worker/server_test.go @@ -22,7 +22,6 @@ import ( "github.com/osbuild/images/pkg/manifest" "github.com/osbuild/images/pkg/osbuild" "github.com/osbuild/images/pkg/rpmmd" - "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/jobqueue/fsjobqueue" "github.com/osbuild/osbuild-composer/internal/target" "github.com/osbuild/osbuild-composer/internal/test" @@ -1626,7 +1625,7 @@ func TestWorkerWatch(t *testing.T) { server := newTestServer(t, t.TempDir(), config, false) - reply := test.TestRouteWithReply(t, server.Handler(), false, "POST", "/api/worker/v1/workers", fmt.Sprintf(`{"arch":"%s"}`, common.CurrentArch()), 201, `{"href":"/api/worker/v1/workers","kind":"WorkerID","id": "15"}`, "id", "worker_id") + reply := test.TestRouteWithReply(t, server.Handler(), false, "POST", "/api/worker/v1/workers", fmt.Sprintf(`{"arch":"%s"}`, arch.Current().String()), 201, `{"href":"/api/worker/v1/workers","kind":"WorkerID","id": "15"}`, "id", "worker_id") var resp api.PostWorkersResponse require.NoError(t, json.Unmarshal(reply, &resp)) workerID, err := uuid.Parse(resp.WorkerId) @@ -1641,7 +1640,7 @@ func TestWorkerWatch(t *testing.T) { func TestRequestJobForWorker(t *testing.T) { server := newTestServer(t, t.TempDir(), defaultConfig, false) - reply := test.TestRouteWithReply(t, server.Handler(), false, "POST", "/api/worker/v1/workers", fmt.Sprintf(`{"arch":"%s"}`, common.CurrentArch()), 201, `{"href":"/api/worker/v1/workers","kind":"WorkerID","id": "15"}`, "id", "worker_id") + reply := test.TestRouteWithReply(t, server.Handler(), false, "POST", "/api/worker/v1/workers", fmt.Sprintf(`{"arch":"%s"}`, arch.Current().String()), 201, `{"href":"/api/worker/v1/workers","kind":"WorkerID","id": "15"}`, "id", "worker_id") var resp api.PostWorkersResponse require.NoError(t, json.Unmarshal(reply, &resp)) workerID, err := uuid.Parse(resp.WorkerId)