installer: build the builder image on Linux and on release installs - #393
installer: build the builder image on Linux and on release installs#393mpuig wants to merge 8 commits into
Conversation
Source-to-image builds boot builder VMs from hypeman/builder:latest, and the API's fallback for installed (non-source) services is to find that image in the local Docker daemon -- lib/builds/manager.go's own comment says "the installer builds this image before loading the service". The installer only did so in its darwin branch, and only for source (BRANCH) installs. A Linux release install therefore had no builder image and no way to grow one: builder preparation retried forever, and every `hypeman build` failed. The builder-image step now runs on every platform where Docker is present, and release installs fetch the source tarball for the exact installed version to build from -- the builder Dockerfile is part of the same tree the binaries were released from. Skipped when the image already exists, warn-and-continue (matching the existing style) when it cannot be built.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Bugbot's review finding, and it is right: the script's documented Linux invocation elevates privileged operations through $SUDO, but the new builder-image step ran docker as the invoking user -- who may not be in the docker group -- so the step would warn-and-skip and leave exactly the gap this PR fixes. Docker access is now probed and escalated through the same $SUDO the rest of the script uses. Also stop discarding the docker build output: it goes to a log file whose path the failure warning names, instead of 2>/dev/null-ing the only evidence of why a build failed.
|
Good catch from Bugbot — fixed in b651481: docker access is now probed and escalated through the same |
chruffins
left a comment
chruffins
left a comment
There was a problem hiding this comment.
hey, thanks for submitting a PR for this! besides the bugbot finding which should be fixed, everything else looks good to me.
…log path The builder-image build log lives under TMP_DIR, which the EXIT trap removes when the script finishes, so the path named in the warning was gone by the time anyone read it. Print the captured docker build output to stderr on failure, before cleanup runs (per review feedback).
chruffins
left a comment
chruffins
left a comment
There was a problem hiding this comment.
thanks for putting this in!
|
/test |
…log to stdout Addresses the two remaining Bugbot findings on this PR: - Daemon reachability is now acted on, not just used to pick sudo. If neither `docker info` nor `$SUDO docker info` succeeds, warn clearly and skip the builder build — the same early skip the "docker not installed" branch already does. Previously a release install with an unreachable daemon fetched the source tarball and only then failed on `docker build`, wasting work and surfacing a confusing error. - The captured build log prints to stdout, matching every other log dump in this script (the source-build path's `cat "$BUILD_LOG"`) and the warn line directly above it. It was going to stderr while its own warning went to stdout, so the two could detach or the log could vanish under `2>/dev/null`.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 03212df. Configure here.
| info "Builder image built successfully" | ||
| fi | ||
| else | ||
| elif [ -z "$BUILD_CONTEXT" ]; then |
There was a problem hiding this comment.
Misleading second fetch warning
Low Severity
When the release source tarball fetch fails, the script warns that it is skipping the builder build and clears BUILD_CONTEXT, then still hits the empty-context branch and warns that the Dockerfile is not available. The second message points at the wrong cause after a curl or tar failure.
Reviewed by Cursor Bugbot for commit 03212df. Configure here.
2 participants
Problem
Source-to-image builds boot builder VMs from
hypeman/builder:latest, and the API's fallback for installed (non-source) services is to find that image in the local Docker daemon —lib/builds/manager.go's own comment says "the installer builds this image before loading the service". But the installer only does that in its darwin branch, and only for source (BRANCH) installs.A Linux release install therefore has no builder image and no way to grow one:
ensureBuilderImagefinds no source checkout (go.modabsent) and no localhypeman/builder:latest, preparation retries forever, and everyhypeman buildfails. On v0.3.0 the failure is especially opaque — the ready flag is set in adefereven on failure, so builds proceed and die withcreate builder instance: image is required.Reproduced on a GitHub-hosted ubuntu runner installing v0.3.0 via
get.hypeman.sh.Fix
The builder-image step now runs on every platform where Docker is present:
TMP_DIR;$VERSIONand build from it — the builder Dockerfile is part of the same tree the binaries were released from;hypeman/builder:latestalready exists; warn-and-continue when it cannot be built (matching the existing style).On Linux the service is already running by this point; the startup preparation loop picks the image up on its next retry. On macOS the ordering (build before service load) is unchanged.
Testing
bash -nclean.docker build -t hypeman/builder:latest -f lib/builds/images/generic/Dockerfile) is what we run in barista's CI as a workaround, and it takes a fresh ubuntu runner from "every build fails" to green source-to-image builds.Found while building barista on hypeman. An alternative worth considering: publish a pinnable pre-built builder image and default
build.builder_imageto it, which would remove the Docker dependency from installs entirely.Note
Medium Risk
Installer now fetches a GitHub source tarball and runs
docker build(possibly via sudo) during install. Failures still warn-and-continue, so a broken daemon does not abort install, but source-to-image builds depend on this step succeeding.Overview
Fixes Linux release installs so
hypeman buildcan work: the installer now buildshypeman/builder:lateston all platforms with Docker, not only the macOS source-install path.Release installs fetch the tagged source tarball for the installed
$VERSIONand build fromlib/builds/images/generic/Dockerfile. Source installs still use the existing checkout. The step is skipped if the image already exists, uses$SUDO dockerwhen the user cannot talk to the daemon, and warn-and-continues if Docker is missing, unreachable, or the build fails (with the docker log printed beforeTMP_DIRis cleaned up).Reviewed by Cursor Bugbot for commit 67dd80f. Bugbot is set up for automated code reviews on this repo. Configure here.