-
Notifications
You must be signed in to change notification settings - Fork 112
DB Backup - Design doc and initial changes #1717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
1a886ad
to
39140b4
Compare
WalkthroughAdds database backup and recovery configuration to the NooBaa CRD and API types, supporting scheduled volume snapshot backups and recovery from a snapshot. Updates generated deepcopy methods and CRD bundle constants. Introduces two documentation files detailing CNPG integration and the backup/recovery design. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant NooBaa CR as NooBaa CR (dbSpec)
participant Operator as NooBaa Operator
participant CNPG as CNPG Controller
participant K8s as Kubernetes API
participant VS as VolumeSnapshot
rect rgb(245,248,255)
note over User,Operator: Configure automated DB backups
User->>K8s: Apply NooBaa with spec.dbSpec.dbBackup
K8s-->>Operator: Reconcile NooBaa
Operator->>K8s: Create/Update CNPG ScheduledBackup (VolumeSnapshotClass, schedule)
CNPG->>K8s: Create VolumeSnapshot per schedule
K8s-->>Operator: List snapshots
Operator->>Operator: Enforce maxSnapshots (delete oldest)
Operator->>K8s: Update NooBaa.status.dbStatus.backupStatus
end
alt Recovery configured
note over User,Operator: Recovery from a snapshot
User->>K8s: Update NooBaa spec.dbSpec.dbRecovery.volumeSnapshotName
K8s-->>Operator: Reconcile NooBaa
Operator->>K8s: Initiate CNPG bootstrap from VolumeSnapshot
Operator->>K8s: Update recoveryStatus (Pending/Running)
CNPG->>K8s: Restore PVCs/DB from snapshot
CNPG-->>Operator: Completion/Failure event
Operator->>K8s: Update recoveryStatus (Completed/Failed) and db cluster status
else No recovery
note over Operator: No recovery flow executed
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Signed-off-by: Danny Zaken <[email protected]> doc changes
Signed-off-by: Danny Zaken <[email protected]>
39140b4
to
1cda15d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
deploy/crds/noobaa.io_noobaas.yaml
(4 hunks)doc/design/noobaa-cnpg-db-backup.md
(1 hunks)doc/noobaa-cnpg-integration.md
(1 hunks)pkg/apis/noobaa/v1alpha1/noobaa_types.go
(3 hunks)pkg/apis/noobaa/v1alpha1/zz_generated.deepcopy.go
(4 hunks)pkg/bundle/deploy.go
(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/apis/noobaa/v1alpha1/zz_generated.deepcopy.go (1)
pkg/apis/noobaa/v1alpha1/noobaa_types.go (4)
DBBackupSpec
(378-387)DBBackupStatus
(557-569)DBRecoverySpec
(390-393)DBRecoveryStatus
(572-581)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: run-azure-vault-test
- GitHub Check: run-cli-tests
- GitHub Check: run-core-config-map-tests
- GitHub Check: cnpg-deployment-test
- GitHub Check: run-kms-tls-sa-test
- GitHub Check: run-hac-test
- GitHub Check: run-kms-kmip-test
- GitHub Check: run-admission-test
- GitHub Check: golangci-lint
- GitHub Check: run-kms-dev-test
- GitHub Check: run-kms-tls-token-test
- GitHub Check: run-kms-key-rotate-test
r.CNPGCluster.Spec.Backup = &cnpgv1.BackupConfiguration{ | ||
VolumeSnapshot: &cnpgv1.VolumeSnapshotConfiguration{ | ||
Class: backupSpec.VolumeSnapshotClass, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix VolumeSnapshotConfiguration field name
cnpgv1.VolumeSnapshotConfiguration
uses ClassName
, not Class
. Using Class
will not compile against CNPG v1 APIs and misleads anyone implementing from this design. Please update the snippet to reference ClassName
.
🤖 Prompt for AI Agents
In doc/design/noobaa-cnpg-db-backup.md around lines 190 to 194, the code snippet
sets VolumeSnapshot configuration using the non-existent field Class; update it
to use the correct CNPG v1 API field name ClassName instead. Replace the Class:
backupSpec.VolumeSnapshotClass entry with ClassName:
backupSpec.VolumeSnapshotClass so the snippet compiles against CNPG v1 and
accurately reflects the API.
r.CNPGCluster.Spec.Bootstrap = &cnpgv1.BootstrapConfiguration{ | ||
Recovery: &cnpgv1.BootstrapRecovery{ | ||
Source: snapshotName, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct BootstrapRecovery structure
cnpgv1.BootstrapRecovery
does not accept a bare Source
string for snapshot recovery. It expects the nested VolumeSnapshot
field (with Name
, optional Kind
/APIGroup
). Documenting Source
here will produce invalid code/templates. Please revise the example to set VolumeSnapshot: &cnpgv1.VolumeSnapshotSource{Name: snapshotName}
(and related fields as needed).
🤖 Prompt for AI Agents
In doc/design/noobaa-cnpg-db-backup.md around lines 304 to 308, the example sets
Recovery.Source to a bare string which is invalid; update the BootstrapRecovery
to populate the nested VolumeSnapshot field instead (e.g., VolumeSnapshot:
&cnpgv1.VolumeSnapshotSource{Name: snapshotName} and include Kind/APIGroup if
needed) so the structure matches the cnpgv1.BootstrapRecovery schema.
Explain the changes
noobaa-cnpg-integration.md
doc.noobaa-cnpg-db-backup.md
NooBaaDBSpec
andNooBaaDBStatus
types to support backup and recovery.Issues: Fixed #xxx / Gap #xxx
Testing Instructions:
Summary by CodeRabbit
New Features
Documentation