-
Notifications
You must be signed in to change notification settings - Fork 159
feat: Add Dockerfile for OTEL enabled container images #318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Copy/pasting is very much the name of the game here since the images are fundamentally meant to be identical to the upstream Docker images. I've made a couple comments but it seems almost ready to go!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the other comment I left, can you also update https://github.com/nginx/docker-nginx-unprivileged/pull/318/files#diff-6456e6e208552591349a09da1db573bb5de5fdfe76776686baf7558f547f4287L31 to grep the eight iteration of mainline (grep -m7
to grep -m8
), and similarly, change the Debian workflow to grep the seventh iteration of mainline? The GH workflow won't work correctly otherwise.
… `FROM` instruction
Co-authored-by: Copilot <[email protected]>
Co-Authored-By: Alessandro Fael Garcia <[email protected]>
There was a problem hiding this 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 support for building unprivileged NGINX container images with OpenTelemetry (OTEL) modules, including template and CI pipeline updates.
- Extend
update.sh
to generate OTEL-enabled Dockerfiles and inject OTEL versions into package lists - Introduce new OTEL-specific Dockerfile templates and generated Dockerfiles for both Debian and Alpine images
- Update GitHub Actions workflows to build and publish OTEL variants alongside existing images
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
update.sh | Added otel version map, extended get_packages , get_packagerepo , and get_buildtarget for OTEL |
Dockerfile-debian.template Dockerfile-debian-perl.template |
Fixed spelling and updated template comments for OTEL support |
Dockerfile-debian-otel.template Dockerfile-alpine-otel.template |
New OTEL templates for Debian and Alpine |
stable/debian-otel/Dockerfile stable/alpine-otel/Dockerfile |
Generated OTEL-enabled Dockerfiles for stable branch |
mainline/debian-otel/Dockerfile mainline/alpine-otel/Dockerfile |
Generated OTEL-enabled Dockerfiles for mainline branch |
.github/workflows/debian-stable.yml | Added OTEL build job and adjusted version parsing for stable Debian |
.github/workflows/debian-mainline.yml | Added OTEL build job and adjusted version parsing for mainline Debian |
.github/workflows/alpine-stable.yml | Added OTEL build job and adjusted version parsing for stable Alpine |
.github/workflows/alpine-mainline.yml | Added OTEL build job and adjusted version parsing for mainline Alpine |
Comments suppressed due to low confidence (4)
update.sh:33
- [nitpick] The local associative array
otel
shadows the global OTEL version map. Consider renaming the local variable tootel_pkg
or similar to avoid confusion.
declare -A otel=(
.github/workflows/debian-stable.yml:255
- [nitpick] The new OTEL job duplicates much of the core workflow. Consider extracting common steps into a reusable workflow or using YAML anchors to reduce duplication.
otel:
update.sh:32
- [nitpick] Add a brief comment explaining the purpose of the
otel
associative array and how it's used in package generation for future maintainers.
# Current otel versions
update.sh:127
- The loop
for p in $otel
will run once with an empty string if no OTEL package is set. Wrap this loop in anif [ -n "$otel" ]
guard to prevent unintended empty iterations.
for p in $otel; do
Sorry, hadn't noticed that |
Proposed changes
Add support for building unprivileged images with OTEL support.
Checklist
Before creating a PR, run through this checklist and mark each as complete:
update.sh
script and ensured all entrypoint/Dockerfile template changes have been applied to the relevant image entrypoint scripts & DockerfilesREADME
for more details)README.md
)Additional Notes
-otel
images from docker-nginx (lots of copy & paste) without much original input from my side.Would close #263