Skip to content

MIG-1963: Add csi.trident.netapp.io to accessModeList for correct access/volume mode detection - #1467

Open
swshende-cmd wants to merge 1 commit into
migtools:masterfrom
swshende-cmd:MIG-1963-add-trident-csi-accessmodes
Open

MIG-1963: Add csi.trident.netapp.io to accessModeList for correct access/volume mode detection#1467
swshende-cmd wants to merge 1 commit into
migtools:masterfrom
swshende-cmd:MIG-1963-add-trident-csi-accessmodes

Conversation

@swshende-cmd

@swshende-cmd swshende-cmd commented Jul 29, 2026

Copy link
Copy Markdown

Summary

MigCluster/MigPlan status incorrectly reports storage classes backed by the NetApp Trident CSI driver (csi.trident.netapp.io) as supporting only ReadWriteOnce + volumeMode: Filesystem, even when the underlying storage supports ReadWriteMany + volumeMode: Block. This blocks VM/Block-mode storage migrations for customers on NetApp Trident CSI.

Root cause: The accessModeList in migcluster_types.go only contained the legacy Trident provisioner name netapp.io/trident with exact string matching. Modern Trident CSI StorageClasses use provisioner csi.trident.netapp.io, which never matches, causing accessModesForProvisioner() to fall back to defaults: [ReadWriteOnce] for Filesystem and nil (dropped) for Block.

Fix: Add a new accessModeList entry for csi.trident.netapp.io alongside the existing legacy entry. This is purely additive — no existing entries or logic are modified.

Changes

File Change
pkg/apis/migration/v1alpha1/migcluster_types.go Added csi.trident.netapp.io entry to accessModeList (+8 lines)
pkg/apis/migration/v1alpha1/migcluster_types_test.go Added 12 table-driven unit tests for accessModesForProvisioner() (+94 lines)

Test Evidence

1. Unit Tests — 12/12 PASS

=== RUN   TestMigCluster_accessModesForProvisioner
=== RUN   TestMigCluster_accessModesForProvisioner/csi.trident.netapp.io_Filesystem_should_return_RWO_and_ROX
=== RUN   TestMigCluster_accessModesForProvisioner/csi.trident.netapp.io_Block_should_return_RWO,_ROX,_and_RWX
=== RUN   TestMigCluster_accessModesForProvisioner/legacy_netapp.io/trident_Filesystem_should_return_RWO_and_ROX
=== RUN   TestMigCluster_accessModesForProvisioner/legacy_netapp.io/trident_Block_should_return_RWO,_ROX,_and_RWX
=== RUN   TestMigCluster_accessModesForProvisioner/CSI_and_legacy_Trident_should_return_identical_Filesystem_modes
=== RUN   TestMigCluster_accessModesForProvisioner/CSI_and_legacy_Trident_should_return_identical_Block_modes
=== RUN   TestMigCluster_accessModesForProvisioner/rbd.csi.ceph.com_suffix_match_for_Filesystem_should_return_RWO
=== RUN   TestMigCluster_accessModesForProvisioner/rbd.csi.ceph.com_suffix_match_for_Block_should_return_RWO,_ROX,_and_RWX
=== RUN   TestMigCluster_accessModesForProvisioner/unknown_provisioner_Filesystem_should_fall_back_to_RWO
=== RUN   TestMigCluster_accessModesForProvisioner/unknown_provisioner_Block_should_fall_back_to_nil
=== RUN   TestMigCluster_accessModesForProvisioner/kubernetes.io/aws-ebs_Filesystem_should_return_RWO
=== RUN   TestMigCluster_accessModesForProvisioner/kubernetes.io/aws-ebs_Block_should_return_nil_(not_in_map)
--- PASS: TestMigCluster_accessModesForProvisioner (0.00s)
    --- PASS: all 12 subtests
PASS

2. Cluster Integration Test — OCP 4.22.3

Tested against live OCP cluster (api.swshende.vmware.gsslab.pnq2.redhat.com:6443, OCP 4.22.3) with mock StorageClasses ontap-san-trident-csi (provisioner: csi.trident.netapp.io) and ontap-san-trident-legacy (provisioner: netapp.io/trident).

BEFORE (unfixed code) — Bug reproduced

GetStorageClasses() returns wrong access modes for csi.trident.netapp.io:

{
  "name": "ontap-san-trident-csi",
  "provisioner": "csi.trident.netapp.io",
  "volumeAccessModes": [
    {
      "volumeMode": "Filesystem",
      "accessModes": ["ReadWriteOnce"]
    }
  ]
}
  • ❌ Filesystem: only ReadWriteOnce (missing ReadOnlyMany)
  • ❌ Block: entirely missing (nil — dropped from output)
  • ✅ Legacy netapp.io/trident: correct
--- FAIL: TestIntegration/csi.trident.netapp.io_has_Filesystem_modes
    FAIL: Expected at least [RWO, ROX] for Filesystem, got [ReadWriteOnce]
--- FAIL: TestIntegration/csi.trident.netapp.io_has_Block_modes_with_RWX
    FAIL: Block access modes are nil — this is the bug!
--- PASS: TestIntegration/legacy_netapp.io/trident_still_works

AFTER (fixed code) — Bug fixed

{
  "name": "ontap-san-trident-csi",
  "provisioner": "csi.trident.netapp.io",
  "volumeAccessModes": [
    {
      "volumeMode": "Filesystem",
      "accessModes": ["ReadWriteOnce", "ReadOnlyMany"]
    },
    {
      "volumeMode": "Block",
      "accessModes": ["ReadWriteOnce", "ReadOnlyMany", "ReadWriteMany"]
    }
  ]
}
  • ✅ Filesystem: [ReadWriteOnce, ReadOnlyMany]
  • ✅ Block: [ReadWriteOnce, ReadOnlyMany, ReadWriteMany] (RWX present)
  • ✅ Legacy netapp.io/trident: still correct (backward compatible)
--- PASS: TestIntegration/csi.trident.netapp.io_has_Filesystem_modes
--- PASS: TestIntegration/csi.trident.netapp.io_has_Block_modes_with_RWX
--- PASS: TestIntegration/legacy_netapp.io/trident_still_works

3. Full Test Suite — No Regressions

All packages that pass on unmodified master continue to pass with this change. Controller package failures (fork/exec /usr/local/kubebuilder/bin/etcd: no such file or directory) are pre-existing and unrelated.

4. Backward Compatibility

  • Legacy netapp.io/trident entry is untouched — existing customers unaffected
  • No logic changes — purely additive data entry in the lookup table
  • No modifications to the fallback behavior for unknown provisioners

Made with Cursor

Summary by CodeRabbit

  • New Features

    • Added support for detecting storage access modes for NetApp Trident CSI provisioners during persistent volume discovery.
    • Supports both filesystem and block volume modes.
  • Bug Fixes

    • Improved access-mode handling for Trident and other known storage provisioners, including appropriate fallbacks for unsupported configurations.

The accessModeList only contained the legacy Trident provisioner
name (netapp.io/trident). Modern NetApp Trident CSI StorageClasses
use csi.trident.netapp.io, which did not match, causing
accessModesForProvisioner() to fall back to ReadWriteOnce/Filesystem
only — silently dropping Block and ReadWriteMany support.

Add a new accessModeList entry for csi.trident.netapp.io alongside
the existing legacy entry to correctly report supported access modes
for both provisioner names.

Closes: https://redhat.atlassian.net/browse/MIG-1963
Ref: migtools#1366
Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 06236bf6-38f1-43df-adf5-ae08fd15a19a

📥 Commits

Reviewing files that changed from the base of the PR and between c2e8dc5 and 46ba35d.

📒 Files selected for processing (2)
  • pkg/apis/migration/v1alpha1/migcluster_types.go
  • pkg/apis/migration/v1alpha1/migcluster_types_test.go

📝 Walkthrough

Walkthrough

Adds CSI Trident filesystem and block access-mode mappings and tests accessModesForProvisioner across supported, legacy, suffix-matched, unknown, and Kubernetes provisioners.

Changes

Storage access mode resolution

Layer / File(s) Summary
Add Trident mapping and resolution tests
pkg/apis/migration/v1alpha1/migcluster_types.go, pkg/apis/migration/v1alpha1/migcluster_types_test.go
Adds CSI Trident access modes for filesystem and block volumes, and validates resolution results across multiple provisioner identifiers and volume modes.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding csi.trident.netapp.io support for access/volume mode detection.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants