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/06/23 06:01:09 UTC

[GitHub] [cloudstack] Pearl1594 opened a new pull request #5142: Fix NPE during removal of VM from Network

Pearl1594 opened a new pull request #5142:
URL: https://github.com/apache/cloudstack/pull/5142


   ### Description
   
   This PR fixes NPE noticed during removal of  VM from a network (KVM) - leading to test failures noticed in test_vpc_redundant.py
   ```
   2021-06-23 00:17:28,726 WARN  [o.a.c.e.o.NetworkOrchestrator] (API-Job-Executor-87:ctx-86b1bf56 job-3057 ctx-cee7aba3) (logid:355b434c) Unable to complete shutdown of the network elements due to element: VpcVirtualRouter
   java.lang.NullPointerException
   	at com.cloud.hypervisor.HypervisorGuruBase.toVirtualMachineTO(HypervisorGuruBase.java:183)
   	at com.cloud.hypervisor.KVMGuru.implement(KVMGuru.java:112)
   	at com.cloud.vm.VirtualMachineManagerImpl.orchestrateRemoveVmFromNetwork(VirtualMachineManagerImpl.java:4171)
   	at com.cloud.vm.VirtualMachineManagerImpl.removeVmFromNetwork(VirtualMachineManagerImpl.java:4156)
   	at com.cloud.network.router.VpcVirtualNetworkApplianceManagerImpl.removeVpcRouterFromGuestNetwork(VpcVirtualNetworkApplianceManagerImpl.java:207)
   	at jdk.internal.reflect.GeneratedMethodAccessor942.invoke(Unknown Source)
   	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   	at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:344)
   	at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:198)
   	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
   	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
   	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
   	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
   	at com.sun.proxy.$Proxy250.removeVpcRouterFromGuestNetwork(Unknown Source)
   	at com.cloud.network.element.VpcVirtualRouterElement.shutdown(VpcVirtualRouterElement.java:302)
   	at org.apache.cloudstack.engine.orchestration.NetworkOrchestrator.shutdownNetworkElementsAndResources(NetworkOrchestrator.java:2949)
   	at org.apache.cloudstack.engine.orchestration.NetworkOrchestrator.shutdownNetwork(NetworkOrchestrator.java:2856)
   	at org.apache.cloudstack.engine.orchestration.NetworkOrchestrator.destroyNetwork(NetworkOrchestrator.java:3038)
   	at com.cloud.network.NetworkServiceImpl.deleteNetwork(NetworkServiceImpl.java:2010)
   	at jdk.internal.reflect.GeneratedMethodAccessor682.invoke(Unknown Source)
   	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   	at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:344)
   	at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:198)
   	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
   	at org.apache.cloudstack.network.contrail.management.EventUtils$EventInterceptor.invoke(EventUtils.java:107)
   	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:175)
   	at com.cloud.event.ActionEventInterceptor.invoke(ActionEventInterceptor.java:51)
   	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:175)
   	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
   	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
   	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
   	at com.sun.proxy.$Proxy169.deleteNetwork(Unknown Source)
   	at org.apache.cloudstack.api.command.user.network.DeleteNetworkCmd.execute(DeleteNetworkCmd.java:80)
   	at com.cloud.api.ApiDispatcher.dispatch(ApiDispatcher.java:156)
   	at com.cloud.api.ApiAsyncJobDispatcher.runJob(ApiAsyncJobDispatcher.java:108)
   	at org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl$5.runInContext(AsyncJobManagerImpl.java:620)
   	at org.apache.cloudstack.managed.context.ManagedContextRunnable$1.run(ManagedContextRunnable.java:48)
   	at org.apache.cloudstack.managed.context.impl.DefaultManagedContext$1.call(DefaultManagedContext.java:55)
   	at org.apache.cloudstack.managed.context.impl.DefaultManagedContext.callWithContext(DefaultManagedContext.java:102)
   	at org.apache.cloudstack.managed.context.impl.DefaultManagedContext.runWithContext(DefaultManagedContext.java:52)
   	at org.apache.cloudstack.managed.context.ManagedContextRunnable.run(ManagedContextRunnable.java:45)
   	at org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl$5.run(AsyncJobManagerImpl.java:568)
   	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
   	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
   	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
   	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
   	at java.base/java.lang.Thread.run(Thread.java:829)
   2021-06-23 00:17:28,737 DEBUG [o.a.c.e.o.NetworkOrchestrator] (API-Job-Executor-87:ctx-86b1bf56 job-3057 ctx-cee7aba3) (logid:355b434c) Lock is released for network Ntwk[336|Guest|73] as a part of network shutdown
   2021-06-23 00:17:28,738 DEBUG [o.a.c.e.o.NetworkOrchestrator] (API-Job-Executor-87:ctx-86b1bf56 job-3057 ctx-cee7aba3) (logid:355b434c) Network is not not in the correct state to be destroyed: Shutdown
   
   ```
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [X] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [X] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   
   
   ### How Has This Been Tested?
   Issue identified due to failure in tests of test_vpc_redundant.py
   Prior fix:
   ![image](https://user-images.githubusercontent.com/10495417/123038950-eade4d00-d40e-11eb-975e-303cb41a9ceb.png)
   
   Post Fix:
   ```
   [root@ref-trl-1171-k-M7-pearl-dsilva-marvin ~]# nosetests --with-xunit --xunit-file=results.xml --with-marvin --marvin-config=/marvin/ref-trl-1171-k-M7-pearl-dsilva-advanced-cfg -s -a tags=xx /marvin/tests/smoke/test_vpc_redundant.py
   
   ==== Marvin Init Started ====
   
   === Marvin Parse Config Successful ===
   
   === Marvin Setting TestData Successful===
   
   ==== Log Folder Path: /marvin/MarvinLogs/Jun_23_2021_04_24_57_URR4YZ. All logs will be available here ====
   
   === Marvin Init Logging Successful===
   
   ==== Marvin Init Successful ====
   Creating a VPC offering..
   Enabling the VPC offering created
   Creating a VPC network in the account: test-TestVPCRedundancy-test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL-AM9T7D
   Starting test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL
   Create NetworkOffering
   
   === TestName: test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Status : SUCCESS ===
   
   === TestName: test_05_rvpc_multi_tiers | Status : SUCCESS ===
   ```
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
   


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5142: Fix NPE during removal of VM from Network

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


   <b>Trillian test result (tid-1189)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 94381 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5142-t1189-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_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.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_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 66 look OK, 22 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` | 3.96 | test_accounts.py
   test_forceDeleteDomain | `Failure` | 3.96 | test_accounts.py
   ContextSuite context=TestRemoveUserFromAccount>:setup | `Error` | 4.97 | test_accounts.py
   ContextSuite context=TestDomainsServiceOfferings>:setup | `Error` | 1514.50 | 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.37 | test_iso.py
   test_02_create_iso_with_checksum_sha256 | `Error` | 66.38 | test_iso.py
   test_03_create_iso_with_checksum_md5 | `Error` | 66.36 | test_iso.py
   test_04_create_iso_with_no_checksum | `Error` | 66.38 | test_iso.py
   test_04_extract_Iso | `Failure` | 128.37 | 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.44 | test_metrics_api.py
   test_list_vms_metrics | `Error` | 0.14 | test_metrics_api.py
   ContextSuite context=TestDeployVMFromISO>:setup | `Error` | 0.00 | test_deploy_vm_iso.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
   test_delete_account | `Error` | 1511.35 | 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.17 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 3.31 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 1.09 | test_network.py
   test_reboot_router | `Failure` | 0.04 | test_network.py
   test_releaseIP | `Error` | 0.41 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 0.46 | test_network.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.80 | test_kubernetes_supported_versions.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 1511.26 | test_multipleips_per_nic.py
   ContextSuite context=TestNestedVirtualization>:setup | `Error` | 0.00 | test_nested_virtualization.py
   ContextSuite context=TestPortForwardingRules>:setup | `Error` | 0.00 | test_portforwardingrules.py
   ContextSuite context=TestPrivateGwACL>:setup | `Error` | 0.00 | test_privategw_acl.py
   test_02_redundant_VPC_default_routes | `Failure` | 1149.21 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 521.46 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 497.60 | 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] Pearl1594 commented on pull request #5142: Fix NPE during removal of VM from Network

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


   @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] Pearl1594 commented on pull request #5142: Fix NPE during removal of VM from Network

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


   > code lgtm
   > 
   > ps: it looks not be related to trillian test failures.
   
   You may be right @weizhouapache, however, I did come across these failures while running the tests and they seemed to be failing during cleanup - however they sometimes also fail to due failure to SSH into instance.


-- 
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] Pearl1594 commented on pull request #5142: Fix NPE during removal of VM from Network

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


   @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] Pearl1594 commented on pull request #5142: Fix NPE during removal of VM from Network

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


   @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 #5142: Fix NPE during removal of VM from Network

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


   @Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


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

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

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #5142: Fix NPE during removal of VM from Network

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



##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -179,9 +179,13 @@ protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) {
         ServiceOffering offering = _serviceOfferingDao.findById(vmProfile.getId(), vmProfile.getServiceOfferingId());
         VirtualMachine vm = vmProfile.getVirtualMachine();
         HostVO host = hostDao.findById(vm.getHostId());
+        boolean divideMemoryByOverprovisioning = true;
+        boolean divideCpuByOverprovisioning = true;
 
-        boolean divideMemoryByOverprovisioning = VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor.valueIn(host.getClusterId());
-        boolean divideCpuByOverprovisioning = VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor.valueIn(host.getClusterId());
+        if (host != null) {

Review comment:
       host.getClusterId() is not referred, only passed. the check should be up to the called method and the method constructing the host object (i think!)




-- 
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 #5142: Fix NPE during removal of VM from Network

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


   <b>Trillian test result (tid-1197)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 32975 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5142-t1197-kvm-centos7.zip
   Smoke tests completed. 88 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


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

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

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

[GitHub] [cloudstack] Pearl1594 commented on pull request #5142: Fix NPE during removal of VM from Network

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


   @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] weizhouapache commented on pull request #5142: Fix NPE during removal of VM from Network

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


   code lgtm
   
   ps: it looks not be related to trillian test failures.


-- 
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] Pearl1594 commented on a change in pull request #5142: Fix NPE during removal of VM from Network

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



##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -179,9 +179,13 @@ protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) {
         ServiceOffering offering = _serviceOfferingDao.findById(vmProfile.getId(), vmProfile.getServiceOfferingId());
         VirtualMachine vm = vmProfile.getVirtualMachine();
         HostVO host = hostDao.findById(vm.getHostId());
+        boolean divideMemoryByOverprovisioning = true;
+        boolean divideCpuByOverprovisioning = true;
 
-        boolean divideMemoryByOverprovisioning = VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor.valueIn(host.getClusterId());
-        boolean divideCpuByOverprovisioning = VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor.valueIn(host.getClusterId());
+        if (host != null) {

Review comment:
       @rhtyd  As @DaanHoogland mentioned, the check for validating if the passed argument is null is done by valueIn() (In ConfigKey class) hence there's no need for a check at this point.




-- 
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 #5142: Fix NPE during removal of VM from Network

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


   <b>Trillian test result (tid-1078)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43952 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5142-t1078-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 88 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5142: Fix NPE during removal of VM from Network

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


   @Pearl1594 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] Pearl1594 closed pull request #5142: Fix NPE during removal of VM from Network

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


   


-- 
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 #5142: Fix NPE during removal of VM from Network

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


   <b>Trillian test result (tid-1057)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 57889 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5142-t1057-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_password_server.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_snapshots.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_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 82 look OK, 6 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_add_primary_storage_disabled_host | `Error` | 0.84 | test_primary_storage.py
   test_01_primary_storage_nfs | `Error` | 0.09 | test_primary_storage.py
   ContextSuite context=TestStorageTags>:setup | `Error` | 0.16 | test_primary_storage.py
   test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 575.12 | test_internal_lb.py
   test_02_list_snapshots_with_removed_data_store | `Error` | 1.15 | test_snapshots.py
   test_01_secure_vm_migration | `Error` | 202.86 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 265.53 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 140.59 | test_vm_life_cycle.py
   test_08_migrate_vm | `Error` | 44.73 | test_vm_life_cycle.py
   test_05_rvpc_multi_tiers | `Failure` | 492.13 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 492.14 | test_vpc_redundant.py
   test_hostha_kvm_host_degraded | `Error` | 694.15 | test_hostha_kvm.py
   test_hostha_kvm_host_fencing | `Error` | 683.55 | 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 #5142: Fix NPE during removal of VM from Network

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


   @Pearl1594 do you have a reproduction-/test scheme for us?


-- 
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 #5142: Fix NPE during removal of VM from Network

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


   @sureshanaparti 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 #5142: Fix NPE during removal of VM from Network

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


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


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5142: Fix NPE during removal of VM from Network

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


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


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

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

[GitHub] [cloudstack] weizhouapache commented on pull request #5142: Fix NPE during removal of VM from Network

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


   > > code lgtm
   > > ps: it looks not be related to trillian test failures.
   > 
   > You may be right @weizhouapache, however, I did come across these failures while running the tests and they seemed to be failing during cleanup - however they sometimes also fail to due failure to SSH into instance.
   
   @Pearl1594 there are same test failures with some prs for 4.15 which does not have this issue.


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

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

[GitHub] [cloudstack] DaanHoogland commented on pull request #5142: Fix NPE during removal of VM from Network

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


   > @DaanHoogland I noticed this exception mainly during the run of the test_vpc_redundant.py smoke test - however, it can be seen occurring at multiple instances on going through the management server logs of the test run. Another way to validate would be to:
   > 
   >     1. Create a VM
   > 
   >     2. Attach the VM to another network
   > 
   >     3. Shutdown the instance and then attempt removing one of the nics
   
   tnx (I've seen those too lately ;) )


-- 
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 #5142: Fix NPE during removal of VM from Network

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



##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -179,9 +179,13 @@ protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) {
         ServiceOffering offering = _serviceOfferingDao.findById(vmProfile.getId(), vmProfile.getServiceOfferingId());
         VirtualMachine vm = vmProfile.getVirtualMachine();
         HostVO host = hostDao.findById(vm.getHostId());
+        boolean divideMemoryByOverprovisioning = true;
+        boolean divideCpuByOverprovisioning = true;
 
-        boolean divideMemoryByOverprovisioning = VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor.valueIn(host.getClusterId());
-        boolean divideCpuByOverprovisioning = VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor.valueIn(host.getClusterId());
+        if (host != null) {

Review comment:
       nit - should we check both host != null and host.getClusterId != null? (that may not be necessary but for writing defensive code)?




-- 
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 #5142: Fix NPE during removal of VM from Network

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


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


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

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5142: Fix NPE during removal of VM from Network

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


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


-- 
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 #5142: Fix NPE during removal of VM from Network

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


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


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

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

[GitHub] [cloudstack] Pearl1594 commented on pull request #5142: Fix NPE during removal of VM from Network

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


   Got it @weizhouapache  - thanks. I'll update the PR description.


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

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

[GitHub] [cloudstack] sureshanaparti commented on pull request #5142: Fix NPE during removal of VM from Network

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


   @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] rhtyd merged pull request #5142: Fix NPE during removal of VM from Network

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


   


-- 
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] Pearl1594 commented on pull request #5142: Fix NPE during removal of VM from Network

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


   @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] Pearl1594 commented on pull request #5142: Fix NPE during removal of VM from Network

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


   @DaanHoogland I noticed this exception mainly during the run of the test_vpc_redundant.py smoke test - however, it can be seen occurring at multiple instances on going through the management server logs of the test run.  Another way to validate would be to:
   1. Create a VM
   2. Attach the VM to another network
   3. Shutdown the instance and then attempt removing one of the nics 


-- 
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 #5142: Fix NPE during removal of VM from Network

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



##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -179,9 +179,13 @@ protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) {
         ServiceOffering offering = _serviceOfferingDao.findById(vmProfile.getId(), vmProfile.getServiceOfferingId());
         VirtualMachine vm = vmProfile.getVirtualMachine();
         HostVO host = hostDao.findById(vm.getHostId());
+        boolean divideMemoryByOverprovisioning = true;
+        boolean divideCpuByOverprovisioning = true;
 
-        boolean divideMemoryByOverprovisioning = VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor.valueIn(host.getClusterId());
-        boolean divideCpuByOverprovisioning = VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor.valueIn(host.getClusterId());
+        if (host != null) {

Review comment:
       host.getClusterId() is not referred, only passed. the check should be up to the called method and the method constructing the host object (i think!)




-- 
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 #5142: Fix NPE during removal of VM from Network

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



##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -179,9 +179,13 @@ protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) {
         ServiceOffering offering = _serviceOfferingDao.findById(vmProfile.getId(), vmProfile.getServiceOfferingId());
         VirtualMachine vm = vmProfile.getVirtualMachine();
         HostVO host = hostDao.findById(vm.getHostId());
+        boolean divideMemoryByOverprovisioning = true;
+        boolean divideCpuByOverprovisioning = true;
 
-        boolean divideMemoryByOverprovisioning = VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor.valueIn(host.getClusterId());
-        boolean divideCpuByOverprovisioning = VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor.valueIn(host.getClusterId());
+        if (host != null) {

Review comment:
       nit - should we check both host != null and host.getClusterId != null? (that may not be necessary but for writing defensive code)?




-- 
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] Pearl1594 commented on a change in pull request #5142: Fix NPE during removal of VM from Network

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



##########
File path: server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java
##########
@@ -179,9 +179,13 @@ protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) {
         ServiceOffering offering = _serviceOfferingDao.findById(vmProfile.getId(), vmProfile.getServiceOfferingId());
         VirtualMachine vm = vmProfile.getVirtualMachine();
         HostVO host = hostDao.findById(vm.getHostId());
+        boolean divideMemoryByOverprovisioning = true;
+        boolean divideCpuByOverprovisioning = true;
 
-        boolean divideMemoryByOverprovisioning = VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor.valueIn(host.getClusterId());
-        boolean divideCpuByOverprovisioning = VmMinCpuSpeedEqualsCpuSpeedDividedByCpuOverprovisioningFactor.valueIn(host.getClusterId());
+        if (host != null) {

Review comment:
       @rhtyd  As @DaanHoogland mentioned, the check for validating if the passed argument is null is done by valueIn() (In ConfigKey class) hence there's no need for a check at this point.




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