publish docker-in-docker (dind) image #4838
Conversation
WalkthroughThe changes update multiple components of the project. The custom dictionary file has been modified by adding and re-adding certain entries. The Makefile now supports new, granular Docker image build and push targets for multi-platform builds. A buildkite script has been revised with a renamed function and adjusted commands. New Dockerfiles and accompanying README documents have been introduced for both the Bacalhau Base and DinD images, and a new entrypoint script for the DinD image implements a startup sequence that checks for privileged mode. An obsolete Bacalhau image README file has been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Container as Container
participant EP as Entrypoint Script
participant DD as Docker Daemon
participant BC as Bacalhau Binary
Container->>EP: Execute entrypoint.sh
EP->>EP: Check for privileged mode (iptables access)
alt Privileged mode detected
EP->>DD: Start Docker daemon (background)
loop Polling for readiness (up to 30s)
EP->>DD: Request docker info
end
EP->>BC: Execute Bacalhau command with arguments
else Not privileged
EP->>Container: Output error message and exit
end
sequenceDiagram
participant BK as Buildkite
participant S as bacalhau_image.sh
participant M as Makefile
BK->>S: Trigger build process
S->>S: Execute setup_buildx (renamed function)
S->>S: Download artifacts (echo start message)
alt BUILDKITE_TAG is present
S->>M: Run "push-bacalhau-images" target
else
S->>S: Skip image build and echo status
end
Poem
Tip 🌐 Web search-backed reviews and chat
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docker/bacalhau-base/Dockerfile (1)
8-8: Consider Using COPY Instead of ADD.
The Dockerfile uses anADDinstruction at line 8 to copy the binary into the image. Docker best practices generally recommend usingCOPYfor local files since it has more predictable semantics. Consider switching to:COPY bin/$TARGETPLATFORM/bacalhau /usr/local/bin/bacalhau🧰 Tools
🪛 Hadolint (2.12.0)
[error] 8-8: Use COPY instead of ADD for files and folders
(DL3020)
docker/bacalhau-dind/Dockerfile (1)
12-14: Consider Replacing ADD with COPY.
At line 14 the Dockerfile uses:ADD bin/$TARGETPLATFORM/bacalhau /usr/local/bin/bacalhauDocker best practices advise using
COPYfor local files. If no special features ofADDare required, consider replacing it with:COPY bin/$TARGETPLATFORM/bacalhau /usr/local/bin/bacalhau🧰 Tools
🪛 Hadolint (2.12.0)
[error] 14-14: Use COPY instead of ADD for files and folders
(DL3020)
docker/bacalhau-dind/README.md (2)
16-18: Specify Language for Fenced Code Blocks.
For better syntax highlighting and clarity, consider adding a language identifier (e.g.,bash) to fenced code blocks. For instance, change:
to:bash
This applies to the code block in lines 16–18 (and similar blocks throughout the document).
68-69: Improve Error Message Wording.
The section beginning with "If you see this error:" could be strengthened by rephrasing to "If you encounter this error:" to provide a more assertive tone.🧰 Tools
🪛 LanguageTool
[style] ~68-~68: Consider an alternative verb to strengthen your wording.
Context: ...n't allowed ## Troubleshooting If you see this error: ``` ERROR: This container m...(IF_YOU_HAVE_THIS_PROBLEM)
🪛 markdownlint-cli2 (0.17.2)
69-69: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.cspell/custom-dictionary.txt(1 hunks)Makefile(1 hunks)buildkite/scripts/bacalhau_image.sh(2 hunks)docker/bacalhau-base/Dockerfile(1 hunks)docker/bacalhau-base/README.md(1 hunks)docker/bacalhau-dind/Dockerfile(1 hunks)docker/bacalhau-dind/README.md(1 hunks)docker/bacalhau-dind/entrypoint.sh(1 hunks)docker/bacalhau-image/README.md(0 hunks)
💤 Files with no reviewable changes (1)
- docker/bacalhau-image/README.md
✅ Files skipped from review due to trivial changes (2)
- .cspell/custom-dictionary.txt
- docker/bacalhau-base/README.md
🧰 Additional context used
🪛 LanguageTool
docker/bacalhau-dind/README.md
[style] ~68-~68: Consider an alternative verb to strengthen your wording.
Context: ...n't allowed ## Troubleshooting If you see this error: ``` ERROR: This container m...
(IF_YOU_HAVE_THIS_PROBLEM)
🪛 markdownlint-cli2 (0.17.2)
docker/bacalhau-dind/README.md
69-69: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 Hadolint (2.12.0)
docker/bacalhau-dind/Dockerfile
[error] 14-14: Use COPY instead of ADD for files and folders
(DL3020)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: build / Build Binary (windows, amd64)
- GitHub Check: build / Build Binary (darwin, arm64)
- GitHub Check: build / Build Binary (darwin, amd64)
- GitHub Check: build / Build Binary (linux, armv6)
- GitHub Check: build / Build Binary (linux, armv7)
- GitHub Check: build / Build Binary (linux, arm64)
- GitHub Check: lint / lint
- GitHub Check: lint / go-lint (ubuntu-latest)
- GitHub Check: build / Build Binary (linux, amd64)
🔇 Additional comments (23)
docker/bacalhau-base/Dockerfile (1)
10-18: Standardizing PATH, ENTRYPOINT, and LABEL Instructions.
The updates set theENV PATH(line 10) to include both/usr/local/binand/usr/binand define the container’s entrypoint to runbacalhau(line 12). The LABEL instructions (lines 14–18) are also reformatted to use the=for assignment, which improves consistency. Ensure that thebacalhaubinary is present at the expected location.docker/bacalhau-dind/entrypoint.sh (4)
1-8: Privileged Mode Check Implemented Correctly.
The script correctly verifies privileged mode by testing access toiptables. The error messages provide clear guidance on how to run the container properly.
10-12: Docker Daemon Startup.
The Docker daemon is launched in the background usingdockerd-entrypoint.sh dockerd &. Double-check that thedockerd-entrypoint.shscript is available in the image’s PATH.
13-19: Effective Timeout Mechanism.
The use oftimeoutwith a polling loop to check fordocker infoprovides a clear and practical way to wait for the Docker daemon. The subsequent check of the exit status (lines 16–19) properly handles timeout failures.
21-28: Delegating to Bacalhau Correctly.
Extracting the bacalhau binary path from the first argument and usingexecto run it (lines 23–28) is an effective pattern that replaces the shell with the intended process.docker/bacalhau-dind/Dockerfile (3)
4-8: Installing Essential Packages.
The Dockerfile installs necessary utilities such ascurl,bash, andcoreutilsfrom Alpine's repositories usingapk. This ensures the image contains all needed dependencies.
17-19: Configuring the Custom Entrypoint.
Copying the custom entrypoint script into/usr/local/bin/(line 17) and making it executable (line 18) is correctly implemented.
20-26: Entrypoint and Metadata Setup.
TheENTRYPOINTdirective (line 20) passesbacalhauas an argument to the custom script, ensuring the proper command is executed. Additionally, the metadata LABELs (lines 22–26) provide clear project details.buildkite/scripts/bacalhau_image.sh (3)
14-17: Renamed Buildx Setup Function.
The function formerly known asdocker_context_createis now renamed tosetup_buildx(lines 14–17), which more accurately reflects its purpose. The logic for creating and selecting the Docker Buildx context is clear.
32-38: Improved Artifact Download Logging.
The updated echo statement at the start of thedownload_artifactsfunction (line 33) improves clarity by indicating when artifact downloading commences. The subsequent artifact download and extraction logic is well-structured.
44-53: Refined Main Function for Image Build and Push.
Within themainfunction, the echo message on line 45 now explicitly announces the start of image building and pushing when aBUILDKITE_TAGis present. Furthermore, the call tomake push-bacalhau-images(line 50) aligns with updated Makefile targets. The alternative branch for cases without a tag is clearly communicated (line 52).docker/bacalhau-dind/README.md (1)
1-77: Comprehensive Documentation Provided.
The README thoroughly explains the purpose, usage, features, and troubleshooting procedures for the Bacalhau DinD image. The structure is well-organized, making it easy for users to understand when and how to deploy this image.🧰 Tools
🪛 LanguageTool
[style] ~68-~68: Consider an alternative verb to strengthen your wording.
Context: ...n't allowed ## Troubleshooting If you see this error: ``` ERROR: This container m...(IF_YOU_HAVE_THIS_PROBLEM)
🪛 markdownlint-cli2 (0.17.2)
69-69: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
Makefile (11)
236-240: Review of "build-http-gateway-image" target:
The inclusion of the--loadflag enables the built image to be immediately available locally. However, note that when building for multiple platforms (i.e.linux/amd64,linux/arm64),--loadwill only load the image for the current host’s architecture. Please verify that this behavior is acceptable for your use case.
241-247: Review of "push-http-gateway-image" target:
The push target is correctly configured using--pushalong with the multi-platform flag. This ensures that the image is pushed to the registry for all specified architectures.
253-261: Validation of Semver Tag Condition:
The conditional block usinggrep -E '^v[0-9]+\.[0-9]+\.[0-9]+$$'effectively checks for a valid semver tag, and then assigns additional tags (latestandlatest-dind). Confirm that all tags in your repository adhere to this semver format to avoid any unintended behavior.
263-267: Review of BACALHAU_IMAGE_FLAGS Definition:
TheBACALHAU_IMAGE_FLAGSnow includes a label for the image version (org.opencontainers.image.version). This is a useful addition for tracking image metadata. Please ensure that the dynamic timestamp and version values are as expected when the build is executed.
268-275: Review of "build-bacalhau-base-image" target:
This target builds the Bacalhau base image usingdocker buildx build --loadalong with caching and tagging options. One point to note is that using--loadmay restrict multi-platform support to the local platform. Verify that building locally (without explicit multi-platform output) meets your requirements for testing or development.
276-283: Review of "build-bacalhau-dind-image" target:
Similar to the base image target, the dind image build uses--loadwhile applying the appropriate dind-specific tags and caching mechanisms. Consider whether limiting the loaded image to the host architecture is intentional, or if a multi-platform build is desired for local usage.
284-293: Review of "push-bacalhau-base-image" target:
The push target for the base image is well-structured, employingdocker buildx build --pushwith the multi-platform flag and caching options. This setup should efficiently publish the image for all supported architectures.
294-302: Review of "push-bacalhau-dind-image" target:
The dind push target mirrors the base image push configuration and correctly handles multi-platform image publishing with caching. It is consistent and clear in its implementation.
303-306: Review of Combined Build Target "build-bacalhau-images":
Consolidating the build of both the Bacalhau base and dind images into a single target improves usability and simplifies the build workflow. This abstraction is clear and aligns with the goal of granular target operations.
307-309: Review of Combined Push Target "push-bacalhau-images":
Grouping the push targets for the base and dind images under a combined target increases clarity and ease-of-use. This approach maintains consistency with the granularity introduced by the new targets.
310-312: Review of "push-docker-images" target:
The updatedpush-docker-imagestarget now delegates topush-http-gateway-image, reflecting the new target structure. Ensure that any documentation or dependent scripts referencing this target are updated accordingly to avoid confusion.
I hear your point. The thing is this is what we call in buildkite to publish the images and we can't ask for approval. The good thing is we only tag these images with latest if they are part of a release. Trying to push locally will just add a tag such as |
|
@wdbaruni I was thinking of having a "-y" flag to support script usage. But that is only a detail . |
Summary by CodeRabbit
New Features
Documentation
Chores