You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/02/03 08:04:03 UTC

[GitHub] [cloudstack] shwstppr opened a new pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance 

shwstppr opened a new pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644


   ### Description
   When a single or last UP host enters into maintenance just stopping SSVM and CPVM will leave behind VMs on hypervisor side. As these system vms will be recreated they can be destroyed.
   
   Fixes #3719
   
   <!--- ********************************************************************************* -->
   <!--- 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)
   
   ### 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 -->
   <!-- see how your change affects other areas of the code, etc. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/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.

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-824993197


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826094189


   @shwstppr a Trillian-Jenkins test job (centos7 mgmt + xcpng74) 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.

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

[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-772331075






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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-806421256


   @shwstppr a Jenkins job has been kicked to build packages. 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.

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

[GitHub] [cloudstack] shwstppr commented on a change in pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#discussion_r627124506



##########
File path: server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java
##########
@@ -795,7 +820,7 @@ private long getRescheduleTime(WorkType workType) {
             case ForceStop:
                 return ((System.currentTimeMillis() >> 10) + _stopRetryInterval);
             case Destroy:
-                return ((System.currentTimeMillis() >> 10) + _restartRetryInterval);
+                return ((System.currentTimeMillis() >> 10) + _stopRetryInterval);

Review comment:
       @sureshanaparti clubbed `Destroy` case with other cases.
   Used `_stopRetryInterval` because it uses value of config - `stop.retry.interval`. The description for that config is - `The time in seconds between retries to stop or destroy a VM.`
   
   https://github.com/apache/cloudstack/blob/4.15/engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java#L50-L51




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

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

[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826093517


   @shwstppr unsupported parameters provided. Supported mgmt server os are: `centos6, centos7, centos8, ubuntu18, ubuntu20`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-centos8, kvm-ubuntu18, kvm-ubuntu20, xenserver-71, xenserver-65sp1, vmware-70u1, vmware-67u3, vmware-65u2, vmware-60u2, vmware-55u3, xcpng76, xcpng80, xcpng81, xenserver-74, xcpng74`


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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-806420983


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

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

[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-806436453






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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825183748


   <b>Trillian Build Failed (tid-506)<b/>


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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-772316035


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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-773071394


   @DaanHoogland yes, probably VR should also be included but right now these changes are not working. Expunging systemvm takes time and between that MS tries to start new systemvm. So still working on fixing...


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

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

[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825168523






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

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

[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#discussion_r626501014



##########
File path: server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java
##########
@@ -795,7 +820,7 @@ private long getRescheduleTime(WorkType workType) {
             case ForceStop:
                 return ((System.currentTimeMillis() >> 10) + _stopRetryInterval);
             case Destroy:
-                return ((System.currentTimeMillis() >> 10) + _restartRetryInterval);
+                return ((System.currentTimeMillis() >> 10) + _stopRetryInterval);

Review comment:
       @shwstppr any reason to change to stop retry interval? if you want to keep it, move the case stmt after 'ForceStop'.
   
   Alternatively, you can use different config (may be "_destroy.retry.interval_") for destroy operation, and set its value to the current value in "_restart.retry.interval_" on upgrade.




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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-832557340


   @blueorangutan package
   
   @DaanHoogland refactored new changes to separate methods where possible. Unable to meaningfully factor out `SecondaryStorageManagerImpl` and `ConsoleProxyManagerImpl`. They need more in-depth refactoring as al lot of common code there, something we can attempt for `main` branch


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-772316665


   @shwstppr a Jenkins job has been kicked to build packages. 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.

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

[GitHub] [cloudstack] DaanHoogland commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-772538298


   code looks ok, do we need to consider VRs?


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

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

[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826093517


   @shwstppr unsupported parameters provided. Supported mgmt server os are: `centos6, centos7, centos8, ubuntu18, ubuntu20`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-centos8, kvm-ubuntu18, kvm-ubuntu20, xenserver-71, xenserver-65sp1, vmware-70u1, vmware-67u3, vmware-65u2, vmware-60u2, vmware-55u3, xcpng76, xcpng80, xcpng81, xenserver-74, xcpng74`


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

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

[GitHub] [cloudstack] rhtyd commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829323187


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

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

[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-824998577






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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825153154


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

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

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#discussion_r623860237



##########
File path: services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java
##########
@@ -811,6 +811,13 @@ public void allocCapacity(long dataCenterId, SecondaryStorageVm.Role role) {
     }
 
     public boolean isZoneReady(Map<Long, ZoneHostInfo> zoneHostInfoMap, long dataCenterId) {
+        List <HostVO> hosts = _hostDao.listByDataCenterId(dataCenterId);
+        if (CollectionUtils.isEmpty(hosts)) {
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug("Zone " + dataCenterId + " has no host available which is enabled and in Up state");
+            }
+            return false;
+        }

Review comment:
       separate method?

##########
File path: server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java
##########
@@ -682,29 +687,44 @@ public void cancelDestroy(VMInstanceVO vm, Long hostId) {
 
     protected Long destroyVM(final HaWorkVO work) {
         final VirtualMachine vm = _itMgr.findById(work.getInstanceId());
-        s_logger.info("Destroying " + vm.toString());
+        if (vm == null) {
+            s_logger.info("No longer can find VM " + work.getInstanceId() + ". Throwing away " + work);
+            work.setStep(Step.Done);
+            return null;
+        }
+        boolean expunge = VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())
+                || VirtualMachine.Type.ConsoleProxy.equals(vm.getType());
+        if (!expunge && VirtualMachine.State.Destroyed.equals(work.getPreviousState())) {
+            s_logger.info("VM " + vm.getUuid() + " already in " + vm.getState() + " state. Throwing away " + work);
+            work.setStep(Step.Done);
+            return null;
+        }
         try {
-            if (vm.getState() != State.Destroyed) {
-                s_logger.info("VM is no longer in Destroyed state " + vm.toString());
-                return null;
+            if (VirtualMachine.State.Running.equals(work.getPreviousState())) {
+                _itMgr.advanceStop(vm.getUuid(), true);
             }
-
-            if (vm.getHostId() != null) {
-                _itMgr.destroy(vm.getUuid(), false);
-                s_logger.info("Successfully destroy " + vm);
-                return null;
-            } else {
-                if (s_logger.isDebugEnabled()) {
-                    s_logger.debug(vm + " has already been stopped");
+            s_logger.info("Destroying " + vm.toString());
+            if (!VirtualMachine.State.Expunging.equals(work.getPreviousState())) {
+                s_logger.info("Destroying " + vm.getUuid());
+                if (VirtualMachine.Type.ConsoleProxy.equals(vm.getType())) {
+                    consoleProxyManager.destroyProxy(vm.getId());
+                } else if (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())) {
+                    secondaryStorageVmManager.destroySecStorageVm(vm.getId());
+                } else {

Review comment:
       the total method has grown from 23 to 40 lines, can you factor the new bits out please.

##########
File path: server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java
##########
@@ -1006,6 +1006,13 @@ private void allocCapacity(long dataCenterId) {
     }
 
     public boolean isZoneReady(Map<Long, ZoneHostInfo> zoneHostInfoMap, long dataCenterId) {
+        List <HostVO> hosts = _hostDao.listByDataCenterId(dataCenterId);
+        if (CollectionUtils.isEmpty(hosts)) {
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug("Zone " + dataCenterId + " has no host available which is enabled and in Up state");
+            }
+            return false;
+        }

Review comment:
       this is the exact same code as inSecondaryStorageManagerImpl.isZoneReady() can you factor out and reuse?

##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1267,8 +1266,14 @@ private boolean doMaintain(final long hostId) {
                 if (hosts == null || hosts.isEmpty() || !answer.getMigrate()
                         || _serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.vgpuType.toString()) != null) {
                     // Migration is not supported for VGPU Vms so stop them.
-                    // for the last host in this cluster, stop all the VMs
-                    s_logger.error("Maintenance: No hosts available for migrations. Scheduling shutdown instead of migrations.");
+                    // for the last host in this cluster, destroy SSVM/CPVM and stop all other VMs
+                    if (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())
+                            || VirtualMachine.Type.ConsoleProxy.equals(vm.getType())) {
+                        s_logger.error(String.format("Maintenance: VM is of type %s. Destroying VM %s (ID: %s) immediately instead of migration.", vm.getType().toString(), vm.getInstanceName(), vm.getUuid()));
+                        _haMgr.scheduleDestroy(vm, host.getId());
+                        continue;
+                    }

Review comment:
       can you factor this out, including the comment, please?

##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -606,10 +610,18 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti
             if (hostId != null) {
                 final Commands cmds = new Commands(Command.OnError.Stop);
                 for (final Command command : finalizeExpungeCommands) {
+                    if (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType()) ||
+                            VirtualMachine.Type.ConsoleProxy.equals(vm.getType())) {
+                        command.setBypassHostMaintenance(true);
+                    }
                     cmds.addCommand(command);
                 }
                 if (nicExpungeCommands != null) {
                     for (final Command command : nicExpungeCommands) {
+                        if (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType()) ||
+                                VirtualMachine.Type.ConsoleProxy.equals(vm.getType())) {
+                            command.setBypassHostMaintenance(true);
+                        }

Review comment:
       I'm seeing this code fragment a lot, please factor out and reuse.

##########
File path: plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java
##########
@@ -214,6 +216,16 @@ public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCal
     @Override
     public void deleteAsync(DataStore dataStore, DataObject data, AsyncCompletionCallback<CommandResult> callback) {
         DeleteCommand cmd = new DeleteCommand(data.getTO());
+        if (DataObjectType.VOLUME.equals(data.getType())) {
+            Volume volume = (Volume)data;
+            if (volume.getInstanceId() != null) {
+                VMInstanceVO vm = vmDao.findById(volume.getInstanceId());
+                if (vm != null && (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType()) ||
+                        VirtualMachine.Type.ConsoleProxy.equals(vm.getType()))) {
+                    cmd.setBypassHostMaintenance(true);
+                }
+            }
+        }

Review comment:
       can you factor out and give a meaningful name?




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

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

[GitHub] [cloudstack] shwstppr removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826093456


   @blueorangutan test centos7 xs71


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-824994240


   @shwstppr a Jenkins job has been kicked to build packages. 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.

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825168523


   @shwstppr a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, 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.

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826086477


   <b>Trillian test result (tid-517)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34370 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4644-t517-vmware-65u2.zip
   Smoke tests completed. 87 look OK, 0 have error(s)
   Only failed 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.

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826093456






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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829198010


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 487


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825420978


   @shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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.

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826081805


   <b>Trillian test result (tid-516)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 32720 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4644-t516-kvm-centos7.zip
   Smoke tests completed. 87 look OK, 0 have error(s)
   Only failed 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.

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-824998577


   Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_multiplication_x: debian. SL-JID 449


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

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

[GitHub] [cloudstack] rhtyd commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-840424495


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829526969


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 493


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

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

[GitHub] [cloudstack] shwstppr removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-772316035


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-837501683


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 573


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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-773071394


   @DaanHoogland yes, probably VR should also be included but right now these changes are not working. Expunging systemvm takes time and between that MS tries to start new systemvm. So still working on fixing...


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

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

[GitHub] [cloudstack] shwstppr removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-824993197






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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829858700


   @blueorangutan test centos7 vmware-67u3


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829323571


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826019598


   @shwstppr a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, 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.

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825420388


   @blueorangutan test centos7 vmware-67u3


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-840829452


   <b>Trillian test result (tid-680)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 41201 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4644-t680-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Smoke tests completed. 86 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 71.50 | test_kubernetes_clusters.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.

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825763921


   <b>Trillian test result (tid-511)</b>
   Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34095 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4644-t511-vmware-67u3.zip
   Smoke tests completed. 87 look OK, 0 have error(s)
   Only failed 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.

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

[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#discussion_r631788363



##########
File path: plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java
##########
@@ -211,10 +213,22 @@ public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCal
         }
     }
 
+    private boolean commandCanBypassHostMaintenance(DataObject data) {
+        if (DataObjectType.VOLUME.equals(data.getType())) {
+            Volume volume = (Volume)data;
+            if (volume.getInstanceId() != null) {
+                VMInstanceVO vm = vmDao.findById(volume.getInstanceId());
+                return vm != null && (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType()) ||
+                        VirtualMachine.Type.ConsoleProxy.equals(vm.getType()));
+            }
+        }
+        return false;
+    }
+

Review comment:
       ok @shwstppr how other drivers set this parameter ?




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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825153808


   @shwstppr a Jenkins job has been kicked to build packages. 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.

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826094096


   @blueorangutan test centos7 xcpng74


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826081805






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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829858860


   @shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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.

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

[GitHub] [cloudstack] shwstppr commented on a change in pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#discussion_r627125816



##########
File path: plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java
##########
@@ -211,10 +213,22 @@ public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCal
         }
     }
 
+    private boolean commandCanBypassHostMaintenance(DataObject data) {
+        if (DataObjectType.VOLUME.equals(data.getType())) {
+            Volume volume = (Volume)data;
+            if (volume.getInstanceId() != null) {
+                VMInstanceVO vm = vmDao.findById(volume.getInstanceId());
+                return vm != null && (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType()) ||
+                        VirtualMachine.Type.ConsoleProxy.equals(vm.getType()));
+            }
+        }
+        return false;
+    }
+

Review comment:
       @sureshanaparti yes, bypasshostmaintenance flag could be true for volumes in managed stores as well if the volume is of cpvm or ssvm




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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825166415


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 452


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-832572642


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 521


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

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

[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#discussion_r626496552



##########
File path: server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java
##########
@@ -680,31 +685,51 @@ public void cancelDestroy(VMInstanceVO vm, Long hostId) {
         _haDao.delete(vm.getId(), WorkType.Destroy);
     }
 
+    private void stopVMWithCleanup(VirtualMachine vm, VirtualMachine.State state) throws OperationTimedoutException, ResourceUnavailableException {
+        if (VirtualMachine.State.Running.equals(state)) {
+            _itMgr.advanceStop(vm.getUuid(), true);
+        }
+    }
+
+    private void destroyVM(VirtualMachine vm, boolean expunge) throws OperationTimedoutException, AgentUnavailableException {
+        s_logger.info("Destroying " + vm.toString());
+        if (VirtualMachine.Type.ConsoleProxy.equals(vm.getType())) {
+            consoleProxyManager.destroyProxy(vm.getId());
+        } else if (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())) {
+            secondaryStorageVmManager.destroySecStorageVm(vm.getId());
+        } else {
+            _itMgr.destroy(vm.getUuid(), expunge);
+        }
+    }
+
     protected Long destroyVM(final HaWorkVO work) {
         final VirtualMachine vm = _itMgr.findById(work.getInstanceId());
-        s_logger.info("Destroying " + vm.toString());
+        if (vm == null) {
+            s_logger.info("No longer can find VM " + work.getInstanceId() + ". Throwing away " + work);
+            return null;
+        }
+        boolean expunge = VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())

Review comment:
       ```suggestion
           boolean expungeSystemVM = VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())
   ```




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

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

[GitHub] [cloudstack] shwstppr closed pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr closed pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644


   


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825184653


   <b>Trillian Build Failed (tid-507)<b/>


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826019797


   <b>Trillian Build Failed (tid-515)<b/>


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829184359


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 486


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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826093456


   @blueorangutan test centos7 xs71


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829738761


   <b>Trillian test result (tid-560)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 35351 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4644-t560-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Smoke tests completed. 87 look OK, 0 have error(s)
   Only failed 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.

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-836833847


   @rhtyd a Jenkins job has been kicked to build packages. 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.

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826093957


   @blueorangutan centos7 xcpng74


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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826019487


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

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

[GitHub] [cloudstack] rhtyd commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-836832120


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-840425043


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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825167963


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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829512837


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

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

[GitHub] [cloudstack] shwstppr commented on a change in pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#discussion_r627122839



##########
File path: server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java
##########
@@ -680,31 +685,51 @@ public void cancelDestroy(VMInstanceVO vm, Long hostId) {
         _haDao.delete(vm.getId(), WorkType.Destroy);
     }
 
+    private void stopVMWithCleanup(VirtualMachine vm, VirtualMachine.State state) throws OperationTimedoutException, ResourceUnavailableException {
+        if (VirtualMachine.State.Running.equals(state)) {
+            _itMgr.advanceStop(vm.getUuid(), true);
+        }
+    }
+
+    private void destroyVM(VirtualMachine vm, boolean expunge) throws OperationTimedoutException, AgentUnavailableException {
+        s_logger.info("Destroying " + vm.toString());
+        if (VirtualMachine.Type.ConsoleProxy.equals(vm.getType())) {
+            consoleProxyManager.destroyProxy(vm.getId());
+        } else if (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())) {
+            secondaryStorageVmManager.destroySecStorageVm(vm.getId());
+        } else {
+            _itMgr.destroy(vm.getUuid(), expunge);
+        }
+    }
+
     protected Long destroyVM(final HaWorkVO work) {
         final VirtualMachine vm = _itMgr.findById(work.getInstanceId());
-        s_logger.info("Destroying " + vm.toString());
+        if (vm == null) {
+            s_logger.info("No longer can find VM " + work.getInstanceId() + ". Throwing away " + work);
+            return null;
+        }
+        boolean expunge = VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())

Review comment:
       @sureshanaparti I think `expunge` name is better here as it is use irrespective of vm type in `destroyVM` method. Through it holds value - `true` only for ssvm and cpvm




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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-833278271


   @shwstppr a Jenkins job has been kicked to build packages. 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.

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-832557703


   @shwstppr a Jenkins job has been kicked to build packages. 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.

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

[GitHub] [cloudstack] shwstppr commented on a change in pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#discussion_r631835831



##########
File path: plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java
##########
@@ -211,10 +213,22 @@ public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCal
         }
     }
 
+    private boolean commandCanBypassHostMaintenance(DataObject data) {
+        if (DataObjectType.VOLUME.equals(data.getType())) {
+            Volume volume = (Volume)data;
+            if (volume.getInstanceId() != null) {
+                VMInstanceVO vm = vmDao.findById(volume.getInstanceId());
+                return vm != null && (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType()) ||
+                        VirtualMachine.Type.ConsoleProxy.equals(vm.getType()));
+            }
+        }
+        return false;
+    }
+

Review comment:
       @sureshanaparti other drivers don't seem to be using `DeleteCommand`.
   ```
   ~/lab/shapeblue/cloudstack|fix-uncleared-sys-vms-3719⚡ 
   ⇒  grep "new DeleteCommand" -R .
   ./engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java:                            DeleteCommand dtCommand = new DeleteCommand(tmplTO);
   ./engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java:                        DeleteCommand dtCommand = new DeleteCommand(tmplTO);
   ./engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java:            DeleteCommand cmd = new DeleteCommand(data.getTO());
   ./server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:                    cmd = new DeleteCommand(volumeInfo.getTO());
   ./plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java:        DeleteCommand cmd = new DeleteCommand(data.getTO());
   ./plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java:         * DeleteCommand cmd = new DeleteCommand(vo.getUri());
   ./plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java:                        DeleteCommand deleteCommand = new DeleteCommand(template);
   ./plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java:                    DeleteCommand deleteCommand = new DeleteCommand(newSnapshot);
   ./plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:                    DeleteCommand cmd = new DeleteCommand(volumeInfo.getTO());
   ./plugins/hypervisors/ovm3/src/test/java/com/cloud/hypervisor/ovm3/resources/Ovm3StorageProcessorTest.java:        DeleteCommand delete = new DeleteCommand(vol);
   ./plugins/hypervisors/ovm3/src/test/java/com/cloud/hypervisor/ovm3/resources/Ovm3StorageProcessorTest.java:        delete = new DeleteCommand(template);
   ./plugins/hypervisors/ovm3/src/test/java/com/cloud/hypervisor/ovm3/resources/Ovm3StorageProcessorTest.java:        delete = new DeleteCommand(snap);
   ./services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java:            DeleteCommand deleteCommand = new DeleteCommand(newTemplate)
   ```
   PS: SamplePrimaryDataStoreDriverImpl code is commented




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

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

[GitHub] [cloudstack] rhtyd merged pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644


   


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

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

[GitHub] [cloudstack] shwstppr removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825167963


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

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

[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#discussion_r626502317



##########
File path: plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java
##########
@@ -211,10 +213,22 @@ public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCal
         }
     }
 
+    private boolean commandCanBypassHostMaintenance(DataObject data) {
+        if (DataObjectType.VOLUME.equals(data.getType())) {
+            Volume volume = (Volume)data;
+            if (volume.getInstanceId() != null) {
+                VMInstanceVO vm = vmDao.findById(volume.getInstanceId());
+                return vm != null && (VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType()) ||
+                        VirtualMachine.Type.ConsoleProxy.equals(vm.getType()));
+            }
+        }
+        return false;
+    }
+

Review comment:
       @shwstppr Is bypass host maintenance applicable for volumes in managed storage as well?




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

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

[GitHub] [cloudstack] rhtyd commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-773156790


   Edge case, not critical issue moved to next milestone.


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-825182748


   <b>Trillian Build Failed (tid-505)<b/>


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

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

[GitHub] [cloudstack] shwstppr commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-833277563


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

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

[GitHub] [cloudstack] shwstppr removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826093957






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

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

[GitHub] [cloudstack] shwstppr removed a comment on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
shwstppr removed a comment on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826093957


   @blueorangutan centos7 xcpng74


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-772331075


   Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2644


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-833294205


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 526


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826241018


   <b>Trillian test result (tid-521)</b>
   Environment: xcpng74 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 47682 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4644-t521-xcpng74.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
   Smoke tests completed. 85 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 3607.41 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 3614.88 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.07 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.14 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 119.63 | test_kubernetes_clusters.py
   test_01_scale_vm | `Failure` | 13.35 | test_scale_vm.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.

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

[GitHub] [cloudstack] rhtyd commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-773156790


   Edge case, not critical issue moved to next milestone.


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-826093517


   @shwstppr unsupported parameters provided. Supported mgmt server os are: `centos6, centos7, centos8, ubuntu18, ubuntu20`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-centos8, kvm-ubuntu18, kvm-ubuntu20, xenserver-71, xenserver-65sp1, vmware-70u1, vmware-67u3, vmware-65u2, vmware-60u2, vmware-55u3, xcpng76, xcpng80, xcpng81, xenserver-74, xcpng74`


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: [WIP] server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-806436453


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 258


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #4644: server: destroy ssvm, cpvm on last host maintenance

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4644:
URL: https://github.com/apache/cloudstack/pull/4644#issuecomment-829513675


   @shwstppr a Jenkins job has been kicked to build packages. 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.

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