docs(platform): add guides related managed apps backups#536
Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds tenant and admin documentation for BackupClass-driven, data-only backups and restores for managed Postgres, MariaDB, ClickHouse, and FoundationDB, plus deprecation callouts and scope clarifications in related application and VM backup docs. ChangesBackup and Recovery Framework Documentation
Sequence Diagram(s)sequenceDiagram
participant Admin
participant BackupController
participant Tenant
participant DriverOperator
participant S3
Admin->>BackupController: create BackupClass (strategy + parameters)
Tenant->>BackupController: submit BackupJob / Plan referencing BackupClass
BackupController->>DriverOperator: render and apply driver strategy (CR or sidecar call)
DriverOperator->>S3: write backup artefacts to bucket
DriverOperator->>BackupController: create Backup CR / report status
Tenant->>BackupController: submit RestoreJob (optional targetApplicationRef)
BackupController->>DriverOperator: trigger restore using Backup artefact
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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. 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 introduces a new data-only backup and recovery framework for managed databases (Postgres, MariaDB, ClickHouse, and FoundationDB) in Cozystack, transitioning from legacy chart-level configurations to a centralized system using BackupClass and strategy-based resources. It provides detailed documentation for both tenants and administrators. Reviewers suggested several improvements, including standardizing FoundationDB account names for better multi-tenancy, enhancing security by using non-root users in backup pods, and clarifying S3 endpoint terminology to prevent configuration errors.
| compression: gzip | ||
| ``` | ||
|
|
||
| The `endpoint` is **path-style without scheme** (e.g. `seaweedfs-s3.<seaweedfs-namespace>.svc:8333` for the default in-cluster SeaweedFS — substitute the namespace where SeaweedFS is deployed in your environment). Drop the `tls` block entirely when the endpoint serves a publicly-trusted certificate. |
There was a problem hiding this comment.
The term "path-style" in the context of S3 usually refers to the addressing mode (e.g., s3.amazonaws.com/bucket vs bucket.s3.amazonaws.com). Here, it seems to be used to describe the endpoint format (host and port without scheme). To avoid confusion, it might be clearer to explicitly state that the endpoint should contain only the host and port.
| The `endpoint` is **path-style without scheme** (e.g. `seaweedfs-s3.<seaweedfs-namespace>.svc:8333` for the default in-cluster SeaweedFS — substitute the namespace where SeaweedFS is deployed in your environment). Drop the `tls` block entirely when the endpoint serves a publicly-trusted certificate. | |
| The `endpoint` should be provided as **host:port without scheme** (e.g. `seaweedfs-s3.<seaweedfs-namespace>.svc:8333` for the default in-cluster SeaweedFS — substitute the namespace where SeaweedFS is deployed in your environment). Drop the `tls` block entirely when the endpoint serves a publicly-trusted certificate. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
content/en/docs/next/applications/backup-and-recovery.md (1)
36-43: ⚡ Quick winConsider adding a language identifier to the fenced code block.
The output example is missing a language identifier. Adding
textorconsolewould satisfy linters and improve rendering:-``` +```text NAME AGE postgres-data-backup 14m🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@content/en/docs/next/applications/backup-and-recovery.md` around lines 36 - 43, The fenced code block in the backup-and-recovery.md example lacks a language identifier; update the triple-backtick fence around the output listing (the block showing NAME / AGE entries like "postgres-data-backup" and "velero") to include a language such as "text" or "console" (e.g., change ``` to ```text) so linters/renderers recognize it as plain output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@content/en/docs/next/applications/backup-and-recovery.md`:
- Around line 36-43: The fenced code block in the backup-and-recovery.md example
lacks a language identifier; update the triple-backtick fence around the output
listing (the block showing NAME / AGE entries like "postgres-data-backup" and
"velero") to include a language such as "text" or "console" (e.g., change ``` to
```text) so linters/renderers recognize it as plain output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f871a89b-ab72-46ba-b44a-b1b590bb3620
📒 Files selected for processing (8)
content/en/docs/next/applications/backup-and-recovery.mdcontent/en/docs/next/applications/clickhouse.mdcontent/en/docs/next/applications/foundationdb.mdcontent/en/docs/next/applications/mariadb.mdcontent/en/docs/next/applications/postgres.mdcontent/en/docs/next/operations/services/managed-app-backup-configuration.mdcontent/en/docs/next/operations/services/velero-backup-configuration.mdcontent/en/docs/next/virtualization/backup-and-recovery.md
androndo
force-pushed
the
feat/apps-backups-guides
branch
2 times, most recently
from
0b9ead0 to
237d627
Compare
|
NOT LGTM Reviewing against Primary blocker — tenant guide is unrunnable as a tenant
Neither is permitted by the aggregated tenant roles. Verified against
Meanwhile the COSI flow in So the tenant guide is broken at step one for every driver except ClickHouse (which routes through chart values, not a hand-rolled Secret). A user following this guide on a real cluster sees 403 on the very first This is not a wording fix. The doc as written exposes a missing piece in the implementation: there is no tenant-visible path from a VerificationEnd-to-end test should run the tenant guide as a tenant ServiceAccount (one of `cozy:tenant:admin` / `cozy:tenant:use`), impersonated via `--as`, against a real cluster, and complete a Postgres / MariaDB / FoundationDB backup and restore without ever using a cluster-admin kubeconfig. A reproducible CI variant lives in `cozystack/cozystack/examples/backups//run-all.sh`; those scripts currently assume cluster-admin and would need a parallel `run-all-as-tenant.sh` variant under the chosen ServiceAccount. Secondary issues (correctness)These are real, but each one sits inside text that will move or vanish when the primary blocker is addressed. Worth folding into the rewrite rather than spot-patching now.
Out of scope / fine as-is
|
androndo
force-pushed
the
feat/apps-backups-guides
branch
from
237d627 to
a3e538b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@content/en/docs/next/operations/services/managed-app-backup-configuration.md`:
- Around line 271-272: The pod spec currently sets securityContext.runAsUser: 0
which runs the backup_agent as root; change the container/pod security context
to run as a non-root UID (use the FoundationDB canonical UID 4059) by replacing
runAsUser: 0 with runAsUser: 4059 (or another non-root UID) and ensure any
filesystem permissions and capabilities for the backup_agent are adjusted
accordingly; update the securityContext section for the backup_agent
container/pod to reflect the non-root runAsUser and keep other securityContext
settings intact.
- Around line 369-372: Update the readiness checks after applying bucket.yaml to
wait not only for the HelmRelease (hr/bucket-db-backups) ready condition but
also for the BucketClaim and BucketAccess conditions that ensure credentials are
populated; add kubectl -n tenant-user wait commands targeting the BucketClaim
resource (check .status.bucketReady=true) and the BucketAccess resource (check
.status.accessGranted=true) using the same logical names as the HelmRelease so
the Secret is guaranteed to be populated before proceeding.
🪄 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: 0d813be1-12ca-41ac-a247-4943f81e047c
📒 Files selected for processing (8)
content/en/docs/next/applications/backup-and-recovery.mdcontent/en/docs/next/applications/clickhouse.mdcontent/en/docs/next/applications/foundationdb.mdcontent/en/docs/next/applications/mariadb.mdcontent/en/docs/next/applications/postgres.mdcontent/en/docs/next/operations/services/managed-app-backup-configuration.mdcontent/en/docs/next/operations/services/velero-backup-configuration.mdcontent/en/docs/next/virtualization/backup-and-recovery.md
✅ Files skipped from review due to trivial changes (7)
- content/en/docs/next/virtualization/backup-and-recovery.md
- content/en/docs/next/applications/postgres.md
- content/en/docs/next/applications/mariadb.md
- content/en/docs/next/applications/foundationdb.md
- content/en/docs/next/applications/clickhouse.md
- content/en/docs/next/operations/services/velero-backup-configuration.md
- content/en/docs/next/applications/backup-and-recovery.md
| ```bash | ||
| kubectl apply -f bucket.yaml | ||
| kubectl -n tenant-user wait hr/bucket-db-backups --for=condition=ready --timeout=300s | ||
| ``` |
There was a problem hiding this comment.
Add bucketclaim and bucketaccess readiness checks.
The current wait only checks HelmRelease ready status. Upstream examples also wait for the bucketclaim .status.bucketReady=true and bucketaccess .status.accessGranted=true conditions to guarantee credential population before proceeding to read the Secret.
🔧 Proposed fix to add complete readiness checks
kubectl apply -f bucket.yaml
kubectl -n tenant-user wait hr/bucket-db-backups --for=condition=ready --timeout=300s
+kubectl -n tenant-user wait bucketclaim/bucket-db-backups --for=jsonpath='{.status.bucketReady}'=true --timeout=60s
+kubectl -n tenant-user wait bucketaccess/bucket-db-backups-backup --for=jsonpath='{.status.accessGranted}'=true --timeout=60s📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```bash | |
| kubectl apply -f bucket.yaml | |
| kubectl -n tenant-user wait hr/bucket-db-backups --for=condition=ready --timeout=300s | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/en/docs/next/operations/services/managed-app-backup-configuration.md`
around lines 369 - 372, Update the readiness checks after applying bucket.yaml
to wait not only for the HelmRelease (hr/bucket-db-backups) ready condition but
also for the BucketClaim and BucketAccess conditions that ensure credentials are
populated; add kubectl -n tenant-user wait commands targeting the BucketClaim
resource (check .status.bucketReady=true) and the BucketAccess resource (check
.status.accessGranted=true) using the same logical names as the HelmRelease so
the Secret is guaranteed to be populated before proceeding.
a3e538b to
5eef4d3
Compare
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
androndo
force-pushed
the
feat/apps-backups-guides
branch
from
5eef4d3 to
90c390a
Compare
Adds the missing entry for #2649 (OpenSearch wired into PaaS bundle, plus StorageClass dropdown override for the legacy 1.3 openapi-ui dashboard), corrects six website-PR author attributions against gh pr view, picks up two website docs PRs merged just before the v1.3.4 cut (cozystack/website#536 and #538), updates the contributors list, and refreshes the date to today. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
3 participants
Summary by CodeRabbit