You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "vishesh92 (via GitHub)" <gi...@apache.org> on 2023/10/12 12:19:40 UTC

[PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

vishesh92 opened a new pull request, #8085:
URL: https://github.com/apache/cloudstack/pull/8085

   ### Description
   
   In case of a failure while deploying VM, we reset the host_id for the failed VM to null but not the pod_id. This results in failure when there is enough capacity in another pod, but not in the existing pod.
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### 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
   - [ ] Major
   - [x] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   
   
   ### How Has This Been Tested?
   
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   
   #### How did you try to break this feature and the system with this change?
   This needs an environment with 2 pods to reproduce the issue and test the fix.
   
   1. On management server, set a debugger here:
   https://github.com/apache/cloudstack/blob/9df580cef457cdb767aa5bea926500fa8b1263ca/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java#L383
   2. Deploy a VM. When the debugger reaches the line above, do the following:
       1) Run `SELECT id, state, pod_id, host_id, last_host_id FROM vm_instance ORDER BY id DESC LIMIT 1;` on the `cloud` database.
       2) Get the pod_id from the above and run this query for that pod_id `UPDATE host_pod_ref SET allocation_state = 'Disabled' WHERE id = <pod id>`.
       3) Set `hostHasCpuCapability = false` in the debugger to throw an error in the first run.
       4) VM is retried again once more after this failure. Before the fix, it won't stop at the debugger since it no longer has any available resources to deploy on. After the fix, it will stop again at the debugger. At this point, you can check that pod_id is different.
   
   
   
   <!-- see how your change affects other areas of the code, etc. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1761006040

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7350


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1778891790

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1792063635

   not sure if the errors are related;
   @blueorangutan  test alma9 kvm-alma9


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1777198835

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1778985582

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7506


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1767969610

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7412


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1777197607

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#discussion_r1358050239


##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1460,6 +1469,11 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
         }
     }
 
+    private boolean checkThatAllVolumesAreAllocated(long vmId) {

Review Comment:
   sorry to be a nag (not really) I think 
   ```suggestion
       private boolean areAllVolumesAllocated(long vmId) {
   ```
   is even better



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#discussion_r1357313019


##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1443,6 +1443,12 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
                 }
                 if (canRetry) {
                     try {
+                        // Setting pod id to null will result in migration of Volumes across pods
+                        // We set it to null only if migration of volumes across cluster is enabled
+                        // Or volumes are still in allocated state for that VM (in case of failure during deployment)
+                        if (MIGRATE_VM_ACROSS_CLUSTERS.valueIn(vm.getDataCenterId()) || checkForNonAllocatedVolumes(vm.getId())) {
+                            vm.setPodIdToDeployIn(null);
+                        }

Review Comment:
   Let me update the comment in detail about why we are doing this here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1760923803

   @vishesh92 a [SF] 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#discussion_r1358052528


##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1460,6 +1469,11 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
         }
     }
 
+    private boolean checkThatAllVolumesAreAllocated(long vmId) {

Review Comment:
   or
   ```suggestion
       private boolean allVolumesAreAllocatedFor(long vmId) {
   ```
   a boolean method starting with `check...()` seems off a bit. I would expect an exception for those `check..`-methods .



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1761109903

   @rohityadavcloud a [SF] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1798599182

   >> @blueorangutan test alma9 kvm-alma9
   
   > this didn´t work :exploding_head: , so started it manually
   
   results:
   Smoke tests completed. 108 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_migrate_VM_and_root_volume | `Error` | 89.26 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 51.66 | test_vm_life_cycle.py
   test_08_migrate_vm | `Error` | 0.07 | test_vm_life_cycle.py
   
   These error are all over the place at the moment, not specific to this issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1794242847

   @blueorangutan  test alma9 kvm-alma9


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1792772365

   JFYI @DaanHoogland @vishesh92 I had some issues using Alma Linux (due to repo/mirror issue) but OL8/OL9 seems to work fine with backend CI/CD.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1767842883

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#discussion_r1363613454


##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1443,6 +1443,15 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
                 }
                 if (canRetry) {
                     try {
+                        // Setting pod id to null can result in migration of Volumes across pods. This is not desirable for VMs which
+                        // have a volume in Ready state (happens when a VM is shutdown and started again).
+                        //
+                        // So, we set it to null only when
+                        //   migration of volumes across cluster is enabled
+                        //   Or, volumes are still in allocated state for that VM (happens when VM is Starting/deployed for the first time)
+                        if (MIGRATE_VM_ACROSS_CLUSTERS.valueIn(vm.getDataCenterId()) || areAllVolumesAllocated(vm.getId())) {
+                            vm.setPodIdToDeployIn(null);

Review Comment:
   shall we make this a method and add the comment as javadoc?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1761108539

   @blueorangutan test matrix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1778332436

   <b>[SF] Trillian test result (tid-8070)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 42125 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8085-t8070-kvm-centos7.zip
   Smoke tests completed. 112 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_vm_wrong_checksum | `Error` | 39.55 | test_templates.py
   test_09_list_templates_download_details | `Failure` | 0.07 | test_templates.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland merged PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1759523705

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8085?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#8085](https://app.codecov.io/gh/apache/cloudstack/pull/8085?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (b5a54e6) into [main](https://app.codecov.io/gh/apache/cloudstack/commit/9df580cef457cdb767aa5bea926500fa8b1263ca?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (9df580c) will **decrease** coverage by `24.31%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##               main   #8085       +/-   ##
   ============================================
   - Coverage     29.15%   4.84%   -24.31%     
   ============================================
     Files          5101     343     -4758     
     Lines        358724   25819   -332905     
     Branches      52349    4447    -47902     
   ============================================
   - Hits         104598    1252   -103346     
   + Misses       239764   24432   -215332     
   + Partials      14362     135    -14227     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8085/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8085/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8085/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.84% <ø> (ø)` | |
   | [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8085/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   [see 4758 files with indirect coverage changes](https://app.codecov.io/gh/apache/cloudstack/pull/8085/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1778949985

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1790711853

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1792662145

   @blueorangutan test alma9 kvm-alma9


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1790832388

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7611


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1794416917

   @DaanHoogland [SL] unsupported parameters provided. Supported mgmt server os are: `centos7, centos6, suse15, alma8, ubuntu18, ubuntu22, ubuntu20, rocky8, alma9`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-rocky8, kvm-alma8, kvm-alma9, kvm-ubuntu18, kvm-ubuntu20, kvm-ubuntu22, kvm-suse15, vmware-55u3, vmware-60u2, vmware-65u2, vmware-67u3, vmware-70u1, vmware-70u2, vmware-70u3, vmware-80, vmware-80u1, xenserver-65sp1, xenserver-71, xenserver-74, xcpng74, xcpng76, xcpng80, xcpng81, xcpng82`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1780711556

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7520


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1779043232

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7508


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1794415770

   > @blueorangutan test alma9 kvm-alma9
   
   this didn´t work :exploding_head: , so started it manually


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1798600743

   tested according to spec in the description.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1762185627

   <b>[SF] Trillian test result (tid-7953)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43151 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8085-t7953-xenserver-71.zip
   Smoke tests completed. 113 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1792663340

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1790838929

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1778890501

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1778948454

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#discussion_r1371897424


##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1443,6 +1443,7 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
                 }
                 if (canRetry) {
                     try {
+                        isMigratingAllocatedVolumesAcrossClusters(vm);

Review Comment:
   :D my suggested method name was for a method checking the condition only. I think this one is mare a `conditionalySetPodToDeployIn()`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1780616154

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#discussion_r1356873391


##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1443,6 +1443,12 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
                 }
                 if (canRetry) {
                     try {
+                        // Setting pod id to null will result in migration of Volumes across pods
+                        // We set it to null only if migration of volumes across cluster is enabled
+                        // Or volumes are still in allocated state for that VM (in case of failure during deployment)
+                        if (MIGRATE_VM_ACROSS_CLUSTERS.valueIn(vm.getDataCenterId()) || checkForNonAllocatedVolumes(vm.getId())) {
+                            vm.setPodIdToDeployIn(null);
+                        }

Review Comment:
   can this be a separate method and javadoc?
   also,
   ```
                           // We set it to null only if migration of volumes across cluster is enabled
   ```
   is clear, but
   ```
                           // Or volumes are still in allocated state for that VM (in case of failure during deployment)
   ```
   is Hindi to me ;) Would you mean
   ```suggestion
                           // Setting pod id to null will result in migration of Volumes across pods
                           // We set it to null only if migration of volumes across cluster is enabled,
                           // or in case some volumes are not in allocated state for that VM.
                           // In which case deployment would fail?
                           if (MIGRATE_VM_ACROSS_CLUSTERS.valueIn(vm.getDataCenterId()) || checkForNonAllocatedVolumes(vm.getId())) {
                               vm.setPodIdToDeployIn(null);
                           }
   ```



##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1460,6 +1466,11 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
         }
     }
 
+    private boolean checkForNonAllocatedVolumes(long vmId) {
+        final List<VolumeVO> vols = _volsDao.findByInstance(vmId);
+        return CollectionUtils.isEmpty(vols) || vols.stream().allMatch(v -> Volume.State.Allocated.equals(v.getState()));

Review Comment:
   so false if any are not in  allocated-state? How does this logically is a `checkForNonAllocatedVolumes()`? seems like a `checkThatAllVolumesAreAllocated()` to me.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1760923139

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1767846805

   @vishesh92 a [SF] 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1790715472

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1790836852

   @blueorangutan test alma9 kvm-alma9


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1791854274

   <b>[SF] Trillian test result (tid-8220)</b>
   Environment: kvm-alma9 (x2), Advanced Networking with Mgmt server a9
   Total time taken: 45623 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8085-t8220-kvm-alma9.zip
   Smoke tests completed. 108 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_migrate_VM_and_root_volume | `Error` | 90.35 | test_vm_life_cycle.py
   test_02_migrate_VM_with_two_data_disks | `Error` | 52.72 | test_vm_life_cycle.py
   test_08_migrate_vm | `Error` | 46.04 | test_vm_life_cycle.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1762151652

   <b>[SF] Trillian test result (tid-7955)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 41619 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8085-t7955-kvm-centos7.zip
   Smoke tests completed. 112 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_vm_wrong_checksum | `Error` | 40.62 | test_templates.py
   test_09_list_templates_download_details | `Failure` | 0.04 | test_templates.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix: Select another pod if all hosts in the pod becomes unavailable [cloudstack]

Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8085:
URL: https://github.com/apache/cloudstack/pull/8085#issuecomment-1780614020

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org