Skip to content

fix(plg): explicitly stop an existing api before installation#1841

Merged
elibosley merged 1 commit into
mainfrom
fix/nodemon
Dec 15, 2025
Merged

fix(plg): explicitly stop an existing api before installation#1841
elibosley merged 1 commit into
mainfrom
fix/nodemon

Conversation

@pujitm

@pujitm pujitm commented Dec 15, 2025

Copy link
Copy Markdown
Member

Necessary for "clean" upgrades to api orchestration (eg changing how the api is daemonized).

Prior to this, rc.unraid-api start would also restart a running api, which sufficed for application updates, but is insufficient for orchestration updates.

Summary by CodeRabbit

  • Bug Fixes
    • Improved update reliability by ensuring services are properly stopped before system modifications occur.

✏️ Tip: You can customize this high-level summary in your review settings.

Necessary for "clean" upgrades to api orchestration (eg changing how the
api is daemonized).

Prior to this, `rc.unraid-api start` would also restart a running api,
which sufficed for application updates, but is insufficient for
orchestration updates.
@coderabbitai

coderabbitai Bot commented Dec 15, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This change adds a pre-mutation safeguard to the Unraid plugin script. Before modifying /usr/local/unraid-api, the script now checks if the Unraid API service script is executable and stops it if present, with logging and warning emission on failure.

Changes

Cohort / File(s) Summary
Unraid API Service Safeguard
plugin/plugins/dynamix.unraid.net.plg
Adds conditional logic to stop the Unraid API service before mutating /usr/local/unraid-api, including service detection, stop attempt, and warning logging on failure

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify the file path /etc/rc.d/rc.unraid-api is correct and consistent with actual system layout
  • Ensure the service stop logic handles edge cases (e.g., service already stopped, permission issues)
  • Confirm warning message is appropriately logged and doesn't interrupt the mutation process

Poem

🐰 A guard now stands before the mutation's door,
Checking if services dance and more,
Stopping them gently ere changes take place,
Safety's the rabbit's most favorite grace! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding an explicit stop of the existing API service before installation, which directly addresses the PR's objective of enabling clean orchestration upgrades.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/nodemon

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6e2939 and 69a5b09.

📒 Files selected for processing (1)
  • plugin/plugins/dynamix.unraid.net.plg (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*

📄 CodeRabbit inference engine (.cursor/rules/default.mdc)

Never add comments unless they are needed for clarity of function

Files:

  • plugin/plugins/dynamix.unraid.net.plg
🧠 Learnings (7)
📓 Common learnings
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations. It includes try/catch blocks, detailed error logging, and safe rollback mechanisms. Individual FileModification implementations (like LogRotateModification) should allow errors to propagate to this service layer rather than handling them internally.
Learnt from: pujitm
Repo: unraid/api PR: 1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations, including detailed error logging with stack traces and modification IDs. Individual FileModification implementations should focus on their core functionality without duplicating error handling.
Learnt from: pujitm
Repo: unraid/api PR: 1658
File: plugin/plugins/dynamix.unraid.net.plg:73-79
Timestamp: 2025-09-04T18:42:53.531Z
Learning: In the dynamix.unraid.net plugin, versions 6.12.1-6.12.14 and 6.12.15 prereleases are intentionally allowed to install with warnings (rather than immediate cleanup) to provide users with a grace period and notice before functionality is completely removed. This is a deliberate UX decision to avoid immediately breaking existing setups while encouraging upgrades.
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:19-24
Timestamp: 2025-05-08T19:28:54.365Z
Learning: The directory `/usr/local/emhttp/plugins/dynamix.my.servers` is a valid directory that exists as part of the Unraid API plugin installation and should be included in verification checks.
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: In the Unraid API, FileModification implementations (apply/rollback methods) don't need to implement their own error handling as it's handled by the UnraidFileModifierService caller.
Learnt from: elibosley
Repo: unraid/api PR: 1120
File: plugin/plugins/dynamix.unraid.net.plg:35-38
Timestamp: 2025-02-05T21:10:48.136Z
Learning: When providing error handling guidance for Unraid plugins, direct users to use the web GUI (Plugins > Installed Plugins) rather than suggesting command-line actions.
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: In the Unraid API, FileModification implementations (like LogRotateModification) don't need to handle errors internally as error handling is managed at the UnraidFileModifierService level.
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.
📚 Learning: 2025-09-04T18:42:53.531Z
Learnt from: pujitm
Repo: unraid/api PR: 1658
File: plugin/plugins/dynamix.unraid.net.plg:73-79
Timestamp: 2025-09-04T18:42:53.531Z
Learning: In the dynamix.unraid.net plugin, versions 6.12.1-6.12.14 and 6.12.15 prereleases are intentionally allowed to install with warnings (rather than immediate cleanup) to provide users with a grace period and notice before functionality is completely removed. This is a deliberate UX decision to avoid immediately breaking existing setups while encouraging upgrades.

Applied to files:

  • plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-01-29T16:36:04.777Z
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations. It includes try/catch blocks, detailed error logging, and safe rollback mechanisms. Individual FileModification implementations (like LogRotateModification) should allow errors to propagate to this service layer rather than handling them internally.

Applied to files:

  • plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-09-04T15:26:34.416Z
Learnt from: elibosley
Repo: unraid/api PR: 1657
File: web/scripts/deploy-dev.sh:37-41
Timestamp: 2025-09-04T15:26:34.416Z
Learning: In web/scripts/deploy-dev.sh, the command `rm -rf /usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/*` intentionally removes all contents of the unraid-components directory before deploying standalone components. This broader cleanup is desired behavior according to the maintainer elibosley.

Applied to files:

  • plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-05-08T19:28:54.365Z
Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:19-24
Timestamp: 2025-05-08T19:28:54.365Z
Learning: The directory `/usr/local/emhttp/plugins/dynamix.my.servers` is a valid directory that exists as part of the Unraid API plugin installation and should be included in verification checks.

Applied to files:

  • plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-01-29T16:35:43.699Z
Learnt from: elibosley
Repo: unraid/api PR: 1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations, including detailed error logging with stack traces and modification IDs. Individual FileModification implementations should focus on their core functionality without duplicating error handling.

Applied to files:

  • plugin/plugins/dynamix.unraid.net.plg
📚 Learning: 2025-06-11T14:14:30.348Z
Learnt from: pujitm
Repo: unraid/api PR: 1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.

Applied to files:

  • plugin/plugins/dynamix.unraid.net.plg
🔇 Additional comments (1)
plugin/plugins/dynamix.unraid.net.plg (1)

481-486: LGTM! Solid implementation for preventing upgrade race conditions.

The explicit stop before mutating /usr/local/unraid-api correctly addresses the orchestration update scenario described in the PR objectives. The implementation is defensive (checks script executability) and resilient (warns but continues on failure, allowing the start command at line 597 to handle restart). The conditional also naturally handles fresh installs where the script doesn't yet exist.


Comment @coderabbitai help to get the list of available commands and usage tips.

@pujitm pujitm requested a review from elibosley December 15, 2025 18:12
@codecov

codecov Bot commented Dec 15, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.84%. Comparing base (317e0fa) to head (69a5b09).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1841      +/-   ##
==========================================
- Coverage   52.04%   51.84%   -0.20%     
==========================================
  Files         876      878       +2     
  Lines       50509    50841     +332     
  Branches     5023     5051      +28     
==========================================
+ Hits        26285    26357      +72     
- Misses      24149    24408     +259     
- Partials       75       76       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1841/dynamix.unraid.net.plg

@elibosley elibosley merged commit 99ce88b into main Dec 15, 2025
12 of 13 checks passed
@elibosley elibosley deleted the fix/nodemon branch December 15, 2025 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants