Skip to content

Add devcontainer workflow for local builds and integration tests#60

Open
pawelchcki wants to merge 5 commits intographite-base/60from
pc/devcontainer
Open

Add devcontainer workflow for local builds and integration tests#60
pawelchcki wants to merge 5 commits intographite-base/60from
pc/devcontainer

Conversation

@pawelchcki
Copy link
Copy Markdown
Contributor

@pawelchcki pawelchcki commented Apr 21, 2026

Adds a .devcontainer/ image definition and a thin Makefile + runner script so developers can reproduce the GitLab build + integration-test pipeline locally. The image bakes in the LLVM+musl sysroot, Rust+cbindgen for RUM, uv, and a prebuilt httpd.

Two non-obvious bits in the runner script:

  • UV_PROJECT_ENVIRONMENT points outside the bind-mounted repo, so a host-side uv sync on darwin can't leave mac wheels in .venv/ that the container's Linux Python then tries to import.
  • Pytest options use --flag=VALUE — the space-separated form is captured by pytest's early arg parser as a positional test path before conftest registers the custom option, which silently skips conftest.

The GitLab build-ci-image job's hash input and Dockerfile path are retargeted to the new .devcontainer/ location.

Copy link
Copy Markdown
Contributor Author

pawelchcki commented Apr 21, 2026

@datadog-official
Copy link
Copy Markdown

datadog-official Bot commented Apr 21, 2026

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 50.93% (+0.00%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 1b72823 | Docs | Datadog PR Page | Give us feedback!

@pawelchcki pawelchcki marked this pull request as ready for review April 21, 2026 09:45
@pawelchcki pawelchcki requested a review from a team as a code owner April 21, 2026 09:45
@pawelchcki pawelchcki requested review from Copilot and dmehala and removed request for a team April 21, 2026 09:45
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a reproducible devcontainer-based environment and local runner to mirror the GitLab build + integration-test pipeline, while retargeting GitLab CI’s image build inputs from .gitlab/ to .devcontainer/.

Changes:

  • Introduces a .devcontainer/ Dockerfile + devcontainer configuration for local builds/tests.
  • Adds a Makefile include + helper devcontainer.mk targets to build/pull the image, open a shell, and run integration tests.
  • Updates .gitlab-ci.yml to build/tag the devcontainer image from .devcontainer/ inputs and use it for build/test jobs.

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
Makefile Switches to including .devcontainer/devcontainer.mk as the primary entrypoint for dev workflows.
.gitlab-ci.yml Retargets CI image build inputs to .devcontainer/, adds nydus conversion in the per-arch build job, and updates downstream jobs to use the devcontainer image.
.gitignore Ignores container-specific build/dist outputs and logs generated by the new runner.
.devcontainer/run-integration-tests.sh New script to build (optionally) and run integration tests inside the devcontainer image, with uv/pytest configuration.
.devcontainer/devcontainer.mk New Make targets for pulling/building the devcontainer image, launching shells, and running tests.
.devcontainer/devcontainer.json Devcontainer definition for VS Code / devcontainer tooling.
.devcontainer/Dockerfile New image definition baking in LLVM+musl sysroot, Rust+cbindgen, uv, and a built httpd tree.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .gitlab-ci.yml Outdated
Comment thread .devcontainer/devcontainer.mk
Comment thread .devcontainer/devcontainer.mk Outdated
Comment thread .devcontainer/devcontainer.mk
Comment thread .devcontainer/devcontainer.mk
Comment thread Makefile
@pawelchcki pawelchcki changed the base branch from main to graphite-base/60 April 21, 2026 13:45
@pawelchcki pawelchcki changed the base branch from graphite-base/60 to pc/tri-state-merge April 21, 2026 13:45
@pawelchcki pawelchcki force-pushed the pc/tri-state-merge branch 2 times, most recently from 02bf4c1 to 5929c41 Compare April 21, 2026 14:40
@pawelchcki pawelchcki force-pushed the pc/tri-state-merge branch 2 times, most recently from 8aecbac to 7851f6f Compare April 21, 2026 18:43
@pawelchcki pawelchcki force-pushed the pc/tri-state-merge branch 2 times, most recently from 066310c to 130bcf8 Compare April 21, 2026 18:44
Introduce a .devcontainer image definition (moved from .gitlab/) and a
thin Makefile + runner script so developers can reproduce the GitLab
build + integration-test pipeline locally. The image bakes in the
LLVM+musl sysroot, Rust+cbindgen for RUM, uv for Python test setup,
and a prebuilt httpd 2.4.

Notable details in the runner script:

- UV_PROJECT_ENVIRONMENT is pointed outside the bind-mounted repo so a
  host-side `uv sync` on darwin can't leave mac wheels sitting in
  .venv/ that the container's Linux Python then tries to import.

- Pytest options are passed with `--flag=VALUE` (single token). The
  space-separated form gets captured by pytest's early arg parser as
  a positional test path before conftest registers the custom option,
  which makes pytest use the file's parent as rootdir and silently
  skip conftest.

The GitLab build-ci-image job's hash input and Dockerfile path are
retargeted to the new .devcontainer/ location.
Merge the three-job ci-image flow (build-ci-image + nydusify-ci-image
+ create-ci-manifest) into two: a single `devcontainer-image` matrix
job that builds + pushes + nydusifies each arch inline, followed by
`create-ci-manifest` which still stitches the per-arch nydus images
into a multi-arch index. Drops the separate nydusify job entirely.

Key changes:

- Single job uses the NYDUS_DD_IMAGE bundle (buildx + crane +
  nydus-convert all in one) instead of the plain docker image, so the
  build-push-nydusify sequence happens in one runner.

- Per-arch nydus conversion lands directly on
  $CI_DOCKER_IMAGE_BASE/$ARCH:$HASH before the manifest is assembled,
  so the final multi-arch manifest references nydus layers without a
  separate manifest-level conversion step.

- Export DEVCONTAINER_IMAGE=$base:$HASH (full ref) via dotenv so
  downstream jobs can pin the image by a single variable and don't
  have to re-construct the ref from a base+hash pair.

Also prepares run-integration-tests.sh for CI reuse: if MODULE_PATH
is set in the environment, the cmake build is skipped and pytest
consumes the pre-built module at that path. Default behavior (no
envvar) is unchanged, so local dev still gets a full build-and-test.
Move the `devcontainer-image` build-and-nydusify job plus the
`create-ci-manifest` multi-arch assembly into their own include file
`.gitlab/devcontainer.yml`, and reference it from the main
`.gitlab-ci.yml` via `include: - local:`. The NYDUS_DD_IMAGE variable
is declared inside the include since it's only used by these jobs.

Groups the devcontainer image pipeline into a single-responsibility
file separate from the build/test/upload orchestration. Matches the
pattern established by inject-browser-sdk's `.gitlab/devcontainer.yml`
and keeps the main `.gitlab-ci.yml` readable (drops 72 lines).

No behavioral change — job names, dependencies, env exports, and
runtime semantics are identical; only file layout moves.
Per-arch build/test jobs used to wait for both devcontainer-image
matrix slots plus create-ci-manifest before they could start. That
serialized the pipeline needlessly: the amd64 test doesn't need the
arm64 image to exist, and neither needs the multi-arch manifest.

Export DEVCONTAINER_IMAGE per arch (\$base/\$arch:\$hash) rather than
multi-arch (\$base:\$hash), and scope each consumer's `needs:` to its
own arch slot via `parallel: matrix:`. Drop create-ci-manifest from
the needs list entirely — it's still built for external consumers but
doesn't gate the internal pipeline.

Effect: when a devcontainer input (Dockerfile, nginx-datadog build_env,
setup-httpd.py) changes, the matching arch's image rebuilds, and that
arch's build + test jobs pick it up as soon as they're ready; the
other arch proceeds in parallel on its own rebuild.
Collects a few follow-ups to the devcontainer branch that were spotted
while testing the flow end-to-end.

devcontainer.mk:
- Fail fast with actionable guidance when deps/nginx-datadog isn't
  initialized. Without this, the DEV_CONTAINER_HASH `find` emits
  warnings and silently hashes an incomplete input set, producing a
  tag that doesn't match what CI computed — the result was a confusing
  pull-then-fall-back-to-build loop.
- Mount $(DEV_CONTAINER_GIT_MOUNT) into dev-shell so submodule init
  works from inside the container.
- Move test-integration to the always-available section. $(DEVCONTAINER_RUN)
  is empty inside the container (script runs directly) and `docker run …`
  outside, so one definition works in both modes.

.github/workflows/{dev,release,system-tests}.yml:
- Replace the unhelpful "See in Makefile where this image comes from."
  comment with one that names the upstream (GitLab-built image from
  .devcontainer/Dockerfile, hash-computed by .gitlab/devcontainer.yml)
  and documents the bump procedure.

.gitlab-ci.yml:
- Switch pytest `--flag VALUE` to `--flag=VALUE` in .test-rum-template
  for the same reason as run-integration-tests.sh: pytest's early arg
  parser runs before conftest registers these custom options, so the
  space-separated form is read as a positional test path and skips
  conftest entirely.

# Map host arch (uname -m) to the Dockerfile's ARCH build-arg.
DEV_CONTAINER_HOST_ARCH := $(shell uname -m)
ifeq ($(DEV_CONTAINER_HOST_ARCH),arm64)
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.

nit: simplification:

ifneq ($(filter $(DEV_CONTAINER_HOST_ARCH),arm64 aarch64),)
DEV_CONTAINER_ARCH := aarch64
DEV_CONTAINER_PLATFORM := linux/arm64

# Map host arch (uname -m) to the Dockerfile's ARCH build-arg.
DEV_CONTAINER_HOST_ARCH := $(shell uname -m)
ifeq ($(DEV_CONTAINER_HOST_ARCH),arm64)
DEV_CONTAINER_ARCH := aarch64
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.

nit, opt: indent (with spaces, not tabs) for improved readability:

ifneq ($(filter $(DEV_CONTAINER_HOST_ARCH),arm64 aarch64),)
  DEV_CONTAINER_ARCH := aarch64
  DEV_CONTAINER_PLATFORM := linux/arm64
else
  DEV_CONTAINER_ARCH := x86_64
  DEV_CONTAINER_PLATFORM := linux/amd64
endif

Some in other small conditionals, lines 22, 56-58.

dev-image:
@true

else
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.

nit, opt: This is a big conditional. I think it's worth adding a comment in the else line 14, and in the endif line 111.

.PHONY: dev-image
dev-image:
@if ! docker image inspect $(DEV_CONTAINER_IMAGE) >/dev/null 2>&1; then \
stale=$$(docker images --format '{{.Repository}}:{{.Tag}}' $(DEV_CONTAINER_IMAGE_NAME) 2>/dev/null | grep -v ':$(DEV_CONTAINER_TAG)$$' || true); \
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.

I think we can replace lines 73-77 to a call to dev-image-clean:

		$(MAKE) --no-print-directory dev-image-clean; \

endif

DEVCONTAINER_RUN = docker run --rm \
-v $(DEV_CONTAINER_REPO_ROOT):$(DEV_CONTAINER_REPO_ROOT) \
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.

Lines 62-65 are the same than lines 95-98. This could be factorized in a variable such as DEV_CONTAINER_MOUNTS.

# silently hashes an incomplete input set, producing a tag that doesn't
# match what CI computed and triggering confusing pull-then-fall-back-to-
# build behavior.
ifeq ($(wildcard $(DEV_CONTAINER_REPO_ROOT)/deps/nginx-datadog/build_env/Toolchain.cmake.x86_64),)
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.

This could be simplified as:

ifeq ($(wildcard $(DEV_CONTAINER_REPO_ROOT)/deps/nginx-datadog/build_env),)

-DCMAKE_TOOLCHAIN_FILE="/sysroot/${ARCH}-none-linux-musl/Toolchain.cmake" \
-B "$BUILD_DIR" .
cmake --build "$BUILD_DIR" -j
cmake --install "$BUILD_DIR" --prefix "$DIST_DIR"
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.

Maybe add a clean DIST_DIR before this to be more deterministic:

rm -rf "$DIST_DIR"

Comment thread .github/workflows/dev.yml
runs-on: ubuntu-22.04
container:
# See in Makefile where this image comes from.
# Public mirror of the GitLab-built image defined in .devcontainer/Dockerfile
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.

This comment is repeated in 4 locations (also in .github/workflows/release.yml and .github/workflows/system-tests.yml). Please try to find a common place to avoid duplication.
Also:

  • Could these commands be put in make target (as before) or in a shell script?
  • Could you please update the related internal documentation page (but maybe this is not the most adequate place for such documentation)?

Comment thread .gitlab/devcontainer.yml
TOOLCHAIN_ARCH: aarch64
script:
- |
FILES=$(find .devcontainer/Dockerfile deps/nginx-datadog/build_env/ scripts/setup-httpd.py -type f | sort)
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.

To avoid duplication (and so possible divergence), the hash computing could be factorized in a dedicated script:

.devcontainer/compute-image-hash.sh:

#!/bin/sh
set -eu

REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
cd "$REPO_ROOT"

find .devcontainer/Dockerfile \
     .devcontainer/compute-image-hash.sh \
     deps/nginx-datadog/build_env/ \
     scripts/setup-httpd.py \
     -type f | sort | \
while IFS= read -r f; do
  printf -- '--- %s ---\n' "$f"
  cat "$f"
done | \
  (command -v sha256sum >/dev/null 2>&1 && sha256sum || shasum -a 256) | \
  cut -d' ' -f1

Then, in devcontainer/devcontainer.mk (line 28):

DEV_CONTAINER_HASH := $(shell $(DEV_CONTAINER_REPO_ROOT)/.devcontainer/compute-image-hash.sh)

In .gitlab/devcontainer.yml (line 29):

HASH=$(.devcontainer/compute-image-hash.sh)

Comment thread .gitlab-ci.yml
needs:
- build-ci-image
- create-ci-manifest
- job: devcontainer-image
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.

This duplicated block can be factorized in:

.needs-devcontainer-amd64: &needs-devcontainer-amd64
  job: devcontainer-image
  parallel:
    matrix:
      - ARCH: amd64
        TOOLCHAIN_ARCH: x86_64
  artifacts: true

@pawelchcki pawelchcki changed the base branch from pc/tri-state-merge to graphite-base/60 April 22, 2026 19:40
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.

3 participants