Support systemd-journal-gatewayd using a TCP socket #5576
Conversation
agners
force-pushed
the
support-systemd-journal-gatewayd-tcp-socket
branch
from
e4fd79c to
a3c0ab6
Compare
📝 WalkthroughWalkthroughThe changes in the Changes
Sequence DiagramsequenceDiagram
participant Logs as LogsControl
participant Env as Environment
participant Session as ClientSession
participant Journal as systemd-journal-gatewayd
Logs->>Env: Check SUPERVISOR_SYSTEMD_JOURNAL_GATEWAYD_URL
alt URL is set
Logs->>Logs: Set available = True
Logs->>Session: Create session with HTTP connector
else URL not set
Logs->>Logs: Check socket availability
Logs->>Session: Create session with Unix socket
end
Logs->>Journal: Request logs
Journal-->>Logs: Return log data
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
supervisor/host/logs.py (3)
12-12: Remove the unused import
TCPConnectoris never used. You can safely remove it to address the static analysis warning.-from aiohttp import ClientError, ClientSession, ClientTimeout, TCPConnector +from aiohttp import ClientError, ClientSession, ClientTimeout🧰 Tools
🪛 Ruff (0.8.2)
12-12:
aiohttp.TCPConnectorimported but unusedRemove unused import:
aiohttp.TCPConnector(F401)
168-173: Consider validating the environment variable
Using the environment variable directly may lead to errors if misconfigured or injected. As a good practice, consider validating the URL format and handling exceptions gracefully.
178-178: Ensure consistent path usage
Usingf"{path}"is straightforward. If you do not need string interpolation, consider passingpathdirectly for cleaner readability.- f"{path}", + path,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supervisor/host/logs.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
supervisor/host/logs.py
12-12: aiohttp.TCPConnector imported but unused
Remove unused import: aiohttp.TCPConnector
(F401)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run tests Python 3.13.1
- GitHub Check: Build armv7 supervisor
- GitHub Check: Build armhf supervisor
- GitHub Check: Build aarch64 supervisor
🔇 Additional comments (2)
supervisor/host/logs.py (2)
9-9: No issues with the new import
The import ofosis valid as it is used to check environment variables.
57-59: Conditional check for environment variable is valid
Checking forSUPERVISOR_SYSTEMD_JOURNAL_GATEWAYD_URLto determine availability is consistent with the new feature for TCP support.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
supervisor/host/logs.py (1)
57-57: Fix typo in docstring.There's a typo in the docstring: "gatwayd" should be "gatewayd".
- """Check if systemd-journal-gatwayd is available.""" + """Check if systemd-journal-gatewayd is available."""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supervisor/host/logs.py(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run tests Python 3.13.1
🔇 Additional comments (2)
supervisor/host/logs.py (2)
9-9: LGTM!The
osmodule import is correctly placed with other standard library imports and is necessary for accessing environment variables.
168-181: Add tests for TCP connection functionality.Please add unit tests to verify:
- URL validation in the
availableproperty- TCP connection setup in
journald_logs- Error handling for invalid URLs and connection failures
Would you like me to help generate the test cases?
Proposed change
This allows to use regular TCP/IP sockets to connect to the systemd-journal-gatewayd.
The drive to add this support is to have advanced logs available in devcontainers as well. Since we don't have systemd in devcontainers, socket activation with UNIX domains sockets is not easily doable: The daemon
systemd-journal-gatewayddoes not support it natively, and work arounds usingsocatdid not work reliably.With this change devcontainers can simply tell Supervisor to use TCP/IP instead by setting
SUPERVISOR_SYSTEMD_JOURNAL_GATEWAYD_URLtohttp://172.30.32.1:19531/.Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit
New Features
Improvements