Skip to content

USHIFT-7518: Support volume group snapshots - #7381

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
jsafrane:USHIFT-7518-volume-group-snapshots
Sep 16, 2026
Merged

openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
jsafrane:USHIFT-7518-volume-group-snapshots

Conversation

@jsafrane

@jsafrane jsafrane commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • sync the CSI snapshot controller deployment arguments needed for volume group snapshots
  • install the VolumeGroupSnapshot, VolumeGroupSnapshotContent, and VolumeGroupSnapshotClass v1 CRDs
  • keep the auto-rebase asset recipe and post-processing aligned with the upstream snapshot controller operator
  • verify the group snapshot APIs and controller feature gate in the configuration test
  • confirm the existing controller RBAC already matches the upstream group snapshot permissions

Summary by CodeRabbit

  • New Features

    • Added Kubernetes VolumeGroupSnapshot support for CSI storage.
    • Added v1 resources for volume group snapshot classes, contents, and snapshots.
    • Enabled the CSI volume group snapshot feature in the snapshot controller.
    • Added validation for snapshot sources, references, deletion policies, and resource immutability.
  • Bug Fixes

    • Improved snapshot controller request handling by configuring API request rate and burst limits.

Sync the CSI snapshot controller deployment and VolumeGroupSnapshot CRDs from the cluster CSI snapshot controller operator.
Register the new APIs during startup and verify the controller feature gate in the configuration test.

Refs: USHIFT-7518
Commit-Message-Assisted-by: Claude (via Claude Code)
Signed-off-by: Jan Safranek <jsafrane@redhat.com>
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 15, 2026
@openshift-ci-robot

openshift-ci-robot commented Sep 15, 2026

Copy link
Copy Markdown

@jsafrane: This pull request references USHIFT-7518 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 bug to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Summary

  • sync the CSI snapshot controller deployment arguments needed for volume group snapshots
  • install the VolumeGroupSnapshot, VolumeGroupSnapshotContent, and VolumeGroupSnapshotClass v1 CRDs
  • keep the auto-rebase asset recipe and post-processing aligned with the upstream snapshot controller operator
  • verify the group snapshot APIs and controller feature gate in the configuration test
  • confirm the existing controller RBAC already matches the upstream group snapshot permissions

Testing

  • go test ./pkg/assets ./pkg/components
  • make verify-sh
  • make verify-rf
  • python3 scripts/auto-rebase/presubmit.py
  • Robot Framework dry run of test/suites/configuration1/configuration.robot
  • YAML parsing and byte-for-byte upstream manifest comparisons

Jira

https://redhat.atlassian.net/browse/USHIFT-7518

Always review AI generated responses prior to use.
AI-assisted response via openshift-developer plugin

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.

@openshift-ci

openshift-ci Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 15d15ddc-9660-48a3-a43c-81eefab8826b

📥 Commits

Reviewing files that changed from the base of the PR and between c2c9986 and 91001c7.

📒 Files selected for processing (8)
  • assets/components/csi-snapshot-controller/csi_controller_deployment.yaml
  • assets/components/csi-snapshot-controller/volumegroupsnapshotclasses.yaml
  • assets/components/csi-snapshot-controller/volumegroupsnapshotcontents.yaml
  • assets/components/csi-snapshot-controller/volumegroupsnapshots.yaml
  • pkg/assets/crd.go
  • scripts/auto-rebase/assets.yaml
  • scripts/auto-rebase/rebase.sh
  • test/suites/configuration1/configuration.robot

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The change adds three v1 volume group snapshot CRDs, enables the controller feature gate and API request limits, registers the manifests, and validates the deployment configuration.

Changes

CSI volume group snapshot support

Layer / File(s) Summary
Volume group snapshot CRD definitions
assets/components/csi-snapshot-controller/volumegroupsnapshotclasses.yaml, assets/components/csi-snapshot-controller/volumegroupsnapshotcontents.yaml, assets/components/csi-snapshot-controller/volumegroupsnapshots.yaml
Adds the VolumeGroupSnapshotClass, VolumeGroupSnapshotContent, and VolumeGroupSnapshot CRDs. The v1 schemas include validation, status fields, and immutability rules. Beta versions remain commented out.
Controller configuration and asset registration
assets/components/csi-snapshot-controller/csi_controller_deployment.yaml, pkg/assets/crd.go, scripts/auto-rebase/assets.yaml, scripts/auto-rebase/rebase.sh
Enables CSIVolumeGroupSnapshot, sets API QPS to 30 and burst to 60, and registers the three CRD manifests.
Deployment support validation
test/suites/configuration1/configuration.robot
Checks that the three CRDs exist and that the controller deployment enables CSIVolumeGroupSnapshot=true.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Suggested reviewers: pacevedom

Merge Risk: ⚪ Minimal · up to 91001

The group snapshot assets, registration, controller configuration, and validation are consistent, with no actionable merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (6 skipped: 6 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PASS. The pull request adds a static Robot Framework keyword name, CSI Volume Group Snapshot Support Is Enabled, and invokes it from the existing static test case. The added test body uses fixed CRD…
Test Structure And Quality ✅ Passed PASS: The pull request does not add or modify Ginkgo tests or Go It blocks. Its only test change is a Robot Framework keyword in test/suites/configuration1/configuration.robot, so the Ginkgo-speci…
Microshift Test Compatibility ✅ Passed The pull request adds no Ginkgo e2e tests. The only changed test file is a Robot Framework configuration test, and the changed Go file only registers CRD assets. Therefore, the MicroShift Test Compati…
Single Node Openshift (Sno) Test Compatibility ✅ Passed PASS: The pull request adds no new Ginkgo e2e tests. The only changed test file is a Robot Framework configuration suite, and the remaining changes are YAML manifests or asset-registration code. There…
Topology-Aware Scheduling Compatibility ✅ Passed No topology scheduling constraint is introduced. The deployment diff adds only -kube-api-qps=30, -kube-api-burst=60, and CSIVolumeGroupSnapshot=true. Its existing maxUnavailable: 1, preferred …
Ote Binary Stdout Contract ✅ Passed PASS. The pull request does not add or modify OTE process-level Go code. The only Go change adds three CRD path strings to the existing crds variable initializer in pkg/assets/crd.go; it adds no s…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PASS: The PR adds no Ginkgo e2e tests. The only test change is a Robot Framework keyword that queries in-cluster CRDs and the CSI snapshot controller deployment. It adds no IPv4 assumptions and no ext…
No-Weak-Crypto ✅ Passed PASS: The PR adds Kubernetes CRDs, controller arguments, asset registrations, and a configuration test. The changed Go file only adds CRD asset paths. The changed shell and YAML files contain no MD5, …
Container-Privileges ✅ Passed No custom-check violation is introduced. The deployment diff adds only API QPS, burst, and feature-gate arguments. Its existing security context remains non-root (runAsNonRoot: true, `runAsUser: 655…
No-Sensitive-Data-In-Logs ✅ Passed The pull request adds CRD schemas, controller arguments, asset registrations, and configuration checks. It does not add logging calls or log sensitive values. The Robot test reads only CRD names and t…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: adding support for volume group snapshots.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Comment on lines +55 to +56
- -kube-api-qps=30
- -kube-api-burst=60

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now present in OpenShift's yaml files, see openshift/cluster-csi-snapshot-controller-operator#261
It's probably unnecessary in Microshift - the load won't be that high. On the other hand, it should not harm anything. I kept it to be in sync with OpenShift.

@jsafrane

Copy link
Copy Markdown
Contributor Author

/payload-job periodic-ci-openshift-microshift-release-5.1-periodics-e2e-aws-ovn-ocp-conformance

@openshift-ci

openshift-ci Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

@jsafrane: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-microshift-release-5.1-periodics-e2e-aws-ovn-ocp-conformance

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/de3af4b0-b0de-11f1-8e12-95ca108e7c48-0

@pacevedom

Copy link
Copy Markdown
Contributor

/test test-unit

@jsafrane

Copy link
Copy Markdown
Contributor Author

I would cosider the bug as fixed - group snapshot tests passed in microshift-main-ocp-full-conformance-rhel-eus, while they failed in an unrelated PR.

@pacevedom

Copy link
Copy Markdown
Contributor

/pipeline auto

@openshift-ci

openshift-ci Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Pipeline controller notification

The pipeline-auto label has been added to this PR. Second-stage tests will be triggered automatically when all first-stage tests pass.

@openshift-ci

openshift-ci Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-tests
/test e2e-aws-tests-arm
/test e2e-aws-tests-bootc-arm-el10
/test e2e-aws-tests-bootc-arm-el9
/test e2e-aws-tests-bootc-el10
/test e2e-aws-tests-bootc-el9

1 similar comment
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-tests
/test e2e-aws-tests-arm
/test e2e-aws-tests-bootc-arm-el10
/test e2e-aws-tests-bootc-arm-el9
/test e2e-aws-tests-bootc-el10
/test e2e-aws-tests-bootc-el9

@pacevedom

Copy link
Copy Markdown
Contributor

/test e2e-aws-tests

1 similar comment
@pacevedom

Copy link
Copy Markdown
Contributor

/test e2e-aws-tests

@pacevedom

Copy link
Copy Markdown
Contributor

/override ci/prow/e2e-aws-tests
/override ci/prow/e2e-aws-tests-arm

Unrelated tuned issues

@openshift-ci

openshift-ci Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@pacevedom: Overrode contexts on behalf of pacevedom: ci/prow/e2e-aws-tests, ci/prow/e2e-aws-tests-arm

Details

In response to this:

/override ci/prow/e2e-aws-tests
/override ci/prow/e2e-aws-tests-arm

Unrelated tuned issues

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 kubernetes-sigs/prow repository.

@pacevedom pacevedom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@pacevedom

Copy link
Copy Markdown
Contributor

/verified by CI

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Sep 16, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@pacevedom: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI

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.

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 16, 2026
@openshift-ci

openshift-ci Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane, pacevedom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 16, 2026
@openshift-ci

openshift-ci Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@jsafrane: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit 7295d8c into openshift:main Sep 16, 2026
16 checks passed
@pacevedom

Copy link
Copy Markdown
Contributor

/cherry-pick release-5.0

@openshift-cherrypick-robot

Copy link
Copy Markdown

@pacevedom: new pull request created: #7385

Details

In response to this:

/cherry-pick release-5.0

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 kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. pipeline-auto verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants