Skip to content

Block Metro volume from Metro PV or Snapshot#378

Merged
santhoshatdell merged 21 commits into
mainfrom
metro-snap-clone
Nov 21, 2024
Merged

Block Metro volume from Metro PV or Snapshot#378
santhoshatdell merged 21 commits into
mainfrom
metro-snap-clone

Conversation

@santhoshatdell

@santhoshatdell santhoshatdell commented Nov 13, 2024 •
edited by suryagupta4
Loading

Copy link
Copy Markdown
Contributor

Description

  • When user requests a volume with dataSource (snap or clone), the driver will fail with an error message if the storage class is Metro.
    • Reason: PowerStore array does not allow configuring Metro on cloned volume or volume restored from Metro snap.
  • If it is non-metro SC, the request will go through like it is done currently.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1586

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained at least 90% code coverage
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

How Has This Been Tested?

Added unit tests.
Verified that the create request is failed with Metro storage class and data source is metro snap or metro PV.
Verified that the create request succeeds with non-Metro storage class and data source is metro snap or metro PV.

Metro e2e tests at 'tests/e2e/driver' passed.

Log

{"level":"info","msg":"/csi.v1.Controller/CreateVolume: REQ 0086: Name=csivol-sl-6653d5e3b1, CapacityRange=required_bytes:16106127360 , VolumeCapabilities=[mount:\u003cfs_type:\"ext4\" \u003e access_mode:\u003cmode:SINGLE_NODE_MULTI_WRITER \u003e ], Parameters=map[arrayID:PS00001 csi.storage.k8s.io/pv/name:csivol-sl-6653d5e3b1 csi.storage.k8s.io/pvc/name:pvcps2-from-snap csi.storage.k8s.io/pvc/namespace:nsps2 replication.storage.dell.com/isReplicationEnabled:true replication.storage.dell.com/mode:METRO replication.storage.dell.com/remoteSystem:RT-D9999], VolumeContentSource=snapshot:\u003csnapshot_id:\"a7b6f6be-b424-4335-b2a4-5a6e6803778b/PS00001/scsi\" \u003e , .......... , XXX_NoUnkeyedLiteral={}, XXX_sizecache=0","time":"2024-11-12T19:53:41.445097328Z"}
{"level":"info","msg":"/csi.v1.Controller/CreateVolume: REP 0086: rpc error: code = InvalidArgument desc = Configuring Metro is not supported on clones or volumes created from Metro snapshot. Choose a non-Metro storage class.","time":"2024-11-12T19:53:41.445251325Z"}

PVC list (blocked case)

NAMESPACE   NAME               STATUS    VOLUME                 CAPACITY   ACCESS MODES   STORAGECLASS                   VOLUMEATTRIBUTESCLASS   AGE
nsps2       pvcps2             Bound     csivol-sl-a8d576781c   14Gi       RWO            powerstore-replication-metro   <unset>                 79m
nsps2       pvcps2-cloned      Pending                                                    powerstore-replication-metro   <unset>                 20s
nsps2       pvcps2-from-snap   Pending                                                    powerstore-replication-metro   <unset>                 91s

PVC list (success case)

NAMESPACE   NAME               STATUS   VOLUME                 CAPACITY   ACCESS MODES   STORAGECLASS                   VOLUMEATTRIBUTESCLASS   AGE
nsps2       pvcps2             Bound    csivol-sl-8452b0f7bd   14Gi       RWO            powerstore-replication-metro   <unset>                 36m
nsps2       pvcps2-cloned      Bound    csivol-sl-65834279c3   14Gi       RWO            powerstore-ext4                <unset>                 12m
nsps2       pvcps2-from-snap   Bound    csivol-sl-2157aeb102   14Gi       RWO            powerstore-ext4                <unset>                 14m

@santhoshatdell santhoshatdell changed the title Metro snap clone Block Metro volume from Metro PV or Snapshot Nov 13, 2024
@santhoshatdell santhoshatdell mentioned this pull request Nov 13, 2024
Merged
1 task
@santhoshatdell santhoshatdell marked this pull request as ready for review November 13, 2024 21:30
@santhoshatdell

santhoshatdell commented Nov 13, 2024

Copy link
Copy Markdown
Contributor Author

Yaml lint and unit test code coverage checks are failing with the new workflow checks. Looking into them.

lukeatdell
lukeatdell previously approved these changes Nov 14, 2024
View reviewed changes

@lukeatdell lukeatdell 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.

Only suggestion, maybe add a passing unit test for provisioning from snap/clone to non-metro storage class.

@santhoshatdell

santhoshatdell commented Nov 15, 2024

Copy link
Copy Markdown
Contributor Author

Moving to Draft since we are working on increasing code coverage.

@santhoshatdell santhoshatdell marked this pull request as draft November 15, 2024 22:09
@suryagupta4 suryagupta4 marked this pull request as ready for review November 21, 2024 10:38
@santhoshatdell santhoshatdell merged commit c6afdbd into main Nov 21, 2024
@santhoshatdell santhoshatdell deleted the metro-snap-clone branch November 21, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adarsh-dell adarsh-dell Awaiting requested review from adarsh-dell

@AkshaySainiDell AkshaySainiDell Awaiting requested review from AkshaySainiDell

@donatwork donatwork Awaiting requested review from donatwork

@jooseppi-luna jooseppi-luna Awaiting requested review from jooseppi-luna

@kumarkgosa kumarkgosa Awaiting requested review from kumarkgosa

@shanmydell shanmydell Awaiting requested review from shanmydell

3 more reviewers

@abhi16394 abhi16394 abhi16394 approved these changes

@mbasha-dell mbasha-dell mbasha-dell approved these changes

@lukeatdell lukeatdell lukeatdell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@santhoshatdell @abhi16394 @mbasha-dell @lukeatdell @suryagupta4