PowerFlex/ScaleIO - Wait after SDC service start/restart/stop, and retry to fetch SDC id/guid#11099
Conversation
|
@blueorangutan package |
|
@sureshanaparti a [SL] 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. |
|
Packaging result [SF]: ✔️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 13932 |
sureshanaparti
changed the title
|
@blueorangutan package |
|
@sureshanaparti a [SL] 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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13938 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds explicit wait periods after starting, stopping, or restarting the SDC service and introduces a retry loop to fetch the SDC ID or GUID in the PowerFlex/ScaleIO integration.
- Introduce
waitForSecshelper inScaleIOUtiland invoke it after SDC service commands - Update
prepareStorageClientto restart or start the SDC service and then retry fetching SDC details - Adjust existing test to expect failure when no SDC details are found
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| plugins/storage/volume/scaleio/.../ScaleIOUtil.java | Added waitForSecs method and invoked delays in start/stop/restart |
| plugins/hypervisors/kvm/.../ScaleIOStorageAdaptor.java | Refactored client preparation flow, added retry loop for SDC details |
| plugins/hypervisors/kvm/.../ScaleIOStorageAdaptorTest.java | Updated assertions to match new failure path |
Comments suppressed due to low confidence (5)
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java:641
- [nitpick] The variable name
sdcGuIdis inconsistent (mixed casing). Consider renaming it tosdcGuidfor clarity and consistency with the constantSDC_GUID.
String sdcGuId = ScaleIOUtil.getSdcGuid();
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java:629
StringUtilsmay not be imported in this file, leading to a compile error. Add the appropriate import (e.g.,org.apache.commons.lang3.StringUtils).
if (StringUtils.isEmpty(storageSystemId)) {
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java:650
- Catching broad
Exceptioninside the retry loop can mask unexpected errors. Narrow this toInterruptedExceptionor handle specific exceptions only.
} catch (Exception ignore) {
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java:250
- Suppressing
InterruptedExceptionwithout restoring the thread's interrupt status can lead to unpredictable thread behavior. Consider callingThread.currentThread().interrupt()in the catch block.
} catch (InterruptedException ignore) {
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java:633
- [nitpick] The hardcoded retry count (
5) is a magic number. Extract it into a named constant or configuration to clarify its purpose and facilitate changes.
int waitTimeInSecs = 5;
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-13632)
|
…top, and retries to fetch SDC id/guid
sureshanaparti
force-pushed
the
powerflex_scaleio-wait-time-after-sdc-service-start-stop
branch
from
f59fd55 to
6273234
Compare
…cs) to wait after SDC service start/restart/stop
|
@blueorangutan package |
|
@sureshanaparti a [SL] 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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #11099 +/- ##
==========================================
Coverage 16.15% 16.16%
Complexity 13275 13275
==========================================
Files 5657 5656 -1
Lines 497948 497771 -177
Branches 60386 60365 -21
==========================================
- Hits 80463 80444 -19
+ Misses 408521 408374 -147
+ Partials 8964 8953 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14004 |
|
@blueorangutan test |
|
@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-13681)
|
|
@blueorangutan package |
|
@sureshanaparti a [SL] 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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14159 |
|
@blueorangutan test |
|
@vladimirpetrov a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-13778)
|
vladimirpetrov
left a comment
vladimirpetrov
left a comment
There was a problem hiding this comment.
LGTM based on manual testing.
DaanHoogland
deleted the
powerflex_scaleio-wait-time-after-sdc-service-start-stop
branch
…try to fetch SDC id/guid (apache#11099) * [PowerFlex/ScaleIO] Added wait time after SDC service start/restart/stop, and retries to fetch SDC id/guid * Added agent property 'powerflex.sdc.service.wait' for the time (in secs) to wait after SDC service start/restart/stop * code improvements
5 participants
Description
This PR adds agent property 'powerflex.sdc.service.wait' for the time (in secs) to wait after SDC service start/restart/stop, and retries to fetch SDC id/guid for PowerFlex/ScaleIO storage.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?