CNTRLPLANE-3173: persist lastSuccessfulEtcdBackupURL in HostedCluster status#8179
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Skipping CI for Draft Pull Request. |
|
@jparrill: This pull request references CNTRLPLANE-3173 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds persistence of the most recent successful etcd backup URL to HostedCluster status via a new optional field Sequence DiagramsequenceDiagram
participant BackupJob as Backup Job/Pod
participant CloudStorage as Cloud Storage
participant Reconciler as HCPEtcdBackupReconciler
participant HCP as HostedControlPlane
participant HC as HostedCluster
BackupJob->>CloudStorage: Upload etcd snapshot
CloudStorage-->>BackupJob: Return snapshot URL
BackupJob->>BackupJob: Expose snapshot URL (logs/status)
Reconciler->>BackupJob: Poll job status
BackupJob-->>Reconciler: JobComplete with snapshot info
Reconciler->>Reconciler: Extract snapshotURL from pod/logs
Reconciler->>HCP: Read hosted-cluster annotation
HCP-->>Reconciler: Provide HostedCluster reference
Reconciler->>HC: Get HostedCluster object
HC-->>Reconciler: Return HostedCluster
Reconciler->>HC: Update Status.LastSuccessfulEtcdBackupURL = snapshotURL
HC-->>Reconciler: Persisted status
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@jparrill: This pull request references CNTRLPLANE-3173 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8179 +/- ##
=======================================
Coverage 33.89% 33.90%
=======================================
Files 768 768
Lines 93163 93177 +14
=======================================
+ Hits 31581 31591 +10
- Misses 58924 58928 +4
Partials 2658 2658
🚀 New features to boost your workflow:
|
|
/label tide/merge-method-squash |
a8fa2e8 to
1d16710
Compare
1d16710 to
254d57a
Compare
|
@jparrill: This pull request references CNTRLPLANE-3173 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
27459ec to
fe32925
Compare
fe32925 to
ab75fad
Compare
ab75fad to
98a0c83
Compare
Add LastSuccessfulEtcdBackupURL field to HostedClusterStatus so the most recent successful etcd backup snapshot URL survives HCPEtcdBackup CR retention/deletion. The etcdbackup controller writes the URL directly to the HostedCluster status on backup success, resolving the URL from the HCP's HostedClusterAnnotation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Regenerate after adding LastSuccessfulEtcdBackupURL to HostedClusterStatus. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Add CEL validation rule to prevent day-2 modifications of the restoreSnapshotURL field. This ensures the field can only be set at creation time, which aligns with the existing API contract and makes the OADP restoration workflow safe without changing the field semantics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
…bility Generated CRD manifests reflecting the new CEL validation rule on the restoreSnapshotURL field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
98a0c83 to
647381e
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre, jparrill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
Scheduling tests matching the |
|
/verified bypass More info in here: https://redhat-internal.slack.com/archives/G01QS0P2F6W/p1775814810815969?thread_ts=1775808691.857879&cid=G01QS0P2F6W |
|
@jparrill: The DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest-required |
|
@jparrill: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
… status (openshift#8179) * feat(CNTRLPLANE-3173): persist lastSuccessfulEtcdBackupURL in HC status Add LastSuccessfulEtcdBackupURL field to HostedClusterStatus so the most recent successful etcd backup snapshot URL survives HCPEtcdBackup CR retention/deletion. The etcdbackup controller writes the URL directly to the HostedCluster status on backup success, resolving the URL from the HCP's HostedClusterAnnotation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * chore(generated): regenerate CRDs, clients, vendor, and docs Regenerate after adding LastSuccessfulEtcdBackupURL to HostedClusterStatus. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * feat(CNTRLPLANE-2678): enforce restoreSnapshotURL immutability via CEL Add CEL validation rule to prevent day-2 modifications of the restoreSnapshotURL field. This ensures the field can only be set at creation time, which aligns with the existing API contract and makes the OADP restoration workflow safe without changing the field semantics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * chore(CNTRLPLANE-2678): regenerate CRDs for restoreSnapshotURL immutability Generated CRD manifests reflecting the new CEL validation rule on the restoreSnapshotURL field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> --------- Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
… status (openshift#8179) * feat(CNTRLPLANE-3173): persist lastSuccessfulEtcdBackupURL in HC status Add LastSuccessfulEtcdBackupURL field to HostedClusterStatus so the most recent successful etcd backup snapshot URL survives HCPEtcdBackup CR retention/deletion. The etcdbackup controller writes the URL directly to the HostedCluster status on backup success, resolving the URL from the HCP's HostedClusterAnnotation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * chore(generated): regenerate CRDs, clients, vendor, and docs Regenerate after adding LastSuccessfulEtcdBackupURL to HostedClusterStatus. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * feat(CNTRLPLANE-2678): enforce restoreSnapshotURL immutability via CEL Add CEL validation rule to prevent day-2 modifications of the restoreSnapshotURL field. This ensures the field can only be set at creation time, which aligns with the existing API contract and makes the OADP restoration workflow safe without changing the field semantics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * chore(CNTRLPLANE-2678): regenerate CRDs for restoreSnapshotURL immutability Generated CRD manifests reflecting the new CEL validation rule on the restoreSnapshotURL field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> --------- Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
… status (openshift#8179) * feat(CNTRLPLANE-3173): persist lastSuccessfulEtcdBackupURL in HC status Add LastSuccessfulEtcdBackupURL field to HostedClusterStatus so the most recent successful etcd backup snapshot URL survives HCPEtcdBackup CR retention/deletion. The etcdbackup controller writes the URL directly to the HostedCluster status on backup success, resolving the URL from the HCP's HostedClusterAnnotation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * chore(generated): regenerate CRDs, clients, vendor, and docs Regenerate after adding LastSuccessfulEtcdBackupURL to HostedClusterStatus. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * feat(CNTRLPLANE-2678): enforce restoreSnapshotURL immutability via CEL Add CEL validation rule to prevent day-2 modifications of the restoreSnapshotURL field. This ensures the field can only be set at creation time, which aligns with the existing API contract and makes the OADP restoration workflow safe without changing the field semantics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * chore(CNTRLPLANE-2678): regenerate CRDs for restoreSnapshotURL immutability Generated CRD manifests reflecting the new CEL validation rule on the restoreSnapshotURL field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> --------- Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
What this PR does / why we need it:
HCPEtcdBackupCRs are ephemeral — the retention mechanism deletes old CRs onceMaxBackupCountis exceeded. When a CR is deleted, itsstatus.snapshotURLis lost. During an OADP restore workflow, the restore operator needs the URL of the last successful etcd backup snapshot, but theHCPEtcdBackupCR that held it may no longer exist.This PR adds a
lastSuccessfulEtcdBackupURLfield toHostedClusterStatusand updates the etcdbackup controller to persist the snapshot URL directly in the HostedCluster status on backup success.Changes:
LastSuccessfulEtcdBackupURLstring field toHostedClusterStatus, feature-gated behindHCPEtcdBackupupdateHostedClusterBackupURL()method that resolves the HostedCluster via the HCP'sHostedClusterAnnotationand writes the URL toHC.Status.LastSuccessfulEtcdBackupURLon backup successTestUpdateHostedClusterBackupURL(3 cases) and update existing success test to verify HC status propagationWhich issue(s) this PR fixes:
Fixes CNTRLPLANE-3173
Dependencies
The field is only added to
HostedClusterStatus(notHostedControlPlaneStatus) since the etcdbackup controller runs in the HO and there is no precedent for the HO writing to HCP status.Checklist:
🤖 Generated with Claude Code via
/jira:solve [CNTRLPLANE-3173](https://redhat.atlassian.net/browse/CNTRLPLANE-3173)Summary by CodeRabbit
New Features
Improvements
Tests