Fix issue when restoring backup after migration of volume #12549
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12549 +/- ##
=========================================
Coverage 16.26% 16.26%
Complexity 13428 13428
=========================================
Files 5660 5660
Lines 499907 499907
Branches 60696 60696
=========================================
+ Hits 81316 81318 +2
+ Misses 409521 409518 -3
- Partials 9070 9071 +1
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:
|
| List<Backup.VolumeInfo> backedVolumes = backup.getBackedUpVolumes(); | ||
| List<VolumeVO> volumes = backedVolumes.stream() | ||
| .map(volume -> volumeDao.findByUuid(volume.getUuid())) | ||
| .map(volume -> volumeDao.findByUuid(volume.getPath())) |
There was a problem hiding this comment.
the new uuid or path after migration needs to be updated in the backed-up volumes metadata if any backups existing for them? any case path might also change?
There was a problem hiding this comment.
The new UUID / path for the backed up volume doesn't need to be updated as the uuid - points to the volume UUID - which is always the same on subsequent backups, and the path points to the backup path - which shouldn't vary even if volume is migrated. I don't see the path of the backup changing.
There was a problem hiding this comment.
| .map(volume -> volumeDao.findByUuid(volume.getPath())) | |
| .map(backedVolumeInfo -> volumeDao.findByUuid(backedVolumeInfo.getPath())) |
it's better change to backedVolumeInfo to avoid confusion.
@Pearl1594 Correct, path of the backup doesn't change. I mean, the volume path after migration might change as the volume is checked by its backed up path (which is before migration). cc @abh1sar
|
|
@blueorangutan package |
|
@Pearl1594 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 16637 |
|
@blueorangutan test |
|
@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
| List<Backup.VolumeInfo> backedVolumes = backup.getBackedUpVolumes(); | ||
| List<VolumeVO> volumes = backedVolumes.stream() | ||
| .map(volume -> volumeDao.findByUuid(volume.getUuid())) | ||
| .map(volume -> volumeDao.findByUuid(volume.getPath())) |
There was a problem hiding this comment.
volumeDao.findByUuid() should take volume.getUuid() as an argument not path.
Volume paths are actually set in restoreCommand.setVolumePaths(getVolumePaths(volumes));
which is getting the path from volume.getPath().
So, ideally it should work as is. Can you check?
The bug is obvious in Restore single volume code restoreCommand.setVolumePaths(Collections.singletonList(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID)));
-- here we should use the volume path not uuid
|
[SF] Trillian test result (tid-15340)
|
4 participants
Description
This PR fixes: #12517
Types of changes
Feature/Enhancement Scale or Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?