You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by DaanHoogland <gi...@git.apache.org> on 2015/10/02 11:58:37 UTC

[GitHub] cloudstack pull request: CLOUDSTACK-8848

GitHub user DaanHoogland opened a pull request:

    https://github.com/apache/cloudstack/pull/909

    CLOUDSTACK-8848

    added a null guard to @resmo's #885 A unit test or two would be nice as well but as this is a blocker I want to get it to review asap.
    @koushik-das @wilderrodrigues @anshul1886 @karuturi @remibergsma you all commented on the original, please have a look. @bhaisaab welcome to comment as well.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/DaanHoogland/cloudstack RESMO-CLOUDSTACK-8848

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/909.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #909
    
----
commit 542880ae76c5e6eefbf61d42364e881495e6f3af
Author: Rene Moser <re...@apache.org>
Date:   2015-09-24T19:10:26Z

    CLOUDSTACK-8848: ensure power state is up to date when handling missing VMs in powerReport
    
    There 2 things which has been changed.
    
    * We look on power_state_update_time instead of update_time. Didn't make sense to me at all to look at update_time.
    * Due DB update optimisation, powerState will only be updated if < MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT. That is why we can not rely on these information unless we make sure these are up to date.

commit bde803ee98d3f343652a8486c5e5faad74b0ea11
Author: Daan Hoogland <da...@onecht.net>
Date:   2015-10-02T09:42:42Z

    CLOUDSTACK-8848: added null pointer guard to new public method

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145382427
  
    @karuturi @koushik-das You reviewed PR #885, @dahn has added one commit and made this PR ontop of the previous one. Would you be so kind to review it? Need one more review before we can merge (solves a blocker). Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/909


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/909#discussion_r41109279
  
    --- Diff: engine/orchestration/src/com/cloud/vm/VirtualMachinePowerStateSyncImpl.java ---
    @@ -111,7 +111,19 @@ private void processReport(long hostId, Map<Long, VirtualMachine.PowerState> tra
     
                 for (VMInstanceVO instance : vmsThatAreMissingReport) {
     
    -                Date vmStateUpdateTime = instance.getUpdateTime();
    +                // Make sure powerState is up to date for missing VMs
    +                try {
    +                    if (!_instanceDao.isPowerStateUpToDate(instance.getId())) {
    +                        s_logger.warn("Detected missing VM but power state is outdated, wait for another process report run for VM id: " + instance.getId());
    +                        _instanceDao.resetVmPowerStateTracking(instance.getId());
    +                        continue;
    +                    }
    +                } catch (CloudRuntimeException e) {
    +                    s_logger.warn("Checked for missing powerstate for none existing vm", e);
    --- End diff --
    
    Can you correct the sentence to "Checked for missing powerstate of a non existing vm"?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145324409
  
    Test router internal advanced zone ... === TestName: test_02_router_internal_adv | Status : SUCCESS ===
    ok
    Test restart network ... === TestName: test_03_restart_network_cleanup | Status : SUCCESS ===
    ok
    Test router basic setup ... === TestName: test_05_router_basic | Status : SUCCESS ===
    ok
    Test router advanced setup ... === TestName: test_06_router_advanced | Status : SUCCESS ===
    ok
    Test stop router ... === TestName: test_07_stop_router | Status : SUCCESS ===
    ok
    Test start router ... === TestName: test_08_start_router | Status : SUCCESS ===
    ok
    Test reboot router ... === TestName: test_09_reboot_router | Status : SUCCESS ===
    ok
    test_privategw_acl (integration.smoke.test_privategw_acl.TestPrivateGwACL) ... === TestName: test_privategw_acl | Status : SUCCESS ===
    ok
    Test reset virtual machine on reboot ... === TestName: test_01_reset_vm_on_reboot | Status : SUCCESS ===
    ok
    Test advanced zone virtual router ... === TestName: test_advZoneVirtualRouter | Status : SUCCESS ===
    ok
    Test Deploy Virtual Machine ... === TestName: test_deploy_vm | Status : SUCCESS ===
    ok
    Test Multiple Deploy Virtual Machine ... === TestName: test_deploy_vm_multiple | Status : SUCCESS ===
    ok
    Test Stop Virtual Machine ... === TestName: test_01_stop_vm | Status : SUCCESS ===
    ok
    Test Start Virtual Machine ... === TestName: test_02_start_vm | Status : SUCCESS ===
    ok
    Test Reboot Virtual Machine ... === TestName: test_03_reboot_vm | Status : SUCCESS ===
    ok
    Test destroy Virtual Machine ... === TestName: test_06_destroy_vm | Status : SUCCESS ===
    ok
    Test recover Virtual Machine ... === TestName: test_07_restore_vm | Status : SUCCESS ===
    ok
    Test migrate VM ... SKIP: At least two hosts should be present in the zone for migration
    Test destroy(expunge) Virtual Machine ... === TestName: test_09_expunge_vm | Status : SUCCESS ===
    ok
    Test VPN in VPC ... === TestName: test_vpc_remote_access_vpn | Status : SUCCESS ===
    ok
    Test VPN in VPC ... === TestName: test_vpc_site2site_vpn | Status : SUCCESS ===
    ok
    Test to create service offering ... === TestName: test_01_create_service_offering | Status : SUCCESS ===
    ok
    Test to update existing service offering ... === TestName: test_02_edit_service_offering | Status : SUCCESS ===
    ok
    Test to delete service offering ... === TestName: test_03_delete_service_offering | Status : SUCCESS ===
    ok
    Test create VPC offering ... === TestName: test_01_create_vpc_offering | Status : SUCCESS ===
    ok
    Test VPC offering without load balancing service ... === TestName: test_03_vpc_off_without_lb | Status : SUCCESS ===
    ok
    Test VPC offering without static NAT service ... === TestName: test_04_vpc_off_without_static_nat | Status : SUCCESS ===
    ok
    Test VPC offering without port forwarding service ... === TestName: test_05_vpc_off_without_pf | Status : SUCCESS ===
    ok
    Test VPC offering with invalid services ... === TestName: test_06_vpc_off_invalid_services | Status : SUCCESS ===
    ok
    Test update VPC offering ... === TestName: test_07_update_vpc_off | Status : SUCCESS ===
    ok
    Test list VPC offering ... === TestName: test_08_list_vpc_off | Status : SUCCESS ===
    ok
    test_09_create_redundant_vpc_offering (integration.component.test_vpc_offerings.TestVPCOffering) ... === TestName: test_09_create_redundant_vpc_offering | Status : SUCCESS ===
    ok
    Test start/stop of router after addition of one guest network ... === TestName: test_01_start_stop_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test reboot of router after addition of one guest network ... === TestName: test_02_reboot_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test to change service offering of router after addition of one guest network ... === TestName: test_04_chg_srv_off_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test destroy of router after addition of one guest network ... === TestName: test_05_destroy_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test to stop and start router after creation of VPC ... === TestName: test_01_stop_start_router_after_creating_vpc | Status : SUCCESS ===
    ok
    Test to reboot the router after creating a VPC ... === TestName: test_02_reboot_router_after_creating_vpc | Status : SUCCESS ===
    ok
    Tests to change service offering of the Router after ... === TestName: test_04_change_service_offerring_vpc | Status : SUCCESS ===
    ok
    Test to destroy the router after creating a VPC ... === TestName: test_05_destroy_router_after_creating_vpc | Status : SUCCESS ===
    ok
    
    ----------------------------------------------------------------------
    Ran 40 tests in 6361.029s
    
    OK (SKIP=1)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145037606
  
    @DaanHoogland Could you please add a bit more descriptive title?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145436050
  
    I will merge this once travis is done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145423925
  
    one minor comment. otherwise, LGTM (only did code review)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145381656
  
    Tested everything I tested in PR #885 again in this PR. @dahn already run the tests, I verified the functionality manually by destoying VMs out-of-band and see how it recovers. It works well.
    
    Logs:
    
    ```
    2015-10-04 19:07:06,168 DEBUG [c.c.v.VirtualMachinePowerStateSyncImpl] (AgentManager-Handler-5:null) Detected missing VM. host: 1, vm id: 4, power state: PowerReportMissing, last state update: 1443985566000
    2015-10-04 19:07:06,168 DEBUG [c.c.v.VirtualMachinePowerStateSyncImpl] (AgentManager-Handler-5:null) vm id: 4 - time since last state update(60167ms) has not passed graceful period yet
    2015-10-04 19:07:06,169 DEBUG [c.c.v.VirtualMachinePowerStateSyncImpl] (AgentManager-Handler-5:null) Detected missing VM. host: 1, vm id: 6, power state: PowerReportMissing, last state update: 1443985566000
    2015-10-04 19:07:06,169 DEBUG [c.c.v.VirtualMachinePowerStateSyncImpl] (AgentManager-Handler-5:null) vm id: 6 - time since last state update(60167ms) has not passed graceful period yet
    
    2015-10-04 19:08:06,542 INFO  [c.c.v.VirtualMachineManagerImpl] (AgentManager-Handler-10:null) VM i-2-6-VM is at Running and we received a power-off report while there is no pending jobs on it
    2015-10-04 19:08:06,542 INFO  [c.c.v.VirtualMachineManagerImpl] (AgentManager-Handler-10:null) Detected out-of-band stop of a HA enabled VM i-2-6-VM, will schedule restart
    2015-10-04 19:08:06,557 INFO  [c.c.h.HighAvailabilityManagerImpl] (AgentManager-Handler-10:null) Schedule vm for HA:  VM[User|i-2-6-VM]
    2015-10-04 19:08:06,557 DEBUG [c.c.v.VirtualMachinePowerStateSyncImpl] (AgentManager-Handler-10:null) Done with process of VM state report. host: 1
    2015-10-04 19:08:06,601 INFO  [c.c.h.HighAvailabilityManagerImpl] (HA-Worker-1:ctx-f0996892 work-2) Processing work HAWork[2-HA-6-Running-Investigating]
    2015-10-04 19:08:06,603 INFO  [c.c.h.HighAvailabilityManagerImpl] (HA-Worker-1:ctx-f0996892 work-2) HA on VM[User|i-2-6-VM]
    
    2015-10-04 19:09:07,022 DEBUG [c.c.v.VirtualMachineManagerImpl] (Work-Job-Executor-9:ctx-1278576b job-36/job-38 ctx-21e14d55) Start completed for VM VM[User|i-2-6-VM]
    
    2015-10-04 19:20:38,647 INFO  [c.c.v.VirtualMachineManagerImpl] (AgentManager-Handler-10:null) Detected out-of-band stop of a HA enabled VM s-2-VM, will schedule restart
    2015-10-04 19:20:38,838 INFO  [c.c.h.HighAvailabilityManagerImpl] (HA-Worker-2:ctx-379f80bc work-11) Processing work HAWork[11-HA-2-Running-Investigating]
    2015-10-04 19:20:38,840 INFO  [c.c.h.HighAvailabilityManagerImpl] (HA-Worker-2:ctx-379f80bc work-11) HA on VM[SecondaryStorageVm|s-2-VM]
    2015-10-04 19:20:38,841 INFO  [c.c.h.HighAvailabilityManagerImpl] (AgentManager-Handler-10:null) Schedule vm for HA:  VM[SecondaryStorageVm|s-2-VM]
    2015-10-04 19:20:38,891 INFO  [c.c.h.HighAvailabilityManagerImpl] (HA-Worker-2:ctx-379f80bc work-11) SimpleInvestigator found VM[SecondaryStorageVm|s-2-VM] to be alive? false
    2015-10-04 19:24:28,759 DEBUG [c.c.v.VirtualMachineManagerImpl] (Work-Job-Executor-12:ctx-49cc4b77 job-40/job-42 ctx-b7bd08fc) Start completed for VM VM[SecondaryStorageVm|s-2-VM]
    ```
    
    A router, a user instance and a system vm were tested and recovered fine.
    
    I make my LGTM of the previous one also count here: LGTM



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145369924
  
    ```bash
    nosetests --with-marvin --marvin-config=/data/shared/marvin/mct-zone1-kvm1.cfg -s -a \
    tags=advanced,required_hardware=false smoke/test_routers.py smoke/test_network_acl.py \
    smoke/test_privategw_acl.py smoke/test_reset_vm_on_reboot.py smoke/test_vm_life_cycle.py \
    smoke/test_vpc_vpn.py smoke/test_service_offerings.py component/test_vpc_offerings.py \
    component/test_vpc_routers.py
    ```
    ran with one skip and the rest success


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145324612
  
    one test skipped, that requires two hosts.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145434199
  
    commited, squashed and force-pushed the change requested by Rajani


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8848 ensure power state is up ...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/909#issuecomment-145154565
  
    Hi @DaanHoogland can you please run the same tests as we did on PR #885 and post output? Let me know if you need help.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---