Skip to content

Fix Docker runtime link failures, add smoke test - #57

Merged
gmarzot merged 15 commits into
mainfrom
bookworm-docker
Mar 16, 2026
Merged

Fix Docker runtime link failures, add smoke test#57
gmarzot merged 15 commits into
mainfrom
bookworm-docker

Conversation

@gmarzot

@gmarzot gmarzot commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

Related: #53

Background

The o-rly Docker image (ghcr.io/openmoq/o-rly) fails at runtime with:

error while loading shared libraries: libsodium.so.23: cannot open shared object file

Root cause: The moxygen tarball dynamically linked libfmt and libsodium.
Fixed on the moxygen side (openmoq/moxygen#77, merged): fmt built from source
via FetchContent, sodium_USE_STATIC_LIBS=ON. libunwind stays dynamic
intentionally (libunwind.a not PIC on x86_64).

Changes

  • Dockerfile: Remove libfmt9 — only libunwind8 needed at runtime
  • Docker smoke test: ldd check + start relay with test config, hit admin
    /info endpoint, verify "service":"o-rly" response. Gates Docker push.
  • Rename formatcheck-format in both ci-main and ci-pr workflows
  • Simplify notifications: 3 statuses — verify (check-format + build),
    publish (tarball + Docker + smoke), release
  • Add update-moxygen-submodule.yml: workflow_dispatch to create a PR
    updating deps/moxygen to latest main
  • Update moxygen submodule to e7a59145 (static linking + unified matrix)
  • Simplify publish job (remove single-item matrix)

Test plan

  • CI builds against new moxygen tarball
  • ldd shows no missing shared libs
  • Admin /info health check passes in Docker
  • Docker image pushes to GHCR

This change is Reviewable

Split docker build/push in publish job with ldd + dump-config-schema
smoke test between them. Remove libfmt9 from Dockerfile since moxygen
tarball now bundles fmt statically. Simplify publish job by removing
single-item matrix.
@gmarzot gmarzot changed the title Add Docker smoke test, drop libfmt9 dep Fix Docker runtime link failures, add smoke test Mar 14, 2026
…flow

- Rename format job to check-format in ci-main and ci-pr workflows
- Simplify notifications to 3 statuses: verify, publish, release
  (verify combines check-format + build results)
- Replace dump-config-schema smoke test with admin /info health check
  (starts relay, hits GET /info, verifies "service":"o-rly" response)
- Add update-moxygen-submodule.yml workflow_dispatch
- Update moxygen submodule to e7a5914 (static fmt/sodium, unified matrix)
@gmarzot
gmarzot marked this pull request as draft March 14, 2026 13:47
gmarzot added 5 commits March 14, 2026 23:51
- Dockerfile: build o-rly inside debian:bookworm against bookworm
  moxygen tarball, copy binary to bookworm-slim runtime image
- ci-main.yml publish: download bookworm tarball via ORLY_PLATFORM
  override, docker build does compilation (no C++ toolchain on runner)
- setup-deps-tarball.sh: support ORLY_PLATFORM env var override
- .dockerignore: exclude build dirs, keep fbcode_builder/CMake
- Update moxygen submodule to b3e8ab9 (find_dependency(fmt) fix)
glog dual-linking conflict under ASAN — will revert once glog
moves to FetchContent in the moxygen tarball.
Tests multi-stage Docker build + smoke test in CI where network
is fast. Downloads bookworm tarball, builds image, verifies ldd
and admin /info endpoint. Remove after validation.
The o-rly binary dynamically links libsodium.so.23 and
libboost_context.so.1.74.0 — present in the builder stage
from -dev packages but missing in bookworm-slim.
The san preset had CMAKE_FIND_LIBRARY_SUFFIXES=".a" which caused
find_dependency(Glog) to find libglog.a, while the tarball's
folly-targets.cmake hardcodes libglog.so. Both copies loaded under
ASAN, crashing on glog's logtostderr flag double-registration.

Removing the static preference lets the san build link consistently
against .so files for system deps. Also reverts the temporary
continue-on-error for asan.
@gmarzot
gmarzot marked this pull request as ready for review March 15, 2026 12:19
@gmarzot

gmarzot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor Author

NOTE: Requires moxygen PR #80 to be merged first, and fully complete CI/publish

gmarzot added 3 commits March 15, 2026 16:51
Temporarily download from PR #80 publish-test artifacts (run 23109570855)
instead of setup-deps-tarball.sh to validate the .so preference fix.

Add libgoogle-glog0v5, libgflags2.2, libdouble-conversion3 to Docker
runtime stage (needed now that tarball hardcodes .so paths).

Revert TEMPORARY ci-pr.yml changes after PR #80 merges to main.
@gmarzot
gmarzot requested a review from afrind March 15, 2026 23:17

@afrind afrind left a comment

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.

dogscience

@gmarzot
gmarzot merged commit aa047d2 into main Mar 16, 2026
5 checks passed
@gmarzot
gmarzot deleted the bookworm-docker branch April 12, 2026 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants