fix flb_utils test in docker environments #11098
Conversation
Signed-off-by: dietWall <[email protected]>
dietWall
requested review from
edsiper,
niedbalski and
patrick-stephens
as code owners
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "mounts": [ | ||
| "source=/etc/machine-id,target=/etc/machine-id,ro,type=bind,consistency=cached" | ||
| ], |
There was a problem hiding this comment.
The new mounts entry assumes /etc/machine-id exists on the host, but many contributors use Docker Desktop on macOS or Windows where that file is absent. In those environments docker run will refuse to start the dev container because the bind source does not exist, rendering the default devcontainer unusable. This solves the WSL case but regresses other platforms; consider making the mount optional or creating the file when present instead of requiring it globally.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.devcontainer/devcontainer.json (1)
36-38: Consider documenting the new host-file requirement.The addition of a
/etc/machine-idmount introduces a dependency on this file existing on the host system. It would be helpful to document this requirement (e.g., inREADME.mdor a devcontainer setup guide) so developers on systems without this file or with it in a different location understand why the setup might fail or need adjustment.
| "mounts": [ | ||
| "source=/etc/machine-id,target=/etc/machine-id,ro,type=bind,consistency=cached" | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
Verify devcontainer mounts behavior when /etc/machine-id is absent.
The new mounts configuration binds /etc/machine-id from the host into the container. This directly addresses the stated problem of unstable machine-id in the test environment. However, confirm the following:
- Syntax correctness: Verify the mount string format is valid for devcontainer (it appears correct, but consult official devcontainer spec if unsure).
-
Host file existence: Clarify the behavior if
/etc/machine-iddoes not exist on the host system—will the container fail to start, or will the mount silently fail/be skipped? -
Alternative locations: The PR description notes some systems have
machine-idin/var/lib/dbus/machine-id. Consider whether a second mount entry would improve robustness, or document this known limitation.
The fix looks sound for the target environment (WSL/devcontainer). Run this verification to confirm the mount syntax is correct per the devcontainer specification:
🏁 Script executed:
#!/bin/bash
# Description: Verify the mounts syntax in devcontainer.json against the spec.
# Check if the JSON is valid
jq empty .devcontainer/devcontainer.json && echo "JSON is valid"
# Extract and display the mounts section for manual inspection
jq '.mounts' .devcontainer/devcontainer.jsonLength of output: 113
🏁 Script executed:
#!/bin/bash
# Display the first ~10 lines to identify the parse error at line 5, col 4
echo "=== First 15 lines of devcontainer.json ==="
head -15 .devcontainer/devcontainer.json
echo -e "\n=== Lines 30-45 (mounts section) ==="
sed -n '30,45p' .devcontainer/devcontainer.json
echo -e "\n=== Full file with line numbers ==="
cat -n .devcontainer/devcontainer.jsonLength of output: 2340
🌐 Web query:
devcontainer.json mounts syntax specification format
💡 Result:
Short answer
- In devcontainer.json, "mounts" is an array. Each entry may be:
- A string using Docker CLI --mount syntax (key=value pairs separated by commas): e.g. "source=/host/path,target=/container/path,type=bind,consistency=cached". [1]
- Or a JSON object with fields like type, source, target (and optional bind/volume/tmpfs options): e.g. { "type": "volume", "source": "my-vol", "target": "/data" }. [2]
Notes
- The string form accepts the same options as Docker's --mount (and supports VS Code variable interpolation like ${localEnv:...} and ${localWorkspaceFolder}). [1][3]
- Use bind for host paths, volume for named volumes, tmpfs for tmpfs, etc.; consistency/read_only and other mount-specific options follow Docker semantics. [1][2]
Sources
[1] VS Code / devcontainers docs (mount examples and --mount string form).
[2] microsoft/vscode devcontainers examples & issues (object-form example and fields).
[3] VS Code docs / metadata reference (notes on interpolation and behavior).
🌐 Web query:
devcontainer.json comments support JSON5
💡 Result:
Short answer: Yes — devcontainer.json supports comments, but via JSONC (JSON with comments), not necessarily full JSON5 syntax. Use // or /* */ comments and keep the file as JSONC; if you run a linter, set its file type to JSONC (not strict JSON). [1][2][3]
Sources:
- VS Code / Dev Containers docs: devcontainer.json is "JSONC (JSON with comments)". [1]
- GitHub Codespaces docs: devcontainer.json written using JSONC; linters should be set to JSONC. [2]
- JSON5 spec (for comparison): JSON5 allows comments plus other extensions (unquoted keys, trailing commas, etc.). [3]
---
🌐 Web query:
```web
Docker devcontainer bind mount fails if source file missing behavior
💡 Result:
Short answer
- Docker bind mounts require the host path (file or directory) to exist when using --mount (the daemon will error if the source is missing). VS Code Dev Containers passes mounts through to Docker, so you get the same failure. [1][2][3]
- Behavior differences: the legacy -v/--volume syntax will create a missing host path (as a directory) in some cases; the --mount long form will not — it errors instead. For swarm services a host bind path must exist on every node. [3][2]
What to do - Create the file/path on the host before opening the devcontainer (mkdir/touch or a prep script), or use a named Docker volume/volume mount instead of a host-file bind if you need automatic creation. [4][2]
Sources
- VS Code / devcontainer example showing Docker error when source empty. [turn0search0]
- "Invalid mount config for type 'bind': bind source path does not exist" — documentation/answers explaining host path must exist (and swarm behavior). [turn1search0]
- Explanation of difference between -v/--volume and --mount (--mount does not auto-create; -v may). [turn1search4]
- Practical guidance (create file beforehand / scripts) and examples. [turn1search3]
Docker bind mount will fail container startup if /etc/machine-id is missing on the host.
The mount syntax is valid per devcontainer specification, using correct Docker --mount format. However, Docker's --mount (used by Dev Containers) requires the source path to exist on the host—if /etc/machine-id is absent, the devcontainer will error during startup rather than silently skip the mount.
This breaks the devcontainer for systems without /etc/machine-id (noted in the PR as a known limitation on some systems). Consider:
- Document the requirement clearly in README or devcontainer.json comments so users on affected systems understand they need to create the file.
- Use a named volume instead of a bind mount if a fallback for missing host files is needed.
- Add a setup script to create
/etc/machine-idif missing (though this may not align with the read-only intent).
🤖 Prompt for AI Agents
.devcontainer/devcontainer.json lines 36-38: the current bind mount of
/etc/machine-id will cause container startup to fail on hosts where that file
doesn't exist; update the setup to avoid a hard host bind or document the
requirement. Fix options: (1) replace the host bind with a named volume (e.g.,
source=devcontainer-machine-id,type=volume,target=/etc/machine-id) and add a
postStartCommand or Dockerfile step to populate the file inside the container if
the volume is empty; or (2) remove the mount and document in README that
/etc/machine-id must exist on the host before starting the devcontainer (with
clear creation instructions), or (3) add a devcontainer postCreateCommand script
that checks for /etc/machine-id and creates it on the host prior to mounting (if
you choose to keep the bind) — implement one of these fixes and update
devcontainer.json and README accordingly.
1 participant
This addition adds the file /etc/machine-id from the host to the devcontainer configuration with read-only permissions.
In my environment: devcontainer on wsl, the test flb_utils has failed consistently. Reason was, that without this file fluent-bit have falled back to a random generated machine-id. As this is read twice, 2 random generated uuids never matched.
However, this might yet fail on some other systems. as some other OSes might have the machine-id in another location. This needs to be added.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
Note: This release contains only development infrastructure updates with no user-facing changes.