Skip to content
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

removing the usage of volumeFreeze StorPool API call #8575

Open
wants to merge 3 commits into
base: 4.20
Choose a base branch
from

Conversation

slavkap
Copy link
Contributor

@slavkap slavkap commented Jan 30, 2024

Description

Deprecate the usage of the volumeFreeze API call.

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 or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Sorry, something went wrong.

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

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

Project coverage is 23.10%. Comparing base (8f6721e) to head (23e3f20).

Files Patch % Lines
...tastore/driver/StorPoolPrimaryDataStoreDriver.java 0.00% 36 Missing ⚠️
...ack/storage/motion/StorPoolDataMotionStrategy.java 0.00% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8575      +/-   ##
============================================
- Coverage     30.90%   23.10%   -7.80%     
+ Complexity    33382    23318   -10064     
============================================
  Files          5355     5219     -136     
  Lines        375727   353392   -22335     
  Branches      54915    50877    -4038     
============================================
- Hits         116109    81663   -34446     
- Misses       244219   259918   +15699     
+ Partials      15399    11811    -3588     
Flag Coverage Δ
simulator-marvin-tests 24.77% <0.00%> (+0.01%) ⬆️
uitests 4.36% <ø> (ø)
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.

Copy link

github-actions bot commented Feb 8, 2024

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

@slavkap slavkap force-pushed the sp-deprecated-volume-freeze branch from 573ad8e to 23e3f20 Compare February 23, 2024 07:59
@slavkap slavkap force-pushed the sp-deprecated-volume-freeze branch from 23e3f20 to 3bcfaa1 Compare April 12, 2024 07:29
@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2024

Codecov Report

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

Project coverage is 16.07%. Comparing base (ab76d3c) to head (f8020dc).

Files with missing lines Patch % Lines
...tastore/driver/StorPoolPrimaryDataStoreDriver.java 0.00% 38 Missing ⚠️
...ack/storage/motion/StorPoolDataMotionStrategy.java 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               4.20    #8575   +/-   ##
=========================================
  Coverage     16.07%   16.07%           
- Complexity    12883    12884    +1     
=========================================
  Files          5639     5639           
  Lines        494193   494179   -14     
  Branches      59925    59919    -6     
=========================================
+ Hits          79419    79420    +1     
+ Misses       405944   405929   -15     
  Partials       8830     8830           
Flag Coverage Δ
uitests 4.02% <ø> (ø)
unittests 16.91% <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.

@sureshanaparti
Copy link
Contributor

Ping @slavkap Any further work on this?

@sureshanaparti sureshanaparti added this to the 4.20.0.0 milestone Jun 27, 2024
@slavkap
Copy link
Contributor Author

slavkap commented Jun 27, 2024

hey @sureshanaparti, no, I think all needed is included

@sureshanaparti
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@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

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

Copy link

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

@slavkap slavkap force-pushed the sp-deprecated-volume-freeze branch from 3bcfaa1 to d099649 Compare July 2, 2024 10:49
@JoaoJandre JoaoJandre modified the milestones: 4.20.0.0, 4.21.0.0 Sep 10, 2024
@slavkap
Copy link
Contributor Author

slavkap commented Sep 30, 2024

@JoaoJandre, is it possible for this to get in at 4.20? It doesn't affect other storage plugins or CloudStack core; a StorPool API call is deprecated, and this is a cleanup.

@JoaoJandre
Copy link
Contributor

@JoaoJandre, is it possible for this to get in at 4.20? It doesn't affect other storage plugins or CloudStack core; a StorPool API call is deprecated, and this is a cleanup.

@slavkap , I understand your point, but as this is not fixing a bug and could introduce new ones (as any PR could), I think that it is safer to wait for the next release. From the user perspective, the functionality will not change, right?

Copy link

github-actions bot commented Jan 8, 2025

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

@DaanHoogland
Copy link
Contributor

@slavkap @JoaoJandre does it make sense to add this to 4.20.1?

@slavkap
Copy link
Contributor Author

slavkap commented Jan 9, 2025

@DaanHoogland, for me, yes. But I was following the set milestone. Can I rebase it on branch 4.20?

@DaanHoogland
Copy link
Contributor

@slavkap , i see no reason not to, but it is your choice. Last time you were in a freeze for the release, hence @JoaoJandre's reservation.

@slavkap slavkap force-pushed the sp-deprecated-volume-freeze branch from 0e78ba9 to 3d31305 Compare January 10, 2025 08:16
@slavkap slavkap changed the base branch from main to 4.20 January 10, 2025 08:17
@slavkap slavkap modified the milestones: 4.21.0, 4.20.1 Jan 10, 2025
@slavkap
Copy link
Contributor Author

slavkap commented Jan 10, 2025

Thanks, @DaanHoogland, I moved it to 4.20.1

@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@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

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

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

clgtm

@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

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

@apache apache deleted a comment from blueorangutan Jan 10, 2025
@blueorangutan
Copy link

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

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm

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.

None yet

6 participants