-
Notifications
You must be signed in to change notification settings - Fork 33
perf(ci): parallelize e2e operator build prerequisites #993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # CI-only runtime image for host-compiled Go binaries. | ||
| # | ||
| # BIN selects which binary becomes the default entrypoint. | ||
| # COPY ${BIN} /entrypoint works because COPY supports ARG expansion, | ||
| # while ENTRYPOINT does not (and ubi-micro has no shell for shell-form). | ||
| FROM registry.access.redhat.com/ubi9/ubi-micro:9.8-1784702951@sha256:b1e86b97028b8fcfb6d85f997c39e6b6b67496163ef8d80d243220a4918e8bef | ||
| ARG BIN | ||
| WORKDIR / | ||
| COPY . . | ||
| # ENTRYPOINT can't expand ARG; fixed path lets one file serve all three images | ||
| COPY ${BIN} /entrypoint | ||
| USER 65532:65532 | ||
| ENTRYPOINT ["/entrypoint"] | ||
|
Comment on lines
+11
to
+13
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 3 '(/manager|/entrypoint)' controllerRepository: jumpstarter-dev/jumpstarter Length of output: 7688 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Containerfiles ---'
cat -n controller/Containerfile.prebuilt
cat -n controller/Containerfile.operator
printf '%s\n' '--- Operator manager manifest ---'
cat -n controller/deploy/operator/config/manager/manager.yaml
printf '%s\n' '--- Relevant build targets ---'
sed -n '135,152p' controller/Makefile
sed -n '176,187p' controller/deploy/operator/Makefile
printf '%s\n' '--- All operator command/entrypoint/probe references ---'
rg -n -C 4 'command:|/manager|/entrypoint|livenessProbe|readinessProbe|startupProbe' controller/deploy/operator controller/Containerfile.prebuilt controller/Containerfile.operatorRepository: jumpstarter-dev/jumpstarter Length of output: 18058 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
prebuilt = Path("controller/Containerfile.prebuilt").read_text()
operator = Path("controller/Containerfile.operator").read_text()
manager = Path("controller/deploy/operator/config/manager/manager.yaml").read_text()
copy_all = bool(re.search(r"^COPY\s+\.\s+\.$", prebuilt, re.M))
copy_bin = re.search(r"^COPY\s+\$\{BIN\}\s+(\S+)$", prebuilt, re.M)
prebuilt_entrypoint = re.search(r"^ENTRYPOINT\s+(.+)$", prebuilt, re.M)
operator_entrypoint = re.search(r"^ENTRYPOINT\s+(.+)$", operator, re.M)
manager_command = re.search(r"^\s*-\s+(/(?:manager|entrypoint))\s*$", manager, re.M)
print({
"prebuilt_COPY_dot_dot": copy_all,
"prebuilt_binary_destination": copy_bin.group(1) if copy_bin else None,
"prebuilt_entrypoint": prebuilt_entrypoint.group(1) if prebuilt_entrypoint else None,
"operator_entrypoint": operator_entrypoint.group(1) if operator_entrypoint else None,
"generated_manager_command": manager_command.group(1) if manager_command else None,
"prebuilt_contains_manager_via_context_copy": copy_all,
})
PYRepository: jumpstarter-dev/jumpstarter Length of output: 417 Remove the duplicate binary copy.
🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,8 @@ else | |
| GOBIN=$(shell go env GOBIN) | ||
| endif | ||
|
|
||
| GOARCH ?= $(shell go env GOARCH) | ||
|
|
||
|
Comment on lines
+74
to
+75
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- Makefile context ---'
sed -n '1,230p' controller/deploy/operator/Makefile
printf '%s\n' '--- Containerfile.prebuilt locations and contents ---'
fd -i 'Containerfile.prebuilt' .
for f in $(fd -i 'Containerfile.prebuilt' .); do
echo "### $f"
cat -n "$f"
done
printf '%s\n' '--- architecture-related references ---'
rg -n --glob 'Makefile' --glob 'Containerfile*' --glob '*.yaml' --glob '*.yml' \
'GOARCH|GOARM|TARGET_PLATFORM|platform=|Containerfile\.prebuilt|bin/ci-stage|entrypoint' .Repository: jumpstarter-dev/jumpstarter Length of output: 15697 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
makefile = Path("controller/deploy/operator/Makefile").read_text()
containerfile = Path("controller/Containerfile.prebuilt").read_text()
target = re.search(
r"docker-build-ci:.*?\n(?P<body>(?:\t.*\n)+)",
makefile,
)
assert target, "docker-build-ci target not found"
body = target.group("body")
assert "GOARCH=$(GOARCH)" in body, "docker-build-ci does not compile with GOARCH"
build = next(
line for line in body.splitlines()
if "$(CONTAINER_TOOL) build" in line
)
assert "--platform" not in build, "build command already specifies --platform"
assert re.search(r"^FROM\s+(?!--platform=)", containerfile, re.M), \
"Containerfile.prebuilt unexpectedly sets a FROM platform"
assert "COPY ${BIN} /entrypoint" in containerfile, \
"Containerfile.prebuilt does not copy the prebuilt binary"
assert "ENTRYPOINT [\"/entrypoint\"]" in containerfile, \
"Containerfile.prebuilt does not execute the copied binary"
print("docker-build-ci compiles with configurable GOARCH")
print("its container build has no --platform option")
print("Containerfile.prebuilt has no FROM --platform override")
print("the copied binary is the image entrypoint")
PY
printf '%s\n' '--- docker-build-ci invocations and GOARCH overrides ---'
rg -n -C 3 'docker-build-ci|GOARCH=|make .*docker-build-ci' \
.github controller --glob '*.yaml' --glob '*.yml' --glob 'Makefile' \
--glob '*.mk' || trueRepository: jumpstarter-dev/jumpstarter Length of output: 3823 Set the image platform from When Proposed fix GOARCH ?= $(shell go env GOARCH)
+TARGET_PLATFORM ?= linux/$(GOARCH)
...
- $(CONTAINER_TOOL) build --build-arg BIN=manager -t $(IMG) -f ../../Containerfile.prebuilt bin/ci-stage
+ $(CONTAINER_TOOL) build --platform=$(TARGET_PLATFORM) --build-arg BIN=manager -t $(IMG) -f ../../Containerfile.prebuilt bin/ci-stageMap variants such as 🤖 Prompt for AI Agents |
||
| # CONTAINER_TOOL defines the container tool to be used for building images. | ||
| # Be aware that the target commands are only tested with Docker which is | ||
| # scaffolded by default. However, you might want to replace it to use other | ||
|
|
@@ -177,6 +179,12 @@ docker-build: ## Build docker image with the manager. | |
| --build-arg BUILD_DATE=$(BUILD_DATE) \ | ||
| -t ${IMG} ../../ -f ../../Containerfile.operator | ||
|
|
||
| .PHONY: docker-build-ci | ||
| docker-build-ci: ## CI-optimized: host-compiled binary, no multi-stage build. | ||
| rm -rf bin/ci-stage && mkdir -p bin/ci-stage | ||
| CGO_ENABLED=0 GOOS=linux GOARCH=$(GOARCH) go build -ldflags "$(LDFLAGS)" -o bin/ci-stage/manager cmd/main.go | ||
| $(CONTAINER_TOOL) build --build-arg BIN=manager -t $(IMG) -f ../../Containerfile.prebuilt bin/ci-stage | ||
|
|
||
| .PHONY: docker-push | ||
| docker-push: ## Push docker image with the manager. | ||
| $(CONTAINER_TOOL) push ${IMG} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
COPY . .copies everything from the build context, including the binaries, since${BIN}is copied again as/entrypoint, we end up shipping duplicatemanager/routerbinaries that aren't used.So it probably can be simplified to just
COPY ${BIN} /entrypointThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coderabbit commented on this too,
the controller image has manager and router, and we have a third exporter-set-controller
so in order to get router too we copy it in COPY . .
at the expense of an extra 40-50 throwaway MBs
I'll add a comment to clarify