Skip to content

fix(docker): stop clobbering the base image's /usr/lib, and build images on main #3157

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.

Sign up for GitHub

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

Merged
wrn14897 merged 5 commits into from
Sep 19, 2026
Merged

fix(docker): stop clobbering the base image's /usr/lib, and build images on main #3157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
015cd52
fix(docker): stop clobbering the base image's /usr/lib with node's
wrn14897 Sep 18, 2026
3dc71d8
ci: build the release images on every push to main
wrn14897 Sep 18, 2026
85e50b2
ci: correct what the concurrency group actually guarantees
wrn14897 Sep 18, 2026
47e95ac
ci: state the real reason the concurrency group keys on the event
wrn14897 Sep 18, 2026
8af1390
ci: inline the build condition and drop the unproven pull: true
wrn14897 Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/fix-alpine-libapk-clobber.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@hyperdx/app': patch
---

fix: stop the all-in-one image copying node's /usr/lib over the base image

The all-in-one and local images copied the whole `/usr/lib` out of `node:22.22-alpine`, overwriting the ClickHouse base image's `libapk.so`, `libssl`, `libcrypto` and `libz`. Once the two images landed on different Alpine point releases (3.24.1 vs 3.24.2), the older `libapk.so` no longer satisfied the newer `/sbin/apk`, and every image build failed with `Error relocating /sbin/apk: apk_fs_is_malicious_filename: symbol not found`.

Node only needed `libstdc++`/`libgcc` from that copy, so those are now installed with apk from the image's own Alpine release and the blanket copy is gone. This also stops ClickHouse's OpenSSL being silently swapped for node's.
83 changes: 58 additions & 25 deletions .github/workflows/docker-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,45 @@
name: Docker Build
# Verifies the release Docker images actually build on PRs, without pushing.
# Verifies the release Docker images actually build, without pushing.
# A source-level type check (`make ci-lint`) cannot catch build failures that
# only surface inside the image — e.g. a declaration file that exists in the
# repo but was never COPYed into the Docker build stage. Each image is built
# only when files that affect it change, so unrelated PRs stay fast.
# repo but was never COPYed into the Docker build stage.
#
# On pull requests each image is built only when files that affect it change,
# so unrelated PRs stay fast.
#
# On pushes to main every image is built regardless of what changed. The path
# filter is the wrong instrument there, because these builds can break with no
# repo change at all: the Dockerfile pins floating upstream tags
# (`node:${NODE_VERSION}-alpine`, `clickhouse-server:${CLICKHOUSE_VERSION}-alpine`)
# and the registry re-points them whenever upstream publishes a patch. That is
# exactly how the 26.8-alpine break reached a release: the tag moved from
# ClickHouse 26.8.6 to 26.8.7 mid-afternoon, upgrading apk-tools 3.0.6 -> 3.0.8
# underneath us. The PR check and the nightly had both already passed against
# the older digest, two commits then merged to main with no image build between
# them, and the failure only appeared in the release run hours later. Building
# unconditionally on main turns that into a red build on the next merge.
on:
pull_request:
branches: [main]
push:

@github-actions github-actions Bot Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — The new push-to-main drift gate has no failure notification, unlike the nightly it is meant to backstop

release-nightly.yml:340-402 posts to SLACK_WEBHOOK_URL_ENG_NOTIFS on failure precisely because an unwatched drift detector is no detector. Replay the incident in the header comment with this change in place: upstream re-tags at 15:01, the 16:17 merge turns Docker Build red on main, and the only signal is a red X in main's commit list plus a default Actions email to whoever clicked merge on an unrelated PR — the 19:31 release still fails. Add a slack-notify-failure job modelled on the nightly's, gated on if: failure() && github.event_name == 'push'.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

@wrn14897 wrn14897 Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best point in the review, and I am deliberately not acting on it — it is a team call about alerting, not a defect.

The gap is real: a red main is only a signal if someone looks, and the repo already concedes that by wiring Slack into the nightly.

What gives me pause is that the nightly and this are not the same situation. A 00:35 cron has no owner, which is exactly why it needs a webhook. A push-to-main failure has an obvious one — the person who just merged, who gets GitHub's default failure mail and is watching main anyway. And if: failure() && github.event_name == 'push' fires on any Docker Build failure on main, not just drift, so most of what it sends would be telling someone something they already know. Whether that trade is worth it depends on how the team currently treats a red main and how noisy SLACK_WEBHOOK_URL_ENG_NOTIFS already is, which I should not be guessing at from inside a build-break fix.

Flagging it for a maintainer rather than deciding it here. If the answer is yes, it is a small follow-up.

@brandon-pereira brandon-pereira Sep 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will there be a large increase in gh actions usage from this? assuming thats fine

@wrn14897 wrn14897 Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its fine. Let me pull out a bit more analysis before merging it.

branches: [main]
workflow_dispatch:
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
# Keyed on the event as well as the ref to separate a manual run on main from
# a merge: both carry `refs/heads/main`, so they would otherwise contend for
# one group. (A pull request run never collides with either — its ref is
# `refs/pull/<n>/merge`.)
#
# Pushes to main do not cancel in progress, so a merge cannot kill the build
# verifying the commit before it. This guarantees the tip of main gets built,
# not that every individual commit does: GitHub holds only one pending run per
# group, so a third merge arriving during a long build supersedes the queued
# one. That is fine for what this job is for — an upstream base-image move
# affects every commit equally, and a Dockerfile break in a superseded commit
# is still present at the tip. Per-commit attribution comes from the pull
# request run, not from here.
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}

@github-actions github-actions Bot Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — cancel-in-progress: false does not guarantee every main commit is built

GitHub keeps at most one pending run per concurrency group: when a third push arrives while one run is in progress and one is queued, the queued run is cancelled, not deferred. So with three merges inside one ~40-minute all-in-one build, the middle commit is still never verified — the outcome the comment above says this change prevents. Either add github.sha to the group for pushes (${{ github.event_name == 'pull_request' && github.ref || github.sha }}) so every commit gets its own group, or reword the comment to claim only that the tip commit is always built.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

@wrn14897 wrn14897 Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and fixed in 85e50b2 — the comment overclaimed.

GitHub holds one pending run per group, so a third merge during a long build supersedes the queued one rather than joining a queue. The comment now says that plainly and says why it is acceptable rather than pretending otherwise: an upstream base move affects every commit equally, and a Dockerfile break in a superseded commit is still present at the tip, so building the tip is what this job needs. Per-commit attribution comes from the pull request run.

Took the reword over adding github.sha to the group deliberately — per-commit groups would fan out N concurrent 40-minute builds on a busy day to buy attribution we already get from PR CI.

permissions:
contents: read
jobs:
Expand All @@ -24,20 +53,21 @@ jobs:
uses: actions/checkout@v6
- name: Get changed files
id: changed-files
if: github.event_name == 'pull_request'
uses: tj-actions/changed-files@v47.0.6
with:
files: |
docker/otel-collector/**
packages/otel-collector/**
- name: Setup Docker Buildx
if:
steps.changed-files.outputs.any_changed == 'true' || github.event_name
== 'workflow_dispatch'
github.event_name != 'pull_request' ||
steps.changed-files.outputs.any_changed == 'true'
uses: docker/setup-buildx-action@v3
- name: Build (no push)
if:
steps.changed-files.outputs.any_changed == 'true' || github.event_name
== 'workflow_dispatch'
github.event_name != 'pull_request' ||
steps.changed-files.outputs.any_changed == 'true'
uses: docker/build-push-action@v6
with:
context: .
Expand All @@ -62,6 +92,7 @@ jobs:
uses: actions/checkout@v6
- name: Get changed files
id: changed-files
if: github.event_name == 'pull_request'
uses: tj-actions/changed-files@v47.0.6
with:
files: |
Expand All @@ -79,13 +110,13 @@ jobs:
nx.json
- name: Setup Docker Buildx
if:
steps.changed-files.outputs.any_changed == 'true' || github.event_name
== 'workflow_dispatch'
github.event_name != 'pull_request' ||
steps.changed-files.outputs.any_changed == 'true'
uses: docker/setup-buildx-action@v3
- name: Build (no push)

@github-actions github-actions Bot Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — On push to main the App Image job is fully subsumed by the All-in-One job, duplicating the ~8-minute Node build on every merge

all-in-one-base does COPY --from=prod /app /app (docker/hyperdx/Dockerfile:214), so building all-in-one-auth builds the entire prod stage first — any failure the App Image job would catch on a push also fails the All-in-One job. The comment at lines 146-153 says the narrow filter exists precisely to avoid paying that Node build twice, and "Pushes to main ignore the filter entirely" reinstates the duplication on every merge, now on two runners at once. Either gate the App Image job's build on github.event_name == 'pull_request' && steps.changed-files.outputs.any_changed == 'true', or note in the comment that the duplicate build on main is deliberately kept for per-job attribution and for warming the docker-build-pr-app cache scope.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

@wrn14897 wrn14897 Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct that it is subsumed, but leaving it — the duplication buys a faster signal.

App Image finishes in roughly 8 minutes; All-in-One takes 15-40. For the failure mode this PR is about, a moved node:*-alpine or clickhouse-server:*-alpine base, App Image goes red first and points at the Node build directly rather than at a 22-layer image. They run on separate runners in parallel, so wall-clock to first signal is the cheaper of the two, not the sum.

It is runner-minutes against time-to-detect, and for a drift gate I would rather know sooner. Happy to gate it to pull_request if a maintainer would rather have the minutes back.

if:
steps.changed-files.outputs.any_changed == 'true' || github.event_name
== 'workflow_dispatch'
github.event_name != 'pull_request' ||
steps.changed-files.outputs.any_changed == 'true'
uses: docker/build-push-action@v6
with:
context: .
Expand Down Expand Up @@ -115,10 +146,11 @@ jobs:
#
# This target's base does `COPY --from=prod /app /app`, so it already
# rebuilds the entire App/prod image. To avoid duplicating that ~8-minute
# Node build on every app change, this job is gated ONLY on the incremental
# bundling inputs (ClickHouse / OTel / the shared Dockerfile) — NOT on
# api/app/common-utils/deps. Those are covered by the App Image job above,
# and they can't break the bundling layers, which only COPY the prod output.
# Node build on every app change, the pull-request path filter below is
# scoped ONLY to the incremental bundling inputs (ClickHouse / OTel / the
# shared Dockerfile) — NOT api/app/common-utils/deps. Those are covered by
# the App Image job above, and they can't break the bundling layers, which
# only COPY the prod output. Pushes to main ignore the filter entirely.
all-in-one-image:
name: Build All-in-One Image
runs-on: ubuntu-24.04
Expand All @@ -128,6 +160,7 @@ jobs:
uses: actions/checkout@v6
- name: Get changed files
id: changed-files
if: github.event_name == 'pull_request'
uses: tj-actions/changed-files@v47.0.6
with:
files: |
Expand All @@ -138,13 +171,13 @@ jobs:
.vex/**
- name: Setup Docker Buildx
if:
steps.changed-files.outputs.any_changed == 'true' || github.event_name
== 'workflow_dispatch'
github.event_name != 'pull_request' ||
steps.changed-files.outputs.any_changed == 'true'
uses: docker/setup-buildx-action@v3
- name: Build all-in-one-auth (no push)
if:
steps.changed-files.outputs.any_changed == 'true' || github.event_name
== 'workflow_dispatch'
github.event_name != 'pull_request' ||
steps.changed-files.outputs.any_changed == 'true'
uses: docker/build-push-action@v6
with:
context: .
Expand All @@ -169,8 +202,8 @@ jobs:
# the final noauth COPY layer.
- name: Build all-in-one-noauth / Local (no push)
if:
steps.changed-files.outputs.any_changed == 'true' || github.event_name
== 'workflow_dispatch'
github.event_name != 'pull_request' ||
steps.changed-files.outputs.any_changed == 'true'
uses: docker/build-push-action@v6
with:
context: .
Expand All @@ -195,7 +228,7 @@ jobs:
# ClickHouse. Proves the artefact we ship actually runs, not just builds.
- name: Smoke test the built image
if:
steps.changed-files.outputs.any_changed == 'true' || github.event_name
== 'workflow_dispatch'
github.event_name != 'pull_request' ||
steps.changed-files.outputs.any_changed == 'true'
timeout-minutes: 10
run: scripts/ci/smoke-all-in-one.sh hyperdx-aio-smoke:pr
15 changes: 11 additions & 4 deletions docker/hyperdx/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,14 @@ ENV OTEL_RESOURCE_ATTRIBUTES="service.version=$CODE_VERSION"
COPY --from=ocb-builder --chmod=755 /build/output/otelcol-hyperdx /otelcontribcol
COPY --from=otel_collector_opampsupervisor_base --chmod=755 /usr/local/bin/opampsupervisor /usr/local/bin/opampsupervisor

# Copy Node.js runtime from node base
# Copy Node.js runtime from node base.
# Deliberately NOT copying node's /usr/lib: it overwrites this image's
# libapk.so, libssl, libcrypto and libz with whatever Alpine point release the
# node image happens to be built on. When the two drift, apk stops linking
# ("Error relocating /sbin/apk: ... symbol not found") and the build dies.
# Node only needs libstdc++/libgcc from there; those are installed via apk below.
COPY --from=node_base --link /usr/local/bin /usr/local/bin
COPY --from=node_base --link /usr/local/lib /usr/local/lib
COPY --from=node_base /usr/lib /usr/lib
COPY --from=node_base /usr/local/include /usr/local/include

# Set up Clickhouse
Expand All @@ -186,8 +190,11 @@ COPY --from=goose /bin/goose /usr/local/bin/goose
# Copy migrate binary (Go-based goose migration tool with full TLS support)
COPY --from=migrate-builder /migrate /usr/local/bin/migrate

# Install MongoDB and other dependencies (consolidated into a single RUN command)
RUN echo 'http://dl-cdn.alpinelinux.org/alpine/v3.9/main' >> /etc/apk/repositories && \
# Install MongoDB and other dependencies (consolidated into a single RUN command).
# libstdc++/libgcc are the Node.js runtime deps, installed before the v3.9
# repositories are appended so they resolve from this image's own Alpine release.
RUN apk add --no-cache libstdc++ libgcc && \
echo 'http://dl-cdn.alpinelinux.org/alpine/v3.9/main' >> /etc/apk/repositories && \
echo 'http://dl-cdn.alpinelinux.org/alpine/v3.9/community' >> /etc/apk/repositories && \
apk update && \
apk add --no-cache mongodb yaml-cpp=0.6.2-r2 ca-certificates && \
Expand Down
Loading