PowerFlex: Handle missing volumes gracefully during delete volume #7924
Conversation
yadvr
left a comment
yadvr
left a comment
There was a problem hiding this comment.
Code LGTM didn't test it though
|
@blueorangutan package |
|
@rohityadavcloud a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report
@@ Coverage Diff @@
## 4.18 #7924 +/- ##
============================================
- Coverage 13.06% 13.06% -0.01%
Complexity 9093 9093
============================================
Files 2720 2720
Lines 257431 257437 +6
Branches 40141 40142 +1
============================================
Hits 33622 33622
- Misses 219582 219588 +6
Partials 4227 4227
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6943 |
|
@blueorangutan test |
|
@rohityadavcloud a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
harikrishna-patnala
left a comment
harikrishna-patnala
left a comment
There was a problem hiding this comment.
Quickly verified the issue and the fix. Volume delete with expunge completed gracefully.
2023-08-31 06:19:26,717 WARN [o.a.c.s.d.c.ScaleIOGatewayClientImpl] (API-Job-Executor-1:ctx-c77978d8 job-154 ctx-2c92ed87) (logid:2628581a) API says deleting volume 4ace104b00000005 does not exist, handling gracefully
| return removeVolumeStatus; | ||
| } | ||
| } catch (Exception ex) { | ||
| if (ex instanceof ServerApiException && ex.getMessage().contains("Could not find the volume")) { |
There was a problem hiding this comment.
depending on the textual contents of the exception message seems a bit unrelyable and maintenance sensitive, is there no return code that can be queried? (excuse my lack of scaleIO/PowerFlex knowledge) (and clgtm otherwise)
There was a problem hiding this comment.
@DaanHoogland Marcus mentioned this in the PR description
"We catch this error case and match the error message to determine if the volume is already gone. There may be some document for handling the error codes more directly, but currently the PowerFlex client doesn't handle errors in this way."
There was a problem hiding this comment.
sorry, only reviewed the code :(
weizhouapache
left a comment
weizhouapache
left a comment
There was a problem hiding this comment.
code lgtm
This will be merged when trillian tests finish.
|
[SF] Trillian test result (tid-7610)
|
6 participants
Description
This PR allows the PowerFlex plugin to gracefully handle the case where volume has already been removed from the backend PowerFlex storage and a user attempts to expunge volume.
Without this, attempting to also clean up in cloudstack results in volumes that are stuck in
Destroystate, unless an admin manually edits the database. If the volume is missing on the back end and admins want to recover, they can still attempt to fix this in some way and the record would be preserved in CloudStack as long as a volume expunge is not attempted.We catch this error case and match the error message to determine if the volume is already gone. There may be some document for handling the error codes more directly, but currently the PowerFlex client doesn't handle errors in this way.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?