[Veeam] Fix restore of reimported VMs

[SadiJr]

Description

Using the VMware hypervisor with the Veeam plugin active, when restoring a VM that has been unmanaged from ACS and reimported, an error occurs because ACS duplicates the register in the database of this VM. This PR aims to fix this behavior.

Types of changes

• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] New feature (non-breaking change which adds functionality)
• [x] 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)

Feature/Enhancement Scale or Bug Severity

Bug Severity

• [ ] BLOCKER
• [ ] Critical
• [ ] Major
• [x] Minor
• [ ] Trivial

Screenshots (if appropriate):

How Has This Been Tested?

It was tested in a local lab:

1. I created one new VM via ACS, unmanaged this VM, and then reimported this VM again to ACS;
2. I added this VM to one backup offering and made one manual backup;
3. I tried to restore this VM backup;
4. Before ACS failed to restore the VM and duplicate the VM in the UI and database;
5. ACS now completes the restore successfully, and there is no VM duplication.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 19.04762% with 17 lines in your changes missing coverage. Please review.

Project coverage is 30.81%. Comparing base (1925040) to head (07c325c). Report is 313 commits behind head on 4.19.

Files	Patch %	Lines
...ain/java/com/cloud/hypervisor/guru/VMwareGuru.java	0.00%	13 Missing :warning:
.../main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java	50.00%	4 Missing :warning:

Additional details and impacted files

@@             Coverage Diff              @@
##               4.19    #7239      +/-   ##
============================================
- Coverage     30.82%   30.81%   -0.02%     
+ Complexity    34014    33998      -16     
============================================
  Files          5341     5341              
  Lines        375027   375033       +6     
  Branches      54554    54554              
============================================
- Hits         115592   115549      -43     
- Misses       244155   244194      +39     
- Partials      15280    15290      +10     

Flag	Coverage Δ
simulator-marvin-tests	24.68% <50.00%> (-0.03%)	:arrow_down:
uitests	4.39% <ø> (ø)
unit-tests	16.50% <19.04%> (+<0.01%)	:arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland a 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]

Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5592

[DaanHoogland]

@blueorangutan test rocky8 vmware-67u3

[blueorangutan]

@DaanHoogland a Trillian-Jenkins test job (rocky8 mgmt + vmware-67u3) has been kicked to run smoke tests

[blueorangutan]

Trillian test result (tid-6189) Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8 Total time taken: 44569 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7239-t6189-vmware-67u3.zip Smoke tests completed. 108 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

[sonarqubecloud[bot]]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

0.0% Coverage
0.0% Duplication

[weizhouapache]

@SadiJr if you want this in 4.18.1.0, can you rebase with 4.18 branch ? thanks

[github-actions[bot]]

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

[weizhouapache]

moved to 4.19.0.0 as the target branch is main

[shwstppr]

@SadiJr can you please check review comments

[github-actions[bot]]

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

[DaanHoogland]

@SadiJr can you answer @harikrishna-patnala 's concerns?

[DaanHoogland]

ping @SadiJr

[SadiJr]

@harikrishna-patnala @DaanHoogland Sorry for the delay, I will check the suggestions and respond/work on this PR

[weizhouapache]

@SadiJr @harikrishna-patnala @DaanHoogland I tried to reproduce the issue on veeam12 environment with some changes (see https://github.com/apache/cloudstack/pull/8241) However, the restore of backup succeeds in my testing

• unmanage a running vm
• import it to cloudstack got two VMs with same instance name

+-----+---------------+--------------------------------------+---------------+---------------+---------------------+---------------------+
| id  | name          | uuid                                 | instance_name | display_name  | created             | removed             |
+-----+---------------+--------------------------------------+---------------+---------------+---------------------+---------------------+
| 162 | admin-001-001 | c13f4662-dd08-4f24-abd3-c1b581a3ecd2 | i-2-162-VM    | admin-001-001 | 2023-12-01 08:22:55 | 2023-12-07 20:00:16 |
| 225 | i-2-162-VM    | a974a38d-5b9a-40d2-a0fb-feb80ef88762 | i-2-162-VM    | NULL          | 2023-12-07 20:01:01 | NULL                |
+-----+---------------+--------------------------------------+---------------+---------------+---------------------+---------------------+
2 rows in set (0.00 sec)

• assign backup offering
• create a backup
• stop the vm, and restore the backup.

I checked the description and code again, I think we should not fix an issue with veeam backup/restore by changing the processes of unmanage/manage VM. We should fix the issue in veeam plugin, if the issue can be reproduced.

[SadiJr]

@weizhouapache can you please share the state of VMs admin-001-001 and i-2-162-VM in your database after doing your test? And about fixing an issue with Veeam by changing the unmanage/manage process, I think having two VMs with same instance_name in database is a inconsistency that is being generated by the manage/import VMware VMs. The internal name is expected to be unique.

[weizhouapache]

@weizhouapache can you please share the state of VMs admin-001-001 and i-2-162-VM in your database after doing your test? And about fixing an issue with Veeam by changing the unmanage/manage process, I think having two VMs with same instance_name in database is a inconsistency that is being generated by the manage/import VMware VMs. The internal name is expected to be unique.

@SadiJr here you are

mysql> select id,name,uuid,instance_name,display_name,state,created,removed from vm_instance where id in (162,225);
+-----+---------------+--------------------------------------+---------------+---------------+-----------+---------------------+---------------------+
| id  | name          | uuid                                 | instance_name | display_name  | state     | created             | removed             |
+-----+---------------+--------------------------------------+---------------+---------------+-----------+---------------------+---------------------+
| 162 | admin-001-001 | c13f4662-dd08-4f24-abd3-c1b581a3ecd2 | i-2-162-VM    | admin-001-001 | Expunging | 2023-12-01 08:22:55 | 2023-12-07 20:00:16 |
| 225 | i-2-162-VM    | a974a38d-5b9a-40d2-a0fb-feb80ef88762 | i-2-162-VM    | NULL          | Stopped   | 2023-12-07 20:01:01 | NULL                |
+-----+---------------+--------------------------------------+---------------+---------------+-----------+---------------------+---------------------+
2 rows in set (0.00 sec)

the original one is in Expunging state and removed field is not empty. so it does not impact the new vm.

[SadiJr]

@weizhouapache that is strange. Your results differ a lot from mine. I review the changes in #8241, but, from what I understand, they should not impact this behavior. I will repeat my tests, using the most recent community version, this PR of mine and #8241, and I will return an opinion. Unfortunately, this may take some time as I am without my VMware environment at the moment.

[github-actions[bot]]

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

[sureshanaparti]

Hi @SadiJr Please resolve conflicts, and let me know if this PR can be targeted for 4.19.1 (if so, rebase with 4.19 branch).

[DaanHoogland]

@harikrishna-patnala , can you check if your queries are answerred?

[DaanHoogland]

ping @harikrishna-patnala

[sureshanaparti]

Hi @harikrishna-patnala Please check/confirm if your comments are addressed. Thanks.

[sureshanaparti]

@blueorangutan package

[blueorangutan]

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

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

[sureshanaparti]

@blueorangutan test alma9 vmware-70u3

[blueorangutan]

@sureshanaparti a [SL] Trillian-Jenkins test job (alma9 mgmt + vmware-70u3) has been kicked to run smoke tests