Conversation
Owner
tis24dev
commented
Feb 5, 2026
tis24dev
commented
- Clean up parse comment and fill UpgradeResult
- logging: capture issues and add counters
- Add granular PBS collection toggles & log summary
- Improve PVE manifest logging and cluster handling
- Remove BASE_DIR & CRON_ from backup.env*
- Improve network IP parsing and rollback UI
- Remove live countdowns from prompts
Remove duplicated comment lines above Config.parse to avoid redundant documentation. In computeConfigUpgrade, ensure UpgradeResult is fully populated (MissingKeys, ExtraKeys, CaseConflictKeys, PreservedValues, Warnings) when no content changes are detected so callers receive complete upgrade metadata.
Add an issueLines slice to Logger and capture WARNING/ERROR/CRITICAL log entries in logWithLabel as single-line entries (sanitizing CR/LF) for end-of-run summaries. Introduce thread-safe accessors WarningCount, ErrorCount and IssueLines to retrieve counts and the captured issue lines. Import strings to support sanitization. Also includes an updated .backup.lock metadata change (pid/time).
Introduce fine-grained PBS collection flags (S3 endpoints, node config, ACME accounts/plugins, metric servers, traffic control, PBS network config) across config, collector, orchestrator and env templates so subcomponents can be independently enabled/disabled. Update collector logic to respect these flags (exclude files, skip commands and snapshots when disabled) and extend collectPBSConfigFile to accept a disable-hint used in clearer log messages. Add a final-run summary that prints WARNING/ERROR log lines to help surface root causes. Update docs, tests and templates to reflect the new options.
Refactor populatePVEManifest to always initialize the manifest and introduce manifestLogOpts + logEntry for consistent, descriptive logging and counting of missing files. Update record(...) calls to include descriptions, disable hints, and not-found counting for PVE config items. Rework collectPVEDirectories cluster logic to separate BackupClusterConfig checks from clustered/node-specific actions, use c.systemPath for authkey, and adjust copy/skip logging behavior. Tune log levels/messages for firewall, VZDump and Ceph detection. Add a note in the backup.env template explaining that enabled-but-unconfigured features emit warnings and can be disabled via BACKUP_* flags.
Stop writing BASE_DIR and CRON_* values into backup.env since BASE_DIR is now auto-detected and cron scheduling is managed via crontab. Changes include: - CLI and TUI installers no longer set BASE_DIR or CRON_* in generated env files; added unsetEnvValue helper to strip keys from templates. - Config upgrade now prunes deprecated keys (BASE_DIR, CRON_SCHEDULE, CRON_HOUR, CRON_MINUTE), emits a warning, and only rewrites the file when there are missing keys or pruned lines. - Added unit tests to verify pruning behavior and updated TUI tests to expect these keys to be removed. - Documentation updated to note BASE_DIR is auto-detected. - Minor update to internal/orchestrator/.backup.lock (runtime metadata).
Enhance network snapshot IP extraction to return the primary address with CIDR and handle token parsing/whitespace robustly, preferring IPv4 when available. Add unit tests covering IPv4/IPv6/CIDR cases and missing snapshots. Introduce a suppressPVEChecks flag for cluster restores to skip PVE-specific health checks, adjust network health options accordingly, and enrich UI messages with observed/original IP and reconnect host guidance. Update applyNetworkWithRollbackWithUI signature and return the constructed NetworkApplyNotCommittedError for richer reporting. Minor test call adjustment and backup lock timestamp update.
Replace live-updating countdown UI in CLI and TUI prompts with a single-line deadline display and simplified input handling. Prompts now show a human-readable deadline (HH:MM:SS) and total seconds, and use ReadLineWithContext directly instead of a ticker + goroutine/select loop to avoid issues on terminals that don't handle repeated carriage-return updates (e.g. IPMI/serial). Logging and timeout/cancel handling preserved; TUI text updated to include the deadline.
There was a problem hiding this comment.
Pull request overview
This PR syncs the development branch to main with several focused improvements to ProxSave's configuration management, logging, PBS collection capabilities, and user experience during network operations.
Changes:
- Removes deprecated BASE_DIR and CRON_* keys from backup.env (now auto-detected at runtime)
- Adds issue line capturing and warning/error counters with end-of-run summary
- Introduces granular PBS collection toggles for S3 endpoints, node config, ACME, metrics, and traffic control
- Improves network IP parsing to preserve CIDR notation and enhances rollback UI with clearer reconnection guidance
- Enhances PVE manifest logging with detailed status messages and disable hints
- Removes live countdowns from CLI prompts for better compatibility with IPMI/serial terminals
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/tui/wizard/install.go | Adds unsetEnvValue function and removes deprecated BASE_DIR/CRON_* from generated configs |
| internal/tui/wizard/install_test.go | Updates tests to verify BASE_DIR and CRON_* keys are properly removed |
| internal/config/upgrade.go | Implements pruning logic for deprecated keys during config upgrades |
| internal/config/upgrade_test.go | Adds comprehensive tests for BASE_DIR and CRON_* pruning |
| internal/config/config.go | Adds new PBS collection toggle fields and removes duplicate comment |
| internal/config/templates/backup.env | Documents new granular PBS toggles and updates BASE_DIR comment |
| internal/backup/collector.go | Adds new PBS-specific collection config fields |
| internal/backup/collector_pbs.go | Implements granular toggles with enhanced logging and disable hints |
| internal/backup/collector_pbs_extra_test.go | Updates test to include new disableHint parameter |
| internal/backup/collector_pve.go | Improves manifest logging with detailed status messages and cluster handling |
| internal/orchestrator/orchestrator.go | Propagates new PBS collection toggles to collector config |
| internal/orchestrator/network_apply.go | Enhances IP extraction to preserve CIDR and removes live countdown |
| internal/orchestrator/network_apply_snapshot_test.go | Adds tests for IPv4/IPv6 CIDR preservation |
| internal/orchestrator/network_apply_workflow_ui.go | Improves rollback UI with IP parsing and reconnection guidance |
| internal/orchestrator/network_apply_preflight_rollback_test.go | Updates test to include suppressPVEChecks parameter |
| internal/orchestrator/restore_workflow_ui.go | Improves network rollback messages and fixes indentation |
| internal/orchestrator/restore_tui.go | Adds deadline timestamp to countdown display |
| internal/orchestrator/prompts_cli.go | Removes live countdown ticker for better terminal compatibility |
| internal/logging/logger.go | Adds issue line capture and warning/error count tracking |
| cmd/proxsave/main.go | Implements end-of-run warning/error summary |
| cmd/proxsave/install.go | Removes explicit BASE_DIR setting from CLI wizard |
| docs/CONFIGURATION.md | Updates BASE_DIR documentation to reflect auto-detection |
| docs/BACKUP_ENV_MAPPING.md | Documents new PBS collection toggles |
| internal/orchestrator/.backup.lock | Runtime lock file (should not be committed) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/orchestrator/.backup.lock
Outdated
Comment on lines
1
to
3
| pid=192209 | ||
| host=pve | ||
| time=2026-02-03T19:42:03+01:00 | ||
| time=2026-02-05T18:34:26+01:00 |
There was a problem hiding this comment.
This file appears to be a runtime lock file that should not be committed to the repository. Lock files containing PIDs and timestamps are typically generated at runtime and should be added to .gitignore.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Implement automatic email recipient discovery for root@pam across Proxmox platforms. The Email notifier now tries pvesh (PVE API) first, falls back to legacy CLIs (pveum / proxmox-backup-manager), and finally parses user.cfg (/etc/pve/user.cfg or /etc/proxmox-backup/user.cfg). Added helper functions (redactEmail, runCombinedOutput, truncateForLog), new configurable paths and diagnostic limits, and improved debug logging with truncated diagnostics. Updated tests to cover pvesh and user.cfg fallbacks and adjusted existing tests. Documentation (CONFIGURATION and TROUBLESHOOTING) now describes auto-detection behavior and provides quick CLI checks. Minor update to internal/orchestrator/.backup.lock timestamp.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.