-
Notifications
You must be signed in to change notification settings - Fork 1.3k
PowerFlex/ScaleIO client initialization, authentication and command execution improvements #12391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.22
Are you sure you want to change the base?
PowerFlex/ScaleIO client initialization, authentication and command execution improvements #12391
Conversation
…xecution improvements
|
@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 Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12391 +/- ##
============================================
+ Coverage 17.59% 17.62% +0.02%
- Complexity 15598 15669 +71
============================================
Files 5910 5917 +7
Lines 529692 531272 +1580
Branches 64716 64947 +231
============================================
+ Hits 93202 93638 +436
- Misses 425998 427097 +1099
- Partials 10492 10537 +45
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:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances PowerFlex/ScaleIO client initialization, authentication, and command execution by implementing lazy authentication, improving busy-wait handling, and adding default command timeouts. The changes address potential deadlock scenarios and ensure commands complete within asynchronous timeout limits.
Key Changes:
- Deferred client authentication from constructor to first API call with proper session expiry detection
- Added 5-minute default timeout for PowerFlex command execution to fit within async command timeouts
- Improved connection pool synchronization to prevent concurrent client initialization
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| ScaleIOGatewayClientImpl.java | Removed eager authentication from constructor, added volatile flag and busy-wait with Thread.yield(), enhanced session expiry checks |
| ScaleIOGatewayClientConnectionPool.java | Improved synchronization with explicit lock object, refactored client creation with Optional, added debug logging |
| ScaleIOUtil.java | Added DEFAULT_TIMEOUT_MS constant, updated all command executions to use timeout, added --file parameter to query commands, simplified removeMdms error handling |
| ScaleIOSDCManagerImpl.java | Fixed canUnprepareSDC logic to return false when multiple pools share same SDC, improved isHostSdcConnected loop structure and logging |
| ScaleIOPrimaryDataStoreDriver.java | Removed throws Exception declarations from method signatures |
| ScaleIOGatewayClientImplTest.java | Moved authentication verifications after API calls to align with lazy authentication |
| ScaleIOStoragePoolTest.java | Updated mocks to include timeout parameter in Script.runSimpleBashScript calls |
| ScaleIOStorageAdaptorTest.java | Updated mocks to include timeout parameter and --file parameter in commands |
| ConfigKey.java | Added valueInDomain convenience method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...eio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManagerImpl.java
Outdated
Show resolved
Hide resolved
...e/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java
Show resolved
Hide resolved
...o/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java
Show resolved
Hide resolved
...o/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java
Show resolved
Hide resolved
framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java
Show resolved
Hide resolved
...c/test/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImplTest.java
Outdated
Show resolved
Hide resolved
...c/test/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImplTest.java
Outdated
Show resolved
Hide resolved
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16303 |
|
@blueorangutan test |
|
@borisstoyanov a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-15207)
|
|
@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 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 16806 |
|
@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 16809 |
|
@blueorangutan package |
|
@vladimirpetrov 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 16811 |
Description
This PR improves PowerFlex/ScaleIO client initialization , authentication and command execution.
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?