Repository navigation
Conversation
jack-berg
commented
Oct 22, 2026
The image tests could pass without examining the image under test. Go's test cache does not track the container image: the test reaches it through the container engine, so neither a rebuilt image nor a retagged one invalidates a cached result. Running the Makefile's command twice with the image swapped for a bare busybox in between printed "ok (cached)" and PASS. Run with -count=1. docker create had no --pull=never against the production GHCR reference, so a pruned or wrong-platform local image sends the engine to the registry and the test validates the previously published image instead of the build. The same call captured the container ID with CombinedOutput, so the pull progress ended up prepended to the ID, leaving export and the cleanup rm to fail on a mangled identifier and leak the container. The installation-state assertion listed three paths, two of which the final stage cannot reach because it copies only /payload/ out of the extract stage. Only /etc/ld.so.preload was live, and only the injector's postinst writes it, so for every other component the check asserted nothing. Compare against the Dockerfile's base image instead: the image may carry the package payload and the base, and nothing else. That holds for every component and catches state at any path, verified against an image carrying /etc/alternatives/java and a stray .pyc that the previous list let through.
packaging/builder has five test files and no target or CI job that runs them: go-unit-tests listed ./cmd/... and ./packaging/images/, and build.yml invokes only that target, so a regression in the builder could merge green. Add the package. It needs no containers or network and adds under half a second. Verified by mutation: weakening a behaviour spec_test.go covers now fails make go-unit-tests, which it did not before. Note that coverage is not uniform across the package -- the sha256 prefix rejection in fetchGitHubAssetDigest, for one, survives mutation untested. Running the package is the prerequisite for closing those gaps, not the closing of them. Also document git as a prerequisite. The image-metadata tests construct git histories through require.NoError, so go-unit-tests -- described as fast and container-free -- hard-fails without git on PATH, and the Prerequisites list did not mention it.
A publication job skipped its own tag whenever any newer first-parent commit pinned the same upstream version and touched the component's inputs. Intermediate commits do not get workflow runs: GitHub creates one run per push, at the push head. So a run could stand down in favour of a build that never happens. Reproduced in a sandbox repository. A run is in flight for a commit pinning java 2.32.0 when one push lands two commits: a Makefile edit that leaves the pin at 2.32.0, and a bump to 2.33.0. GitHub creates a single run, at the bump. The in-flight run walks back, skips the bump as a different version, matches the Makefile commit on a shared path, and reports publish=false. The run at the tip publishes 2.33.0. Nothing ever publishes 2.32.0, both runs are green, and manual dispatch cannot recover it because dispatch republishes whatever main pins now. Any rebase-merge of a multi-commit PR, merge-queue batch, or direct multi-commit push triggers it. RELEASING.md promises every merged upstream version its own image tag. Consult the tip alone. The tip always has a run, so yielding to it is safe; it reclaims a tag only by rebuilding that same version itself. An older tag now publishes once the version has moved on. Every other case the suite covers -- same-tag rebuilds, rollbacks, unmerged PR history, unrelated commits, different components -- keeps its existing expectation. The test named "same-tag rebuild before newer version still supersedes" encoded the defect as intended behaviour; it is now two cases asserting that an intermediate commit does not supersede, and both fail against the previous implementation. Also report git's stderr. Every git call used Output(), discarding it, so a publish job that failed here left only "exit status 128" -- no command, commit, or path. A missing ref now reports the command it ran and git's own "invalid object name".
Two ways one component's trouble withheld another's image tag. The publish matrix had no fail-fast: false, which the build job sets explicitly. A transient GHCR error on one component's push cancelled every sibling leg, including ones queued in unrelated per-component concurrency groups. The cancelled component's tag is then lost: no later run republishes it, because the next commit to affect that component carries a different version. The job also needed the whole build matrix, so the implicit success() skipped publication for every component whenever any single (component, arch) leg failed -- including components that built and tested green on both architectures. A rate-limited asset lookup on injector/arm64 withheld java's tag. Gate on plan instead and let each leg stand or fall on its own inputs. A leg whose builds produced no DEBs now fails on an explicit completeness check rather than reaching the image build with a partial context: the check requires exactly one DEB per configured architecture, which also denies the Dockerfile's COPY an ambiguous glob to resolve. Verified by extracting the check from the workflow and running it against real build output: complete context passes, a missing architecture fails, and two DEBs for one architecture fail.
Three ways a bad input reached a provisioned runner, or worse. image-packages-% was the only build-packages caller in the Makefile without the otel-config-check prerequisite, which deb-package-%, rpm-package-%, and packages all declare. packaging/images/README.md carries an open TODO to configure python, whose package ships that cross-compiled validator: adding it would abort every image build on a clean tree, and on a tree where an earlier build left a binary behind, silently package a stale one built at a different commit or architecture. Load accepted any component the builder knows, without checking it has an upstream version pin. Appending python and meta to config.json left make go-unit-tests green, printed four components, and generated a full build matrix; every per-component job then failed on a missing release.txt. python's pins live in requirements.txt and packaging/common/ meta does not exist. CheckPins closes this at the gate that was green -- go-unit-tests, which build.yml and every build matrix cell run. Load also accepted any non-empty architecture string. A typo like x86_64 generated a matrix and only failed deep inside build-packages after CI had provisioned a runner, and the value reaches Make recipes and $GITHUB_OUTPUT unquoted: make -n image-packages-java with ARCH='amd64; echo INJECTED' expanded to rm -rf followed by a second command. Validate against the builder's own list, which build-packages now shares rather than keeping a second copy of, and quote the expansions.
Contributor
|
@jack-berg I found some issues and took a stab at them, would you mind counter-reviewing? ;-) |
COPY resolved ${PACKAGE}_*.deb to a single file when the glob matched
more than one, with no error. Verified on BuildKit: a context holding
pkg_1.0.0 and pkg_2.0.0 produced one /tmp/package.deb containing the
latter, exit 0. The build job's test asserts exactly one match, but the
publish job builds straight from merged download-artifact output and ran
no such check, so an artifact-name collision or a re-run merging two
builds would publish whichever filename sorts last. Copy the
architecture's directory and assert the match count in the stage itself,
where both the Makefile and the publish job go through it.
Also pin the extract stage's platform explicitly. FROM --platform with
BUILDPLATFORM keeps dpkg-deb off emulation when cross-building. BuildKit
sets BUILDPLATFORM, and so does buildah: 1.28.2, 1.33.7, and 1.43.5 were
checked in an Ubuntu 24.04 VM, reporting BUILDPLATFORM=linux/arm64/v8
against TARGETPLATFORM=linux/amd64, and all three already pinned this
stage to the host. Falling back to TARGETPLATFORM therefore changes
nothing on any engine tested; it is kept because ${VAR:-} also covers a
defined-but-empty value, and an empty platform does resolve to the
target and emulate silently -- an undefined variable built x86_64 under
--platform linux/amd64 on an arm64 host.
Verified under BuildKit that the fallback preserves existing behaviour:
cross-building linux/amd64 on an arm64 host still runs the extract stage
as aarch64, and the resulting image passes the integration test.
Not covered: buildah older than 1.28, and an amd64 host building arm64.
The shared-path list missed two inputs the Python package ships: cmd/otel-config-check, whose cross-compiled binary lands at /usr/lib/opentelemetry/python/otel-config-check, and the repository-root NOTICE. Once python is configured -- an open TODO in the image README -- a commit that only fixed the validator would select nothing, skip build and publish, and leave the published image serving the previous validator with no signal that anything had changed. Both are latent today because python is not yet configured. They are also the kind of gap that surfaces as a silently stale image rather than a failure, so close them now; the new cases fail against the previous list. The list's over-approximation is left alone deliberately. Entries like packaging/tests/images and the image README cannot change image contents, but the same list also selects what CI rebuilds and retests, where test and documentation paths belong. Separating "rebuild and retest this" from "republish this" is a design decision rather than a fix, and it changes expectations the suite currently encodes.
RELEASING.md named manual dispatch as the recovery mechanism, but dispatch cannot recover the case that needs recovering. The plan step passes an empty base for a dispatch, selecting every configured package, and the version comes from reading release.txt out of the checked-out tree -- main's tip. So a dispatch after the pin has moved on republishes the newer version and can never produce the missed tag. The procedure that works is re-running the run that was meant to publish it, because a re-run uses the commit the original event carried, and so reads that commit's pin. Re-running only the failed jobs reuses the tested DEBs within their 7-day retention; re-running all jobs rebuilds them from the same commit after that. Restoring the pin in a fresh commit remains the fallback once the run is gone. Worth recording that this escape hatch was itself shut before the preceding commit: under the previous rule a re-run at an older commit matched an intermediate same-version commit and reported publish=false.
The man page .TH line renders as "opentelemetry-java-autoinstrumentation <version>", which reads to anyone running man(1) as the version of the agent the package ships. It carried cfg.Version instead -- the distribution packaging version -- so the same agent described itself differently depending on which build produced the package. Package images made this visible. They version their DEBs by the upstream pin while released DEBs use the git tag, so the java image shipped a page saying 2.32.0 where the DEB in the APT repository said 0.0.3, for byte-identical agent contents. The image integration test could not catch it: it diffs the image against the DEB built in the same run with the same -version, so both sides carried the same value. The conflation predates the images; passing a second value into the same placeholder is what surfaced it. Give the templates @UPSTREAM_VERSION@, resolved per component from the Renovate-managed pin. Python has no release.txt -- its payload is a set of wheels -- so it reports the opentelemetry-distro pin, that being the auto-instrumentation distribution itself and the direct analogue of the Java agent. @Version@ keeps its meaning as the packaging version, and GenerateManPage stays source-compatible for vendors through a variadic substitutions argument. Verified end to end: building java as an image DEB (2.32.0) and as a released DEB (0.0.3) now produces byte-identical man pages, and a test asserts that equality for all five components carrying a man page.
mmanciop
force-pushed
the
publish-images
branch
from
October 25, 2026 16:31
7218e68 to
f54b263
Compare
jack-berg
commented
Oct 28, 2026
jack-berg
left a comment
jack-berg
left a comment
Member
Author
There was a problem hiding this comment.
Happy to defer to you @mmanciop and the other @open-telemetry/packaging-maintainers.
A few high level comments:
What can we do to make image tags immutable? Currently, various parts of the flow rely on them being mutable.
I think we should punt on all the bits of code related to images we're not publishing yet. There will need to be language specific considerations, and I think it makes sense to keep the PRs smaller / more reviewable such that when we have to review python specific considerations, we can consider those properly.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #87, open-telemetry/opentelemetry-injector#70.
Add scaffolding to publish images corresponding to the packages.
How it works:
packaging/images/config.jsonlists the packages we want to publish as images, and which architectures we want to support.packaging/images/Dockerfilewhich accepts a target package, extracts its contents to a busybox image (needed because operator needscpsupport for backwards compatibility) in a standardized waymake images, and to test the go scripts that are involved in image creationimages.yml, publishes multi-architecture images when upstream version-pin renovate PRs merge into main and tests pass. Every merged upstream version receives a matching image tag, independently of packaging release. For example:opentelemetry-java-instrumentationreleases 2.32.0 and the version pin update PR is merged,ghcr.io/open-telemetry/opentelemetry-packaging/opentelemetry-java-autoinstrumentation:2.32.0is published.opentelemetry-java-instrumentationreleases 2.33.0 and the version pin update PR is merged,ghcr.io/open-telemetry/opentelemetry-packaging/opentelemetry-java-autoinstrumentation:2.33.0is published.There is followup work needed after this:
packaging/images/config.jsonto include all the auto instrumentation packages.