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/09/15 05:26:32 UTC

[GitHub] [cloudstack] GutoVeronezi opened a new pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

GutoVeronezi opened a new pull request #4966:
URL: https://github.com/apache/cloudstack/pull/4966


   ### Description
   This PR intends to refactor a few process, like extract repeated code to methods, remove unnecessary logic... Of class `VirtualMachineManagerImpl` and improve logging to facilitate troubleshooting.
   
   ### Types of changes
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [x] Enhancement (improves an existing feature and functioanlity)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   - [ ] Major
   - [x] Minor
   
   ### How Has This Been Tested?
   It has been tested locally on a test lab.


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



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4390,13 +4390,18 @@ public VMInstanceVO reConfigureVm(final String vmUuid, final ServiceOffering old
 
             VirtualMachine vm = retrieveVmFromJobOutcome(outcome, vmUuid, "reconfigureVm");
 
+            Object result = null;
             try {
-                retrieveResultFromJobOutcomeAndThrowExceptionIfNeeded(outcome);
-            } catch (InsufficientCapacityException ex) {
+                result = retrieveResultFromJobOutcomeAndThrowExceptionIfNeeded(outcome);
+
+                if (result == null) {
+                    return (VMInstanceVO)vm;
+                }
+            } catch (Exception ex) {

Review comment:
       I'm not particular fond of this. much rather see the statement outside the try/catch and be
   ```
   if (result != null) {
       throw new RuntimeException("Unexpected job execution result");
   }
   return (VMInstanceVO)vm;
   ```
   
   (no :-1: by any means though)




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



[GitHub] [cloudstack] rhtyd commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @nvazquez General comment - closer to an RC you want to exclude any non-essential PRs you aren't confident on wrt stability and potential regressions. We can move such PRs to merging right after cutting the RC, and move them to the next major or minor release which should give enough time for community to test the changes until the next major/minor release.


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-639)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 100678 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t639-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic_adapter_type.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 64 look OK, 24 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAccounts>:setup | `Error` | 0.00 | test_accounts.py
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 0.00 | test_accounts.py
   test_DeleteDomain | `Error` | 4.01 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 4.06 | test_accounts.py
   ContextSuite context=TestRemoveUserFromAccount>:setup | `Error` | 5.08 | test_accounts.py
   ContextSuite context=TestDomainsServiceOfferings>:setup | `Error` | 1514.52 | test_domain_service_offerings.py
   ContextSuite context=TestInternalLb>:setup | `Error` | 0.00 | test_internal_lb.py
   ContextSuite context=TestDeployVmWithAffinityGroup>:setup | `Error` | 0.00 | test_affinity_groups_projects.py
   test_01_create_iso_with_checksum_sha1 | `Error` | 66.36 | test_iso.py
   test_02_create_iso_with_checksum_sha256 | `Error` | 66.37 | test_iso.py
   test_03_create_iso_with_checksum_md5 | `Error` | 66.38 | test_iso.py
   test_04_create_iso_with_no_checksum | `Error` | 66.37 | test_iso.py
   test_01_create_iso | `Failure` | 1511.71 | test_iso.py
   ContextSuite context=TestISO>:setup | `Error` | 3022.80 | test_iso.py
   ContextSuite context=TestAsyncJob>:setup | `Error` | 0.00 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   ContextSuite context=TestDeployVirtioSCSIVM>:setup | `Error` | 0.00 | test_deploy_virtio_scsi_vm.py
   test_list_clusters_metrics | `Error` | 1511.52 | test_metrics_api.py
   test_list_vms_metrics | `Error` | 0.39 | test_metrics_api.py
   ContextSuite context=TestDeployVMFromISO>:setup | `Error` | 0.00 | test_deploy_vm_iso.py
   test_delete_account | `Error` | 1511.54 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 1.11 | test_network.py
   test_deploy_vm_l2network | `Error` | 1.10 | test_network.py
   test_l2network_restart | `Error` | 2.18 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 3.33 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 1.01 | test_network.py
   test_reboot_router | `Failure` | 0.04 | test_network.py
   test_releaseIP | `Error` | 0.43 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 0.49 | test_network.py
   ContextSuite context=TestDeployVmWithVariedPlanners>:setup | `Error` | 0.00 | test_deploy_vms_with_varied_deploymentplanners.py
   ContextSuite context=TestDeployVmWithUserData>:setup | `Error` | 0.00 | test_deploy_vm_with_userdata.py
   ContextSuite context=TestAdapterTypeForNic>:setup | `Error` | 0.00 | test_nic_adapter_type.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 60.67 | test_kubernetes_supported_versions.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 1511.58 | test_multipleips_per_nic.py
   ContextSuite context=TestPortForwardingRules>:setup | `Error` | 0.00 | test_portforwardingrules.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 350.44 | test_privategw_acl.py
   ContextSuite context=TestProjectSuspendActivate>:setup | `Error` | 1519.81 | test_projects.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 331.75 | test_routers_network_ops.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 536.19 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 506.30 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 502.14 | test_vpc_redundant.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] DaanHoogland removed a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Restart jenkins...


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-1650)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 39486 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t1650-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 86 look OK, 3 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_add_primary_storage_disabled_host | `Error` | 0.53 | test_primary_storage.py
   test_01_primary_storage_nfs | `Error` | 0.10 | test_primary_storage.py
   ContextSuite context=TestStorageTags>:setup | `Error` | 0.18 | test_primary_storage.py
   test_02_list_snapshots_with_removed_data_store | `Error` | 1.15 | test_snapshots.py
   test_01_secure_vm_migration | `Error` | 153.96 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 270.80 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 145.99 | test_vm_life_cycle.py
   test_08_migrate_vm | `Error` | 43.64 | 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



[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -1129,27 +1080,24 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
                     if (planChangedByVolume) {
                         plan = originalPlan;
                         planChangedByVolume = false;
-                        //do not enter volume reuse for next retry, since we want to look for resources outside the volume's cluster
                         reuseVolume = false;
                         continue;
                     }
                     throw new InsufficientServerCapacityException("Unable to create a deployment for " + vmProfile, DataCenter.class, plan.getDataCenterId(),
                             areAffinityGroupsAssociated(vmProfile));
                 }
 
-                if (dest != null) {

Review comment:
       This null check is not necessary indeed :+1:

##########
File path: api/src/main/java/com/cloud/vm/NicProfile.java
##########
@@ -430,16 +430,6 @@ public void deallocate() {
 
     @Override
     public String toString() {
-        return new StringBuilder("NicProfile[").append(id)
-                .append("-")
-                .append(vmId)
-                .append("-")
-                .append(reservationId)
-                .append("-")
-                .append(iPv4Address)
-                .append("-")
-                .append(broadcastUri)
-                .append("]")
-                .toString();
+        return String.format("NicProfile {\"id\": %s, \"vmId\": %s, \"reservationId\": \"%s\", \"iPv4Address\": \"%s\", \"broadcastUri\": \"%s\"}", id, vmId, reservationId, iPv4Address, broadcastUri);

Review comment:
       `NicProfile {\"id\": %s, \"vmId\": %s` will result in `NicProfile {"id": 123, "vmId": 456, ...`
   
   I would go to something as:
   `NicProfile {id: \"%s\", vmId: \"%s\"` :arrow_right: `NicProfile {id: "123", vmId: "456", ...`
   OR
   `NicProfile {id: %s, vmId: %s` :arrow_right: `NicProfile {id: 123, vmId: 456, ...`
   

##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -618,43 +607,43 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti
 
         final VirtualMachineGuru guru = getVmGuru(vm);
         guru.finalizeExpunge(vm);
-        //remove the overcommit details from the uservm details
+
         userVmDetailsDao.removeDetails(vm.getId());
 
-        // Remove deploy as-is (if any)
         userVmDeployAsIsDetailsDao.removeDetails(vm.getId());
 
-        // send hypervisor-dependent commands before removing
-        final List<Command> finalizeExpungeCommands = hvGuru.finalizeExpunge(vm);
-        if (finalizeExpungeCommands != null && finalizeExpungeCommands.size() > 0) {
-            if (hostId != null) {
-                final Commands cmds = new Commands(Command.OnError.Stop);
-                for (final Command command : finalizeExpungeCommands) {
-                    command.setBypassHostMaintenance(expungeCommandCanBypassHostMaintenance(vm));
-                    cmds.addCommand(command);
-                }
-                if (nicExpungeCommands != null) {
-                    for (final Command command : nicExpungeCommands) {
-                        command.setBypassHostMaintenance(expungeCommandCanBypassHostMaintenance(vm));
-                        cmds.addCommand(command);
-                    }
-                }
-                _agentMgr.send(hostId, cmds);
-                if (!cmds.isSuccessful()) {
-                    for (final Answer answer : cmds.getAnswers()) {
-                        if (!answer.getResult()) {
-                            s_logger.warn("Failed to expunge vm due to: " + answer.getDetails());
-                            throw new CloudRuntimeException("Unable to expunge " + vm + " due to " + answer.getDetails());
-                        }
-                    }
-                }
-            }
-        }
-
         if (s_logger.isDebugEnabled()) {
             s_logger.debug("Expunged " + vm);
         }
+    }
+
+    protected void handleUnsuccessfulCommands(Commands cmds, VMInstanceVO vm) throws CloudRuntimeException {
+        String cmdsStr = cmds.toString();
+        String vmToString = vm.toString();
+
+        if (cmds.isSuccessful()) {
+            s_logger.debug(String.format("The commands [%s] to %s were successful.", cmdsStr, vmToString));
+            return;
+        }
+
+        s_logger.info(String.format("The commands [%s] to %s were unsuccessful. Handling answers.", cmdsStr, vmToString));
 
+        Answer[] answers = cmds.getAnswers();
+        if (answers == null) {
+            s_logger.debug(String.format("There are no answers to commands [%s] to %s.", cmdsStr, vmToString));
+            return;
+        }
+
+        for (Answer answer : answers) {
+            String details = answer.getDetails();
+            if (!answer.getResult()) {
+                String message = String.format("Unable to expunge %s due to [%s].", vmToString, details);
+                s_logger.warn(message);

Review comment:
       What do you think of moving this log level from "warn" to "error"? This log look like could be an Error considering the thrown exception.




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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-1804)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 38389 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t1804-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Smoke tests completed. 88 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_list_snapshots_with_removed_data_store | `Error` | 51.68 | test_snapshots.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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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






-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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


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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @sureshanaparti done, thanks!


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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Can anyone review this?


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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @GabrielBrascher done, thanks!


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @DaanHoogland 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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 1233


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 874


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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   not sure about this failure
   @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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-639)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 100678 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t639-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic_adapter_type.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 64 look OK, 24 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAccounts>:setup | `Error` | 0.00 | test_accounts.py
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 0.00 | test_accounts.py
   test_DeleteDomain | `Error` | 4.01 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 4.06 | test_accounts.py
   ContextSuite context=TestRemoveUserFromAccount>:setup | `Error` | 5.08 | test_accounts.py
   ContextSuite context=TestDomainsServiceOfferings>:setup | `Error` | 1514.52 | test_domain_service_offerings.py
   ContextSuite context=TestInternalLb>:setup | `Error` | 0.00 | test_internal_lb.py
   ContextSuite context=TestDeployVmWithAffinityGroup>:setup | `Error` | 0.00 | test_affinity_groups_projects.py
   test_01_create_iso_with_checksum_sha1 | `Error` | 66.36 | test_iso.py
   test_02_create_iso_with_checksum_sha256 | `Error` | 66.37 | test_iso.py
   test_03_create_iso_with_checksum_md5 | `Error` | 66.38 | test_iso.py
   test_04_create_iso_with_no_checksum | `Error` | 66.37 | test_iso.py
   test_01_create_iso | `Failure` | 1511.71 | test_iso.py
   ContextSuite context=TestISO>:setup | `Error` | 3022.80 | test_iso.py
   ContextSuite context=TestAsyncJob>:setup | `Error` | 0.00 | test_async_job.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   ContextSuite context=TestDeployVirtioSCSIVM>:setup | `Error` | 0.00 | test_deploy_virtio_scsi_vm.py
   test_list_clusters_metrics | `Error` | 1511.52 | test_metrics_api.py
   test_list_vms_metrics | `Error` | 0.39 | test_metrics_api.py
   ContextSuite context=TestDeployVMFromISO>:setup | `Error` | 0.00 | test_deploy_vm_iso.py
   test_delete_account | `Error` | 1511.54 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 1.11 | test_network.py
   test_deploy_vm_l2network | `Error` | 1.10 | test_network.py
   test_l2network_restart | `Error` | 2.18 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 3.33 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 1.01 | test_network.py
   test_reboot_router | `Failure` | 0.04 | test_network.py
   test_releaseIP | `Error` | 0.43 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 0.49 | test_network.py
   ContextSuite context=TestDeployVmWithVariedPlanners>:setup | `Error` | 0.00 | test_deploy_vms_with_varied_deploymentplanners.py
   ContextSuite context=TestDeployVmWithUserData>:setup | `Error` | 0.00 | test_deploy_vm_with_userdata.py
   ContextSuite context=TestAdapterTypeForNic>:setup | `Error` | 0.00 | test_nic_adapter_type.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 60.67 | test_kubernetes_supported_versions.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 1511.58 | test_multipleips_per_nic.py
   ContextSuite context=TestPortForwardingRules>:setup | `Error` | 0.00 | test_portforwardingrules.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 350.44 | test_privategw_acl.py
   ContextSuite context=TestProjectSuspendActivate>:setup | `Error` | 1519.81 | test_projects.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 331.75 | test_routers_network_ops.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 536.19 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 506.30 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 502.14 | test_vpc_redundant.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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @GutoVeronezi you've submitted many PRs which are the same class of PRs around log improvements and refactorings and changing the resource/vo/object `toString()` that returns hand-written json and add inconsistency across other VOs/objects/... Why is that? Are the json output from log statements consumed somewhere?
   
   Can you club all such PRs into a single PR with separate/individual commits? They make it difficult to review and test individually and take a lot of time and resources from reviewers and BO/lab.


-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @rhtyd please let's make changes per PR as little as possible. smaller PRs are usually easier to review. Only when the code is very intertwined should we combine PRs into bigger ones.


-- 
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] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   > @GutoVeronezi you've submitted many PRs which are the same class of PRs around log improvements and refactorings and changing the resource/vo/object `toString()` that returns hand-written json and add inconsistency across other VOs/objects/... Why is that? Are the json output from log statements consumed somewhere?
   > 
   > Can you club all such PRs into a single PR with separate/individual commits? They make it difficult to review and test individually and take a lot of time and resources from reviewers and BO/lab.
   
   @rhtyd
   
   Most of `toString()` implementations uses a simple hyphen (`-`) to separete values, but what `NicProfile[1-2-3-192.168.0.1-...]` means? Operators get confused and need to open the code to know what each value means. Moreover, if a string contains a hyphen, it will print as an separator and can confuse much more the operator.
   JSON was chosen because it is an easy format to read key-value pairs. `NicProfile {"id": 1, "vmId": 2, "reservationId": "3", "iPv4Address": "192.168.0.1", "broadcastUri": "..."}` tells much more than a simple `-----`.
   
   But that is not the focus of the PR. The focus is improve logging of these classes, as they do not supply an appropriate context to make the operation feasible.
   
   About join all this PRs in only one (separated in commits):
   Each PR is separted by context. Each one has their own peculiarity. Each peaculiarity refactored is separated by commits. Join this all in a single commit will make it even more difficult to review, as there will be a lot of changes without pontual descriptions.


-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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] GutoVeronezi edited a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

Posted by GitBox <gi...@apache.org>.
GutoVeronezi edited a comment on pull request #4966:
URL: https://github.com/apache/cloudstack/pull/4966#issuecomment-837061808


   > @GutoVeronezi you've submitted many PRs which are the same class of PRs around log improvements and refactorings and changing the resource/vo/object `toString()` that returns hand-written json and add inconsistency across other VOs/objects/... Why is that? Are the json output from log statements consumed somewhere?
   > 
   > Can you club all such PRs into a single PR with separate/individual commits? They make it difficult to review and test individually and take a lot of time and resources from reviewers and BO/lab.
   
   @rhtyd
   
   Most of `toString()` implementations uses a simple hyphen (`-`) to separete values, but what `NicProfile[1-2-3-192.168.0.1-...]` means? Operators get confused and need to open the code to know what each value means. Moreover, if a string contains a hyphen, it will print as an separator and can confuse much more the operator.
   JSON was chosen because it is an easy format to read key-value pairs. `NicProfile {"id": 1, "vmId": 2, "reservationId": "3", "iPv4Address": "192.168.0.1", "broadcastUri": "..."}` tells much more than a simple `-----`.
   
   But that is not the focus of the PR. The focus is improve logging of these classes, as they do not supply an appropriate context to make the operation feasible.
   
   About join all this PRs in only one (separated in commits):
   Each PR is separted by context. Each one has their own peculiarity. Each peculiarity refactored is separated by commits. Join this all in a single commit will make it even more difficult to review, as there will be a lot of changes without pontual descriptions.


-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-2029)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34697 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t2029-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 0 have errors
   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.

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

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



[GitHub] [cloudstack] nvazquez commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Thanks for reviewing @rhtyd I see your points - opening for discussion @GutoVeronezi @sureshanaparti @rhtyd @GabrielBrascher I think we all agree this PR will need some extensive testing for regressions on VM lifecycle operations. A good sign is that smoke tests are returning good results. Given that we are close to cutting the RC, do you think we can complete all the required testing in time for 4.16?


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



[GitHub] [cloudstack] rhtyd commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Rekicked Travis.
   
   Let's do another round, merge if next round of smoketests pass.
   @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



[GitHub] [cloudstack] nvazquez commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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






-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 668


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



[GitHub] [cloudstack] rhtyd commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @DaanHoogland I've just shared my preference that is around optimising around reviewers' and lab resources time. One can get the same thing with a single PR with individual small commits that are easier to review (Github allows you to review each commit individually). The benefit of the latter is you save a lot of overhead in people's time in tracking/test, and on smoketest resources.
   
   @GutoVeronezi what you've raised is a valid issue, however, there was no discussion around a new standard/consistent format for people to follow for new code. For example and as a suggestion, instead of `NicProfile {"id": 1, "vmId": 2, "reservationId": "3", "iPv4Address": "192.168.0.1", "broadcastUri": "..."}`, something like `NicProfile [id=1, vmId=2, reservationId=3, iPv4Address=192.168.0.1, ...]` may read better, and developers don't need to work with handwritten json. This is merely an example - I'm not asking you to change the string format.
   
   Here something to think about:
   - You can present the enhancement cleanups under a standard initiative, start discussion on dev@, i.e. what should be a new standard on improving logs, what new developers, new feature/changes should follow. For example, Gabriel's suggestion on a standard logging pattern is great feedback.
   - A central utility/method that can help switch between formats of objects (json, csv, ...), if that is even feasible? Do a poc with your proposal.
   - Should we log internal DB IDs in logs? Is the cleanup PRs is replacing old bad logging style in a new way?
   - I've seen customers/users who pass logs through an aggregation system such as Splunk where they may have rules to make sense of things. Many old dinosaurs in the community like me are used to a pattern/style of reading logs and code i.e. have we considered what could/would break for existing users and developers, or their approach as their eyes are set looking for something they'll not adjust (written with humour - this point can be completely ignored we don't maintain backward compatibility for logs)
   
   On the issue of context, frankly, any PR that is more than a 100 lines of changes is not going to get a thorough review for each and every line by 2xreviewers, it's simply not feasible at least for me and people I know. Instead, we use smoketests and other means (manual QA, design/architecture review, consistency/pattern of changes, ...) to validate such PRs. I would club all such PRs around the same initiative which in this particular case is around logging/refactoring improvements. Splitting the work around the same initiative in 10 different PRs camouflaging under different `contexts` will just make the process a lot slower and you're essentially in some way telling the community (of dinosaurs :D) that you want them to work hard in their "free" time and you essentially don't care of the overhead it causes them on reviewing/tracking/testing/...


-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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






-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-2054)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 41239 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t2054-kvm-centos7.zip
   Smoke tests completed. 84 look OK, 5 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_add_primary_storage_disabled_host | `Error` | 0.63 | test_primary_storage.py
   test_01_primary_storage_nfs | `Error` | 0.12 | test_primary_storage.py
   ContextSuite context=TestStorageTags>:setup | `Error` | 0.21 | test_primary_storage.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 77.00 | test_kubernetes_clusters.py
   test_02_list_snapshots_with_removed_data_store | `Error` | 1.19 | test_snapshots.py
   test_01_secure_vm_migration | `Error` | 178.17 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 278.34 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 149.01 | test_vm_life_cycle.py
   test_08_migrate_vm | `Error` | 44.78 | test_vm_life_cycle.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 303.83 | test_hostha_kvm.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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @DaanHoogland 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] rhtyd edited a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on pull request #4966:
URL: https://github.com/apache/cloudstack/pull/4966#issuecomment-918837390


   We need test matrix on this @nvazquez
   General note: such a refactoring PR worries me closer to a RC, I've seen my share of blockers which get introduced due to refactoring changes as there are so many paths which may not be covered 100% by smoketests. I suggest the RM to discuss with the author and other reviewers and take the call whether we want the risk or if there's a risk.
   
   One specific review feedback - I didn't like is manual json strings, for example use a utility to convert specific field of a class to json? 
   
   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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] DaanHoogland merged pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-589)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 365677 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t589-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_create_list_domain_account_project.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_extra_config_data.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_direct_download.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_disk_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_network_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_vpc_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_migration.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_persistent_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_storage_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_deployment_planner.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 32 look OK, 56 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 38.92 | test_accounts.py
   test_DeleteDomain | `Error` | 34.34 | test_accounts.py
   test_DeleteDomain | `Error` | 53.88 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 27.72 | test_accounts.py
   test_forceDeleteDomain | `Error` | 33.97 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 33.59 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 31.76 | test_affinity_groups_projects.py
   test_DeployVmAffinityGroup | `Error` | 32.82 | test_affinity_groups.py
   test_DeployVmAntiAffinityGroup | `Error` | 6.25 | test_affinity_groups.py
   test_query_async_job_result | `Error` | 32.16 | test_async_job.py
   test_01_create_list_domain_account_project | `Error` | 55.75 | test_create_list_domain_account_project.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   ContextSuite context=TestDeployVirtioSCSIVM>:setup | `Error` | 0.00 | test_deploy_virtio_scsi_vm.py
   test_01_deploy_vm_with_extraconfig_throws_exception_kvm | `Error` | 2.28 | test_deploy_vm_extra_config_data.py
   test_02_deploy_vm_with_extraconfig_kvm | `Error` | 6.30 | test_deploy_vm_extra_config_data.py
   test_03_update_vm_with_extraconfig_kvm | `Error` | 5.26 | test_deploy_vm_extra_config_data.py
   test_list_vms_metrics | `Error` | 8.42 | test_metrics_api.py
   test_deploy_vm_from_iso | `Error` | 10.74 | test_deploy_vm_iso.py
   test_01_native_to_native_network_migration | `Error` | 6.86 | test_migration.py
   test_02_native_to_native_vpc_migration | `Error` | 2.77 | test_migration.py
   ContextSuite context=TestNetworkMigration>:teardown | `Error` | 3.90 | test_migration.py
   test_00_deploy_vm_root_resize | `Error` | 4.31 | test_deploy_vm_root_resize.py
   test_deployvm_firstfit | `Error` | 4.31 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 4.25 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 3.25 | test_deploy_vms_with_varied_deploymentplanners.py
   test_network_acl | `Error` | 1.14 | test_network_acl.py
   test_deployvm_userdata | `Error` | 4.36 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 3.31 | test_deploy_vm_with_userdata.py
   test_delete_account | `Error` | 19.96 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 3.21 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 4.27 | test_network.py
   test_deploy_vm_l2network | `Error` | 5.24 | test_network.py
   test_deploy_vm_l2network | `Error` | 6.30 | test_network.py
   test_l2network_restart | `Error` | 4.48 | test_network.py
   test_l2network_restart | `Error` | 5.55 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 6.64 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 10.39 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 5.32 | test_network.py
   test_reboot_router | `Error` | 4.76 | test_network.py
   test_releaseIP | `Error` | 3.74 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 7.46 | test_network.py
   test_01_deployVMInSharedNetwork | `Failure` | 31.69 | test_network.py
   test_02_verifyRouterIpAfterNetworkRestart | `Failure` | 34.66 | test_network.py
   test_03_destroySharedNetwork | `Failure` | 1.06 | test_network.py
   ContextSuite context=TestSharedNetwork>:teardown | `Error` | 2.13 | test_network.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_01_deploy_vm_from_direct_download_template_nfs_storage | `Error` | 8.33 | test_direct_download.py
   ContextSuite context=TestDirectDownloadTemplates>:teardown | `Error` | 2.14 | test_direct_download.py
   ContextSuite context=TestCreateDomainsDiskOffering>:teardown | `Error` | 60.17 | test_domain_disk_offerings.py
   ContextSuite context=TestDomainsDiskOfferings>:teardown | `Error` | 41.58 | test_domain_disk_offerings.py
   test_01_nic | `Error` | 4.08 | test_nic.py
   ContextSuite context=TestCreateDomainsNetworkOffering>:teardown | `Error` | 60.24 | test_domain_network_offerings.py
   ContextSuite context=TestDomainsNetworkOfferings>:teardown | `Error` | 41.67 | test_domain_network_offerings.py
   ContextSuite context=TestCreateDomainsServiceOffering>:teardown | `Error` | 43.94 | test_domain_service_offerings.py
   test_03_deploy_vm_domain_service_offering | `Error` | 5.13 | test_domain_service_offerings.py
   ContextSuite context=TestDomainsServiceOfferings>:teardown | `Error` | 70.53 | test_domain_service_offerings.py
   ContextSuite context=TestCreateDomainsVpcOffering>:teardown | `Error` | 53.12 | test_domain_vpc_offerings.py
   test_03_create_vpc_domain_vpc_offering | `Error` | 1.86 | test_domain_vpc_offerings.py
   ContextSuite context=TestDomainsVpcOfferings>:teardown | `Error` | 65.26 | test_domain_vpc_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Failure` | 2.40 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 2.40 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Failure` | 3.36 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Failure` | 3.37 | test_internal_lb.py
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 1.29 | test_kubernetes_clusters.py
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 1.15 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 1.21 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 46.27 | test_kubernetes_clusters.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 19.92 | test_multipleips_per_nic.py
   test_01_isolated_persistent_network | `Error` | 0.12 | test_persistent_network.py
   test_03_deploy_and_destroy_VM_and_verify_network_resources_persist | `Error` | 0.04 | test_persistent_network.py
   ContextSuite context=TestL2PersistentNetworks>:teardown | `Error` | 0.08 | test_persistent_network.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 3.98 | test_portforwardingrules.py
   test_01_add_primary_storage_disabled_host | `Error` | 35.67 | test_primary_storage.py
   test_01_primary_storage_nfs | `Error` | 0.12 | test_primary_storage.py
   ContextSuite context=TestStorageTags>:setup | `Error` | 0.22 | test_primary_storage.py
   test_01_vpc_privategw_acl | `Failure` | 2.59 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Failure` | 2.58 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 2.62 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 2.58 | test_privategw_acl.py
   ContextSuite context=TestCrossDomainAccountAdd>:teardown | `Error` | 65.44 | test_projects.py
   test_04_delete_domain_with_project | `Error` | 61.20 | test_projects.py
   test_09_project_suspend | `Error` | 4.38 | test_projects.py
   test_10_project_activation | `Error` | 4.28 | test_projects.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   test_01_sys_vm_start | `Failure` | 60.23 | test_secondary_storage.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.13 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_list_sec_storage_vm | `Failure` | 0.03 | test_ssvm.py
   test_02_list_cpvm_vm | `Failure` | 0.02 | test_ssvm.py
   test_03_ssvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_05_stop_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_06_stop_cpvm | `Failure` | 0.02 | test_ssvm.py
   test_07_reboot_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_08_reboot_cpvm | `Failure` | 0.02 | test_ssvm.py
   test_09_reboot_ssvm_forced | `Failure` | 0.02 | test_ssvm.py
   test_10_reboot_cpvm_forced | `Failure` | 0.02 | test_ssvm.py
   test_11_destroy_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_12_destroy_cpvm | `Error` | 1.11 | test_ssvm.py
   ContextSuite context=TestVMWareStoragePolicies>:setup | `Error` | 0.00 | test_storage_policy.py
   test_02_create_template_with_checksum_sha1 | `Error` | 65.37 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 65.39 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 65.39 | test_templates.py
   test_05_create_template_with_no_checksum | `Error` | 65.39 | test_templates.py
   test_02_deploy_vm_from_direct_download_template | `Error` | 1.24 | test_templates.py
   ContextSuite context=TestTemplates>:setup | `Error` | 10.66 | test_templates.py
   ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestNatRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestPublicIPUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestSnapshotUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVmUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVolumeUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVpnUsage>:setup | `Error` | 0.00 | test_usage.py
   test_01_deploy_vm_on_specific_host | `Error` | 9.40 | test_vm_deployment_planner.py
   test_02_deploy_vm_on_specific_cluster | `Error` | 10.44 | test_vm_deployment_planner.py
   test_03_deploy_vm_on_specific_pod | `Error` | 13.48 | test_vm_deployment_planner.py
   test_04_deploy_vm_on_host_override_pod_and_cluster | `Error` | 8.38 | test_vm_deployment_planner.py
   test_05_deploy_vm_on_cluster_override_pod | `Error` | 10.41 | test_vm_deployment_planner.py
   ContextSuite context=TestDeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   test_01_secure_vm_migration | `Error` | 1.08 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 0.07 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 1.07 | test_vm_life_cycle.py
   test_04_nonsecured_to_secured_vm_migration | `Error` | 0.07 | test_vm_life_cycle.py
   ContextSuite context=TestVMLifeCycle>:setup | `Error` | 1.78 | test_vm_life_cycle.py
   ContextSuite context=TestVmSnapshot>:setup | `Error` | 1.84 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
   test_disable_oobm_ha_state_ineligible | `Error` | 1511.58 | test_hostha_kvm.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 301.52 | test_hostha_kvm.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] DaanHoogland commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] GabrielBrascher commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Here follow my 2cents.
   
   #### 1 - Number of PRs
   
   @rhtyd raised an interesting point regartding the quantity of PRs.
   I agree that there are quite a lot of PRs, and that this can make our life harder to track, test, and review them all. However, I understand that many of them hold their particular contexts.
   
   I think that creating one issue to track them all would help on testing and reviewing them all. The idea would be to link any PR that fits the issue efforts on enhancing log messsages as well as keeping a track of each PR (merged, under review, draft, etc).
   
   #### 2 - Creating an log message pattern
   
   I have been running 4.16-SNAPSHOT already with a couple of these toString() methods. Personally I am happy to see log messages with standars and solid information. Maybe we can find better formatations than the _JSON like_. However, I find them interesting as it is a globally understood pattern for anyone at IT.
   
   The biggest plus in adopting `toString` is the standardtization on log messages. Nowadays we see all kind of ways of logging resources. Some logs present the name, or the ID, sometimes just the UUID, or combinations of them. It is about time to promote a standard (which would be possible mostly due to `toString`).


-- 
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 a change in pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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



##########
File path: api/src/main/java/com/cloud/vm/NicProfile.java
##########
@@ -430,16 +430,6 @@ public void deallocate() {
 
     @Override
     public String toString() {
-        return new StringBuilder("NicProfile[").append(id)
-                .append("-")
-                .append(vmId)
-                .append("-")
-                .append(reservationId)
-                .append("-")
-                .append(iPv4Address)
-                .append("-")
-                .append(broadcastUri)
-                .append("]")
-                .toString();
+        return String.format("NicProfile {\"id\": %s, \"vmId\": %s, \"reservationId\": \"%s\", \"iPv4Address\": \"%s\", \"broadcastUri\": \"%s\"}", id, vmId, reservationId, iPv4Address, broadcastUri);

Review comment:
       @GutoVeronezi why are you converting them into hand-written json?




-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4390,13 +4390,18 @@ public VMInstanceVO reConfigureVm(final String vmUuid, final ServiceOffering old
 
             VirtualMachine vm = retrieveVmFromJobOutcome(outcome, vmUuid, "reconfigureVm");
 
+            Object result = null;
             try {
-                retrieveResultFromJobOutcomeAndThrowExceptionIfNeeded(outcome);
-            } catch (InsufficientCapacityException ex) {
+                result = retrieveResultFromJobOutcomeAndThrowExceptionIfNeeded(outcome);
+
+                if (result == null) {
+                    return (VMInstanceVO)vm;
+                }
+            } catch (Exception ex) {

Review comment:
       come to think of it, maybe even add the result, parse the result and add some info? (or maybe later)




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



[GitHub] [cloudstack] nvazquez commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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



[GitHub] [cloudstack] rhtyd commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   We need test matrix on this @nvazquez
   General note: such a refactoring PR worries me closer to a RC, I've seen my share of blockers which get introduced due to refactoring changes as there are so many paths which may not be covered 100% by smoketests. I suggest the RM to discuss with the author and other reviewers and take the call whether we want the risk or if there's a risk. 
   
   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] nvazquez commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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



[GitHub] [cloudstack] GutoVeronezi closed pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   


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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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



[GitHub] [cloudstack] GutoVeronezi edited a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

Posted by GitBox <gi...@apache.org>.
GutoVeronezi edited a comment on pull request #4966:
URL: https://github.com/apache/cloudstack/pull/4966#issuecomment-837061808


   > @GutoVeronezi you've submitted many PRs which are the same class of PRs around log improvements and refactorings and changing the resource/vo/object `toString()` that returns hand-written json and add inconsistency across other VOs/objects/... Why is that? Are the json output from log statements consumed somewhere?
   > 
   > Can you club all such PRs into a single PR with separate/individual commits? They make it difficult to review and test individually and take a lot of time and resources from reviewers and BO/lab.
   
   @rhtyd
   
   Most of `toString()` implementations uses a simple hyphen (`-`) to separete values, but what `NicProfile[1-2-3-192.168.0.1-...]` means? Operators get confused and need to open the code to know what each value means. Moreover, if a string contains a hyphen, it will print as an separator and can confuse much more the operator.
   JSON was chosen because it is an easy format to read key-value pairs. `NicProfile {"id": 1, "vmId": 2, "reservationId": "3", "iPv4Address": "192.168.0.1", "broadcastUri": "..."}` tells much more than a simple `-----`.
   
   But that is not the focus of the PR. The focus is improve logging of these classes, as they do not supply an appropriate context to make the operation feasible.
   
   About join all this PRs in only one (separated in commits):
   Each PR is separted by context. Each one has their own peculiarity. Each peculiarity refactored is separated by commits. Join this all in a single commit will make it even more difficult to review, as there will be a lot of changes without punctual descriptions.


-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-589)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 365677 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t589-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_affinity_groups.py
   Intermittent failure detected: /marvin/tests/smoke/test_async_job.py
   Intermittent failure detected: /marvin/tests/smoke/test_create_list_domain_account_project.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_extra_config_data.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_direct_download.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_disk_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_network_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_domain_vpc_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_migration.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
   Intermittent failure detected: /marvin/tests/smoke/test_persistent_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
   Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_projects.py
   Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_storage_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_deployment_planner.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 32 look OK, 56 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestAddVmToSubDomain>:setup | `Error` | 38.92 | test_accounts.py
   test_DeleteDomain | `Error` | 34.34 | test_accounts.py
   test_DeleteDomain | `Error` | 53.88 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 27.72 | test_accounts.py
   test_forceDeleteDomain | `Error` | 33.97 | test_accounts.py
   test_01_user_remove_VM_running | `Error` | 33.59 | test_accounts.py
   test_DeployVmAntiAffinityGroup_in_project | `Error` | 31.76 | test_affinity_groups_projects.py
   test_DeployVmAffinityGroup | `Error` | 32.82 | test_affinity_groups.py
   test_DeployVmAntiAffinityGroup | `Error` | 6.25 | test_affinity_groups.py
   test_query_async_job_result | `Error` | 32.16 | test_async_job.py
   test_01_create_list_domain_account_project | `Error` | 55.75 | test_create_list_domain_account_project.py
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   ContextSuite context=TestDeployVirtioSCSIVM>:setup | `Error` | 0.00 | test_deploy_virtio_scsi_vm.py
   test_01_deploy_vm_with_extraconfig_throws_exception_kvm | `Error` | 2.28 | test_deploy_vm_extra_config_data.py
   test_02_deploy_vm_with_extraconfig_kvm | `Error` | 6.30 | test_deploy_vm_extra_config_data.py
   test_03_update_vm_with_extraconfig_kvm | `Error` | 5.26 | test_deploy_vm_extra_config_data.py
   test_list_vms_metrics | `Error` | 8.42 | test_metrics_api.py
   test_deploy_vm_from_iso | `Error` | 10.74 | test_deploy_vm_iso.py
   test_01_native_to_native_network_migration | `Error` | 6.86 | test_migration.py
   test_02_native_to_native_vpc_migration | `Error` | 2.77 | test_migration.py
   ContextSuite context=TestNetworkMigration>:teardown | `Error` | 3.90 | test_migration.py
   test_00_deploy_vm_root_resize | `Error` | 4.31 | test_deploy_vm_root_resize.py
   test_deployvm_firstfit | `Error` | 4.31 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userconcentrated | `Error` | 4.25 | test_deploy_vms_with_varied_deploymentplanners.py
   test_deployvm_userdispersing | `Error` | 3.25 | test_deploy_vms_with_varied_deploymentplanners.py
   test_network_acl | `Error` | 1.14 | test_network_acl.py
   test_deployvm_userdata | `Error` | 4.36 | test_deploy_vm_with_userdata.py
   test_deployvm_userdata_post | `Error` | 3.31 | test_deploy_vm_with_userdata.py
   test_delete_account | `Error` | 19.96 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 3.21 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 4.27 | test_network.py
   test_deploy_vm_l2network | `Error` | 5.24 | test_network.py
   test_deploy_vm_l2network | `Error` | 6.30 | test_network.py
   test_l2network_restart | `Error` | 4.48 | test_network.py
   test_l2network_restart | `Error` | 5.55 | test_network.py
   ContextSuite context=TestL2Networks>:teardown | `Error` | 6.64 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 10.39 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 5.32 | test_network.py
   test_reboot_router | `Error` | 4.76 | test_network.py
   test_releaseIP | `Error` | 3.74 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 7.46 | test_network.py
   test_01_deployVMInSharedNetwork | `Failure` | 31.69 | test_network.py
   test_02_verifyRouterIpAfterNetworkRestart | `Failure` | 34.66 | test_network.py
   test_03_destroySharedNetwork | `Failure` | 1.06 | test_network.py
   ContextSuite context=TestSharedNetwork>:teardown | `Error` | 2.13 | test_network.py
   ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
   test_01_deploy_vm_from_direct_download_template_nfs_storage | `Error` | 8.33 | test_direct_download.py
   ContextSuite context=TestDirectDownloadTemplates>:teardown | `Error` | 2.14 | test_direct_download.py
   ContextSuite context=TestCreateDomainsDiskOffering>:teardown | `Error` | 60.17 | test_domain_disk_offerings.py
   ContextSuite context=TestDomainsDiskOfferings>:teardown | `Error` | 41.58 | test_domain_disk_offerings.py
   test_01_nic | `Error` | 4.08 | test_nic.py
   ContextSuite context=TestCreateDomainsNetworkOffering>:teardown | `Error` | 60.24 | test_domain_network_offerings.py
   ContextSuite context=TestDomainsNetworkOfferings>:teardown | `Error` | 41.67 | test_domain_network_offerings.py
   ContextSuite context=TestCreateDomainsServiceOffering>:teardown | `Error` | 43.94 | test_domain_service_offerings.py
   test_03_deploy_vm_domain_service_offering | `Error` | 5.13 | test_domain_service_offerings.py
   ContextSuite context=TestDomainsServiceOfferings>:teardown | `Error` | 70.53 | test_domain_service_offerings.py
   ContextSuite context=TestCreateDomainsVpcOffering>:teardown | `Error` | 53.12 | test_domain_vpc_offerings.py
   test_03_create_vpc_domain_vpc_offering | `Error` | 1.86 | test_domain_vpc_offerings.py
   ContextSuite context=TestDomainsVpcOfferings>:teardown | `Error` | 65.26 | test_domain_vpc_offerings.py
   ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
   test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Failure` | 2.40 | test_internal_lb.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 2.40 | test_internal_lb.py
   test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Failure` | 3.36 | test_internal_lb.py
   test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Failure` | 3.37 | test_internal_lb.py
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 1.29 | test_kubernetes_clusters.py
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 1.15 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 1.21 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 1.16 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 46.27 | test_kubernetes_clusters.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 19.92 | test_multipleips_per_nic.py
   test_01_isolated_persistent_network | `Error` | 0.12 | test_persistent_network.py
   test_03_deploy_and_destroy_VM_and_verify_network_resources_persist | `Error` | 0.04 | test_persistent_network.py
   ContextSuite context=TestL2PersistentNetworks>:teardown | `Error` | 0.08 | test_persistent_network.py
   test_01_create_delete_portforwarding_fornonvpc | `Error` | 3.98 | test_portforwardingrules.py
   test_01_add_primary_storage_disabled_host | `Error` | 35.67 | test_primary_storage.py
   test_01_primary_storage_nfs | `Error` | 0.12 | test_primary_storage.py
   ContextSuite context=TestStorageTags>:setup | `Error` | 0.22 | test_primary_storage.py
   test_01_vpc_privategw_acl | `Failure` | 2.59 | test_privategw_acl.py
   test_02_vpc_privategw_static_routes | `Failure` | 2.58 | test_privategw_acl.py
   test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 2.62 | test_privategw_acl.py
   test_04_rvpc_privategw_static_routes | `Failure` | 2.58 | test_privategw_acl.py
   ContextSuite context=TestCrossDomainAccountAdd>:teardown | `Error` | 65.44 | test_projects.py
   test_04_delete_domain_with_project | `Error` | 61.20 | test_projects.py
   test_09_project_suspend | `Error` | 4.38 | test_projects.py
   test_10_project_activation | `Error` | 4.28 | test_projects.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   test_01_sys_vm_start | `Failure` | 60.23 | test_secondary_storage.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.13 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_list_sec_storage_vm | `Failure` | 0.03 | test_ssvm.py
   test_02_list_cpvm_vm | `Failure` | 0.02 | test_ssvm.py
   test_03_ssvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.02 | test_ssvm.py
   test_05_stop_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_06_stop_cpvm | `Failure` | 0.02 | test_ssvm.py
   test_07_reboot_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_08_reboot_cpvm | `Failure` | 0.02 | test_ssvm.py
   test_09_reboot_ssvm_forced | `Failure` | 0.02 | test_ssvm.py
   test_10_reboot_cpvm_forced | `Failure` | 0.02 | test_ssvm.py
   test_11_destroy_ssvm | `Failure` | 0.02 | test_ssvm.py
   test_12_destroy_cpvm | `Error` | 1.11 | test_ssvm.py
   ContextSuite context=TestVMWareStoragePolicies>:setup | `Error` | 0.00 | test_storage_policy.py
   test_02_create_template_with_checksum_sha1 | `Error` | 65.37 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 65.39 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 65.39 | test_templates.py
   test_05_create_template_with_no_checksum | `Error` | 65.39 | test_templates.py
   test_02_deploy_vm_from_direct_download_template | `Error` | 1.24 | test_templates.py
   ContextSuite context=TestTemplates>:setup | `Error` | 10.66 | test_templates.py
   ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestNatRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestPublicIPUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestSnapshotUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVmUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVolumeUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVpnUsage>:setup | `Error` | 0.00 | test_usage.py
   test_01_deploy_vm_on_specific_host | `Error` | 9.40 | test_vm_deployment_planner.py
   test_02_deploy_vm_on_specific_cluster | `Error` | 10.44 | test_vm_deployment_planner.py
   test_03_deploy_vm_on_specific_pod | `Error` | 13.48 | test_vm_deployment_planner.py
   test_04_deploy_vm_on_host_override_pod_and_cluster | `Error` | 8.38 | test_vm_deployment_planner.py
   test_05_deploy_vm_on_cluster_override_pod | `Error` | 10.41 | test_vm_deployment_planner.py
   ContextSuite context=TestDeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   test_01_secure_vm_migration | `Error` | 1.08 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 0.07 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 1.07 | test_vm_life_cycle.py
   test_04_nonsecured_to_secured_vm_migration | `Error` | 0.07 | test_vm_life_cycle.py
   ContextSuite context=TestVMLifeCycle>:setup | `Error` | 1.78 | test_vm_life_cycle.py
   ContextSuite context=TestVmSnapshot>:setup | `Error` | 1.84 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
   test_disable_oobm_ha_state_ineligible | `Error` | 1511.58 | test_hostha_kvm.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 301.52 | test_hostha_kvm.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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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


-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @DaanHoogland 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 removed a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @DaanHoogland 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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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


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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @rhtyd I changed `toString` implementation to use `ReflectionToStringBuilderUtils`.


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



[GitHub] [cloudstack] rhtyd closed pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   


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



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -1243,15 +1216,11 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
                                 throw new ConcurrentOperationException("Failed to deploy VM"+ vm.getUuid());
                             }
 
-                            // Update GPU device capacity
                             final GPUDeviceTO gpuDevice = startAnswer.getVirtualMachine().getGpuDevice();
                             if (gpuDevice != null) {
                                 _resourceMgr.updateGPUDetails(destHostId, gpuDevice.getGroupDetails());
                             }
 
-                            // Remove the information on whether it was a deploy vm request.The deployvm=true information

Review comment:
       Some comments may be kept




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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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


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



[GitHub] [cloudstack] GutoVeronezi closed pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4390,13 +4390,18 @@ public VMInstanceVO reConfigureVm(final String vmUuid, final ServiceOffering old
 
             VirtualMachine vm = retrieveVmFromJobOutcome(outcome, vmUuid, "reconfigureVm");
 
+            Object result = null;
             try {
-                retrieveResultFromJobOutcomeAndThrowExceptionIfNeeded(outcome);
-            } catch (InsufficientCapacityException ex) {
+                result = retrieveResultFromJobOutcomeAndThrowExceptionIfNeeded(outcome);
+
+                if (result == null) {
+                    return (VMInstanceVO)vm;
+                }
+            } catch (Exception ex) {

Review comment:
       @DaanHoogland makes sense in this case. I made the changes, thanks!




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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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] blueorangutan removed a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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






-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-2055)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 41000 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t2055-vmware-65u2.zip
   Smoke tests completed. 89 look OK, 0 have errors
   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.

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

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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @DaanHoogland @GabrielBrascher @nvazquez @rhtyd is there anything else to do?


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



[GitHub] [cloudstack] nvazquez commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @rhtyd makes sense, thanks. If no objections let's move this PR to the next milestone to prevent unexpected regressions close to the RC cut date


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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @nvazquez I had in mind the following logic to remove these codes:
   
   The `nicExpungeCommands` is instantied here:
   https://github.com/apache/cloudstack/blob/2a4c2c250696a154ccf8a7c8fff8604d893e9258/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L576
   
   There are 2 implementation for this method:
   - `HypervisorGuruBase`, which returns `null`:
   https://github.com/apache/cloudstack/blob/2a4c2c250696a154ccf8a7c8fff8604d893e9258/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java#L293-L296
   
   - `VMWareGuru`, which returns some commands:
   https://github.com/apache/cloudstack/blob/2a4c2c250696a154ccf8a7c8fff8604d893e9258/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java#L342-L358
   
   `nicExpungeCommands` is only used in the following block:
   https://github.com/apache/cloudstack/blob/2a4c2c250696a154ccf8a7c8fff8604d893e9258/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L628-L652
   
   However, only `HypervisorGuruBase` implements the method `finalizeExpunge`, which returns `null`:
   https://github.com/apache/cloudstack/blob/2a4c2c250696a154ccf8a7c8fff8604d893e9258/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java#L288-L291
   
   Therefore, the block where `nicExpungeCommands` is used is never called, therefore it's not needed. 
   
   If I'm missing something, please let me know.


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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


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



[GitHub] [cloudstack] rhtyd commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 650


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



[GitHub] [cloudstack] nvazquez commented on a change in pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -572,19 +571,19 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti
 
         final HypervisorGuru hvGuru = _hvGuruMgr.getGuru(vm.getHypervisorType());
 
-        s_logger.debug("Cleaning up NICS");
-        final List<Command> nicExpungeCommands = hvGuru.finalizeExpungeNics(vm, profile.getNics());

Review comment:
       Why are we removing these commands? Checked the Vmware guru may need `UnregisterNicCommand` processing




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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-1408)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 61774 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t1408-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Smoke tests completed. 85 look OK, 4 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3606.74 | test_kubernetes_clusters.py
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 3603.67 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.06 | 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` | 90.84 | test_kubernetes_clusters.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 348.66 | test_routers_network_ops.py
   test_04_nonsecured_to_secured_vm_migration | `Error` | 158.48 | test_vm_life_cycle.py
   test_10_attachAndDetach_iso | `Failure` | 816.74 | test_vm_life_cycle.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 473.58 | test_vpc_redundant.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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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] GabrielBrascher edited a comment on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

Posted by GitBox <gi...@apache.org>.
GabrielBrascher edited a comment on pull request #4966:
URL: https://github.com/apache/cloudstack/pull/4966#issuecomment-837209817


   Here follow my 2cents.
   
   #### 1 - Number of PRs
   
   @rhtyd raised an interesting point regartding the quantity of PRs.
   I agree that there are quite a lot of PRs, and that this can make our life harder to track, test, and review them all. However, I understand that many of them hold their particular contexts.
   
   I think that creating one issue to track these PRs would help on testing and reviewing them all. The idea would be to link any PR that fits this effort on enhancing log messages as well as keeping a track of each PR (merged, under review, draft, etc).
   
   #### 2 - Creating an log message pattern
   
   I have been running 4.16-SNAPSHOT already with a couple of these toString() methods. Personally I am happy to see log messages with a pattern and holding solid information. Maybe we can find better formatations than the _JSON like_. However, I find them interesting as it is a globally understood pattern for anyone at IT.
   
   The biggest plus in adopting `toString` is the standardtization on log messages. Nowadays we see all kind of ways of logging resources. Some logs present the name, or the ID, sometimes just the UUID, or combinations of them. It is about time to promote a standard (which would be possible mostly due to `toString`).


-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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


-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-2573)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 29783 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t2573-kvm-centos7.zip
   Smoke tests completed. 91 look OK, 0 have errors
   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.

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

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



[GitHub] [cloudstack] nvazquez commented on a change in pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -618,43 +607,43 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti
 
         final VirtualMachineGuru guru = getVmGuru(vm);
         guru.finalizeExpunge(vm);
-        //remove the overcommit details from the uservm details
+
         userVmDetailsDao.removeDetails(vm.getId());
 
-        // Remove deploy as-is (if any)
         userVmDeployAsIsDetailsDao.removeDetails(vm.getId());
 
-        // send hypervisor-dependent commands before removing
-        final List<Command> finalizeExpungeCommands = hvGuru.finalizeExpunge(vm);

Review comment:
       Can you also explain why these are not needed?




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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Restart jenkins...


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Can anyone review this?


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



[GitHub] [cloudstack] sureshanaparti commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Hi @GutoVeronezi can you fix the conflicts.


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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @nvazquez @rhtyd does it make sense to move this to the milestone 4.16.1.0? it is not a new feature and was built for 4.16.


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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   Can anyone review this?


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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


-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-2053)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40902 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t2053-xenserver-71.zip
   Smoke tests completed. 89 look OK, 0 have errors
   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.

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

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



[GitHub] [cloudstack] GutoVeronezi commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   As @nvazquez and me discussed offline, the block should be called, which characterize https://github.com/apache/cloudstack/pull/4966#issuecomment-897615227 as a bug. I created issue #5308 reporting this bug  and will undo the code remotion.


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   <b>Trillian test result (tid-1392)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 33992 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4966-t1392-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_nic.py
   Smoke tests completed. 88 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_nic | `Error` | 140.10 | test_nic.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



[GitHub] [cloudstack] nvazquez commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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






-- 
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 #4966: Refactor few process of VirtualMachineManagerImpl and improve logs

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


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

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

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