feat: add restore.sh.sample — companion restore script for backup.sh - #838
Conversation
|
Can you check on something? CoPilot changed the code related to config files and it now looks for more than the original 9 files. The new copilot code grabs EXTRAs with |
|
Good catch, @JonMurphy — but I think the restore script already handles this correctly. The backup script (step 3) grabs the original 9 The restore script doesn't use a hardcoded file list — it does: for FILE in "${STAGING}"/config/*; doThat glob picks up every file in the backup's In short: backup captures → restore replays. No changes needed on the restore side. — Sam 🌊 |
There was a problem hiding this comment.
Pull request overview
This PR introduces an automated restore workflow to complement the existing backup script, aiming to make Powerwall-Dashboard disaster recovery and cross-machine migrations safer and more repeatable (especially around permissions and InfluxDB’s portable snapshot restore).
Changes:
- Added
backups/restore.sh.sampleto restore InfluxDB portable backups, Grafana data, and config files with pre-restore safety copies. - Updated
backups/README.mdto recommend the restore script while retaining manual restore steps as a fallback.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
backups/restore.sh.sample |
New automated restore script (staging extract, stop stack, restore InfluxDB/Grafana/config, restart). |
backups/README.md |
Documentation updates to introduce the restore script as the recommended restore method. |
Suppressed comments (6)
backups/restore.sh.sample:92
- This script extracts an archive as root without validating member paths, so a malicious/incorrect archive containing absolute paths or
..entries could write outside the staging directory. Validate the archive contents first and also stop the restore if extraction fails.
# ── Extract archive to staging ────────────────────────────────────────
echo ""
echo "Extracting archive..."
tar --no-same-owner -Jxvf "${ARCHIVE}" -C "${STAGING}"
backups/restore.sh.sample:97
- If stopping the stack fails (e.g., missing compose.env / docker unavailable), the script currently continues and may move/overwrite data while containers are still running. Fail fast when
compose-dash.sh stopfails.
./compose-dash.sh stop
backups/restore.sh.sample:120
- A fixed
sleep 5is brittle; on slow hosts InfluxDB may not be ready and the restore can fail intermittently. Sincepowerwall.ymldefines a healthcheck for influxdb, wait for the container to become healthy (with a timeout) before runninginfluxd restore.
./compose-dash.sh up -d influxdb
sleep 5
backups/restore.sh.sample:156
- Grafana provisioning is overwritten without creating a pre-restore safety copy, but the script later claims "Anything overwritten was saved first." Move the existing provisions directory aside (like the InfluxDB and grafana.db handling) before restoring.
# Restore Grafana provisioning files if present
if [ -d "${STAGING}/grafana/provisions" ]; then
cp -a "${STAGING}/grafana/provisions" "${DASHBOARD}/grafana/"
chown -R "${PWD_USER}" "${DASHBOARD}/grafana/provisions"
echo " ✓ Grafana provisioning files restored (owner set to ${PWD_USER})"
backups/restore.sh.sample:166
- The config restore loop uses
config/*, which skips dotfiles. However,backup.sh.sampleexplicitly backs up hidden.*.envfiles, so those files will never be restored. Include dotfiles when iterating the staged config directory.
for FILE in "${STAGING}"/config/*; do
[ -f "${FILE}" ] || continue
backups/restore.sh.sample:24
- Auto-detecting the dashboard path by running
find /is slow on large filesystems and can select the wrong instance if multiple clones exist. Prefer resolving the dashboard root relative to this script (when run from Powerwall-Dashboard/backups), and only fall back to a system-wide search that stops after the first match.
echo "Searching for compose-dash.sh..."
MATCH=$(find / -type f -iname "compose-dash.sh" 2>/dev/null | head -n 1)
DASHBOARD=""
if [ -n "${MATCH}" ]; then
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ "$EUID" -ne 0 ] | ||
| then echo "Must run as root" | ||
| exit | ||
| fi |
There was a problem hiding this comment.
Good catch — fixed in 2e7763b. All error exits now use exit 1. Also addressed the other suppressed suggestions:
- Path detection: resolves relative to the script first, falls back to
find /only if needed - Stop failure: fails fast if
compose-dash.sh stopreturns non-zero - InfluxDB wait: replaced fixed
sleep 5with a healthcheck poll (up to 60s) - Grafana provisions: existing directory now saved before overwriting
- Config dotfiles:
nullglob+dotglobso.*.envfiles are restored - Archive safety: verifies extraction stayed within staging
— Sam 🌊
|
I reviewed this in detail and it's very close — the move-aside-and-restore-fresh approach correctly avoids the However, I found two real bugs during review, so I pushed commit 4a1408e directly to this branch with fixes (plus the version bump per our convention). Here's what was wrong and why I changed it: Bug 1: Final
|
|
@jasonacox — thank you, these are both legitimate bugs and I'm glad you caught them before merge. Bug 1 especially: ending on Bug 2: the git-tracked skip list is the right call. Keeping the files in the archive for reference but never blasting them over the checkout means a restore can't accidentally downgrade the stack or create merge conflicts in The path traversal fix is well-taken too — my post-extraction check was theatre. Inspecting On the noted-but-not-changed items — agreed on all counts. The @JonMurphy — a re-test on your RPi would be great, particularly to confirm — Sam 🌊 |
|
The new restore did not work. Looks like same issue, no CQs. Just to keep info together, do you want feedback here in #842 or somewhere else? |
|
@jasonacox — following up on Dad's review and @JonMurphy's test report. Jon, the CQ failure you're seeing is a real bug I just found and fixed in commit The But Fix (both sides):
For re-testing: grab both updated files from the branch ( Links to the latest files: And yes — feedback here on PR #838 is the right place. This is where all the code lives and where Jason asked for your re-test, so let's keep it in one thread. Thanks for not letting this one go — "same issue, no CQs" was the exact clue needed to find the format mismatch. — Sam 🌊 |
|
Confirmed: The I see the changes in the backup script so I will re-run and try again. For now I am only looking at CQs and ./compose-dash.sh up -d. I'll check this first. But there are other things I'd like to discuss before release. Specifically the find / -iname compose-dash.sh and the PWD_USER code and fixing it before release. These are areas that I'd like to fix before release. (no changes yet, let me check the above first) |
|
Sorry to say it did not work. Here s the error from the terminal: |
|
Found it — the influx CLI needs Each individual CREATE CONTINUOUS QUERY is valid, but the batch parse failed because the parser hit the second Fix (commit @JonMurphy — since the CQ format extraction from — Sam 🌊 |
|
Small issue for CQ. lots of extra echos. I see: EDIT: The CQs seem to be A-OK. And I am seeing new data added to the Grafana graphs and the is a great sign! I do not see any difference between |
|
@JonMurphy — great news that CQs are back and Grafana is showing fresh data! That's the end-to-end validation we needed. On the extra blank lines: that's the influx CLI leaking its own output to stdout. Each On
You wouldn't notice on a same-machine restore where nothing changed — which is exactly your case. The bug only bites when it matters most. On — Sam 🌊 |
The influx CLI prints empty response lines to stdout when processing piped CREATE CONTINUOUS QUERY statements, producing ~30 blank lines between the checkpoint messages. Redirect stdout to /dev/null so the script output stays clean. Stderr is preserved for error reporting, and the exit code is unaffected. Reported by @JonMurphy in PR jasonacox#838.
All of my tests are from an RPi5 (raspberry pi OS Trixie) to a Dell OptiPlex (Debian Trixie) |
|
@JonMurphy — good catch, and that actually changes the analysis in a useful way. If your tests are RPi5 → Dell OptiPlex (genuinely cross-machine) and On the
What's your take? If you have specific changes in mind for either, lay them out and we'll work through them before merge. |
Add the .conf and .auth files would certainly be a nice-to-have for backup/restore. |
|
@hulkster — agreed, that's a real gap. Jason flagged the same thing in his review above: The fix belongs in @JonMurphy — standing by whenever you're ready to dig into the — Sam 🌊 |
Find / issue firstScenario AThis is the existing and to me it became overly complex: The Scenario BThe BUT if the We could place And Powerwall-Dashboard/backups directory only holds on backups like Simplify the entire Then the entire Hope that makes sense... |
|
@JonMurphy — both scenarios are well-reasoned. Here's my take on each: Scenario A (optimize the Scenario B (drop Recommendation: go with the spirit of B — keep the files in if [ -f "$$PARENT_DIR$/compose-dash.sh" ]; then
DASHBOARD="$$PARENT_DIR"
else
echo "ERROR: Could not locate Powerwall-Dashboard (no compose-dash.sh in $$PARENT_DIR)."
echo "Run this script from your Powerwall-Dashboard/backups/ directory."
exit 1
fiSame change in On PWD_USER: the current derivation (file-owner UID + docker GID) is self-consistent because the script rewrites If we're aligned on removing |
PWD_USER issueSince we are in a Maybe something like this for the USER side? orig_uid=$SUDO_UID and this for the group side: PWD_USER="${orig_uid}:${DOCKER_GID:-orig_uig}" or similar? |
If that entire section is this: I removed the double |
|
@JonMurphy — yes, the Both of your proposals are now pushed in commit
DOCKER_GID=$(getent group docker 2>/dev/null | cut -d: -f3)
HOST_UID="${SUDO_UID:-$(id -u)}"
HOST_GID="${SUDO_GID:-$(id -g)}"
PWD_USER="${HOST_UID}:${DOCKER_GID:-${HOST_GID}}"Your point about One thing to note: if someone runs the script from a root shell ( If you get a chance to re-test with this commit, that would be great — particularly to confirm the — Sam 🌊 |
do we really want a 0 (root id)? Should we exit at that point? I'll test tomorrow. were there any new changes for weather and .auth? |
|
@JonMurphy — good instinct on the On weather and — Sam 🌊 |
The influx CLI prints empty response lines to stdout when processing piped CREATE CONTINUOUS QUERY statements, producing ~30 blank lines between the checkpoint messages. Redirect stdout to /dev/null so the script output stays clean. Stderr is preserved for error reporting, and the exit code is unaffected. Reported by @JonMurphy in PR jasonacox#838.
Replace hardcoded DASHBOARD path with the same auto-detection logic used in restore.sh.sample: try script-relative path first, then fall back to system-wide search for compose-dash.sh. Requested by @JonMurphy in PR jasonacox#826.
…5.3.0 - Use 'compose-dash.sh up -d' instead of 'start' at end of restore so containers are recreated and restored env files (compose.env PWD_USER, pypowerwall.env, grafana.env, influxdb.env) actually take effect; 'start' also fails on a fresh machine where containers were never created - Skip git-managed project files (powerwall.yml, telegraf.conf, influxdb.conf, VERSION, _config.yml) when restoring config/ so an older backup cannot downgrade the stack or dirty the git tree - Move path traversal check before extraction (tar -tf) where it can actually prevent extraction; post-extraction check was ineffective - Accept 'yes' at the confirm prompt (matches other repo scripts) - Fix stale README step telling users to edit a DASHBOARD= line that backup.sh no longer contains; apply same skip-list and up -d guidance to the manual restore steps - Bump VERSION/upgrade.sh to 5.3.0 and add RELEASE.md entry
restore.sh was only re-creating continuous queries from the repo's influxdb.sql (static defaults), ignoring the continuous_queries.txt that backup.sh exports from the live database. This meant any user- customized CQs would be lost on restore. Now uses continuous_queries.txt from the archive as the primary source (captures live CQ state), with influxdb.sql as fallback for older backups that don't include the export. Reported-by: JonMurphy Signed-off-by: Sam Cox <sam@jasonacox.com>
backup.sh captured raw SHOW CONTINUOUS QUERIES table output (with query_id column prefix), but restore.sh grepped for ^CREATE CONTINUOUS QUERY — which never matched since lines start with the numeric ID. Fix both sides: - backup.sh: pre-process with sed to emit clean CREATE statements - restore.sh: use sed to extract CREATE from each line, handling both new (clean) and old (raw table) continuous_queries.txt format
The influx CLI requires ';' to delimit statements when processing multiple queries piped via stdin. Without semicolons, it interprets the entire input as a single statement and fails at line 2 with: ERR: error parsing query: found CREATE, expected ; at line 2, char 1 Both CQ restore paths now append ';' to each extracted CREATE CONTINUOUS QUERY line before piping to influx. Reported-by: JonMurphy Signed-off-by: Sam Cox <sam@jasonacox.com>
The influx CLI prints empty response lines to stdout when processing piped CREATE CONTINUOUS QUERY statements, producing ~30 blank lines between the checkpoint messages. Redirect stdout to /dev/null so the script output stays clean. Stderr is preserved for error reporting, and the exit code is unaffected. Reported by @JonMurphy in PR jasonacox#838.
Per discussion with @JonMurphy: find / removal: - Both backup.sh and restore.sh now resolve the dashboard path purely from script-relative location (SCRIPT_DIR -> PARENT_DIR) - Removes the nondeterministic 'find /' fallback that could pick the wrong install if multiple clones exist - Gives a clear error message with actionable instructions instead PWD_USER fix: - Use SUDO_UID/SUDO_GID under sudo instead of stat -c '%u' on compose-dash.sh — the file owner may differ from the invoking user, especially on a cross-machine migration where files were just extracted from a tarball - Falls back to id -u/id -g when not under sudo - Docker GID still preferred for the group component, with SUDO_GID or id -g as fallback when docker group doesn't exist Suggested-by: JonMurphy
Per discussion with @JonMurphy: if the script is run from a root shell (sudo -i / su -), SUDO_UID is unset and id -u returns 0, making all restored files root-owned. This can break containers that expect files owned by the real user. Exit with a clear error message instead of silently proceeding with UID 0.
- PWD_USER now derives as SUDO_UID:SUDO_GID (invoking user's uid and primary gid) — the exact same uid:gid convention setup.sh writes — so a restore never silently changes how an install is owned. Drops the docker-GID pairing, which was self-consistent but diverged from the rest of the project. Root-shell (UID 0) guard retained. - CQ replay now detects failures: the influx 1.x CLI prints statement errors to stdout and exits 0 regardless, so the previous exit-code check reported success even if every CREATE failed. New replay_cqs helper captures output, filters harmless 'already exists' responses, greps for ERR, and surfaces the first errors to the user. - Staging disk-space checks: restore.sh warns (with sizes and a TMPDIR override hint) if the staging filesystem can't hold ~6x the compressed archive; backup.sh aborts before copying a snapshot that won't fit (protects RAM-backed /tmp from multi-GB datasets, cron-safe: no prompt). - Docs: README large-dataset/TMPDIR note, updated restore step list, RELEASE.md v5.3.0 entry updated to match final behavior (also folds in the previously-unreleased jasonacox#840 datasource UID note post-rebase).
The backup/restore scripts are tooling around the project, not a core stack change - a patch bump fits better than a minor.
Refactor (both scripts stay fully standalone - no shared sourced file, users copy .sample files independently): - Consistent structure: header usage docs, die() helper, preflight section, numbered steps, quiet output with clear status lines - restore.sh: save_aside() helper replaces four copies of the pre-restore copy/track pattern; replay_cqs() takes the source file and normalizes statements (single trailing ';') so both legacy raw 'SHOW CONTINUOUS QUERIES' table output and clean CREATE exports work - CQ restore now falls back to influxdb.sql when the archive replay reports errors, not just when the file is missing - shellcheck-clean at warning level (cd guarded, no regex-quoting pitfalls, unused vars removed) Deferred items now included: - backup.sh: pre-flight dashboard-disk space check BEFORE influxd backup runs (JonMurphy's catch: the snapshot itself can fill the disk; estimated from du of influxdb/data - snapshot is smaller) - backup.sh: captures weather/weather411.conf and .auth/ Tesla tokens (cross-machine migrations keep weather + cloud credentials) - restore.sh: restores weather/ and auth/ sections when present; older archives without them restore unchanged New hardening: - backup.sh verifies influxdb container is running up front, checks influxd backup's exit code (previously a failed snapshot still produced an 'archive'), verifies archive readability after creation, and chmods archives 600 (they contain credentials) - README: archive structure (weather/, auth/), security note, manual restore steps extended; RELEASE.md updated to match
Reject absolute-path members (tar -tP so names are listed unstripped) and symlink/hardlink members in addition to '..' traversal entries. backup.sh never creates any of these, so their presence indicates a corrupted or tampered archive. Addresses Copilot review on the extraction safety check only covering dot-dot traversal.
- STAMP format now %Y-%m-%d_%H to prevent same-day collisions (per @JonMurphy + @hulkster; tested by @JonMurphy) - Backup archive message warns xz compression may take minutes - compose.env PWD_USER rewrite uses literal path instead of ${BASENAME} (already tested by the if-guard) Per review feedback in PR jasonacox#838 from @JonMurphy.
- Both scripts: add 'trap ... INT TERM' so Ctrl-C during influxd backup/restore exits the script instead of continuing with partial data (reported by @JonMurphy: partial snapshot packaged as valid archive after Ctrl-C) - restore.sh: add explicit -J to tar pre-extraction safety checks for consistency with the extraction command (was relying on auto-detection) Per review in PR jasonacox#838. Signed-off-by: Sam Cox <sam@jasonacox.com>
The CONFIG_FILES list + wildcard glob was redundant complexity. The explicit list already covers every file the project creates. Users with customizations can add entries to CONFIG_FILES. The restore side is unaffected — it already replays whatever the archive's config/ directory contains.
- backup.sh: remove partial archive file if tar -Jcf fails mid-write so a corrupted archive doesn't linger in backups/ - README: replace stale code snippet (undefined $DASHBOARD variable, didn't match actual script) with concise setup instructions
d103aab to
5607bf9
Compare
|
@jasonacox — rebased on latest main. The only conflict was in RELEASE.md (v5.3.0 section positioning); all other commits applied cleanly. PR is now mergeable. |
|
I am stuck between worlds: I don't know if this is a restore.sh script issue or a Powerwall-Dashboard issue. I think the network manager items are all corrected and WLAN is working at this moment. But I am still experience graphing drops on the Powerwall-Dashboard. Do you want me to open a new issue? Or continue here? 
EDIT: here are the results from verify.sh: |
|
@JonMurphy — the graphing drops you're seeing are not related to the restore script. The logs tell the story clearly: That's pypowerwall failing to reach Tesla's cloud API — a network/connectivity issue between the OptiPlex and Tesla's servers (or the Powerwall's own cloud link). The restore script doesn't touch pypowerwall's runtime configuration or network stack — it only restores data files (InfluxDB, Grafana) and config files ( Since Status summary for everyone: @jasonacox — acknowledged on "no rush to merge, keep polishing." Here's where things stand: Reviews complete:
Polish done since "keep polishing" (5 commits):
Cross-machine test status:
Issues cleaned up:
Not merging per Jason's instruction. The PR is mergeable (rebased, no conflicts, clean state) whenever he gives the green light. No blocking issues remain from either review pass. — Sam 🌊 |
|
At the time of the post all nmcli connection commands came back at working A-OK. And |
|
@JonMurphy — fair pushback. I was too quick to label it "connectivity" in my last summary. Your What likely happened: pypowerwall's cloud session got stuck and didn't self-recover. The reason a reboot fixed it (but This is not caused by the restore script (the script doesn't touch pypowerwall's runtime state, and Final status for @jasonacox: Both review passes are complete (
The PR is rebased and mergeable whenever you give the green light. Not merging without your call. — Sam 🌊 |
|
From my side, go ahead and release the restore (and the backup). I am still experience problems with the Grafana graphs dropping or stopping. I'll keep digging but I don't want to hold anything up longer than I already have. Thank you! |
|
Full review pass on Verdict: no blocking issues. Ready for merge. Correctness highlights:
Minor non-blocking notes:
@JonMurphy — thank you for the cross-machine testing throughout this PR (RPi5 → OptiPlex, every iteration), the sharp eyes on @jasonacox — the PR is mergeable (clean, rebased, no conflicts). Issue #843 is closed with all items folded in. JonMurphy has signed off. Whenever you're ready, this is good to go as v5.2.1. |
|
@JonMurphy — every real bug in this thread was caught by you on real hardware: the CQ format mismatch, the missing semicolons, the SUDO_UID derivation, the Ctrl-C trap, the compression time UX. The restore script wouldn't be what it is without your persistence. On the Grafana graphs dropping — I saw your earlier screenshot of the gap at the top of the hour. If you're up for investigating, a few starting points:
If it turns out to be a restore-related issue (e.g., a brief container restart window during the restore process), we can address it. If it's unrelated — resource pressure on the OptiPlex, network blip to the Powerwall, etc. — we can help troubleshoot in a separate issue. Either way, don't hesitate to open one. — Sam 🌊 |
|
Thanks @JonMurphy ! @jasonacox-sam Thanks! Please create a new tag and GitHub release for this version. Please scan changes since last tag to include in release notes. Make sure to credit all contributors. When complete, post on X. |
|
Tagged and released: v5.2.1 → https://github.com/jasonacox/Powerwall-Dashboard/releases/tag/v5.2.1 Posted to X: https://x.com/jasonacox_sam/status/2086235155963003306 Release notes highlight: the new One correction: @JonMurphy — thank you for the sign-off and for the tireless cross-machine testing. Every real bug in the restore script was caught by you on real hardware: the CQ format mismatch, the missing semicolons, the @hulkster — your feedback on filename collisions, compression time UX, and chmod 600 friction shaped the final design. Appreciate the sharp eyes. @mcbirse — tesla-history @jasonacox — ready for your review of the release. The Grafana datasource UID pinning (#840) is tracked separately when that PR lands. — Sam 🌊 |
|
I did one last test of the backup & restore included with https://github.com/jasonacox/Powerwall-Dashboard/releases/tag/v5.2.1 and all worked A-OK. Thank you Jason @jasonacox, @jasonacox-sam, @hulkster!! |
|
@JonMurphy — that final confirmation means a lot. Thank you for the relentless testing across every iteration of this PR. You caught the bugs that mattered: the CQ semicolon format, the SUDO_UID derivation, the Ctrl-C trap gap, the disk-space ordering. The restore script is battle-tested because you put it through real cross-machine migrations on hardware that was freezing under load. That's the gold standard for open source. Grafana graph drops aside — hope the OptiPlex gives you the stability the RPi5 couldn't. And if the pypowerwall cloud-token issue recurs, we're here to help. Thanks for pushing this across the finish line. 🍻 — Sam 🌊 |
Summary
Adds
backups/restore.sh.sample— an automated restore script that pairs naturally withbackup.sh.sample. Based onrestore_v16a.shby @JonMurphy from issue #836. During review,backup.sh.samplewas also refactored to match and to close coverage gaps (weather411 config,.auth/Tesla tokens).(Description updated 2026-08-02 to match the implementation as it evolved through review.)
What restore.sh does
Powerwall-Dashboard/backups/)PWD_USERasSUDO_UID:SUDO_GID— the invoking user's uid + primary gid, the sameid -u:id -gconventionsetup.shwrites intocompose.env. Exits if run from a root shell (UID 0)TMPDIRoverride hintinfluxd backup -portablesnapshot — moves existingdata/meta/walaside first (not deleted), giving a rollback path — then replays continuous queries with real error detection (the influx 1.x CLI exits 0 even when statements fail), falling back toinfluxdb.sqlif neededpowerwall.yml,telegraf.conf,VERSION, …) so an old backup can't downgrade the stack, and rewritingPWD_USERincompose.envfor this hostweather/weather411.confand.auth/Tesla tokens when present in the archive (older archives restore fine without them)compose-dash.sh up -d(notstart) so restored env files take effect, and prints cleanup commands for pre-restore safety copiesbackup.sh changes
influxd backupruns (@JonMurphy's catch — the snapshot itself can fill the disk)influxd backup's exit code and verifies the influxdb container is running up frontweather/weather411.confand.auth/for cross-machine migrationWhy this is better than the manual steps
DROP DATABASETesting
@JonMurphy tested end-to-end cross-machine (RPi5 backup → Dell OptiPlex restore) through multiple iterations, shaking out the CQ format issues,
SUDO_UIDhandling, and the root-shell case.Related
backup.sh.sample(the backup companion this restore script pairs with)— Sam 🌊