Skip to content
Closed
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
113 changes: 113 additions & 0 deletions scripts/docker/build/ubuntu24/Dockerfile.prof
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
# Auto generated for ubuntu24
Comment thread
marc-casavant marked this conversation as resolved.
# from scripts/docker/m4/profiling.deb.m4
#
# Rebuild this file with `make crossbuild.ubuntu24.profile.regen`
#
Comment thread
marc-casavant marked this conversation as resolved.
ARG from=freeradius40x-build/ubuntu24
FROM ${from}

#
# Install profiling tools
#
# Valgrind/cachegrind
# kcachegrind + KDE/Qt libs
# gperftools
# heaptrack
#
RUN apt-get update && \
apt-get install -y $APT_OPTS \
libgoogle-perftools-dev \
google-perftools \
valgrind \
heaptrack \
psmisc \
kcachegrind \
kio \
libkf5iconthemes5 \
libkf5parts5 \
libkf5textwidgets5 \
libqt5gui5 \
libqt5widgets5 && \
apt-get clean && \
rm -r /var/lib/apt/lists/*

#
# Set up Ubuntu debug symbol repository and install OS library debug symbols.
# These allow profiling tools to resolve system library calls.
#
RUN apt-get update && \
apt-get install -y $APT_OPTS ubuntu-dbgsym-keyring && \
printf 'deb http://ddebs.ubuntu.com noble main restricted universe multiverse\ndeb http://ddebs.ubuntu.com noble-updates main restricted universe multiverse\n' \
> /etc/apt/sources.list.d/ddebs.list && \
apt-get update && \
for pkg in \
libc6-dbg \
zlib1g-dbgsym \
libreadline8t64-dbgsym \
libssl3t64-dbgsym \
libsasl2-2-dbgsym \
libpam0g-dbgsym \
libldap2-dbgsym \
libtalloc2-dbgsym \
libpcre2-8-0-dbgsym \
libpcap0.8t64-dbgsym \
libunbound8-dbgsym \
libsqlite3-0-dbgsym \
libpq5-dbgsym \
libmariadb3-dbgsym \
libgdbm6t64-dbgsym \
libjson-c5-dbgsym \
libbrotli1-dbgsym \
libhiredis1.1.0-dbgsym \
librdkafka1-dbgsym \
libwbclient0-dbgsym \
libcurl4t64-dbgsym; do \
apt-get install -y $APT_OPTS $pkg || echo "WARNING: could not install dbgsym package: $pkg"; \
done && \
apt-get clean && \
rm -r /var/lib/apt/lists/*

#
# Rebuild libkqueue from source with debug symbols.
# CMAKE_BUILD_TYPE must be explicitly set to prevent
# the libkqueue project from overriding the flags.
# libkqueue source currently uses "-O2 -g -DNDEBUG" by default.
#
RUN apt-get update && \
apt-get install -y $APT_OPTS --no-install-recommends cmake git && \
apt-get clean && \
rm -r /var/lib/apt/lists/* && \
git clone --depth 1 https://github.com/mheily/libkqueue.git /tmp/libkqueue && \
cd /tmp/libkqueue && \
cmake -G "Unix Makefiles" \
-DCMAKE_INSTALL_PREFIX=/usr \
-DCMAKE_INSTALL_LIBDIR=lib \
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DCMAKE_C_FLAGS_RELWITHDEBINFO="-g3 -O1 -fno-omit-frame-pointer -DNDEBUG" \
. && \
make && \
cpack -G DEB && \
dpkg -i *.deb && \
cd / && rm -rf /tmp/libkqueue

#
# Install FlameGraph
#
RUN git clone --depth 1 https://github.com/brendangregg/FlameGraph /opt/flamegraph \
&& chmod +x /opt/flamegraph/*.pl /opt/flamegraph/*.sh

# Add FlameGraph to path
ENV PATH="/opt/flamegraph:${PATH}"

#
# Install Inferno for callgrind. Inferno is a Rust port of FlameGraph with broader format support
#
RUN apt-get update && \
apt-get install -y $APT_OPTS --no-install-recommends \
cargo && \
cargo install inferno --version 0.11.21 --locked --root /usr/local && \
apt-get clean && \
rm -r /var/lib/apt/lists/*

EXPOSE 1812/udp 1813/udp
CMD ["/bin/sh", "-c", "while true; do sleep 60; done"]
64 changes: 64 additions & 0 deletions scripts/docker/crossbuild.mk
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ endif
CB_IPREFIX:=freeradius40x-build
CB_CPREFIX:=fr40x-crossbuild-

PROFILE ?= default-profiling

#
# This Makefile is included in-line, and not via the "boilermake"
# wrapper. But it's still useful to use the same process for
Expand Down Expand Up @@ -92,6 +94,11 @@ crossbuild.help: crossbuild.info
@echo " crossbuild.IMAGE.clean - stop container and tidy up"
@echo " crossbuild.IMAGE.distclean - remove Docker image"
@echo ""
@echo "Profiling targets (currently only for ubuntu24):"
@echo " crossbuild.IMAGE.profile.regen - regenerate Dockerfile.prof"
@echo " crossbuild.IMAGE.profile.build - build profiling image"
@echo " crossbuild.IMAGE.profile.reset - remove profiling stamp to force rebuild"
@echo ""
@echo "Use 'make NOCACHE=1 ...' to disregard the Docker cache on build"

#
Expand Down Expand Up @@ -147,6 +154,27 @@ $(DD)/stamp-image.${1}:
${Q}docker build $(DOCKER_BUILD_OPTS) $(if $(CB_FROM_${1}),--build-arg=from=$(CB_FROM_${1})) $(DT)/${1} -f $(DT)/${1}/Dockerfile.cb -t $(CB_IPREFIX)/${1} >$(DD)/build.${1} 2>&1
${Q}touch $(DD)/stamp-image.${1}


#
# Build the profiling image
#

# ubuntu24 profiling target start
ifeq (${1},ubuntu24)

crossbuild.${1}.profile.build: $(DD)/stamp-image.${1}-profile.build

$(DD)/stamp-image.${1}-profile.build: $(DD)/stamp-image.${1} $(DT)/${1}/Dockerfile.prof
${Q}echo "BUILD ${1} (freeradius4-$(PROFILE)/${1}) > $(DD)/build.${1}-profile.build"
${Q}docker build $(DOCKER_BUILD_OPTS) . \
-f $(DT)/${1}/Dockerfile.prof \
-t freeradius4-$(PROFILE)/${1} \
Comment on lines +158 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The new crossbuild.IMAGE.profile.build rule only depends on Dockerfile.prof, not on the base crossbuild image stamp $(DD)/stamp-image.${1}. Since Dockerfile.prof starts with FROM freeradius40x-build/<distro> (a local-only image tag), running make crossbuild.ubuntu24.profile.build on a clean checkout fails because docker tries to pull the non-existent image from a remote registry. Add $(DD)/stamp-image.${1} as a prerequisite of $(DD)/stamp-image.${1}-profile.build so the base image is built first.

Extended reasoning...

What the bug is

The rule introduced at scripts/docker/crossbuild.mk:160:

$(DD)/stamp-image.${1}-profile.build: $(DT)/${1}/Dockerfile.prof
        ${Q}echo "BUILD ${1} (freeradius4-$(PROFILE)/${1}) > $(DD)/build.${1}-profile.build"
        ${Q}docker build $(DOCKER_BUILD_OPTS) . \
                -f $(DT)/${1}/Dockerfile.prof \
                -t freeradius4-$(PROFILE)/${1} \
                >$(DD)/build.${1}-profile.build 2>&1
        ${Q}touch $(DD)/stamp-image.${1}-profile.build

declares only $(DT)/${1}/Dockerfile.prof as a prerequisite. However, the generated Dockerfile.prof (see scripts/docker/build/ubuntu24/Dockerfile.prof:6-7 and scripts/docker/m4/profiling.deb.m4:1-2) begins with:

ARG from=freeradius40x-build/ubuntu24
FROM ${from}

This is a local-only image tag, produced by the $(DD)/stamp-image.${1} rule. The docker build invocation passes no --build-arg to override from, so docker will look for freeradius40x-build/ubuntu24 locally and, on a clean checkout where this image has not been built, fall through to a registry pull which fails (e.g. pull access denied for freeradius40x-build/ubuntu24).

Why existing rules don't prevent it

Compare with the sibling docker.up.${1} rule a few lines below (around line 174):

$(DD)/docker.up.${1}: $(DD)/stamp-image.${1}

That rule correctly chains on the base image stamp, ensuring the base image is built first. The new profile.build rule simply omits this prerequisite.

Impact

The PR description explicitly advertises make crossbuild.ubuntu24.profile.build as a user-facing workflow, and it is listed in crossbuild.help at line 97. A developer following the PR description on a fresh checkout will hit a confusing failure because docker reports a pull error for what appears to be a local image name, rather than a clear "base image not built yet" message. CI is unaffected because it already runs the base crossbuild step first.

Step-by-step proof

  1. Fresh checkout of this branch — scripts/docker/crossbuild/stamp-image.ubuntu24 does not exist and the docker image freeradius40x-build/ubuntu24 is not present in the local docker daemon.
  2. Run make crossbuild.ubuntu24.profile.build.
  3. Make resolves: crossbuild.ubuntu24.profile.build$(DD)/stamp-image.ubuntu24-profile.build.
  4. The only prerequisite is $(DT)/ubuntu24/Dockerfile.prof, which already exists in the tree, so make proceeds directly to the docker build recipe.
  5. docker build -f scripts/docker/build/ubuntu24/Dockerfile.prof -t freeradius4-default-profiling/ubuntu24 . is invoked.
  6. Docker parses FROM ${from} with from=freeradius40x-build/ubuntu24. No local image with that tag exists, so docker attempts a registry pull.
  7. The pull fails with pull access denied for freeradius40x-build/ubuntu24, repository does not exist or may require 'docker login'. The build aborts and build.ubuntu24-profile.build records the failure.

Fix

Add $(DD)/stamp-image.${1} as a prerequisite of the profile build stamp:

$(DD)/stamp-image.${1}-profile.build: $(DD)/stamp-image.${1} $(DT)/${1}/Dockerfile.prof

This matches the convention used by $(DD)/docker.up.${1} and ensures the base crossbuild image exists before docker tries to consume it as the FROM of the profiling image.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added $(DD)/stamp-image.${1} as a dependency.

>$(DD)/build.${1}-profile.build 2>&1
${Q}touch $(DD)/stamp-image.${1}-profile.build

# ubuntu24 profiling target end
endif

#
# Start up the docker container
#
Expand Down Expand Up @@ -256,6 +284,42 @@ $(DT)/${1}/Dockerfile.cb: $(DOCKER_TMPL) $(CB_DIR)/m4/crossbuild.deb.m4 $(CB_DIR
${Q}echo REGEN ${1}
${Q}m4 -I $(CB_DIR)/m4 -D D_NAME=${1} -D D_TYPE=crossbuild $$< > $$@

#
# Regenerate Dockerfile.prof from m4 template
#

# ubuntu24 profiling target start
ifeq (${1},ubuntu24)

.PHONY: crossbuild.${1}.profile.regen
crossbuild.${1}.profile.regen: $(DT)/${1}/Dockerfile.prof

$(DT)/${1}/Dockerfile.prof: $(DOCKER_TMPL) $(CB_DIR)/m4/profiling.deb.m4
${Q}echo REGEN ${1}
${Q}m4 -I $(CB_DIR)/m4 \
-D D_NAME=${1} \
-D D_TYPE=profiling \
-D CB_IMAGE=$(CB_IPREFIX)/${1} \
$$< > $$@

# ubuntu24 profiling target end
endif

#
# Remove profiling stamp so next profile.build starts clean
#

# ubuntu24 profiling target start
ifeq (${1},ubuntu24)

.PHONY: crossbuild.${1}.profile.reset
crossbuild.${1}.profile.reset:
${Q}echo RESET profiling ${1}
${Q}rm -f $(DD)/stamp-image.${1}-profile.build

# ubuntu24 profiling target end
endif

#
# Run the build test
#
Expand Down
4 changes: 3 additions & 1 deletion scripts/docker/m4/Dockerfile.m4
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ divert[]dnl
[#] Auto generated for D_NAME
[#] from scripts/docker/m4/D_TYPE.PKG_TYPE.m4
[#]
[#] Rebuild this file with `make D_TYPE.D_NAME.regen`
ifelse(D_TYPE, [profiling],
[[#] Rebuild this file with `make crossbuild.D_NAME.profile.regen`],
[[#] Rebuild this file with `make D_TYPE.D_NAME.regen`])
[#]
changequote([`], ['])dnl
include(D_TYPE.PKG_TYPE.m4)dnl
108 changes: 108 additions & 0 deletions scripts/docker/m4/profiling.deb.m4
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
ARG from=CB_IMAGE
FROM ${from}

#
# Install profiling tools
#
# Valgrind/cachegrind
# kcachegrind + KDE/Qt libs
# gperftools
# heaptrack
#
RUN apt-get update && \
apt-get install -y $APT_OPTS \
libgoogle-perftools-dev \
google-perftools \
valgrind \
heaptrack \
psmisc \
kcachegrind \
kio \
libkf5iconthemes5 \
libkf5parts5 \
libkf5textwidgets5 \
libqt5gui5 \
libqt5widgets5 && \
apt-get clean && \
rm -r /var/lib/apt/lists/*

#
# Set up Ubuntu debug symbol repository and install OS library debug symbols.
# These allow profiling tools to resolve system library calls.
#
RUN apt-get update && \
apt-get install -y $APT_OPTS ubuntu-dbgsym-keyring && \
printf 'deb http://ddebs.ubuntu.com OS_CODENAME main restricted universe multiverse\ndeb http://ddebs.ubuntu.com OS_CODENAME-updates main restricted universe multiverse\n' \
> /etc/apt/sources.list.d/ddebs.list && \
apt-get update && \
for pkg in \
libc6-dbg \
zlib1g-dbgsym \
libreadline8t64-dbgsym \
libssl3t64-dbgsym \
libsasl2-2-dbgsym \
libpam0g-dbgsym \
libldap2-dbgsym \
libtalloc2-dbgsym \
libpcre2-8-0-dbgsym \
libpcap0.8t64-dbgsym \
libunbound8-dbgsym \
libsqlite3-0-dbgsym \
libpq5-dbgsym \
libmariadb3-dbgsym \
libgdbm6t64-dbgsym \
libjson-c5-dbgsym \
libbrotli1-dbgsym \
libhiredis1.1.0-dbgsym \
librdkafka1-dbgsym \
libwbclient0-dbgsym \
libcurl4t64-dbgsym; do \
apt-get install -y $APT_OPTS $pkg || echo "WARNING: could not install dbgsym package: $pkg"; \
done && \
apt-get clean && \
rm -r /var/lib/apt/lists/*

#
# Rebuild libkqueue from source with debug symbols.
# CMAKE_BUILD_TYPE must be explicitly set to prevent
# the libkqueue project from overriding the flags.
# libkqueue source currently uses "-O2 -g -DNDEBUG" by default.
#
RUN apt-get update && \
apt-get install -y $APT_OPTS --no-install-recommends cmake git && \
apt-get clean && \
rm -r /var/lib/apt/lists/* && \
git clone --depth 1 https://github.com/mheily/libkqueue.git /tmp/libkqueue && \
cd /tmp/libkqueue && \
cmake -G "Unix Makefiles" \
-DCMAKE_INSTALL_PREFIX=/usr \
-DCMAKE_INSTALL_LIBDIR=lib \
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DCMAKE_C_FLAGS_RELWITHDEBINFO="-g3 -O1 -fno-omit-frame-pointer -DNDEBUG" \
. && \
make && \
cpack -G DEB && \
dpkg -i *.deb && \
cd / && rm -rf /tmp/libkqueue

#
# Install FlameGraph
#
RUN git clone --depth 1 https://github.com/brendangregg/FlameGraph /opt/flamegraph \
&& chmod +x /opt/flamegraph/*.pl /opt/flamegraph/*.sh

# Add FlameGraph to path
ENV PATH="/opt/flamegraph:${PATH}"

#
# Install Inferno for callgrind. Inferno is a Rust port of FlameGraph with broader format support
#
RUN apt-get update && \
apt-get install -y $APT_OPTS --no-install-recommends \
cargo && \
cargo install inferno --version 0.11.21 --locked --root /usr/local && \
apt-get clean && \
rm -r /var/lib/apt/lists/*

EXPOSE 1812/udp 1813/udp
CMD ["/bin/sh", "-c", "while true; do sleep 60; done"]
Loading