You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by koushik-das <gi...@git.apache.org> on 2015/12/09 10:52:17 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-9130: Make RebootCommand simil...
GitHub user koushik-das opened a pull request:
https://github.com/apache/cloudstack/pull/1200
CLOUDSTACK-9130: Make RebootCommand similar to start/stop/migrate age…
…nt commands w.r.t. "execute in sequence" flag
RebootCommand now behaves in the same way as start/stop/migrate agent commands w.r.t. to sequential/parallel execution.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/koushik-das/cloudstack CLOUDSTACK-9130
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1200.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 #1200
----
commit 5b3ffeb6cd087c6b4406eae3a8c3ec782cdb80d0
Author: Koushik Das <ko...@apache.org>
Date: 2015-12-09T09:49:10Z
CLOUDSTACK-9130: Make RebootCommand similar to start/stop/migrate agent commands w.r.t. "execute in sequence" flag
RebootCommand now behaves in the same way as start/stop/migrate agent commands w.r.t. to sequential/parallel execution.
----
---
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-9130: Make RebootCommand simil...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1200#discussion_r47190062
--- Diff: core/src/com/cloud/agent/api/RebootCommand.java ---
@@ -19,29 +19,25 @@
package com.cloud.agent.api;
-import com.cloud.vm.VirtualMachine;
-
public class RebootCommand extends Command {
String vmName;
+ protected boolean executeInSequence = false;
protected RebootCommand() {
}
- public RebootCommand(VirtualMachine vm) {
- vmName = vm.getInstanceName();
- }
-
- public RebootCommand(String vmName) {
+ public RebootCommand(String vmName, boolean executeInSequence) {
this.vmName = vmName;
+ this.executeInSequence = executeInSequence;
}
public String getVmName() {
- return vmName;
+ return this.vmName;
}
@Override
public boolean executeInSequence() {
- return true;
+ return this.executeInSequence;
--- End diff --
In StopCommand.java lot of places have 'this.' prefix. Based on that did changes in RebootCommand as well. I am not sure what the general guideline says in this regard.
---
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-9130: Make RebootCommand simil...
Posted by rajesh-battala <gi...@git.apache.org>.
Github user rajesh-battala commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-212842649
LGTM. :+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-9130: Make RebootCommand simil...
Posted by srinivas-gandikota <gi...@git.apache.org>.
Github user srinivas-gandikota commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-212781868
LGTM based on 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-9130: Make RebootCommand simil...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1200
---
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-9130: Make RebootCommand simil...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-173196336
@koushik-das
Is there any integration test that can cover this?
If so, we will run it and get this merged. Otherwise I can help writing one.
Ping @remibergsma @borisroman
Cheers,
Wilder
---
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-9130: Make RebootCommand simil...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-212903509
Thanks, I will merge...
---
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-9130: Make RebootCommand simil...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1200#discussion_r47093362
--- Diff: core/src/com/cloud/agent/api/RebootCommand.java ---
@@ -19,29 +19,25 @@
package com.cloud.agent.api;
-import com.cloud.vm.VirtualMachine;
-
public class RebootCommand extends Command {
String vmName;
+ protected boolean executeInSequence = false;
protected RebootCommand() {
}
- public RebootCommand(VirtualMachine vm) {
- vmName = vm.getInstanceName();
- }
-
- public RebootCommand(String vmName) {
+ public RebootCommand(String vmName, boolean executeInSequence) {
this.vmName = vmName;
+ this.executeInSequence = executeInSequence;
}
public String getVmName() {
- return vmName;
+ return this.vmName;
}
@Override
public boolean executeInSequence() {
- return true;
+ return this.executeInSequence;
--- End diff --
Why the 'this.' prefix? Although it works I thought it was the general idea that you should not use it when not needed
---
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-9130: Make RebootCommand simil...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-212870271
@swill This is ready for merge, has the required LGTMs
---
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-9130: Make RebootCommand simil...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-173955971
Ping @wilderrodrigues see answer above.
---
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-9130: Make RebootCommand simil...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-173474463
@remibergsma RebootCoomand is used by rebootVirtualMachine API. Refer to test_03_reboot_vm in smoke/test_vm_life_cycle.py. It is also used in rebootSystemVm API. Check test_07_reboot_ssvm and test_08_reboot_cpvm in smoke/test_ssvm.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 pull request: CLOUDSTACK-9130: Make RebootCommand simil...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-199189940
Need one more LGTM, anyone?
---
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-9130: Make RebootCommand simil...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-163862343
LGTM based on these tests:
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=true \
component/test_vpc_redundant.py \
component/test_routers_iptables_default_policy.py \
component/test_routers_network_ops.py \
component/test_vpc_router_nics.py \
smoke/test_loadbalance.py \
smoke/test_internal_lb.py \
smoke/test_ssvm.py \
smoke/test_network.py
```
Result:
```
Create a redundant VPC with two networks with two VMs in each network ... === TestName: test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Status : SUCCESS ===
ok
Create a redundant VPC with two networks with two VMs in each network and check default routes ... === TestName: test_02_redundant_VPC_default_routes | Status : SUCCESS ===
ok
Create a redundant VPC with two networks with two VMs in each network ... === TestName: test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | Status : SUCCESS ===
ok
Test iptables default INPUT/FORWARD policy on RouterVM ... === TestName: test_02_routervm_iptables_policies | Status : SUCCESS ===
ok
Test iptables default INPUT/FORWARD policies on VPC router ... === TestName: test_01_single_VPC_iptables_policies | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_01_isolate_network_FW_PF_default_routes_egress_true | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_02_isolate_network_FW_PF_default_routes_egress_false | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_03_RVR_Network_check_router_state | Status : SUCCESS ===
ok
Create a VPC with two networks with one VM in each network and test nics after destroy ... === TestName: test_01_VPC_nics_after_destroy | Status : SUCCESS ===
ok
Create a VPC with two networks with one VM in each network and test default routes ... === TestName: test_02_VPC_default_routes | Status : SUCCESS ===
ok
Check the password file in the Router VM ... === TestName: test_isolate_network_password_server | Status : SUCCESS ===
ok
Check that the /etc/dhcphosts.txt doesn't contain duplicate IPs ... === TestName: test_router_dhcphosts | Status : SUCCESS ===
ok
Test to create Load balancing rule with source NAT ... === TestName: test_01_create_lb_rule_src_nat | Status : SUCCESS ===
ok
Test to create Load balancing rule with non source NAT ... === TestName: test_02_create_lb_rule_non_nat | Status : SUCCESS ===
ok
Test for assign & removing load balancing rule ... === TestName: test_assign_and_removal_lb | Status : SUCCESS ===
ok
Test to verify access to loadbalancer haproxy admin stats page ... === TestName: test02_internallb_haproxy_stats_on_all_interfaces | Status : SUCCESS ===
ok
Test create, assign, remove of an Internal LB with roundrobin http traffic to 3 vm's ... === TestName: test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Status : SUCCESS ===
ok
Test SSVM Internals ... === TestName: test_03_ssvm_internals | Status : SUCCESS ===
ok
Test CPVM Internals ... === TestName: test_04_cpvm_internals | Status : SUCCESS ===
ok
Test stop SSVM ... === TestName: test_05_stop_ssvm | Status : SUCCESS ===
ok
Test stop CPVM ... === TestName: test_06_stop_cpvm | Status : SUCCESS ===
ok
Test reboot SSVM ... === TestName: test_07_reboot_ssvm | Status : SUCCESS ===
ok
Test reboot CPVM ... === TestName: test_08_reboot_cpvm | Status : SUCCESS ===
ok
Test destroy SSVM ... === TestName: test_09_destroy_ssvm | Status : SUCCESS ===
ok
Test destroy CPVM ... === TestName: test_10_destroy_cpvm | Status : SUCCESS ===
ok
Test Remote Access 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_01_vpc_privategw_acl (integration.smoke.test_privategw_acl.TestPrivateGwACL) ... === TestName: test_01_vpc_privategw_acl | Status : SUCCESS ===
ok
test_02_vpc_privategw_static_routes (integration.smoke.test_privategw_acl.TestPrivateGwACL) ... === TestName: test_02_vpc_privategw_static_routes | Status : SUCCESS ===
ok
test_03_rvpc_privategw_static_routes (integration.smoke.test_privategw_acl.TestPrivateGwACL) ... === TestName: test_03_rvpc_privategw_static_routes | Status : SUCCESS ===
ok
Test for port forwarding on source NAT ... === TestName: test_01_port_fwd_on_src_nat | Status : SUCCESS ===
ok
Test for port forwarding on non source NAT ... === TestName: test_02_port_fwd_on_non_src_nat | Status : SUCCESS ===
ok
Test for reboot router ... === TestName: test_reboot_router | Status : SUCCESS ===
ok
Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_1_static_nat_rule | Status : SUCCESS ===
ok
Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_2_nat_rule | Status : SUCCESS ===
ok
Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 38 tests in 20915.680s
OK
```
And:
```
nosetests --with-marvin --marvin-config=${marvinCfg} -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
```
Result:
```
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 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 ... === TestName: test_08_migrate_vm | Status : SUCCESS ===
ok
Test destroy(expunge) Virtual Machine ... === TestName: test_09_expunge_vm | 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 for delete account ... === TestName: test_delete_account | Status : SUCCESS ===
ok
Test for Associate/Disassociate public IP address for admin account ... === TestName: test_public_ip_admin_account | Status : SUCCESS ===
ok
Test for Associate/Disassociate public IP address for user account ... === TestName: test_public_ip_user_account | Status : SUCCESS ===
ok
Test for release public IP address ... === TestName: test_releaseIP | 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 41 tests in 8664.384s
OK
```
I didn't test the actual feature. This just shows it didn't break these 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 pull request: CLOUDSTACK-9130: Make RebootCommand simil...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-163166411
As part of this noticed that RebootRouterCommand is obsolete, will create a separate PR for removing that once this is accepted.
---
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.
---