feat: enforce systemd usage for container start/restart

[mairas]

Summary

• Prevent direct docker compose up commands by requiring HALOS_SYSTEMD_STARTED=1 environment variable
• The sentinel variable is only set by the systemd unit file, ensuring users must use systemctl start/restart
• This guarantees the prestart.sh script runs to set up required runtime environment variables

Changes

• Added Environment=HALOS_SYSTEMD_STARTED=1 to systemd service template
• Created systemd_check.py module with inject_systemd_check() function
• Integrated the check injection into builder.py
• Added comprehensive tests

How It Works

Via systemd (correct way):

systemctl start myapp-container.service
# systemd sets HALOS_SYSTEMD_STARTED=1 → docker compose succeeds

Direct docker compose (blocked):

cd /var/lib/container-apps/myapp && docker compose up
# Error: Container must be started via systemctl, not docker compose directly

Test plan

• Unit tests for inject_systemd_check() function
• All existing tests pass
• Build a test package and verify the check works on a real system

🤖 Generated with Claude Code

[mairas]

PR Review: feat: enforce systemd usage for container start/restart

Summary

This PR implements a clever mechanism to prevent users from running docker compose up directly, ensuring they use systemctl start/restart instead. The approach uses bash's required variable syntax (${VAR:?error}) which fails with a clear error message when the variable is not set. The implementation is clean, well-tested, and follows existing patterns in the codebase.

Code Quality Assessment

Strengths:

1. Elegant approach: The ${VAR:?error} bash syntax is an idiomatic way to enforce required environment variables. When the variable is missing, users get a clear error: HALOS_SYSTEMD_STARTED: Container must be started via systemctl, not docker compose directly

2. Consistent with codebase patterns: The inject_systemd_check() function in systemd_check.py follows the same structure as inject_traefik_network() and inject_homarr_labels() - deep copying to avoid mutation, handling both list and dict environment formats, proper integration point in builder.py.

3. Comprehensive tests: The test suite in test_systemd_check.py covers:

   • Empty service, list format, dict format environments
   • Only adds to first service (efficiency)
   • Idempotency (no duplication)
   • Does not modify original compose dict
   • Edge cases (empty services, no services key)

4. Clear documentation: Both the module docstring and the systemd service template comment explain the purpose.

Minor Observations

1. First service selection relies on dict ordering (/Users/mairas/w/hatlabs/halos/halos-distro/container-packaging-tools/src/generate_container_packages/systemd_check.py:44):

   first_service_name = next(iter(services))

   Since Python 3.7+, dicts preserve insertion order, so this is deterministic. However, in multi-service compose files, the "first" service may not be the "main" service semantically. This is fine for this use case since any service failing will fail the entire docker compose up, but worth noting.

2. Test assumption about service ordering (/Users/mairas/w/hatlabs/halos/halos-distro/container-packaging-tools/tests/test_systemd_check.py:52-72):
   The test test_only_adds_to_first_service assumes "app" is the first service, which works because dict literals in Python preserve order. The test is correct but depends on this behavior.

3. Error message UX (/Users/mairas/w/hatlabs/halos/halos-distro/container-packaging-tools/src/generate_container_packages/systemd_check.py:12-14):
   The error message is clear and actionable. It might be helpful to include the correct command, e.g., "Use 'systemctl start <service-name>' instead of 'docker compose up'", but the current message is sufficient.

Verification

• The bash syntax ${VAR:?error} works correctly - tested locally
• CI tests are passing
• The version bump to 0.5.0 is appropriate for a feature addition

Questions

1. Is there a documented way for advanced users to bypass this check for debugging purposes? For example, they could set HALOS_SYSTEMD_STARTED=1 manually, but this might not be obvious. Consider whether this escape hatch should be documented.

Positive Notes

• Clean, focused PR with a single responsibility
• Good separation of concerns - the check logic is in its own module
• Tests verify behavior rather than implementation details
• The underscore prefix _HALOS_SYSTEMD_CHECK clearly marks it as an internal variable
• Appropriate use of copy.deepcopy() to avoid side effects

Verdict

Approve with comments - This is a well-implemented feature that will prevent user confusion when the prestart.sh environment setup is skipped. The code quality is high, tests are comprehensive, and it follows existing patterns. Can merge once the lintian check passes.

One suggestion for future consideration: document the debugging escape hatch in user-facing docs if users frequently need to troubleshoot container startup issues.