fix: move all startup logic into rc.d for consistency with other apps #72
Conversation
… apps Signed-off-by: Derek Kaser <[email protected]>
WalkthroughAdds a pre-upgrade cleanup to remove an existing rc.tailscale symlink during install, centralizes utility scripts under /usr/local/php/unraid-tailscale-utils, removes legacy wrapper and update-settings scripts, moves cron and event symlinks to the new utility paths, and introduces a pre-startup PHP validation that creates the Taildrop link and gates daemon start. Changes
Sequence Diagram(s)sequenceDiagram
participant Installer
participant rc.tailscale
participant preStartup as pre-startup.php
participant System
participant tailscaled
rect rgb(250,250,240)
note over Installer: Install-time cleanup
Installer->>Installer: if -L /etc/rc.d/rc.tailscale then rm -f symlink
end
rect rgb(230,245,255)
note over rc.tailscale: Runtime startup with validation
rc.tailscale->>preStartup: invoke pre-startup.php
preStartup->>System: createTaildropLink(config)
System->>System: validate dir, ensure parent, unlink old, symlink Taildrop
alt Validation passes (Enable true)
preStartup-->>rc.tailscale: exit(0)
rc.tailscale->>tailscaled: start tailscaled
else Validation fails (Enable false)
preStartup-->>rc.tailscale: exit(1)
rc.tailscale->>rc.tailscale: abort start
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 2
🧹 Nitpick comments (1)
src/install/doinst.sh (1)
1-7: Consider adding error handling for cd commands.The static analysis tool flags that
cdcommands could fail without error handling. While unlikely in a controlled install environment, adding|| exitwould make the script more robust and prevent commands from executing in the wrong directory.Apply this pattern to all cd commands:
-( cd etc/cron.daily ; rm -rf tailscale-daily ) +( cd etc/cron.daily || exit ; rm -rf tailscale-daily ) -( cd etc/cron.daily ; rm -rf check-tailscale-key ) +( cd etc/cron.daily || exit ; rm -rf check-tailscale-key ) -( cd etc/cron.daily ; ln -sf /usr/local/php/unraid-tailscale-utils/daily.sh tailscale-daily ) +( cd etc/cron.daily || exit ; ln -sf /usr/local/php/unraid-tailscale-utils/daily.sh tailscale-daily )And similarly for the other
cdcommands on lines 4-7.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
plugin/plugin.j2(1 hunks)src/install/doinst.sh(1 hunks)src/usr/local/emhttp/plugins/tailscale/daily.sh(0 hunks)src/usr/local/emhttp/plugins/tailscale/restart.sh(1 hunks)src/usr/local/emhttp/plugins/tailscale/update-settings.sh(0 hunks)src/usr/local/etc/rc.d/rc.tailscale(1 hunks)src/usr/local/php/unraid-tailscale-utils/daily.sh(1 hunks)src/usr/local/php/unraid-tailscale-utils/pre-startup.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/System.php(1 hunks)
💤 Files with no reviewable changes (2)
- src/usr/local/emhttp/plugins/tailscale/update-settings.sh
- src/usr/local/emhttp/plugins/tailscale/daily.sh
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-15T03:15:08.212Z
Learnt from: dkaser
Repo: unraid/unraid-tailscale PR: 68
File: src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/System.php:371-373
Timestamp: 2025-11-15T03:15:08.212Z
Learning: In the unraid-tailscale project, tailscaled accepts both single-dash and double-dash formats for command-line flags (e.g., both `-port` and `--port` are valid). The project uses single-dash format for consistency across daemon parameters in the System.php file.
Applied to files:
src/usr/local/etc/rc.d/rc.tailscale
🧬 Code graph analysis (2)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/System.php (2)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Config.php (1)
Config(22-66)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Utils.php (2)
Utils(22-184)logwrap(66-73)
src/usr/local/emhttp/plugins/tailscale/restart.sh (1)
src/usr/local/php/unraid-tailscale-utils/log.sh (1)
log(3-7)
🪛 Shellcheck (0.11.0)
src/install/doinst.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (5)
src/usr/local/php/unraid-tailscale-utils/daily.sh (1)
1-3: LGTM - Simple and appropriate for cron.The script correctly wraps the PHP daily maintenance task with output suppression, which is standard for cron jobs. The silent execution is appropriate since logging is presumably handled within daily.php itself.
plugin/plugin.j2 (1)
85-88: Good migration cleanup logic.The pre-upgrade removal of the legacy
/etc/rc.d/rc.tailscalesymlink ensures a clean transition to the new architecture where startup logic is consolidated inrc.d. The-Ltest correctly identifies symlinks, and-fflag prevents errors if the file doesn't exist.src/usr/local/php/unraid-tailscale-utils/pre-startup.php (1)
37-42: Clean pre-startup gate implementation.The addition of Taildrop link creation and the use of exit codes to control daemon startup is a clean pattern. Exit 0 signals readiness to start, while exit 1 prevents startup when disabled. This aligns well with the refactored startup flow in
rc.tailscale.src/usr/local/emhttp/plugins/tailscale/restart.sh (1)
1-6: LGTM - Clean migration to centralized paths.The updates correctly point to the centralized log helper and use the refactored
rc.tailscale restartcommand instead of the deprecatedupdate-settings.sh. The delayed restart viaatis appropriate to avoid race conditions.src/usr/local/etc/rc.d/rc.tailscale (1)
8-19: Well-structured pre-startup validation gate.The introduction of the pre-startup check creates a clean separation between validation/preparation and daemon startup. The exit code pattern correctly gates daemon startup based on configuration state, and the custom params sourcing is now properly structured with an else clause for initialization.
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/System.php
Outdated
Show resolved
Hide resolved
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/System.php
Outdated
Show resolved
Hide resolved
Signed-off-by: Derek Kaser <[email protected]>
dkaser
changed the title
Summary by CodeRabbit
New Features
Bug Fixes
Refactor