docs: clarify backup and restore coverage#72
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
brin-security-scanner
Bot
added
the
contributor:flagged
|
| Dimension | Score | What it measures |
|---|---|---|
| Identity | 45 | Account age, contribution history, GPG keys, org memberships |
| Behavior | 90 | PR patterns, unsolicited contribution ratio, activity cadence |
| Content | 100 | PR body substance, issue linkage, contribution quality |
| Graph | 30 | Cross-repo trust, co-contributor relationships |
Analyzed by Brin · Full profile
📝 WalkthroughWalkthroughDocumentation updates to backup and restore procedures in Helm and Docker Compose self-hosting guides, adding explicit warnings about backup scope, step-by-step restore instructions, separate guidance for uploading files, and post-restore verification procedures. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the self-hosting documentation for Helm and Docker Compose by adding comprehensive backup and restore procedures, including warnings about file storage and verification steps. The review feedback suggests providing explicit commands to stop Kubernetes workloads, ensuring the $PRIMARY_POD variable is defined for standalone restore steps, and advising users on how to avoid SQL restoration conflicts.
|
|
||
| ### Restore from backup | ||
|
|
||
| 1. Make sure app traffic is stopped or the deployment is in maintenance mode. |
There was a problem hiding this comment.
For a more guided experience, it is helpful to provide the specific command to stop the application workloads. This ensures that no new data is written to the database during the restore process.
Example command:
kubectl scale deployment -n sure sure-web sure-worker --replicas=0|
|
||
| ```bash | ||
| # Copy backup to pod | ||
| kubectl cp backup.sql sure/$PRIMARY_POD:/tmp/backup.sql |
There was a problem hiding this comment.
The $PRIMARY_POD variable is defined in the previous "Manual backup" section. If a user jumps directly to the restore steps, this command will fail because the variable is not set in their current shell session. Including the lookup command here makes the restore process self-contained and more robust.
PRIMARY_POD=$(kubectl get pod -n sure -l cnpg.io/cluster=sure-db,role=primary -o name)
kubectl cp backup.sql sure/$PRIMARY_POD:/tmp/backup.sql
|
|
||
| # Restore | ||
| ```bash | ||
| kubectl exec -n sure $PRIMARY_POD -- psql -U sure sure_production -f /tmp/backup.sql |
There was a problem hiding this comment.
Restoring a SQL dump into an existing database that already contains schema and data can lead to conflicts (e.g., relation already exists errors). It is recommended to mention that the target database should be empty, or suggest using pg_dump --clean --if-exists when creating the backup to ensure a smooth restoration.
| 4. Restore the SQL backup into PostgreSQL: | ||
|
|
||
| ```bash | ||
| docker compose exec -T db psql -U sure_user -d sure_production < /path/to/backup.sql |
There was a problem hiding this comment.
Similar to the Helm restore process, piping a SQL dump into an existing PostgreSQL database via psql may result in errors if the tables already exist. Consider adding a note advising users to ensure the database is empty or to use a 'clean' dump to avoid restoration failures.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@self-hosting-helm.mdx`:
- Around line 361-365: The PRIMARY_POD variable is being set with kubectl -o
name (which yields "pod/<name>") and later used in the kubectl cp command
(kubectl cp backup.sql sure/$PRIMARY_POD:/tmp/backup.sql) causing an invalid
three-part filespec; change how PRIMARY_POD is populated (e.g., use kubectl with
-o jsonpath='{.items[0].metadata.name}' or otherwise strip the "pod/" prefix) so
it contains only the pod name, then keep the kubectl cp invocation as
sure/$PRIMARY_POD:/tmp/backup.sql so the filespec matches the required
[[namespace/]pod:]path format.
In `@self-hosting.mdx`:
- Around line 312-323: The doc currently restarts containers (the "docker
compose up -d web worker" step under the "Restart the app" section) before
instructing users to restore local uploaded files under the "Restore local
uploaded files" heading, which can bring the app up with missing files; change
the flow and wording so the restore happens first and the restart happens after:
move or edit the sentence that says "Restore the same directory that Sure uses
for local storage, then restart the app containers." to explicitly instruct
restoring the host storage directory before starting the app containers and
remove the earlier immediate restart instruction, ensuring the "docker compose
up -d web worker" command appears only after the restore guidance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f78ec7f0-c1f0-49c3-bb07-36a7a1bfd56f
📒 Files selected for processing (2)
self-hosting-helm.mdxself-hosting.mdx
| 2. Copy the SQL backup to the primary PostgreSQL pod: | ||
|
|
||
| ```bash | ||
| # Copy backup to pod | ||
| kubectl cp backup.sql sure/$PRIMARY_POD:/tmp/backup.sql | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="$(fd '^self-hosting-helm\.mdx$' | head -n1)"
rg -n -C2 'PRIMARY_POD|kubectl cp backup\.sql|kubectl exec -n sure \$PRIMARY_POD' "$FILE"Repository: we-promise/docs
Length of output: 536
🌐 Web query:
In Kubernetes, for kubectl cpfile-spec[[namespace/]pod:]file/path, is pod/valid as the pod token, or must it be just?
💡 Result:
No, pod/ is not valid as the pod token in the kubectl cp file-spec [[namespace/]pod:]file/path. The pod token must be just (optionally prefixed by namespace/ if specifying a non-default namespace). The official Kubernetes documentation shows examples using :path for default namespace and /:path for specific namespaces, with no pod/ prefix. The source code parser in pkg/cmd/cp/cp.go splits the part before the colon (:) by / and accepts exactly 1 piece (PodName) or 2 pieces (PodNamespace/PodName). Three pieces like pod/ would result in more than 2, causing "filespec must match the canonical format: [[namespace/]pod:]file/path" error. All examples across docs, man pages, and tutorials consistently use or namespace/, never pod/. The pod/ prefix appears only in explanatory text like "pod token", not in actual syntax.
Citations:
- 1: https://kubernetes.io/docs/reference/kubectl/generated/kubectl_cp/
- 2: https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/cp/cp.go
- 3: https://kubernetes.io/docs/reference/kubectl/generated/kubectl_cp
Fix PRIMARY_POD format for kubectl cp in restore steps.
Line 348 sets PRIMARY_POD using -o name, which outputs pod/<name> format. When this variable is used in line 364 as sure/$PRIMARY_POD:/tmp/backup.sql, it expands to sure/pod/<name>, which violates kubectl cp file-spec syntax. The kubectl cp parser expects either <pod>:path or <namespace>/<pod>:path (two parts when split by /), not three parts. This causes a "filespec must match the canonical format: [[namespace/]pod:]file/path" error.
Extract only the pod name (without the pod/ prefix) using -o jsonpath='{.items[0].metadata.name}' instead of -o name.
Suggested doc patch
-PRIMARY_POD=$(kubectl get pod -n sure -l cnpg.io/cluster=sure-db,role=primary -o name)
+PRIMARY_POD=$(kubectl get pod -n sure -l cnpg.io/cluster=sure-db,role=primary -o jsonpath='{.items[0].metadata.name}')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@self-hosting-helm.mdx` around lines 361 - 365, The PRIMARY_POD variable is
being set with kubectl -o name (which yields "pod/<name>") and later used in the
kubectl cp command (kubectl cp backup.sql sure/$PRIMARY_POD:/tmp/backup.sql)
causing an invalid three-part filespec; change how PRIMARY_POD is populated
(e.g., use kubectl with -o jsonpath='{.items[0].metadata.name}' or otherwise
strip the "pod/" prefix) so it contains only the pod name, then keep the kubectl
cp invocation as sure/$PRIMARY_POD:/tmp/backup.sql so the filespec matches the
required [[namespace/]pod:]path format.
| 5. Restart the app: | ||
|
|
||
| ```bash | ||
| docker compose up -d | ||
| docker compose up -d web worker | ||
| ``` | ||
|
|
||
| ### Restore local uploaded files | ||
|
|
||
| If your Sure instance stores uploaded files on the local filesystem, restoring the database alone is not enough. You must also restore the app's storage directory from the matching file backup. | ||
|
|
||
| The exact host path depends on how you mapped volumes in `compose.yml`. Restore the same directory that Sure uses for local storage, then restart the app containers. | ||
|
|
There was a problem hiding this comment.
Avoid conflicting restart order in restore flow.
Line 315 restarts web/worker, but Line 322 later says restore local storage then restart containers. For full recovery, this is contradictory and can bring the app up before file state is restored.
Suggested doc adjustment
-5. Restart the app:
+5. If you are restoring only PostgreSQL, restart the app:
```bash
docker compose up -d web workerRestore local uploaded files
If your Sure instance stores uploaded files on the local filesystem, restoring the database alone is not enough. You must also restore the app's storage directory from the matching file backup.
-The exact host path depends on how you mapped volumes in compose.yml. Restore the same directory that Sure uses for local storage, then restart the app containers.
+The exact host path depends on how you mapped volumes in compose.yml. Restore the same directory that Sure uses for local storage before starting the app containers.
If you are using external object storage instead of local disk, restore those files using that provider's backup or versioning workflow instead.
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @self-hosting.mdx around lines 312 - 323, The doc currently restarts
containers (the "docker compose up -d web worker" step under the "Restart the
app" section) before instructing users to restore local uploaded files under the
"Restore local uploaded files" heading, which can bring the app up with missing
files; change the flow and wording so the restore happens first and the restart
happens after: move or edit the sentence that says "Restore the same directory
that Sure uses for local storage, then restart the app containers." to
explicitly instruct restoring the host storage directory before starting the app
containers and remove the earlier immediate restart instruction, ensuring the
"docker compose up -d web worker" command appears only after the restore
guidance.
</details>
<!-- fingerprinting:phantom:poseidon:hawk:7b3d8bf9-5981-403d-9ec7-82b63ac1b58f -->
<!-- 4e71b3a2 -->
<!-- This is an auto-generated comment by CodeRabbit -->
1 participant
Summary
Why
The existing self-hosting docs explain how to restore a PostgreSQL dump, but they do not clearly call out that database-only backups do not include local uploaded files or other deployment state. This makes the restore path look more complete than it is.
Summary by CodeRabbit