Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
File renamed without changes.
10 changes: 10 additions & 0 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "httpd-datadog",
"build": {
"dockerfile": "Dockerfile",
"context": "..",
"args": {
"ARCH": "${localEnv:ARCH:x86_64}"
}
}
}
118 changes: 118 additions & 0 deletions .devcontainer/devcontainer.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
DEV_CONTAINER_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
DEV_CONTAINER_REPO_ROOT := $(abspath $(DEV_CONTAINER_DIR)/..)

# When running inside the container, tools are available directly.
# Check for /.dockerenv (Docker) or KUBERNETES_SERVICE_HOST (Kubernetes CI).
ifneq ($(or $(wildcard /.dockerenv),$(KUBERNETES_SERVICE_HOST)),)

DEVCONTAINER_RUN :=
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: naming consistency: DEV_CONTAINER_RUN (same lines 61 and 118).


.PHONY: dev-image
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.


Comment thread
pawelchcki marked this conversation as resolved.
# Fail fast with actionable guidance if deps/nginx-datadog isn't populated —
# without this check, the DEV_CONTAINER_HASH `find` below emits warnings and
# 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),)

$(error deps/nginx-datadog submodule is not initialized. Run: git submodule update --init --recursive)
endif

DEV_CONTAINER_REGISTRY ?= registry.ddbuild.io
DEV_CONTAINER_IMAGE_NAME ?= $(DEV_CONTAINER_REGISTRY)/ci/httpd-datadog

# Match the hash computation in .gitlab/devcontainer.yml (devcontainer-image
# job): sha256 over .devcontainer/Dockerfile, deps/nginx-datadog/build_env/,
# and scripts/setup-httpd.py.
DEV_CONTAINER_HASH := $(shell cd $(DEV_CONTAINER_REPO_ROOT) && \
Comment thread
pawelchcki marked this conversation as resolved.
find .devcontainer/Dockerfile deps/nginx-datadog/build_env/ scripts/setup-httpd.py -type f | sort | \
while IFS= read -r f; do echo "--- $$f ---"; cat "$$f"; done | \
(command -v sha256sum >/dev/null 2>&1 && sha256sum || shasum -a 256) | cut -d' ' -f1)

DEV_CONTAINER_TAG := $(DEV_CONTAINER_HASH)
DEV_CONTAINER_IMAGE := $(DEV_CONTAINER_IMAGE_NAME):$(DEV_CONTAINER_TAG)

# 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

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_CONTAINER_PLATFORM := linux/arm64
else ifeq ($(DEV_CONTAINER_HOST_ARCH),aarch64)
DEV_CONTAINER_ARCH := aarch64
DEV_CONTAINER_PLATFORM := linux/arm64
else
DEV_CONTAINER_ARCH := x86_64
DEV_CONTAINER_PLATFORM := linux/amd64
endif

# For git worktrees, .git is a file pointing to a gitdir outside the repo root;
# bind-mount the git common-dir so `git` works inside the container.
DEV_CONTAINER_GIT_COMMON_DIR := $(shell cd $(DEV_CONTAINER_REPO_ROOT) && git rev-parse --path-format=absolute --git-common-dir 2>/dev/null)
ifneq ($(DEV_CONTAINER_GIT_COMMON_DIR),)
ifneq ($(DEV_CONTAINER_GIT_COMMON_DIR),$(DEV_CONTAINER_REPO_ROOT)/.git)
DEV_CONTAINER_GIT_MOUNT := -v $(DEV_CONTAINER_GIT_COMMON_DIR):$(DEV_CONTAINER_GIT_COMMON_DIR)
endif
endif

DEVCONTAINER_RUN = docker run --rm \
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: We can use :=.

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

$(DEV_CONTAINER_GIT_MOUNT) \
-w $(DEV_CONTAINER_REPO_ROOT) \
$(DEV_CONTAINER_IMAGE)

# Ensure the dev container image is available locally. Prefer pulling from the
# registry (CI builds every tagged hash); fall back to a local build.
# Stale images with different tags are removed first.
.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; \

if [ -n "$$stale" ]; then \
echo "Removing stale dev container image(s): $$stale"; \
docker rmi $$stale || true; \
fi; \
if docker pull $(DEV_CONTAINER_IMAGE) >/dev/null 2>&1; then \
echo "Pulled $(DEV_CONTAINER_IMAGE)"; \
else \
echo "Building dev container image $(DEV_CONTAINER_IMAGE)..."; \
docker buildx build \
--platform $(DEV_CONTAINER_PLATFORM) \
--build-arg ARCH=$(DEV_CONTAINER_ARCH) \
--load \
-t $(DEV_CONTAINER_IMAGE) \
-f $(DEV_CONTAINER_REPO_ROOT)/.devcontainer/Dockerfile \
$(DEV_CONTAINER_REPO_ROOT); \
fi; \
fi

.PHONY: dev-shell
dev-shell: dev-image
docker run --rm -it \
-v $(DEV_CONTAINER_REPO_ROOT):$(DEV_CONTAINER_REPO_ROOT) \
Comment thread
pawelchcki marked this conversation as resolved.
$(DEV_CONTAINER_GIT_MOUNT) \
-w $(DEV_CONTAINER_REPO_ROOT) \
$(DEV_CONTAINER_IMAGE) \
/bin/sh

.PHONY: dev-image-clean
dev-image-clean:
@stale=$$(docker images --format '{{.Repository}}:{{.Tag}}' $(DEV_CONTAINER_IMAGE_NAME) 2>/dev/null | grep -v ':$(DEV_CONTAINER_TAG)$$' || true); \
if [ -n "$$stale" ]; then \
echo "Removing stale dev container image(s): $$stale"; \
docker rmi $$stale; \
else \
echo "No stale dev container images found."; \
fi

endif

# Always-available targets: $(DEVCONTAINER_RUN) is empty inside the container
# (the script runs directly) and `docker run …` outside, so one definition
# works in both modes.
.PHONY: test-integration
test-integration: dev-image
$(DEVCONTAINER_RUN) .devcontainer/run-integration-tests.sh
52 changes: 52 additions & 0 deletions .devcontainer/run-integration-tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#!/bin/sh
# Build mod_datadog (with RUM) and run the full integration-test suite.
# Assumes it runs inside the devcontainer image (Alpine + musl sysroot + httpd
# at /httpd/httpd-build/ + uv).
#
# If MODULE_PATH is set in the environment, the cmake build is skipped and
# pytest is pointed at the pre-built module at that path. CI uses this to
# reuse the artifact produced by the build:$ARCH job instead of rebuilding
# from source inside the test job.
set -eux

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

ARCH="$(uname -m)"

# Use a container-specific build tree to avoid colliding with host builds
# (host cmake would use Unix Makefiles; the ci-release preset uses Ninja).
BUILD_DIR="$REPO_ROOT/build-container"
DIST_DIR="$REPO_ROOT/dist-container"

# Repo root is bind-mounted from the host; cmake complains otherwise.
git config --global --add safe.directory "$REPO_ROOT"

if [ -z "${MODULE_PATH:-}" ]; then
cmake --preset=ci-release \
-DHTTPD_DATADOG_ENABLE_RUM=ON \
-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"

MODULE_PATH="$DIST_DIR/lib/mod_datadog.so"
fi

cd test/integration-test

# Keep the venv outside the bind-mounted repo so a host-side `uv sync`
# (darwin/arm64 wheels) can't shadow the container's Linux one.
export UV_PROJECT_ENVIRONMENT=/root/.venv-httpd-datadog-tests

uv sync

# Pytest's early arg parser runs BEFORE conftest registers `--bin-path`.
# If we pass `--bin-path VALUE` (space-separated), that parser treats the
# value as a positional test path and uses its parent as rootdir, which
# then prevents conftest from loading. The `=` syntax keeps the option and
# its value in a single token and avoids that race.
uv run pytest \
--bin-path=/httpd/httpd-build/bin/apachectl \
--module-path="$MODULE_PATH" \
--log-dir="$REPO_ROOT/logs" \
-v "$@"
10 changes: 8 additions & 2 deletions .github/workflows/dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ jobs:
needs: format
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)?

# (hash-computed by .gitlab/devcontainer.yml). Bump by copying a newer tag
# via `docker pull registry.ddbuild.io/ci/httpd-datadog:<hash> && docker tag
# ... && docker push datadog/docker-library:httpd-datadog-ci-<hash>`.
image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd
steps:
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
Expand All @@ -44,7 +47,10 @@ jobs:
needs: build
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
# (hash-computed by .gitlab/devcontainer.yml). Bump by copying a newer tag
# via `docker pull registry.ddbuild.io/ci/httpd-datadog:<hash> && docker tag
# ... && docker push datadog/docker-library:httpd-datadog-ci-<hash>`.
image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd
env:
DD_ENV: ci
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ jobs:
build:
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
# (hash-computed by .gitlab/devcontainer.yml). Bump by copying a newer tag
# via `docker pull registry.ddbuild.io/ci/httpd-datadog:<hash> && docker tag
# ... && docker push datadog/docker-library:httpd-datadog-ci-<hash>`.
image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd
steps:
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/system-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ jobs:
build-artifacts:
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
# (hash-computed by .gitlab/devcontainer.yml). Bump by copying a newer tag
# via `docker pull registry.ddbuild.io/ci/httpd-datadog:<hash> && docker tag
# ... && docker push datadog/docker-library:httpd-datadog-ci-<hash>`.
image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd
steps:
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@
compile_commands.json

__pycache__/
build-container/
build-rum/
build/
dist-container/
httpd-*/
httpd/
/logs/
venv/

test/integration-test/.coverage
Expand Down
Loading
Loading