Skip to content

Conversation

@kaovilai
Copy link
Member

Signed-off-by: Tiger Kaovilai [email protected]

Why the changes were made

Fixes #1776

How to test the changes made

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Walkthrough

The pull request adds the "csi" plugin to the defaultPlugins list in the Velero configuration across two files: the ClusterServiceVersion manifest and the sample DataProtectionApplication configuration. This enables CSI plugin support in default deployments.

Changes

Cohort / File(s) Change Summary
CSI plugin enablement in default Velero configuration
bundle/manifests/oadp-operator.clusterserviceversion.yaml, config/samples/oadp_v1alpha1_dataprotectionapplication.yaml
Added "csi" to the defaultPlugins list under velero in DataProtectionApplication spec, expanding the set of enabled plugins from [openshift, aws, kubevirt, hypershift] to [openshift, aws, csi, kubevirt, hypershift].

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Simple configuration changes adding a single plugin entry to existing lists across two YAML files
  • Repetitive change pattern with no logic modifications

Assessment against linked issues

Objective Addressed Explanation
Enable CSI support in example bundle DataProtectionApplication (#1776) CSI plugin was added to defaultPlugins, which enables CSI capability. However, the issue title explicitly requests "featureFlags EnableCSI" but the diff only shows plugin addition; unclear if featureFlags section was also configured to enable the EnableCSI feature flag as described in the issue.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 098131f and da807be.

📒 Files selected for processing (2)
  • bundle/manifests/oadp-operator.clusterserviceversion.yaml (1 hunks)
  • config/samples/oadp_v1alpha1_dataprotectionapplication.yaml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • bundle/manifests/oadp-operator.clusterserviceversion.yaml
  • config/samples/oadp_v1alpha1_dataprotectionapplication.yaml
🔇 Additional comments (2)
config/samples/oadp_v1alpha1_dataprotectionapplication.yaml (1)

9-14: Verify completeness: featureFlags.enableCSI may be missing.

The PR objectives state "Add featureFlags: EnableCSI to the example DataProtectionApplication bundle", but the changes only add "csi" to the defaultPlugins list. The objectives indicate that CSI VolumeSnapshot functionality is gated behind a feature flag, suggesting spec.configuration.velero.featureFlags.enableCSI: true should also be added alongside the plugin.

Should the DataProtectionApplication spec also include featureFlags.enableCSI: true to fully address the PR objectives?

bundle/manifests/oadp-operator.clusterserviceversion.yaml (1)

40-46: Consistent with sample file; verify featureFlags alignment.

The change correctly adds "csi" to the defaultPlugins list in the alm-examples annotation, maintaining consistency with the sample DataProtectionApplication file. However, the same concern applies: the PR objectives mention adding featureFlags.enableCSI, which is not present in either the CSV example or the sample file.

Should the embedded DataProtectionApplication example in this CSV also include featureFlags.enableCSI: true?


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

@openshift-ci openshift-ci bot requested review from mrnold and sseago November 12, 2025 20:40
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 12, 2025
@kaovilai
Copy link
Member Author

/retest

ai-retester: The e2e tests failed because the Mongo application DATAMOVER test timed out while waiting for a Pod to become ready, and there were provisioning issues causing problems mounting volumes. This indicates potential problems deploying applications/pods in the test environment or with storage provisioning.

@kaovilai
Copy link
Member Author

/retest

@kaovilai
Copy link
Member Author

Node unavailable
/retest

@kaovilai
Copy link
Member Author

kaovilai commented Dec 2, 2025

/retest

@kaovilai
Copy link
Member Author

kaovilai commented Dec 2, 2025

/retest

ai-retester: The e2e test failed because the DPA CR without Region, without S3ForcePathStyle and with BackupImages false test case consistently found more than one velero pod running. This is accompanied by a more general error indicating a container failure within the e2e-test-aws-e2e pod, and ultimately resulted in the test suite being terminated due to a timeout.

comment for /pull/2030

@openshift-ci
Copy link

openshift-ci bot commented Dec 3, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai, shubham-pampattiwar, weshayutin

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [kaovilai,shubham-pampattiwar]

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

@kaovilai kaovilai added the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2025
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD f8b06e8 and 2 for PR HEAD da807be in total

@openshift-ci
Copy link

openshift-ci bot commented Dec 5, 2025

@kaovilai: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/4.20-e2e-test-cli-aws da807be link true /test 4.20-e2e-test-cli-aws

Full PR test history. Your PR dashboard.

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.

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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add featureFlags EnableCSI to example bundle DataProtectionApplication

4 participants