fix(hypershift): remove unused VolumeSnapshotLocation in OADP setup - #83883
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughThe Hypershift OADP setup scripts no longer create or apply ChangesOADP setup simplification
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to This localized change removes unused OADP resource creation without any supplied indication of a current correctness, availability, security, or deployment risk; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. Full details: Stable And Deterministic Test NamesExplanation PASS: The pull request changes only two shell setup scripts. The diff removes Full details: Test Structure And QualityExplanation PASS: The pull request changes only two shell setup scripts. The exact diff removes Full details: Microshift Test CompatibilityExplanation PASS: The pull request changes only two shell setup scripts. The diff removes Full details: Single Node Openshift (Sno) Test CompatibilityExplanation PASS: The pull request adds no Ginkgo tests. The verified diff contains only 35 deletions from two OADP setup shell scripts, and no added lines or test constructs such as It(), Describe(), Context(), or When(). The SNO compatibility check is therefore not applicable. Full details: Topology-Aware Scheduling CompatibilityExplanation PASS: The pull request changes only two Bash setup scripts, as shown by the HEAD^..HEAD path diff. The patch removes Full details: Ote Binary Stdout ContractExplanation PASS: The pull request changes only two shell setup scripts. The exact diff removes Full details: Ipv6 And Disconnected Network Test CompatibilityExplanation PASS: The pull request adds no Ginkgo e2e tests. The verified diff contains only 35 deletions from two OADP shell setup scripts, with zero additions. Therefore the IPv4 and external-connectivity conditions in this check do not apply. Full details: No-Weak-CryptoExplanation PASS: The PR only deletes Full details: Container-PrivilegesExplanation PASS — The pull request only deletes two Full details: No-Sensitive-Data-In-LogsExplanation PASS: The pull request only removes the ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/pj-rehearse periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-agent-connected-ovn-ipv4-metal-backuprestore periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-kubevirt-metal-ovn-backuprestore |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
Setup failing due to https://redhat.atlassian.net/browse/OCPBUGS-112342 , need to re-run after merging the related PR with the fix. |
|
/pj-rehearse periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-agent-connected-ovn-ipv4-metal-backuprestore periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-kubevirt-metal-ovn-backuprestore |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-agent-connected-ovn-ipv4-metal-backuprestore |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-agent-connected-ovn-ipv4-metal-backuprestore |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse ack The backup-restore tests passed on Baremetal+Kubevirt, only a few conformance tests failed, which is unrelated |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/hold I will update this for AWS too because VolumeSnapshotLocation should not be required either. |
Removes VolumeSnapshotLocation resource creation from OADP v2 setup step for hypershift, simplifying the configuration. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1507a88 to
fab7d8d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/unhold Removed VolumeSnapshotLocation from AWS too. We use CSI snapshots and VSL is not required for that. It is only required for volume snapshots with older non-CSI setup. |
|
[REHEARSALNOTIFIER]
A total of 33 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-v2-aws-backuprestore |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
TODO: Update docs for AWS backup/restore if VolumeSnapshotLocation is not required (using CSI) |
|
/pj-rehearse periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-v2-aws-backuprestore |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-agent-connected-ovn-ipv4-metal-backuprestore |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-agent-connected-ovn-ipv4-metal-backuprestore |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/retest-required |
|
/pj-rehearse periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-kubevirt-metal-ovn-backuprestore periodic-ci-openshift-hypershift-release-4.22-periodics-mce-e2e-agent-connected-ovn-ipv4-metal-backuprestore |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@mgencur: The following tests failed, say
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. |
|
/pj-rehearse ack |
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
The AWS and Baremetal/Agent configurations fully passed. The KubeVirt one has one failure in conformance tests which is unrelated to backup/restore. This is ready for merging. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill, mgencur 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 |
13e5fbe
into
openshift:main
Removes VolumeSnapshotLocation resource creation from OADP v2 setup step for hypershift, simplifying the configuration.
Related to discussion in this thread
Summary by CodeRabbit
The Hypershift Agent and AWS OADP CI setup no longer creates the unused
VolumeSnapshotLocationresource. The setup retainsBackupStorageLocationconfiguration and continues waiting for the required resources to become ready.