You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by nathanejohnson <gi...@git.apache.org> on 2016/08/11 12:54:31 UTC

[GitHub] cloudstack pull request #1635: CLOUDSTACK-9451

GitHub user nathanejohnson opened a pull request:

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

    CLOUDSTACK-9451

    https://issues.apache.org/jira/browse/CLOUDSTACK-9451
    
    Re-doing against 4.8 since this is a bug, not a feature.

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

    $ git pull https://github.com/myENA/cloudstack feature/honor_force_stop_vm

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

    https://github.com/apache/cloudstack/pull/1635.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 #1635
    
----
commit d5471b94e7617565836c1460007cbf6afa4c12ec
Author: Nathan Johnson <nj...@ena.com>
Date:   2016-08-11T12:52:05Z

    CLOUDSTACK-9451
    
    https://issues.apache.org/jira/browse/CLOUDSTACK-9451
    
    Re-doing against 4.8 since this is a bug, not a feature.

----


---
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 #1635: CLOUDSTACK-9451

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

    https://github.com/apache/cloudstack/pull/1635#discussion_r76544594
  
    --- Diff: engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java ---
    @@ -115,6 +115,12 @@ String reserve(DeploymentPlanner plannerToUse, @BeanParam DeploymentPlan plan, E
         boolean stop(String caller) throws ResourceUnavailableException, CloudException;
     
         /**
    +     * Stop the virtual machine, optionally force
    +     *
    +     */
    +    boolean stop(String caller, boolean forced) throws ResourceUnavailableException, CloudException;
    --- End diff --
    
    [Flag arguments](http://martinfowler.com/bliki/FlagArgument.html) are a nasty code smell/anti-pattern.  Please consider changing method to something more descriptive with a flag argument such as ``forceStop(String caller)``.  The contrasting version of the method already exists on the interface so it would be a compatible change.


---
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 issue #1635: CLOUDSTACK-9451

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-78


---
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 issue #1635: CLOUDSTACK-9451

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    I'll let him comment on this, but If I recall, it had more to do with implementation of force stopping with KVM rather than the lack of the parameter being passing into the VM Manager.


---
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 issue #1635: CLOUDSTACK-9451

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @ProjectMoon Jeff, could you help review this, as I believe you were the first to point it out on the list.
    
    From talking to Nathan, it sounds like we're not sure KVM is actually affected by this bug. He was in the process of doing some digging. Regardless of that, it probably affects other hypervisors.
    



---
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 issue #1635: CLOUDSTACK-9451

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-203


---
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 issue #1635: CLOUDSTACK-9451

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    Packaging result: \u2716centos6 \u2714centos7 \u2714debian. JID-248


---
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 #1635: CLOUDSTACK-9451

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

    https://github.com/apache/cloudstack/pull/1635#discussion_r77279256
  
    --- Diff: engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java ---
    @@ -115,6 +115,12 @@ String reserve(DeploymentPlanner plannerToUse, @BeanParam DeploymentPlan plan, E
         boolean stop(String caller) throws ResourceUnavailableException, CloudException;
     
         /**
    +     * Stop the virtual machine, optionally force
    +     *
    +     */
    +    boolean stop(String caller, boolean forced) throws ResourceUnavailableException, CloudException;
    --- End diff --
    
    @rhtyd since this PR adds new methods, let's not carry it forward.  The implementation of ``forceStop`` would end up passing ``true`` for the ``forced`` flag down the stack, but at least we are not expanding an anti-pattern.


---
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 issue #1635: CLOUDSTACK-9451

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    Packaging result: \u2714centos6 \u2714centos7 \u2714debian repo: http://packages.shapeblue.com/cloudstack/pr/1635
    Job ID-95


---
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 #1635: CLOUDSTACK-9451

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

    https://github.com/apache/cloudstack/pull/1635#discussion_r76569173
  
    --- Diff: engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java ---
    @@ -115,6 +115,12 @@ String reserve(DeploymentPlanner plannerToUse, @BeanParam DeploymentPlan plan, E
         boolean stop(String caller) throws ResourceUnavailableException, CloudException;
     
         /**
    +     * Stop the virtual machine, optionally force
    +     *
    +     */
    +    boolean stop(String caller, boolean forced) throws ResourceUnavailableException, CloudException;
    --- End diff --
    
    @jburwell the `forced` flag has always existed, the `stopVirtualMachine` API accepts this and the query layer too has a method with this signature (https://github.com/apache/cloudstack/pull/1635/files#diff-0a1cd9df984252594918eec5acfed08cR3850) but it does not honour or pass this flag to the layers below, so effectively stopVM API with/without the `forced` flag has the same effect.


---
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 #1635: CLOUDSTACK-9451

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

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


---
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 issue #1635: CLOUDSTACK-9451

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @blueorangutan package


---
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 issue #1635: CLOUDSTACK-9451

Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @nathanejohnson I want to get this bug fix into 4.8.2.0.  In order to make it, we need to complete the following:
    
      * [] Resolve the question about the name of the method (``stop`` vs ``forcedStop``)
      * [] Amend the commit message to summarize the change and explain the reason/motivation for it
      * [] Code review LGTM
      * [] Test LGTM
    
    @rhtyd or I will provide the code review LGTM once we resolve the naming question.  Do you have someone who can run the test?  Also, is this change something we can check via a Marvin test case?


---
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 issue #1635: CLOUDSTACK-9451

Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    All hypervisors are affected by this, since the parameter is just not passed into the VM Manager. That means its effects don't work at all anywhere. :) I will try to review it to tomorrow.


---
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 issue #1635: CLOUDSTACK-9451

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


---
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 issue #1635: CLOUDSTACK-9451

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @nathanejohnson can you reply to @jburwell 's comments


---
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 issue #1635: CLOUDSTACK-9451

Posted by kiwiflyer <gi...@git.apache.org>.
Github user kiwiflyer commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    References: https://github.com/apache/cloudstack/pull/1632


---
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 issue #1635: CLOUDSTACK-9451

Posted by nathanejohnson <gi...@git.apache.org>.
Github user nathanejohnson commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @jburwell sorry I've been silent, I've been on vacation for a bit and I will be juggling plans this weekend as well.  I plan to update this PR soon, and as far as a Marvin test I am not aware of any current tests that would cover this.  I haven't forgotten about this but it might be a few days yet before I can dig in fully.


---
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 issue #1635: CLOUDSTACK-9451

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    Checked centos6 failed due to an intermittent unit test failure, we can ignore that for now.
    @blueorangutan test


---
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 issue #1635: CLOUDSTACK-9451

Posted by nathanejohnson <gi...@git.apache.org>.
Github user nathanejohnson commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    I did find one marvin test that does pass forced flag.  Here are the results.
    
    https://gist.github.com/nathanejohnson/c4d1ffb8b61b19a8105dcb5326794b15


---
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 issue #1635: CLOUDSTACK-9451

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


---
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 issue #1635: CLOUDSTACK-9451

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


---
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 issue #1635: CLOUDSTACK-9451

Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @nathanejohnson please amend your commit message to explain the change(s) made and their motivation?


---
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 issue #1635: CLOUDSTACK-9451

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @blueorangutan package


---
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 #1635: CLOUDSTACK-9451

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

    https://github.com/apache/cloudstack/pull/1635#discussion_r76547668
  
    --- Diff: engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java ---
    @@ -115,6 +115,12 @@ String reserve(DeploymentPlanner plannerToUse, @BeanParam DeploymentPlan plan, E
         boolean stop(String caller) throws ResourceUnavailableException, CloudException;
     
         /**
    +     * Stop the virtual machine, optionally force
    +     *
    +     */
    +    boolean stop(String caller, boolean forced) throws ResourceUnavailableException, CloudException;
    --- End diff --
    
    For better or worse, I was just trying to follow the lead of the actual method that does the work in question, namely advanceStop, and some of the methods called from advanceStop.  I have no issue changing the name to have force in it and skip the bool flag (lest Fowler think I'm a bad programmer), but I am not thinking that I'd like to change the signature of advanceStop or any other the other methods that eventually get called in the chain where there is a bool flag in it.  I think the much bigger issue with this bit of code is that the refactor that lead to this issue in the first place abstracted the call to advanceStop under so many layers that it made it really hard to notice that functionality was reduced in the first place, to the point where the author nor anyone reviewing caught it.  If I was to place serious energy into refactoring, I'd probably start with undoing a lot of that abstraction, but that would require me being familiar with a lot more of the codebase than I
  currently am.  Also, more immediately, I want to come up with a good test for this, but so far haven't been able to come up with something reliable / reproducible under kvm.


---
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 issue #1635: CLOUDSTACK-9451

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    <b>Trillian test result (tid-427)</b>
    Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
    Total time taken: 28223 seconds
    Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr1635-t427-kvm-centos7.zip
    Test completed. 41 look ok, 2 have error(s)
    
    
    Test | Result | Time (s) | Test File
    --- | --- | --- | ---
    test_05_rvpc_multi_tiers | `Failure` | 225.67 | test_vpc_redundant.py
    test_10_attachAndDetach_iso | `Failure` | 684.22 | test_vm_life_cycle.py
    test_01_vpc_site2site_vpn | Success | 161.39 | test_vpc_vpn.py
    test_01_vpc_remote_access_vpn | Success | 67.19 | test_vpc_vpn.py
    test_01_redundant_vpc_site2site_vpn | Success | 264.00 | test_vpc_vpn.py
    test_02_VPC_default_routes | Success | 271.42 | test_vpc_router_nics.py
    test_01_VPC_nics_after_destroy | Success | 572.64 | test_vpc_router_nics.py
    test_04_rvpc_network_garbage_collector_nics | Success | 1344.16 | test_vpc_redundant.py
    test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | Success | 576.07 | test_vpc_redundant.py
    test_02_redundant_VPC_default_routes | Success | 753.98 | test_vpc_redundant.py
    test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Success | 1333.54 | test_vpc_redundant.py
    test_09_delete_detached_volume | Success | 15.85 | test_volumes.py
    test_08_resize_volume | Success | 15.67 | test_volumes.py
    test_07_resize_fail | Success | 20.57 | test_volumes.py
    test_06_download_detached_volume | Success | 15.49 | test_volumes.py
    test_05_detach_volume | Success | 100.30 | test_volumes.py
    test_04_delete_attached_volume | Success | 10.42 | test_volumes.py
    test_03_download_attached_volume | Success | 15.48 | test_volumes.py
    test_02_attach_volume | Success | 73.83 | test_volumes.py
    test_01_create_volume | Success | 734.50 | test_volumes.py
    test_deploy_vm_multiple | Success | 285.29 | test_vm_life_cycle.py
    test_deploy_vm | Success | 0.03 | test_vm_life_cycle.py
    test_advZoneVirtualRouter | Success | 0.04 | test_vm_life_cycle.py
    test_09_expunge_vm | Success | 125.27 | test_vm_life_cycle.py
    test_08_migrate_vm | Success | 41.36 | test_vm_life_cycle.py
    test_07_restore_vm | Success | 0.22 | test_vm_life_cycle.py
    test_06_destroy_vm | Success | 126.11 | test_vm_life_cycle.py
    test_03_reboot_vm | Success | 126.36 | test_vm_life_cycle.py
    test_02_start_vm | Success | 10.25 | test_vm_life_cycle.py
    test_01_stop_vm | Success | 40.47 | test_vm_life_cycle.py
    test_CreateTemplateWithDuplicateName | Success | 61.25 | test_templates.py
    test_08_list_system_templates | Success | 0.04 | test_templates.py
    test_07_list_public_templates | Success | 0.04 | test_templates.py
    test_05_template_permissions | Success | 0.08 | test_templates.py
    test_04_extract_template | Success | 5.25 | test_templates.py
    test_03_delete_template | Success | 5.10 | test_templates.py
    test_02_edit_template | Success | 90.20 | test_templates.py
    test_01_create_template | Success | 40.62 | test_templates.py
    test_10_destroy_cpvm | Success | 161.95 | test_ssvm.py
    test_09_destroy_ssvm | Success | 163.64 | test_ssvm.py
    test_08_reboot_cpvm | Success | 101.75 | test_ssvm.py
    test_07_reboot_ssvm | Success | 133.77 | test_ssvm.py
    test_06_stop_cpvm | Success | 132.21 | test_ssvm.py
    test_05_stop_ssvm | Success | 133.95 | test_ssvm.py
    test_04_cpvm_internals | Success | 1.27 | test_ssvm.py
    test_03_ssvm_internals | Success | 3.65 | test_ssvm.py
    test_02_list_cpvm_vm | Success | 0.20 | test_ssvm.py
    test_01_list_sec_storage_vm | Success | 0.26 | test_ssvm.py
    test_01_snapshot_root_disk | Success | 11.40 | test_snapshots.py
    test_04_change_offering_small | Success | 205.01 | test_service_offerings.py
    test_03_delete_service_offering | Success | 0.04 | test_service_offerings.py
    test_02_edit_service_offering | Success | 0.06 | test_service_offerings.py
    test_01_create_service_offering | Success | 0.18 | test_service_offerings.py
    test_02_sys_template_ready | Success | 0.13 | test_secondary_storage.py
    test_01_sys_vm_start | Success | 0.20 | test_secondary_storage.py
    test_09_reboot_router | Success | 40.52 | test_routers.py
    test_08_start_router | Success | 35.43 | test_routers.py
    test_07_stop_router | Success | 10.20 | test_routers.py
    test_06_router_advanced | Success | 0.07 | test_routers.py
    test_05_router_basic | Success | 0.05 | test_routers.py
    test_04_restart_network_wo_cleanup | Success | 5.73 | test_routers.py
    test_03_restart_network_cleanup | Success | 70.83 | test_routers.py
    test_02_router_internal_adv | Success | 1.21 | test_routers.py
    test_01_router_internal_basic | Success | 0.74 | test_routers.py
    test_router_dhcphosts | Success | 310.98 | test_router_dhcphosts.py
    test_router_dhcp_opts | Success | 22.30 | test_router_dhcphosts.py
    test_01_updatevolumedetail | Success | 5.19 | test_resource_detail.py
    test_01_reset_vm_on_reboot | Success | 171.81 | test_reset_vm_on_reboot.py
    test_createRegion | Success | 0.04 | test_regions.py
    test_create_pvlan_network | Success | 5.33 | test_pvlan.py
    test_dedicatePublicIpRange | Success | 0.74 | test_public_ip_range.py
    test_04_rvpc_privategw_static_routes | Success | 694.24 | test_privategw_acl.py
    test_03_vpc_privategw_restart_vpc_cleanup | Success | 547.64 | test_privategw_acl.py
    test_02_vpc_privategw_static_routes | Success | 444.74 | test_privategw_acl.py
    test_01_vpc_privategw_acl | Success | 114.87 | test_privategw_acl.py
    test_01_primary_storage_nfs | Success | 35.97 | test_primary_storage.py
    test_createPortablePublicIPRange | Success | 15.20 | test_portable_publicip.py
    test_createPortablePublicIPAcquire | Success | 15.79 | test_portable_publicip.py
    test_isolate_network_password_server | Success | 90.07 | test_password_server.py
    test_UpdateStorageOverProvisioningFactor | Success | 0.43 | test_over_provisioning.py
    test_extendPhysicalNetworkVlan | Success | 15.45 | test_non_contigiousvlan.py
    test_01_nic | Success | 664.50 | test_nic.py
    test_releaseIP | Success | 290.52 | test_network.py
    test_reboot_router | Success | 493.01 | test_network.py
    test_public_ip_user_account | Success | 10.30 | test_network.py
    test_public_ip_admin_account | Success | 40.43 | test_network.py
    test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Success | 62.52 | test_network.py
    test_network_rules_acquired_public_ip_2_nat_rule | Success | 62.56 | test_network.py
    test_network_rules_acquired_public_ip_1_static_nat_rule | Success | 124.90 | test_network.py
    test_delete_account | Success | 360.91 | test_network.py
    test_02_port_fwd_on_non_src_nat | Success | 56.22 | test_network.py
    test_01_port_fwd_on_src_nat | Success | 142.23 | test_network.py
    test_nic_secondaryip_add_remove | Success | 277.49 | test_multipleips_per_nic.py
    login_test_saml_user | Success | 27.37 | test_login.py
    test_assign_and_removal_lb | Success | 135.31 | test_loadbalance.py
    test_02_create_lb_rule_non_nat | Success | 187.89 | test_loadbalance.py
    test_01_create_lb_rule_src_nat | Success | 188.38 | test_loadbalance.py
    test_07_list_default_iso | Success | 0.29 | test_iso.py
    test_05_iso_permissions | Success | 0.30 | test_iso.py
    test_04_extract_Iso | Success | 6.08 | test_iso.py
    test_03_delete_iso | Success | 95.13 | test_iso.py
    test_02_edit_iso | Success | 0.07 | test_iso.py
    test_01_create_iso | Success | 22.31 | test_iso.py
    test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | Success | 290.85 | test_internal_lb.py
    test_03_vpc_internallb_haproxy_stats_on_all_interfaces | Success | 230.04 | test_internal_lb.py
    test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | Success | 661.07 | test_internal_lb.py
    test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Success | 654.08 | test_internal_lb.py
    test_dedicateGuestVlanRange | Success | 10.37 | test_guest_vlan_range.py
    test_UpdateConfigParamWithScope | Success | 0.18 | test_global_settings.py
    test_04_create_fat_type_disk_offering | Success | 0.23 | test_disk_offerings.py
    test_03_delete_disk_offering | Success | 0.06 | test_disk_offerings.py
    test_02_edit_disk_offering | Success | 0.21 | test_disk_offerings.py
    test_02_create_sparse_type_disk_offering | Success | 0.18 | test_disk_offerings.py
    test_01_create_disk_offering | Success | 0.37 | test_disk_offerings.py
    test_deployvm_userdispersing | Success | 66.86 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userconcentrated | Success | 22.19 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_firstfit | Success | 137.26 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userdata_post | Success | 10.67 | test_deploy_vm_with_userdata.py
    test_deployvm_userdata | Success | 66.55 | test_deploy_vm_with_userdata.py
    test_02_deploy_vm_root_resize | Success | 7.01 | test_deploy_vm_root_resize.py
    test_01_deploy_vm_root_resize | Success | 8.28 | test_deploy_vm_root_resize.py
    test_00_deploy_vm_root_resize | Success | 311.02 | test_deploy_vm_root_resize.py
    test_deploy_vm_from_iso | Success | 245.86 | test_deploy_vm_iso.py
    test_DeployVmAntiAffinityGroup | Success | 204.47 | test_affinity_groups.py
    test_03_delete_vm_snapshots | Skipped | 0.00 | test_vm_snapshots.py
    test_02_revert_vm_snapshots | Skipped | 0.00 | test_vm_snapshots.py
    test_01_test_vm_volume_snapshot | Skipped | 0.00 | test_vm_snapshots.py
    test_01_create_vm_snapshots | Skipped | 0.00 | test_vm_snapshots.py
    test_06_copy_template | Skipped | 0.00 | test_templates.py
    test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
    test_01_primary_storage_iscsi | Skipped | 0.04 | test_primary_storage.py
    test_06_copy_iso | Skipped | 0.00 | test_iso.py
    test_deploy_vgpu_enabled_vm | Skipped | 0.01 | test_deploy_vgpu_enabled_vm.py



---
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 issue #1635: CLOUDSTACK-9451

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    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 issue #1635: CLOUDSTACK-9451

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @blueorangutan package


---
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 issue #1635: CLOUDSTACK-9451

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    Test LGTM, the two failures are intermittent and env related.


---
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 issue #1635: CLOUDSTACK-9451

Posted by nathanejohnson <gi...@git.apache.org>.
Github user nathanejohnson commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @rhtyd @jburwell  I believe the last commit should have addressed his concerns.


---
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 #1635: CLOUDSTACK-9451

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

    https://github.com/apache/cloudstack/pull/1635#discussion_r76550534
  
    --- Diff: engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java ---
    @@ -115,6 +115,12 @@ String reserve(DeploymentPlanner plannerToUse, @BeanParam DeploymentPlan plan, E
         boolean stop(String caller) throws ResourceUnavailableException, CloudException;
     
         /**
    +     * Stop the virtual machine, optionally force
    +     *
    +     */
    +    boolean stop(String caller, boolean forced) throws ResourceUnavailableException, CloudException;
    --- End diff --
    
    @nathanejohnson I wasn't thinking to change any of the existing code.  I am just interested in avoiding incurring any more flag argument technical debt.
    
    In terms of testing, this change is not hypervisor specific.  Therefore, a test case that utilizes the simulator might be a more fruitful path.  @rhtyd may have some thoughts as to how you could accomplish it.


---
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 issue #1635: CLOUDSTACK-9451

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1635
  
    @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


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