Skip to content

Added support for storpool_qos service #8755

Merged
DaanHoogland merged 5 commits into
apache:mainfrom
storpool:sp-support-tiers
Aug 29, 2024
Merged

Added support for storpool_qos service #8755
DaanHoogland merged 5 commits into
apache:mainfrom
storpool:sp-support-tiers

Conversation

@slavkap

@slavkap slavkap commented Mar 7, 2024

Copy link
Copy Markdown
Contributor

Description

This PR provides support to use the storpool_qos service (to change tiers) via the Disk offering's details.
StorPool provides the ‘storpool_qos’ service that tracks and configures the storage tier for all volumes based on a specifically provided qc tag specifying the storage tier for each volume.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI

Feature/Enhancement Scale

  • Major
  • Minor

How Has This Been Tested?

manual and smoke tests

@codecov

codecov Bot commented Mar 7, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 1.42180% with 208 lines in your changes are missing coverage. Please review.

Project coverage is 22.97%. Comparing base (9a73a2f) to head (93618c7).
Report is 26 commits behind head on main.

Files Patch % Lines
...tastore/driver/StorPoolPrimaryDataStoreDriver.java 0.00% 138 Missing ⚠️
...oudstack/storage/datastore/api/StorPoolVolume.java 0.00% 40 Missing ⚠️
...loudstack/storage/datastore/util/StorPoolUtil.java 0.00% 17 Missing ⚠️
...n/java/com/cloud/storage/VolumeApiServiceImpl.java 10.00% 9 Missing ⚠️
.../subsystem/api/storage/PrimaryDataStoreDriver.java 0.00% 2 Missing ⚠️
...udstack/storage/datastore/util/StorPoolHelper.java 0.00% 1 Missing ⚠️
...in/java/com/cloud/storage/ResizeVolumePayload.java 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8755      +/-   ##
============================================
- Coverage     30.98%   22.97%   -8.01%     
+ Complexity    33526    23449   -10077     
============================================
  Files          5361     5262      -99     
  Lines        376250   357229   -19021     
  Branches      54933    51350    -3583     
============================================
- Hits         116564    82062   -34502     
- Misses       244259   263288   +19029     
+ Partials      15427    11879    -3548     
Flag Coverage Δ
simulator-marvin-tests 24.61% <1.42%> (-0.21%) ⬇️
uitests 4.34% <ø> (-0.03%) ⬇️
unit-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

@sureshanaparti sureshanaparti left a comment

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.

clgtm

@sureshanaparti

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8897

@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@codecov-commenter

codecov-commenter commented Apr 12, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 0% with 264 lines in your changes missing coverage. Please review.

Project coverage is 15.57%. Comparing base (b61c3b8) to head (f956477).
Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
...tastore/driver/StorPoolPrimaryDataStoreDriver.java 0.00% 153 Missing ⚠️
...stack/storage/datastore/api/StorPoolVolumeDef.java 0.00% 68 Missing ⚠️
...loudstack/storage/datastore/util/StorPoolUtil.java 0.00% 21 Missing ⚠️
...n/java/com/cloud/storage/VolumeApiServiceImpl.java 0.00% 10 Missing ⚠️
...in/java/com/cloud/storage/ResizeVolumePayload.java 0.00% 6 Missing ⚠️
.../subsystem/api/storage/PrimaryDataStoreDriver.java 0.00% 4 Missing ⚠️
...udstack/storage/datastore/util/StorPoolHelper.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8755      +/-   ##
============================================
- Coverage     15.57%   15.57%   -0.01%     
- Complexity    12035    12041       +6     
============================================
  Files          5501     5505       +4     
  Lines        482219   482543     +324     
  Branches      58938    60302    +1364     
============================================
+ Hits          75119    75148      +29     
- Misses       398798   399091     +293     
- Partials       8302     8304       +2     
Flag Coverage Δ
uitests 4.16% <ø> (-0.01%) ⬇️
unittests 16.35% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@github-actions

github-actions Bot commented May 8, 2024

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✖️ debian ✔️ suse15. SL-JID 9630

Comment thread test/integration/plugins/storpool/TestStorPoolTiers.py Outdated
Comment thread test/integration/plugins/storpool/TestStorPoolTiers.py
Comment thread test/integration/plugins/storpool/TestStorPoolTiers.py Outdated
Comment thread test/integration/plugins/storpool/TestStorPoolTiers.py
Comment thread test/integration/plugins/storpool/TestStorPoolTiers.py
Comment thread test/integration/plugins/storpool/TestStorPoolTiers.py
Comment thread test/integration/plugins/storpool/TestStorPoolTiers.py
Comment thread test/integration/plugins/storpool/TestStorPoolTiers.py
Comment thread test/integration/plugins/storpool/TestStorPoolTiers.py
Comment thread test/integration/plugins/storpool/test-storpool-tiers.py
Comment on lines +184 to +190
@classmethod
def tearDownClass(cls):
cls.cleanUpCloudStack()

@classmethod
def cleanUpCloudStack(cls):
super(TestStorPoolTiers, cls).tearDownClass()

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.

why the double redirection and not just call super() directly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, I've removed the unnecessary method

@DaanHoogland DaanHoogland left a comment

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.

clgtm

@DaanHoogland

Copy link
Copy Markdown
Contributor

not sure if this requires more testing than smoke tests (for other storage solutions cc @harikrishna-patnala @rp- @rg9975 ) ???

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10841

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@rp-

rp- commented Aug 28, 2024

Copy link
Copy Markdown
Contributor

not sure if this requires more testing than smoke tests (for other storage solutions cc @harikrishna-patnala @rp- @rg9975 ) ???

I don't think this can in anyway break the Linstor driver

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-11218)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 48430 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8755-t11218-kvm-ol8.zip
Smoke tests completed. 139 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@DaanHoogland DaanHoogland merged commit 12d9c26 into apache:main Aug 29, 2024
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants