Skip to content

Profiling images can now be generated from crossbuild images#5834

Open
marc-casavant wants to merge 2 commits intoFreeRADIUS:masterfrom
marc-casavant:new-profiling-image-build
Open

Profiling images can now be generated from crossbuild images#5834
marc-casavant wants to merge 2 commits intoFreeRADIUS:masterfrom
marc-casavant:new-profiling-image-build

Conversation

@marc-casavant
Copy link
Copy Markdown
Contributor

@marc-casavant marc-casavant commented Apr 22, 2026

Key Changes:

  • Profiling images are built FROM <CROSSBULD_IMAGE>
  • Profiling images install useful profiling tools
  • Profiling images do not build FreeRADIUS

Build Steps Example:

Generate the crossbuild Dockerfile.cb:
make crossbuild.ubuntu24.regen

Build the crossbuild base image:
make crossbuild.ubuntu24.build

Generate the profiling Dockerfile.prof:
make crossbuild.ubuntu24.profile.regen

Build the profiling image:
make crossbuild.ubuntu24.profile.build

Copy link
Copy Markdown
Member

@mcnewton mcnewton left a comment

Choose a reason for hiding this comment

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

few things to look at

Comment thread scripts/docker/crossbuild.mk Outdated
Comment thread scripts/docker/m4/profiling.deb.m4 Outdated
Comment thread scripts/docker/crossbuild.mk Outdated
@echo " crossbuild.IMAGE.clean - stop container and tidy up"
@echo " crossbuild.IMAGE.wipe - remove Docker image"
@echo ""
@echo "Profiling targets:"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not entirely convinced these should be in the help. They should be used by the CI jobs only, it's not helpful to have extra targets for developers that they should never use.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As a minor point, help is useful. :) Especially if I want to run profiling tools locally.

As seen here, it's a bit of a pain to get the profiling builds up and running. It's a lot easier to do make profiling

So I'm OK with leaving this in.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed on the ones that developers use. But some of these targets are really only for CI, which might be confusing if a developer tries to use them. They're the ones I suggest not adding to help :)

As long as it's clear, then np.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the targets are only for CI, then there's no need to print them out in the help text.

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.

I kinda like having those in the help text :) Helps my brain, but I'm okay to remove the extra info in the help text is that's what we prefer.

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.

I believe the targets are useful for developers, especially when used with profiling multi-server tests.

Comment thread scripts/docker/crossbuild.mk Outdated
Comment thread scripts/docker/docker.mk Outdated
@marc-casavant marc-casavant force-pushed the new-profiling-image-build branch 3 times, most recently from 21643c9 to 64edef6 Compare April 27, 2026 21:26
@arr2036 arr2036 self-requested a review April 29, 2026 17:05
@marc-casavant marc-casavant force-pushed the new-profiling-image-build branch from 34872f1 to f8f3881 Compare May 1, 2026 18:27
@marc-casavant marc-casavant force-pushed the new-profiling-image-build branch from f8f3881 to ff45c05 Compare May 4, 2026 14:25
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