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

ovirt: Respect storage mapping #822

Merged
merged 1 commit into from
Mar 26, 2024
Merged

ovirt: Respect storage mapping #822

merged 1 commit into from
Mar 26, 2024

Conversation

ahadas
Copy link
Member

@ahadas ahadas commented Mar 24, 2024

Previously, when migrating to the cluster Forklift is deployed on using a cold migration, all disks of the migrated VM were transferred to the same storage class, which is one of the target storage class that appear in the storage mapping that is set for the migration plan.

With these changes, each disk will be mapped to a target storage class based on its original storage domain and the storage mapping that is defined for the migration plan.

@ahadas ahadas linked an issue Mar 24, 2024 that may be closed by this pull request
@ahadas ahadas added this to the 2.6.0 milestone Mar 24, 2024
@ahadas
Copy link
Member Author

ahadas commented Mar 24, 2024

@bennyz we have the same issue with OpenStack, could you please post a fix for that as well?

Copy link

codecov bot commented Mar 24, 2024

Codecov Report

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

Project coverage is 16.86%. Comparing base (a55e08e) to head (b7a646d).
Report is 267 commits behind head on main.

Files Patch % Lines
pkg/controller/plan/adapter/ovirt/builder.go 0.00% 16 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main     #822       +/-   ##
=========================================
+ Coverage      0   16.86%   +16.86%     
=========================================
  Files         0       93       +93     
  Lines         0    19402    +19402     
=========================================
+ Hits          0     3272     +3272     
- Misses        0    15865    +15865     
- Partials      0      265      +265     
Flag Coverage Δ
unittests 16.86% <0.00%> (?)

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.

@bennyz
Copy link
Member

bennyz commented Mar 24, 2024

@bennyz we have the same issue with OpenStack, could you please post a fix for that as well?

it looks (somewhat) fine, the next lines override the SC based on the volume type

@ahadas
Copy link
Member Author

ahadas commented Mar 24, 2024

@bennyz we have the same issue with OpenStack, could you please post a fix for that as well?

it looks (somewhat) fine, the next lines override the SC based on the volume type

ok, so we don't use the first storage class that we find in the mapping for all the volumes but why do we use it at all?

@ahadas
Copy link
Member Author

ahadas commented Mar 24, 2024

filed #823 for OpenStack, it's indeed a bit different and less important than the issue this fix is intended for

Previously, when migrating to the cluster Forklift is deployed on using
a cold migration, all disks of the migrated VM were transferred to the
same storage class, which is one of the target storage class that appear
in the storage mapping that is set for the migration plan.

With these changes, each disk will be mapped to a target storage class
based on its original storage domain and the storage mapping that is
defined for the migration plan.

Signed-off-by: Arik Hadas <[email protected]>
Copy link

sonarcloud bot commented Mar 24, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
8.6% Duplication on New Code

See analysis details on SonarCloud

@ahadas ahadas requested a review from bennyz March 25, 2024 10:05
@ahadas ahadas merged commit 3965155 into kubev2v:main Mar 26, 2024
12 checks passed
@ahadas ahadas deleted the issue_427 branch March 26, 2024 14:05
@ahadas ahadas removed this from the 2.6.0 milestone Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple storage domains are always mapped to a single storage class?
3 participants