chore(glances): HA add-on best practices improvements#628
chore(glances): HA add-on best practices improvements#628ChrisHaPunkt wants to merge 18 commits into
Conversation
ChrisHaPunkt
changed the title
|
Warning Review limit reached
More reviews will be available in 25 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughContainer label changed to "app"; build refs updated; config defaults and schema adjusted; startup script runs exporter and webserver as coordinated background processes with shutdown handling; English translations for new configuration fields added. ChangesGlances App Migration & Exporter Refactoring
Sequence DiagramsequenceDiagram
autonumber
participant Init as Init Script
participant Exporter as Glances Exporter (background)
participant Web as Glances Webserver
participant Sys as System Signal
Init->>Exporter: if exporters configured -> start (--export, --quiet)
Init->>Init: store EXPORTER_PID
Init->>Web: start webserver (no --export)
Init->>Init: store WEBSERVER_PID
Sys->>Init: send TERM/INT
Init->>Exporter: forward TERM/INT (kill EXPORTER_PID)
Init->>Web: forward TERM/INT (kill WEBSERVER_PID)
Exporter-->>Init: exit
Web-->>Init: exit
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
glances/Dockerfile (1)
58-70: ⚡ Quick winRemove duplicate
io.hass.versionlabel entry.Line 70 duplicates the same key already defined on Line 58. Keeping a single definition avoids ambiguity and future drift in this label block.
Proposed cleanup
LABEL \ io.hass.name="${BUILD_NAME}" \ io.hass.description="${BUILD_DESCRIPTION}" \ io.hass.arch="${BUILD_ARCH}" \ io.hass.type="app" \ io.hass.version=${BUILD_VERSION} \ maintainer="Franck Nijhof <opensource@frenck.dev>" \ org.opencontainers.image.title="${BUILD_NAME}" \ org.opencontainers.image.description="${BUILD_DESCRIPTION}" \ org.opencontainers.image.vendor="Home Assistant Community Apps" \ org.opencontainers.image.authors="Franck Nijhof <opensource@frenck.dev>" \ org.opencontainers.image.licenses="MIT" \ org.opencontainers.image.url="https://frenck.dev/home-assistant-apps" \ org.opencontainers.image.source="https://github.com/${BUILD_REPOSITORY}" \ org.opencontainers.image.documentation="https://github.com/${BUILD_REPOSITORY}/blob/main/README.md" \ org.opencontainers.image.created=${BUILD_DATE} \ - org.opencontainers.image.revision=${BUILD_REF} \ - io.hass.version=${BUILD_VERSION} + org.opencontainers.image.revision=${BUILD_REF}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@glances/Dockerfile` around lines 58 - 70, The Dockerfile LABEL block contains a duplicate key "io.hass.version" (appears both in the first and last entries of the shown LABEL list); remove the redundant occurrence so only one "io.hass.version=${BUILD_VERSION}" remains, leaving all other labels (maintainer, org.opencontainers.image.* and so on) untouched; update the LABEL block where "io.hass.version" is defined to keep the single, canonical instance and ensure no other duplicate keys are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@glances/rootfs/etc/services.d/glances/run`:
- Around line 34-38: The exporter invocation currently builds its own argv so
the process_info flag (when false) isn't applied; modify the run logic so the
exporter command includes the same process info setting as the main glances
invocation — e.g., reuse the existing options array or append the same
"--disable-process" flag to the exporter argv when process_info=false; update
the code that builds exporters (the exporters variable/array and the glances
start command) to ensure the exporter process is started with the same options
as the main glances instance.
In `@glances/translations/en.yaml`:
- Line 5: The description entry in glances/translations/en.yaml currently uses
the term "addon"; update the user-facing copy for the "description" key to use
"app" instead (for example change "The log level for the addon." to "The log
level for the app.") so the translation matches the PR's reclassification of
Glances as an app.
---
Nitpick comments:
In `@glances/Dockerfile`:
- Around line 58-70: The Dockerfile LABEL block contains a duplicate key
"io.hass.version" (appears both in the first and last entries of the shown LABEL
list); remove the redundant occurrence so only one
"io.hass.version=${BUILD_VERSION}" remains, leaving all other labels
(maintainer, org.opencontainers.image.* and so on) untouched; update the LABEL
block where "io.hass.version" is defined to keep the single, canonical instance
and ensure no other duplicate keys are present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42d01af0-c3e0-4482-b993-aeb4a67ba394
📒 Files selected for processing (5)
glances/Dockerfileglances/build.yamlglances/config.yamlglances/rootfs/etc/services.d/glances/runglances/translations/en.yaml
💤 Files with no reviewable changes (1)
- glances/build.yaml
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
glances/rootfs/etc/services.d/glances/run (1)
61-65: 💤 Low valueOn graceful shutdown (SIGTERM), the log message is misleading and exit code is non-zero.
When the container receives SIGTERM (e.g., HA stops the addon), the trap kills both children, but the script continues to
wait -n, logs "exited unexpectedly", and exits 1. This isn't strictly incorrect—processes are stopped cleanly—but the log is misleading and exit code 1 may confuse orchestration layers expecting 0 for graceful shutdown.♻️ Optional: Exit cleanly from trap
- trap 'kill "${WEBSERVER_PID}" "${EXPORTER_PID}" 2>/dev/null' TERM INT + trap 'kill "${WEBSERVER_PID}" "${EXPORTER_PID}" 2>/dev/null; exit 0' TERM INTThis way, SIGTERM/INT results in immediate clean exit (code 0), while a child crash still logs the fatal message and exits 1.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@glances/rootfs/etc/services.d/glances/run` around lines 61 - 65, The shutdown behavior is misleading: the trap currently kills WEBSERVER_PID and EXPORTER_PID but the main flow still runs wait -n then logs via bashio::log.fatal and exits 1; change this so SIGTERM/INT performs a clean exit (0) instead of treating it as an unexpected crash — either make the trap itself kill the children and immediately exit 0, or have the trap set a flag (e.g., SHUTTING_DOWN=1) and after wait -n check that flag: if set exit 0, otherwise call bashio::log.fatal and exit 1; update references to trap, WEBSERVER_PID, EXPORTER_PID, wait -n, and bashio::log.fatal accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@glances/rootfs/etc/services.d/glances/run`:
- Around line 61-65: The shutdown behavior is misleading: the trap currently
kills WEBSERVER_PID and EXPORTER_PID but the main flow still runs wait -n then
logs via bashio::log.fatal and exits 1; change this so SIGTERM/INT performs a
clean exit (0) instead of treating it as an unexpected crash — either make the
trap itself kill the children and immediately exit 0, or have the trap set a
flag (e.g., SHUTTING_DOWN=1) and after wait -n check that flag: if set exit 0,
otherwise call bashio::log.fatal and exit 1; update references to trap,
WEBSERVER_PID, EXPORTER_PID, wait -n, and bashio::log.fatal accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a2a09cd-d9d1-4c7f-9358-9314aae0c21e
📒 Files selected for processing (2)
glances/rootfs/etc/services.d/glances/runglances/translations/en.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- glances/translations/en.yaml
ChrisHaPunkt
force-pushed
the
fix/influxdb-export-v4
branch
from
36032bd to
781cf29
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
glances/Dockerfile (1)
69-69: 💤 Low valueConsider restoring
org.opencontainers.image.versionfor OCI compliance.
org.opencontainers.image.versionis part of the OCI Image Annotation spec. Removing it means standard OCI tooling (Crane, Skopeo, ghcr.io UI) won't surface the image version from the standard label — only the HA-specificio.hass.versionat line 58 remains. The fix is a single line.♻️ Proposed restoration
org.opencontainers.image.created=${BUILD_DATE} \ - org.opencontainers.image.revision=${BUILD_REF} + org.opencontainers.image.revision=${BUILD_REF} \ + org.opencontainers.image.version=${BUILD_VERSION}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@glances/Dockerfile` at line 69, Restore the OCI standard version label by adding back the label org.opencontainers.image.version to the Dockerfile and set it to the same value used for the HA-specific version label (i.e., mirror the value used for io.hass.version) so standard OCI tooling can surface the image version; locate the label block in the Dockerfile near the existing io.hass.version and add a line with org.opencontainers.image.version=<same-value-as-io.hass.version>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@glances/Dockerfile`:
- Line 69: Restore the OCI standard version label by adding back the label
org.opencontainers.image.version to the Dockerfile and set it to the same value
used for the HA-specific version label (i.e., mirror the value used for
io.hass.version) so standard OCI tooling can surface the image version; locate
the label block in the Dockerfile near the existing io.hass.version and add a
line with org.opencontainers.image.version=<same-value-as-io.hass.version>.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
@frenck I kindly ask for a review :) |
ChrisHaPunkt
force-pushed
the
fix/influxdb-export-v4
branch
from
3d12019 to
978c4b1
Compare
Glances v4 requires explicit --export flag, but combining it with -w (webserver mode) triggers 'Export is only available in standalone or client mode' and exits. Run a separate lightweight Glances instance in quiet/standalone mode for exports while the webserver runs without --export.
This is a Home Assistant Community App, not an addon. The OCI vendor
label already reflects this ("Home Assistant Community Apps").
Enables HA to auto-restart the addon when the Glances web server becomes unresponsive.
Provides user-friendly labels and descriptions for all configuration options in the HA UI.
Build configuration belongs in config.yaml per current HA developer docs. The ARG in the Dockerfile already specifies BUILD_FROM.
Glances only reads system metrics, it never writes to the HA config directory. Removed unnecessary :rw access.
S6-Overlay cannot be used with host_pid: true (requires PID 1). Make this explicit in config.yaml to match the Dockerfile's ENTRYPOINT [] override.
The InfluxDB token is a secret and should be masked in the UI, consistent with the password field.
The exporter process built its own argv and did not include the --disable-process flag when process_info was false, causing inconsistent behavior between the webserver and exporter.
Consistent with the reclassification of Glances as an app.
When InfluxDB export is enabled, both processes now run in the background with `wait -n` to detect the first exit. SIGTERM is forwarded to both children for clean shutdown from the HA UI. Without an exporter, the webserver runs via exec as before.
Use a SHUTTING_DOWN flag so SIGTERM/INT from HA exits cleanly (0) instead of logging a fatal error and exiting 1.
Co-authored-by: Copilot <copilot@github.com>
ChrisHaPunkt
force-pushed
the
fix/influxdb-export-v4
branch
from
978c4b1 to
1bb9342
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
rebased, @frenck I kindly ask for a review :) |
ChrisHaPunkt
changed the title
|
This PR has been split. The InfluxDB bug fix (the This PR now contains only: |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
github-actions
Bot
added
the
stale
1 participant
Overview
Aligns the Glances Community App with current hassio-addons best practices. This is a separate PR from the InfluxDB bug fix — it contains no functional changes to Glances behaviour.
Changes
Dockerfile— correctio.hass.typelabel"addon"→"app": this is a Community App, not a Supervisor-managed add-onbuild.yaml— remove pinned base image versionsghcr.io/hassio-addons/base:20.1.1entries are deprecated in favour of the unpinned base image resolution used by the build systemconfig.yaml— three improvementswatchdog: http://[HOST]:61209— enables Home Assistant to automatically restart the addon if the Glances HTTP endpoint stops respondinginit: false— documents that S6-Overlay init is disabled (was already the runtime behaviour)homeassistant_config:rw→homeassistant_config— drops unnecessary write access to the HA config directory; Glances only reads ittoken: str?→token: password?— masks the InfluxDB token in the UI instead of displaying it in plain texttranslations/en.yaml— add full English translations