Skip to content

Commit e94f5bf

Browse files
authored
Issue #1373: feat(sandbox): add multi-platform support through --platform CLI argument (#1629)
* fixup, add multi-platform support with GOARCH detection and `TARGETPLATFORM` env in gha_pr_changed_files.py
1 parent 82ce3f8 commit e94f5bf

File tree

6 files changed

+60
-12
lines changed

6 files changed

+60
-12
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ define build_image
7171

7272
$(info # Building $(IMAGE_NAME) image...)
7373

74-
$(ROOT_DIR)/scripts/sandbox.py --dockerfile '$(2)' -- \
74+
$(ROOT_DIR)/scripts/sandbox.py --dockerfile '$(2)' --platform '$(BUILD_ARCH)' -- \
7575
$(CONTAINER_ENGINE) build $(CONTAINER_BUILD_CACHE_ARGS) --platform=$(BUILD_ARCH) --label release=$(RELEASE) --tag $(IMAGE_NAME) --file '$(2)' $(BUILD_ARGS) {}\;
7676
endef
7777

ci/cached-builds/gha_pr_changed_files.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
import logging
44
import os
55
import pathlib
6+
import platform
67
import re
78
import shutil
89
import subprocess
910
import unittest
11+
from typing import Literal
1012

1113
PROJECT_ROOT = pathlib.Path(__file__).parent.parent.parent.resolve()
1214
MAKE = shutil.which("gmake") or shutil.which("make")
@@ -87,7 +89,13 @@ def should_build_target(changed_files: list[str], target_directory: str) -> str:
8789
dockerfiles = find_dockerfiles(target_directory)
8890
for dockerfile in dockerfiles:
8991
stdout = subprocess.check_output(
90-
[PROJECT_ROOT / "bin/buildinputs", target_directory + "/" + dockerfile], text=True, cwd=PROJECT_ROOT
92+
[PROJECT_ROOT / "bin/buildinputs", target_directory + "/" + dockerfile],
93+
env={
94+
"TARGETPLATFORM": f"linux/{get_go_arch()}",
95+
**os.environ,
96+
}, # TODO(jdanek): still not ideal for qemu-user
97+
text=True,
98+
cwd=PROJECT_ROOT,
9199
)
92100
logging.debug(f"{target_directory=} {dockerfile=} {stdout=}")
93101
if stdout == "\n":
@@ -116,6 +124,24 @@ def filter_out_unchanged(targets: list[str], changed_files: list[str]) -> list[s
116124
return changed
117125

118126

127+
def get_go_arch() -> Literal["amd64", "arm64", "ppc64le", "s390x"]:
128+
arch = os.environ.get("GOARCH")
129+
if arch is not None:
130+
return arch
131+
match platform.machine().lower():
132+
case "x86_64" | "amd64":
133+
arch = "amd64"
134+
case "aarch64" | "arm64":
135+
arch = "arm64"
136+
case "ppc64le":
137+
arch = "ppc64le"
138+
case "s390x":
139+
arch = "s390x"
140+
case _:
141+
raise Exception(f"Unknown machine architecture: {platform.machine()}")
142+
return arch
143+
144+
119145
class SelfTests(unittest.TestCase):
120146
def test_list_changed_files(self):
121147
"""This is PR #556 in opendatahub-io/notebooks"""

scripts/buildinputs/buildinputs.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,26 @@ package main
33
import (
44
"flag"
55
"fmt"
6+
"os"
7+
"strings"
68
)
79

810
func main() {
11+
targetPlatform := os.Getenv("TARGETPLATFORM")
12+
platformFields := strings.Split(targetPlatform, "/")
13+
if len(platformFields) != 2 {
14+
panic(fmt.Sprint(targetPlatform, "format is invalid, should be os/arch"))
15+
}
16+
targetOs := platformFields[0]
17+
targetArch := platformFields[1]
18+
19+
if targetOs != "linux" {
20+
panic(fmt.Sprint(targetOs, "not supported"))
21+
}
22+
923
flag.Parse()
1024
for _, dockerfile := range flag.Args() {
11-
deps := getDockerfileDeps(dockerfile)
25+
deps := getDockerfileDeps(dockerfile, targetArch)
1226
fmt.Println(deps)
1327
}
1428
}

scripts/buildinputs/buildinputs_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestParseAllDockerfiles(t *testing.T) {
3030

3131
for _, dockerfile := range dockerfiles {
3232
t.Run(dockerfile, func(t *testing.T) {
33-
result := getDockerfileDeps(dockerfile)
33+
result := getDockerfileDeps(dockerfile, "amd64")
3434
if result == "" {
3535
// no deps in the dockerfile
3636
return

scripts/buildinputs/dockerfile.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"log"
88
"os"
9-
"runtime"
109
"strings"
1110

1211
"github.com/containerd/platforms"
@@ -21,7 +20,7 @@ import (
2120
"github.com/pkg/errors"
2221
)
2322

24-
func getDockerfileDeps(dockerfile string) string {
23+
func getDockerfileDeps(dockerfile string, targetArch string) string {
2524
ctx := context.Background()
2625
data := noErr2(os.ReadFile(dockerfile))
2726

@@ -32,11 +31,11 @@ func getDockerfileDeps(dockerfile string) string {
3231
// random digest value
3332
digest: "sha256:a1c7d58d98df3f9a67eda799200655b923ebc7a41cad1d9bb52723ae1c81ad17",
3433
dir: "/",
35-
platform: "linux/" + runtime.GOARCH,
34+
platform: "linux/" + targetArch,
3635
},
3736
Config: dockerui.Config{
3837
BuildArgs: map[string]string{"BASE_IMAGE": "fake-image"},
39-
BuildPlatforms: []ocispecs.Platform{{OS: "linux", Architecture: runtime.GOARCH}},
38+
BuildPlatforms: []ocispecs.Platform{{OS: "linux", Architecture: targetArch}},
4039
},
4140
Warn: func(rulename, description, url, fmtmsg string, location []parser.Range) {
4241
log.Printf(rulename, description, url, fmtmsg, location)

scripts/sandbox.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22

33
import argparse
44
import logging
5+
import os
56
import pathlib
67
import shutil
78
import subprocess
89
import sys
910
import tempfile
1011
import json
11-
from typing import cast
12+
from typing import cast, Literal
1213

1314
ROOT_DIR = pathlib.Path(__file__).parent.parent
1415
MAKE = shutil.which("gmake") or shutil.which("make")
@@ -19,12 +20,16 @@
1920

2021
class Args(argparse.Namespace):
2122
dockerfile: pathlib.Path
23+
platform: Literal["linux/amd64", "linux/arm64", "linux/s390x", "linux/ppc64le"]
2224
remaining: list[str]
2325

2426

2527
def main() -> int:
2628
p = argparse.ArgumentParser(allow_abbrev=False)
2729
p.add_argument("--dockerfile", type=pathlib.Path, required=True)
30+
p.add_argument("--platform", type=str,
31+
choices=["linux/amd64", "linux/arm64", "linux/s390x", "linux/ppc64le"],
32+
required=True)
2833
p.add_argument('remaining', nargs=argparse.REMAINDER)
2934

3035
args = cast(Args, p.parse_args())
@@ -38,7 +43,7 @@ def main() -> int:
3843
print("must give a `{};` parameter that will be replaced with new build context")
3944
return 1
4045

41-
prereqs = buildinputs(dockerfile=args.dockerfile)
46+
prereqs = buildinputs(dockerfile=args.dockerfile, platform=args.platform)
4247

4348
with tempfile.TemporaryDirectory(delete=True) as tmpdir:
4449
setup_sandbox(prereqs, pathlib.Path(tmpdir))
@@ -52,11 +57,15 @@ def main() -> int:
5257
return 0
5358

5459

55-
def buildinputs(dockerfile: pathlib.Path | str) -> list[pathlib.Path]:
60+
def buildinputs(
61+
dockerfile: pathlib.Path | str,
62+
platform: Literal["linux/amd64", "linux/arm64", "linux/s390x", "linux/ppc64le"] = "linux/amd64"
63+
) -> list[pathlib.Path]:
5664
if not (ROOT_DIR / "bin/buildinputs").exists():
5765
subprocess.check_call([MAKE, "bin/buildinputs"], cwd=ROOT_DIR)
5866
stdout = subprocess.check_output([ROOT_DIR / "bin/buildinputs", str(dockerfile)],
59-
text=True, cwd=ROOT_DIR)
67+
text=True, cwd=ROOT_DIR,
68+
env={"TARGETPLATFORM": platform, **os.environ})
6069
prereqs = [pathlib.Path(file) for file in json.loads(stdout)] if stdout != "\n" else []
6170
print(f"{prereqs=}")
6271
return prereqs

0 commit comments

Comments
 (0)