ControllerUnpublish fails to retrieve PV after Upgrade#481
Conversation
…refix mismatch with Auth v2 enabled for powerscale
| return nil, err | ||
| pv, volErr := s.k8sclient.CoreV1().PersistentVolumes().Get(ctx, volumeName, metav1.GetOptions{}) | ||
| if volErr != nil { | ||
| log.Warnf("Failed to get PV %s: %v", volumeName, volErr) |
There was a problem hiding this comment.
@hoppea2 Could you please explain why we're changing the severity from Error to Warning? Just trying to understand the rationale behind this change.
There was a problem hiding this comment.
PV is only needed for lookup the Volume Context for AZNetwork, which is a specific scenario for multi-tenancy on NFS. Unpublish Volume legacy code does not require PV
| pv, volErr := s.k8sclient.CoreV1().PersistentVolumes().Get(ctx, volumeName, metav1.GetOptions{}) | ||
| if volErr != nil { | ||
| log.Warnf("Failed to get PV %s: %v", volumeName, volErr) | ||
| // Not returning error code here as there is an authorization upgrade scenario where PV might not be found when it was created with tenant prefix |
There was a problem hiding this comment.
@hoppea2 Thanks for the comment. Could you please clarify why the PV might not be found during the authorization upgrade scenario?
Specifically, I'm trying to understand how the tenant prefix impacts PV visibility or lookup. Was the PV created under a different identity/namespace and is now inaccessible due to authorization upgrade ?
There was a problem hiding this comment.
In Authorization, the resource is being looked up be VolumeHandle, which includes the tenant prefix.
So while the persistent volume name in Kubernetes is immutable (example: "vol-12345", the volume handle is updated to "tenant-vol-12345".)
In the authorization upgrade scenario, this lookup will fail of the resources have been created pre upgrade. However we should not exit out of unpublish volume because the pv name is not required to remove the resources.
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
Description
Upgrading from CSM 1.14 to current builds on the main branch and executing CSM Authorization tests with CSI PowerScale failed to delete PV in ControllerUnpublish if the resources were created before upgrade.
--> Pod and PVC are deleted, but PV remains
GitHub Issues
List the GitHub issues impacted by this PR:
| GitHub Issue # |
https://github.com/dell/csm/issues/2017
Checklist:
How Has This Been Tested?
SUCCESS! -- 48 Passed | 0 Failed | 0 Pending | 6851 Skipped
PASS
Generating Report:
TestSuite Name: Kubernetes e2e suite
Total Tests Executed: 52
Total Tests Passed: 52
Total Tests Failed: 0