While starting VM with 'considerlasthost' enabled, don't load host tags/details for the last host when it doesn't exist

[sureshanaparti]

Description

This PR doesn't load host tags/details for the last host when it doesn't exist, while starting VM with 'considerlasthost' enabled.

Fixes #9033

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)
• [ ] build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• [ ] Major
• [ ] Minor

Bug Severity

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

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

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

[weizhouapache]

@sureshanaparti #7214 has refactored this code snippet in main/4.20 branch we can consider the backport to 4.18/4.19, and fix the issue like

     private DeployDestination deployInVmLastHost(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids,
             DeploymentPlanner planner, VirtualMachine vm, DataCenter dc, ServiceOffering offering, int cpuRequested, long ramRequested,
             boolean volumesRequireEncryption) throws InsufficientServerCapacityException {
         HostVO host = _hostDao.findById(vm.getLastHostId());
+        if (host == null) {
+            logger.warn(String.format("The last host [id=%s] does not exist, it may have been removed.", vm.getLastHostId()));
+            return null;
+        }
         _hostDao.loadHostTags(host);
         _hostDao.loadDetails(host);

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 54.09836% with 28 lines in your changes missing coverage. Please review.

Project coverage is 12.24%. Comparing base (5c9d79e) to head (d242123). Report is 3 commits behind head on 4.18.

Files	Patch %	Lines
...om/cloud/deploy/DeploymentPlanningManagerImpl.java	54.09%	13 Missing and 15 partials :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##               4.18    #9037     +/-   ##
===========================================
  Coverage     12.24%   12.24%             
- Complexity     9291     9293      +2     
===========================================
  Files          4698     4698             
  Lines        414259   414259             
  Branches      52267    50777   -1490     
===========================================
+ Hits          50707    50719     +12     
+ Misses       357251   357237     -14     
- Partials       6301     6303      +2     

Flag	Coverage Δ
unittests	12.24% <54.09%> (+<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.

[blueorangutan]

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

[sureshanaparti]

@sureshanaparti #7214 has refactored this code snippet in main/4.20 branch we can consider the backport to 4.18/4.19, and fix the issue like

     private DeployDestination deployInVmLastHost(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids,
             DeploymentPlanner planner, VirtualMachine vm, DataCenter dc, ServiceOffering offering, int cpuRequested, long ramRequested,
             boolean volumesRequireEncryption) throws InsufficientServerCapacityException {
         HostVO host = _hostDao.findById(vm.getLastHostId());
+        if (host == null) {
+            logger.warn(String.format("The last host [id=%s] does not exist, it may have been removed.", vm.getLastHostId()));
+            return null;
+        }
         _hostDao.loadHostTags(host);
         _hostDao.loadDetails(host);

@weizhouapache I think, better to fix this issue (no backport for that refactored code). PR #7214 also doesn't fix load tags issue, needs to be fixed here - https://github.com/apache/cloudstack/pull/7214/files#r1593867688

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

[weizhouapache]

@sureshanaparti #7214 has refactored this code snippet in main/4.20 branch we can consider the backport to 4.18/4.19, and fix the issue like

     private DeployDestination deployInVmLastHost(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids,
             DeploymentPlanner planner, VirtualMachine vm, DataCenter dc, ServiceOffering offering, int cpuRequested, long ramRequested,
             boolean volumesRequireEncryption) throws InsufficientServerCapacityException {
         HostVO host = _hostDao.findById(vm.getLastHostId());
+        if (host == null) {
+            logger.warn(String.format("The last host [id=%s] does not exist, it may have been removed.", vm.getLastHostId()));
+            return null;
+        }
         _hostDao.loadHostTags(host);
         _hostDao.loadDetails(host);

@weizhouapache I think, better to fix this issue (no backport for that refactored code). PR #7214 also doesn't fix load tags issue, needs to be fixed here - https://github.com/apache/cloudstack/pull/7214/files#r1593867688

@sureshanaparti right, #7214 does not fix any issue. if no backport (and then fix), this PR will only be applicable for 4.18/4.19, we need a separated pr for 4.20/main

[sureshanaparti]

@sureshanaparti right, #7214 does not fix any issue. if no backport (and then fix), this PR will only be applicable for 4.18/4.19, we need a separated pr for 4.20/main

correct @weizhouapache , separate PR for 4.20/main (will create one)

[weizhouapache]

@sureshanaparti right, #7214 does not fix any issue. if no backport (and then fix), this PR will only be applicable for 4.18/4.19, we need a separated pr for 4.20/main

correct @weizhouapache , separate PR for 4.20/main (will create one)

ok , it should work @sureshanaparti

[blueorangutan]

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

[sureshanaparti]

@sureshanaparti right, #7214 does not fix any issue. if no backport (and then fix), this PR will only be applicable for 4.18/4.19, we need a separated pr for 4.20/main

correct @weizhouapache , separate PR for 4.20/main (will create one)

ok , it should work @sureshanaparti

PR for 4.20/main here: https://github.com/apache/cloudstack/pull/9063

[sureshanaparti]

@blueorangutan test

[blueorangutan]

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

[blueorangutan]

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

Test	Result	Time (s)	Test File

[DaanHoogland]

@shwstppr , is this concern answered

[shwstppr]

@DaanHoogland yes