Skip to content

Snapshot License cache fix#388

Merged
santhoshatdell merged 6 commits into
mainfrom
license-cache-fix
Dec 18, 2024
Merged

Snapshot License cache fix#388
santhoshatdell merged 6 commits into
mainfrom
license-cache-fix

Conversation

@santhoshatdell

@santhoshatdell santhoshatdell commented Dec 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

  • Fixed Snapshot License cache.
  • Earlier, when the capabilities request to Unisphere is made for an array, the driver caches that information and used the same even for other arrays. This becomes an issue if the other array is managed by another Unisphere.

GitHub Issues

List the GitHub issues impacted by this PR:

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

Checklist:

  • Have you run format,vet & lint checks against your submission?
  • Have you made sure that the code compiles?
  • Have you commented your code, particularly in hard-to-understand areas
  • Did you run tests in a real Kubernetes cluster?
  • Backward compatibility is not broken

How Has This Been Tested?

On a setup with 2 arrays that are managed by different Unisphere instances, the issue was reproducible. And then with the fix, the issue is not observed. Clone volume was successfully created.

cert-csi tests passed for clone and other basic test suites. Snap test suite also passed when ran separately.
Please refer the ticket (31001) for test log and cert-csi results.

Also tested creating 2 snapshots on PV from array-1, and restored a snap to another PV, and then deleted them all.

Before:

[root@master-1-0BX8wDGBJidld csm]# k get pvc -A
NAMESPACE   NAME            STATUS    VOLUME            CAPACITY     ACCESS MODES   STORAGECLASS         VOLUMEATTRIBUTESCLASS   AGE
nsps1       pvcps1          Bound     pmax-28fdfaef98   8390400Ki    RWO            powermax-iscsi-602   <unset>                 73m
nsps2       pvcps1          Bound     pmax-a18af3bc32   10487040Ki   RWO            powermax-iscsi-836   <unset>                 57m
nsps2       pvcps1-cloned   Pending                                                 powermax-iscsi-836   <unset>                 3m18s
[root@master-1-0BX8wDGBJidld csm]#
[root@master-1-0BX8wDGBJidld csm]# k describe pvc pvcps1-cloned -n nsps2
Events:
  Normal   Provisioning          59s (x9 over 3m9s)   csi-powermax.dellemc.com_powermax-controller-79b8c76dcc-gz44r_bfad7f04-3aa3-4347-b520-ba1dab3b7a5e  External provisioner is provisioning volume for claim "nsps2/pvcps1-cloned"
  Warning  ProvisioningFailed    59s (x9 over 3m7s)   csi-powermax.dellemc.com_powermax-controller-79b8c76dcc-gz44r_bfad7f04-3aa3-4347-b520-ba1dab3b7a5e  failed to provision volume with StorageClass "powermax-iscsi-836": rpc error: code = Internal desc = PowerMax array (0000000001) is not being managed by Unisphere

After:

[root@master-1-0BX8wDGBJidld ~]# k get pvc -A
NAMESPACE   NAME            STATUS   VOLUME            CAPACITY     ACCESS MODES   STORAGECLASS         VOLUMEATTRIBUTESCLASS   AGE
nsps1       pvcps1          Bound    pmax-28fdfaef98   8390400Ki    RWO            powermax-iscsi-602   <unset>                 6d
nsps2       pvcps1          Bound    pmax-a18af3bc32   10487040Ki   RWO            powermax-iscsi-836   <unset>                 6d
nsps2       pvcps1-cloned   Bound    pmax-6d1f4c3525   10487040Ki   RWO            powermax-iscsi-836   <unset>                 5d23h
[root@master-1-0BX8wDGBJidld ~]#
[root@master-1-0BX8wDGBJidld ~]# k get pv
NAME              CAPACITY     ACCESS MODES   RECLAIM POLICY   STATUS   CLAIM                 STORAGECLASS         VOLUMEATTRIBUTESCLASS   REASON   AGE
pmax-28fdfaef98   8390400Ki    RWO            Delete           Bound    nsps1/pvcps1          powermax-iscsi-602   <unset>                          6d
pmax-6d1f4c3525   10487040Ki   RWO            Delete           Bound    nsps2/pvcps1-cloned   powermax-iscsi-836   <unset>                          2m53s
pmax-a18af3bc32   10487040Ki   RWO            Delete           Bound    nsps2/pvcps1          powermax-iscsi-836   <unset>                          6d
[root@master-1-0BX8wDGBJidld ~]#

@santhoshatdell santhoshatdell marked this pull request as ready for review December 17, 2024 21:28
donatwork
donatwork previously approved these changes Dec 17, 2024
View reviewed changes
Comment thread service/snap.go
donatwork
donatwork reviewed Dec 18, 2024
View reviewed changes
Comment thread service/snap.go
for _, symmCapability := range repCapabilities.SymmetrixCapability {
if symmCapability.SymmetrixID == symID {
symmRepCapabilities[symID] = symmCapability
log.Infof("License information with PowerMax %s is cached", symID)

@donatwork donatwork Dec 18, 2024

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.

The log message does not seem to reflect what the code is doing. Someone in service may see this and not know we are checking a license.

@santhoshatdell santhoshatdell merged commit c273338 into main Dec 18, 2024
@santhoshatdell santhoshatdell deleted the license-cache-fix branch December 18, 2024 17:07
abhi16394 pushed a commit that referenced this pull request Jan 3, 2025
@santhoshatdell @abhi16394
Snapshot License cache fix (#388)
5ad0011
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

@bpjain2004 bpjain2004 Awaiting requested review from bpjain2004

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

@tdawe tdawe Awaiting requested review from tdawe tdawe is a code owner

@Sahiba-Gupta Sahiba-Gupta Awaiting requested review from Sahiba-Gupta

2 more reviewers

@donatwork donatwork donatwork approved these changes

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

3 participants

@santhoshatdell @donatwork @abhi16394