Azure / Azure/unbounded

images: BUILD_TIME is stamped empty in every released image

Open
#634 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
28
Forks
11
Avg merge
1d 8h
Merged PRs (30d)
55

Description

## Summary

Every released container image built from one of nine Containerfiles stamps an
empty `BuildTime`. The value is not merely defaulted to `unknown`, it is set to
the empty string, so `internal/version.String()` reports `built: ` with nothing
after it.

## Evidence

Straight from the published `v0.2.4` image:

```
$ docker create --name x ghcr.io/azure/machina:v0.2.4
$ docker cp x:/unbounded/bin/machina ./machina-bin
$ go version -m machina-bin | grep ldflags
build -ldflags="-X github.com/Azure/unbounded/internal/version.Version=v0.2.4
-X github.com/Azure/unbounded/internal/version.GitCommit=c59fecaa2ba3eb7aba19218df64ec459e81be49c
-X github.com/Azure/unbounded/internal/version.BuildTime="
```

## Cause

Two things combine.

`release.yaml` passes `VERSION` and `GIT_COMMIT` to every image build, but never
`BUILD_TIME`. `grep -n BUILD_TIME .github/workflows/*.yaml` returns nothing.

The Containerfiles were written to tolerate that. They declare

```dockerfile
ARG VERSION=dev
ARG GIT_COMMIT=
ARG BUILD_TIME=
RUN ... make machina-build VERSION=${VERSION} ${GIT_COMMIT:+GIT_COMMIT=${GIT_COMMIT}} ${BUILD_TIME:+BUILD_TIME=${BUILD_TIME}}
```

with the intent, per the comment, that "empty defaults let the Makefile fall back
to its own git/date detection". They do not. An `ARG` declared with an empty
default still enters the `RUN` command's environment as a set-but-empty variable,
and make imports the environment, so `BUILD_TIME ?= $(shell date -u ...)` sees an
already-defined variable and leaves it empty. The `${VAR:+...}` guard cannot help,
because the leak is environmental rather than positional: it correctly avoids
passing `BUILD_TIME=` on the command line, but make has already picked the empty
value up from the environment.

Demonstrable in isolation:

```
$ docker run --rm -e BUILD_TIME= sh -c 'cd /src && make -n machina-build | head -1'
go build -ldflags '... -X .../version.BuildTime=' -o bin/machina ./cmd/machina/main.go
```

The Makefile's own git and date detection cannot rescue this in any case, since
`.dockerignore` excludes `.git/` from the build context, so `GIT_COMMIT` would
only ever resolve to `unknown` inside an image build.

## Scope

Nine Containerfiles share the pattern:

```
images/gantry/Containerfile
images/machina/Containerfile
images/machine-ops-controller/Containerfile
images/metalman/Containerfile
images/net/Containerfile
images/orca/Containerfile
images/playpen/Containerfile
images/unbounded-operator/Containerfile
images/unbounded-storage-supervisor/Containerfile
```

`GIT_COMMIT` happens to be correct in release and nightly builds because both
pass it explicitly. It is empty for anyone running `docker build` without the
arg. `BUILD_TIME` is empty everywhere except the Makefile's own
`image-*-local` targets, which do pass all three.

The four `images/inventory/*` Containerfiles had the same bug and were fixed in
#633; that PR deliberately left these nine alone rather than touching images it
was not otherwise changing.

## Suggested fix

The approach taken in #633: give the ARGs the same defaults `internal/version`
uses and pass all three unconditionally, so an unset arg yields `unknown` rather
than an empty string and no shell guard is needed.

```dockerfile
ARG VERSION=dev
ARG GIT_COMMIT=unknown
ARG BUILD_TIME=unknown
RUN ... make machina-build VERSION=${VERSION} GIT_COMMIT=${GIT_COMMIT} BUILD_TIME=${BUILD_TIME}
```

Optionally also pass `BUILD_TIME` from `release.yaml` and `nightly.yaml` so
released images carry a real timestamp instead of `unknown`. Note that a real
build time makes image builds non-reproducible by construction, so `unknown` may
be the better answer for release builds; the current empty string is the one
option that is neither.

## Impact

Low severity, entirely cosmetic today: nothing parses `BuildTime`, and the
version and commit are both correct in released images. It undermines the version
metadata as a debugging aid, and `String()` renders awkwardly as
`v0.2.4 (commit: c59fecaa, built: )`.

Contributor guide

Open the contributing guide

Research direction

Start with the nine listed images/*/Containerfile files and compare their ARG defaults and machina-build invocations with the already-fixed images/inventory/* Containerfiles in #633. Then inspect .github/workflows/release.yaml and nightly.yaml for image arguments. Done means image builds no longer stamp an empty BuildTime, while version metadata remains correct; validate with the relevant Docker builds and internal/version.String().

Written by the indexing model from the issue text.

Assessment

Tech stack
dockerfile, go
Domain
build-system, release
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.