Backup action changes for UX improvements #1069
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Backup/Restore UX by adding cleanup behavior on termination, introducing an action to import backup destinations from a cluster backup payload, and enriching defaults returned to the UI.
Changes:
- Add SIGTERM-triggered cleanup tasks support to agent task runner and use it to auto-remove half-restored modules.
- Introduce
import-backup-destinationsaction plus JSON schemas for validating its input/output. - Extend
get-defaultsoutput withhas_additional_disksand reuse shared helpers for hostname/volumes.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| core/imageroot/var/lib/nethserver/cluster/actions/restore-module/50restore_module | Registers a SIGTERM cleanup task for restore-module and changes failure exit behavior. |
| core/imageroot/var/lib/nethserver/cluster/actions/import-backup-destinations/10import_backup_destinations | New Python action to decrypt/decompress backup payload and import destinations into Redis. |
| core/imageroot/var/lib/nethserver/cluster/actions/import-backup-destinations/validate-input.json | Adds input JSON schema for the new import action. |
| core/imageroot/var/lib/nethserver/cluster/actions/import-backup-destinations/validate-output.json | Adds output JSON schema for the new import action. |
| core/imageroot/var/lib/nethserver/cluster/actions/get-defaults/50read | Computes has_additional_disks and uses shared hostname helper. |
| core/imageroot/var/lib/nethserver/cluster/actions/get-defaults/validate-input.json | Changes input schema for get-defaults. |
| core/imageroot/var/lib/nethserver/cluster/actions/get-defaults/validate-output.json | Adds has_additional_disks to get-defaults output schema. |
| core/imageroot/usr/local/agent/pypkg/agent/tasks/run.py | Adds on_sigterm_tasks support and refactors progress/extra handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...t/var/lib/nethserver/cluster/actions/import-backup-destinations/10import_backup_destinations
Outdated
Show resolved
Hide resolved
...imageroot/var/lib/nethserver/cluster/actions/import-backup-destinations/validate-output.json
Outdated
Show resolved
Hide resolved
.../imageroot/var/lib/nethserver/cluster/actions/import-backup-destinations/validate-input.json
Outdated
Show resolved
Hide resolved
core/imageroot/var/lib/nethserver/cluster/actions/get-defaults/validate-input.json
Show resolved
Hide resolved
core/imageroot/var/lib/nethserver/cluster/actions/get-defaults/validate-output.json
Outdated
Show resolved
Hide resolved
...t/var/lib/nethserver/cluster/actions/import-backup-destinations/10import_backup_destinations
Show resolved
Hide resolved
core/imageroot/var/lib/nethserver/cluster/actions/restore-module/50restore_module
Show resolved
Hide resolved
Return a new boolean attribute "has_additional_disks" describing the NS8 node storage. This boolean guides the user through the correct cluster disaster recovery procedure path. If an additional disk exists, the procedure must direct the user to the Backup page.
If restore-module action is canceled by user, run a cleanup task that removes the restored instance from the cluster. Introduce on_sigterm_tasks optional array for agent.tasks.run/runp: it registers additional tasks that are submitted in nowait mode when the process receives SIGTERM, without overriding the internal cancel-task behavior. Sanitize kwargs variable in _runp(), pop()ing unexpected arguments to sub-functions.
Import backup destinations from a cluster backup encrypted file. This new action expects a file generated by download-cluster-backup as input and the password to decrypt it. During import, existing backup destinations are skipped. Configurations are not validated, they are assumed to be correct. For this reason, interal cluster destinations are ignored if the cluster backup was generated by another cluster (cluster UUIDs are compared for this purpose).
DavidePrincipi
force-pushed
the
feat-7810
branch
2 times, most recently
from
bcb9f35 to
ae4bedc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses multiple enhancements for the Backup/Restore UX.
See individual commit for details.
Refs
see mattermost discussion: https://mattermost.nethesis.it/nethesis/pl/p8uu8moi47rbxcmg58ersjcywc