Skip to content

Conversation

@sairon
Copy link
Member

@sairon sairon commented Sep 2, 2025

Use the --cidfile Docker CLI argument when starting the container and bind-mount the generated file containing full ID of the container to the container itself.

Using --mount instead of --volume is needed, as --volume is racy and creates empty directory volume at the destination path instead.

This is prerequisite for home-assistant/supervisor#6006 but can come handy for other cases too.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Supervisor startup reliability by removing any stale container ID before launch, preventing single-instance conflicts.
    • Container creation now records its ID to a managed file, enabling more robust lifecycle tracking.
    • The container receives a read-only view of its ID file, aiding diagnostics and ensuring consistent state during restarts and updates.

Use the --cidfile Docker CLI argument when starting the container and
bind-mount the generated file containing full ID of the container to the
container itself.

Using --mount instead of --volume is needed, as --volume is racy and creates
empty directory volume at the destination path instead.

This is prerequisite for home-assistant/supervisor#6006 but can come handy for
other cases too.
@sairon sairon requested a review from agners September 2, 2025 15:36
@sairon sairon added the os label Sep 2, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 2, 2025

📝 Walkthrough

Walkthrough

Adds a SUPERVISOR_CIDFILE variable, removes any existing CID file before container creation, and updates the Docker create command to write the container ID to the CID file and bind-mount it into the container at /run/cid (read-only). Minor formatting changes included.

Changes

Cohort / File(s) Change summary
Supervisor startup script
buildroot-external/rootfs-overlay/usr/sbin/hassos-supervisor
Introduced SUPERVISOR_CIDFILE="${SUPERVISOR_DATA}/hassio_supervisor.cid". Added pre-create cleanup: remove existing CID file if present. Updated docker container create with --cidfile "${SUPERVISOR_CIDFILE}" and --mount type=bind,src="${SUPERVISOR_CIDFILE}",dst=/run/cid,readonly. Minor indentation/comment tweaks.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Init as System Init
    participant Script as hassos-supervisor (script)
    participant Docker as Docker Engine
    participant Ctn as Supervisor Container

    Init->>Script: Start supervisor
    Script->>Script: Define SUPERVISOR_CIDFILE
    alt CID file exists
        Script->>Script: Remove existing CID file
    end
    Script->>Docker: docker create --cidfile CIDFILE<br/>--mount src=CIDFILE,dst=/run/cid,ro …
    Docker-->>Script: Container ID written to CIDFILE
    Docker-->>Ctn: Start container
    Ctn-->>Ctn: Access /run/cid (read-only)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch supervisor-cidfile

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
buildroot-external/rootfs-overlay/usr/sbin/hassos-supervisor (1)

14-14: Introduce a separate temp CID path to avoid bind-mount vs --cidfile conflict

Defining only SUPERVISOR_CIDFILE is fine, but we’ll need a second path for --cidfile to avoid colliding with the bind-mount’s “source must exist” rule (see detailed fix below). Also confirm the directory exists before first use.

Apply:

-SUPERVISOR_CIDFILE="${SUPERVISOR_DATA}/hassio_supervisor.cid"
+SUPERVISOR_CIDFILE="${SUPERVISOR_DATA}/hassio_supervisor.cid"
+# Separate tmp path for --cidfile to avoid conflict with bind mount existence checks.
+SUPERVISOR_CIDFILE_TMP="${SUPERVISOR_CIDFILE}.tmp"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge Base: Disabled due to data retention organization setting

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 700ff77 and 2a08091.

📒 Files selected for processing (1)
  • buildroot-external/rootfs-overlay/usr/sbin/hassos-supervisor (3 hunks)
🔇 Additional comments (1)
buildroot-external/rootfs-overlay/usr/sbin/hassos-supervisor (1)

62-64: Comment tweak LGTM

No functional changes; message reads clearly.

Comment on lines +91 to +94
# We need to remove the CID file here, Docker will refuse to start if the
# file is present. Single instance is ensured by other code paths.
[ -f "${SUPERVISOR_CIDFILE}" ] && rm -f "${SUPERVISOR_CIDFILE}"

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Creation will fail: bind-mounting a non-existent file while using --cidfile to create it

Docker requires the bind source to exist at create time; meanwhile, --cidfile refuses to write if the file already exists. Removing the file (Lines 91–94) and then attempting to bind it (Lines 101–102) in the same docker container create leads to “invalid mount config for type 'bind': bind source path does not exist.”

Fix by:

  • Pre-creating the file used as the bind source.
  • Using a separate temp file for --cidfile.
  • After create, move/copy the temp file’s content into the mounted file before start.

Apply within this block:

-    # We need to remove the CID file here, Docker will refuse to start if the
-    # file is present. Single instance is ensured by other code paths.
-    [ -f "${SUPERVISOR_CIDFILE}" ] && rm -f "${SUPERVISOR_CIDFILE}"
+    # Ensure bind source exists; keep a separate tmp for --cidfile.
+    mkdir -p "$(dirname "${SUPERVISOR_CIDFILE}")"
+    : > "${SUPERVISOR_CIDFILE}"
+    chmod 0644 "${SUPERVISOR_CIDFILE}" || true
+    [ -n "${SUPERVISOR_CIDFILE_TMP}" ] && rm -f "${SUPERVISOR_CIDFILE_TMP}"
-        --cidfile "${SUPERVISOR_CIDFILE}" \
+        --cidfile "${SUPERVISOR_CIDFILE_TMP}" \
         --mount type=bind,src="${SUPERVISOR_CIDFILE}",dst=/run/cid,readonly \

And insert right after the docker container create ... "${SUPERVISOR_IMAGE}:latest" line (before touching SUPERVISOR_STARTSCRIPT_VERSION):

+    # Populate the mounted CID file with the created container ID
+    if [ -s "${SUPERVISOR_CIDFILE_TMP}" ]; then
+        mv -f "${SUPERVISOR_CIDFILE_TMP}" "${SUPERVISOR_CIDFILE}"
+    else
+        echo "[ERROR] CID file was not written by Docker."
+        exit 1
+    fi

Optional: if you prefer not to replace the inode for the mounted file, use cat "${SUPERVISOR_CIDFILE_TMP}" > "${SUPERVISOR_CIDFILE}" && rm -f "${SUPERVISOR_CIDFILE_TMP}".

Also applies to: 101-102

🤖 Prompt for AI Agents
In buildroot-external/rootfs-overlay/usr/sbin/hassos-supervisor around lines
91–94, removing SUPERVISOR_CIDFILE before creating the container causes Docker
to fail because bind sources must exist at create time; instead pre-create the
mount target file and use a separate temporary file for --cidfile during
container create (e.g. SUPERVISOR_CIDFILE_TMP), then after the container is
created copy/move the temp file content into the mounted SUPERVISOR_CIDFILE
before starting the container (or use cat SUPERVISOR_CIDFILE_TMP >
SUPERVISOR_CIDFILE && rm -f SUPERVISOR_CIDFILE_TMP to avoid replacing the
inode).

sairon added a commit to home-assistant/supervisor that referenced this pull request Sep 3, 2025
…un/cid

There is no standard way to get the container ID in the container
itself, which can be needed for instance for #6006. The usual pattern is
to use the --cidfile argument of Docker CLI and mount the generated file
to the container. However, this is feature of Docker CLI and we can't
use it when creating the containers via API. To get container ID to
implement native logging in e.g. Core as well, we need the help of the
Supervisor.

This change implements similar feature fully in Supervisor's DockerAPI
class that orchestrates lifetime of all containers managed by
Supervisor. The files are created in the SUPERVISOR_DATA directory, as
it needs to be persisted between reboots, just as the instances of
Docker containers are.

Supervisor's cidfile must be created when starting the Supervisor
itself, for that see home-assistant/operating-system#4276.
sairon added a commit to home-assistant/supervisor that referenced this pull request Sep 5, 2025
…un/cid

There is no standard way to get the container ID in the container
itself, which can be needed for instance for #6006. The usual pattern is
to use the --cidfile argument of Docker CLI and mount the generated file
to the container. However, this is feature of Docker CLI and we can't
use it when creating the containers via API. To get container ID to
implement native logging in e.g. Core as well, we need the help of the
Supervisor.

This change implements similar feature fully in Supervisor's DockerAPI
class that orchestrates lifetime of all containers managed by
Supervisor. The files are created in the SUPERVISOR_DATA directory, as
it needs to be persisted between reboots, just as the instances of
Docker containers are.

Supervisor's cidfile must be created when starting the Supervisor
itself, for that see home-assistant/operating-system#4276.
agners pushed a commit to home-assistant/supervisor that referenced this pull request Sep 9, 2025
…un/cid (#6154)

* Write cidfiles of Docker containers and mount them individually to /run/cid

There is no standard way to get the container ID in the container
itself, which can be needed for instance for #6006. The usual pattern is
to use the --cidfile argument of Docker CLI and mount the generated file
to the container. However, this is feature of Docker CLI and we can't
use it when creating the containers via API. To get container ID to
implement native logging in e.g. Core as well, we need the help of the
Supervisor.

This change implements similar feature fully in Supervisor's DockerAPI
class that orchestrates lifetime of all containers managed by
Supervisor. The files are created in the SUPERVISOR_DATA directory, as
it needs to be persisted between reboots, just as the instances of
Docker containers are.

Supervisor's cidfile must be created when starting the Supervisor
itself, for that see home-assistant/operating-system#4276.

* Address review comments, fix mounting of the cidfile
Copy link
Member

@agners agners left a comment

Choose a reason for hiding this comment

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

LGTM

@agners agners merged commit 0e3fd2c into dev Sep 9, 2025
3 checks passed
@agners agners deleted the supervisor-cidfile branch September 9, 2025 18:16
@github-actions github-actions bot locked and limited conversation to collaborators Sep 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants