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 <ko...@citrix.com> on 2014/08/04 12:18:50 UTC

Re: Review Request 24152: CLOUDSTACK-7182: NPE while trying to deploy VMs in parallel in isolated network

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24152/
-----------------------------------------------------------

(Updated Aug. 4, 2014, 10:18 a.m.)


Review request for cloudstack, Alena Prokharchyk and Sheng Yang.


Changes
-------

Rebased with latest master


Bugs: CLOUDSTACK-7182
    https://issues.apache.org/jira/browse/CLOUDSTACK-7182


Repository: cloudstack-git


Description
-------

- Check to see if network is implemented changed from 'state == Implementing||Implemented' to 'state == Implemented'. The earlier check was a hack to prevent the issue described next.
- At the time of implementing network (using implementNetwork() method), if the VR needs to be deployed then it follows the same path of regular VM deployment. This leads to a nested call to implementNetwork() while preparing VR nics. This flow creates issues in dealing with network state transitions. The original call puts network in "Implementing" state and then the nested call again tries to put it into same state resulting in issues. In order to avoid it, implementNetwork() call for VR is replaced with below code.


Diffs (updated)
-----

  engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java cdca839 

Diff: https://reviews.apache.org/r/24152/diff/


Testing
-------

Ran isolated network tests using simulator.
Also ran the following VPC test using simulator:
1. created vpc, VR gets started as part of this
2. created network in the vpc
3. deployed vm in network created in step#2


Thanks,

Koushik Das


Re: Review Request 24152: CLOUDSTACK-7182: NPE while trying to deploy VMs in parallel in isolated network

Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24152/#review50853
-----------------------------------------------------------

Ship it!


master -> 8bc7ae695d621b67e1e5d1916a1852cad1f4dda0

- Koushik Das


On Aug. 5, 2014, 5:16 a.m., Koushik Das wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24152/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2014, 5:16 a.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-7182
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7182
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> - Check to see if network is implemented changed from 'state == Implementing||Implemented' to 'state == Implemented'. The earlier check was a hack to prevent the issue described next.
> - At the time of implementing network (using implementNetwork() method), if the VR needs to be deployed then it follows the same path of regular VM deployment. This leads to a nested call to implementNetwork() while preparing VR nics. This flow creates issues in dealing with network state transitions. The original call puts network in "Implementing" state and then the nested call again tries to put it into same state resulting in issues. In order to avoid it, implementNetwork() call for VR is replaced with below code.
> 
> 
> Diffs
> -----
> 
>   engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java cdca839 
> 
> Diff: https://reviews.apache.org/r/24152/diff/
> 
> 
> Testing
> -------
> 
> 1. Ran the network related tests under /test/integration/smoke folder using Simulator. Also the other smoke tests didn’t cause any new failures.
> 
> 2. Also ran the following VPC test using simulator:
> a. created vpc, VR gets started as part of this
> b. created network in the vpc
> c. deployed vm in network created in step#2
> 
> 3. Manually ran the following tests using simulator (for both regular VR and VPC VR):
> a. VR stop/start via API when the network is in Implemented state. Network implement shouldn’t be triggered in this case.
> Deployed VM, network gets implemented, stopped/started VR. Repeated this for VPC as well.
> b. VR start via API when network is Allocated state. It should trigger network implement process. This test case is a good one to test the chicken-egg problem – VR triggers network implement, and network implement in turn triggers the provider implementation – which is the virtual router itself.
> Deployed VM, network gets implemented, destroyed VM, after sometime VR gets stopped and network goes to allocated state. At this point started the VR. Also repeated same for VPC.
> 
> 4. Ran #3 using XS setup as well
> 
> 
> Thanks,
> 
> Koushik Das
> 
>


Re: Review Request 24152: CLOUDSTACK-7182: NPE while trying to deploy VMs in parallel in isolated network

Posted by Koushik Das <ko...@citrix.com>.

> On Aug. 14, 2014, 9:54 p.m., Alena Prokharchyk wrote:
> > Koushik, I did the code review and some additional testing for the process. Following scenarios that I've tested:
> > 
> > * Update guest network when the network is a) implemented b) not implemented.
> > * associate ip address list to account
> > * start previously stopped router when the network is a) implemented b) not implemented 
> > * create nic for vm from the network that is a) implement b) not implemented. 
> > * deploy multiple vms for the same network in parallel (tried for 4)
> > 
> > All test cases are tested on a real Xen setup
> > 
> > Can you change the access to the new method from public to private/protected, as its an internal method, and can't be called from other classes?
> > 
> > After that, please go ahead and commit your fix. Put myself as a reviewer.

Made the method as private.


- Koushik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24152/#review50653
-----------------------------------------------------------


On Aug. 5, 2014, 5:16 a.m., Koushik Das wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24152/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2014, 5:16 a.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-7182
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7182
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> - Check to see if network is implemented changed from 'state == Implementing||Implemented' to 'state == Implemented'. The earlier check was a hack to prevent the issue described next.
> - At the time of implementing network (using implementNetwork() method), if the VR needs to be deployed then it follows the same path of regular VM deployment. This leads to a nested call to implementNetwork() while preparing VR nics. This flow creates issues in dealing with network state transitions. The original call puts network in "Implementing" state and then the nested call again tries to put it into same state resulting in issues. In order to avoid it, implementNetwork() call for VR is replaced with below code.
> 
> 
> Diffs
> -----
> 
>   engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java cdca839 
> 
> Diff: https://reviews.apache.org/r/24152/diff/
> 
> 
> Testing
> -------
> 
> 1. Ran the network related tests under /test/integration/smoke folder using Simulator. Also the other smoke tests didn’t cause any new failures.
> 
> 2. Also ran the following VPC test using simulator:
> a. created vpc, VR gets started as part of this
> b. created network in the vpc
> c. deployed vm in network created in step#2
> 
> 3. Manually ran the following tests using simulator (for both regular VR and VPC VR):
> a. VR stop/start via API when the network is in Implemented state. Network implement shouldn’t be triggered in this case.
> Deployed VM, network gets implemented, stopped/started VR. Repeated this for VPC as well.
> b. VR start via API when network is Allocated state. It should trigger network implement process. This test case is a good one to test the chicken-egg problem – VR triggers network implement, and network implement in turn triggers the provider implementation – which is the virtual router itself.
> Deployed VM, network gets implemented, destroyed VM, after sometime VR gets stopped and network goes to allocated state. At this point started the VR. Also repeated same for VPC.
> 
> 4. Ran #3 using XS setup as well
> 
> 
> Thanks,
> 
> Koushik Das
> 
>


Re: Review Request 24152: CLOUDSTACK-7182: NPE while trying to deploy VMs in parallel in isolated network

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24152/#review50653
-----------------------------------------------------------


Koushik, I did the code review and some additional testing for the process. Following scenarios that I've tested:

* Update guest network when the network is a) implemented b) not implemented.
* associate ip address list to account
* start previously stopped router when the network is a) implemented b) not implemented 
* create nic for vm from the network that is a) implement b) not implemented. 
* deploy multiple vms for the same network in parallel (tried for 4)

All test cases are tested on a real Xen setup

Can you change the access to the new method from public to private/protected, as its an internal method, and can't be called from other classes?

After that, please go ahead and commit your fix. Put myself as a reviewer.

- Alena Prokharchyk


On Aug. 5, 2014, 5:16 a.m., Koushik Das wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24152/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2014, 5:16 a.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-7182
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7182
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> - Check to see if network is implemented changed from 'state == Implementing||Implemented' to 'state == Implemented'. The earlier check was a hack to prevent the issue described next.
> - At the time of implementing network (using implementNetwork() method), if the VR needs to be deployed then it follows the same path of regular VM deployment. This leads to a nested call to implementNetwork() while preparing VR nics. This flow creates issues in dealing with network state transitions. The original call puts network in "Implementing" state and then the nested call again tries to put it into same state resulting in issues. In order to avoid it, implementNetwork() call for VR is replaced with below code.
> 
> 
> Diffs
> -----
> 
>   engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java cdca839 
> 
> Diff: https://reviews.apache.org/r/24152/diff/
> 
> 
> Testing
> -------
> 
> 1. Ran the network related tests under /test/integration/smoke folder using Simulator. Also the other smoke tests didn’t cause any new failures.
> 
> 2. Also ran the following VPC test using simulator:
> a. created vpc, VR gets started as part of this
> b. created network in the vpc
> c. deployed vm in network created in step#2
> 
> 3. Manually ran the following tests using simulator (for both regular VR and VPC VR):
> a. VR stop/start via API when the network is in Implemented state. Network implement shouldn’t be triggered in this case.
> Deployed VM, network gets implemented, stopped/started VR. Repeated this for VPC as well.
> b. VR start via API when network is Allocated state. It should trigger network implement process. This test case is a good one to test the chicken-egg problem – VR triggers network implement, and network implement in turn triggers the provider implementation – which is the virtual router itself.
> Deployed VM, network gets implemented, destroyed VM, after sometime VR gets stopped and network goes to allocated state. At this point started the VR. Also repeated same for VPC.
> 
> 4. Ran #3 using XS setup as well
> 
> 
> Thanks,
> 
> Koushik Das
> 
>


Re: Review Request 24152: CLOUDSTACK-7182: NPE while trying to deploy VMs in parallel in isolated network

Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24152/
-----------------------------------------------------------

(Updated Aug. 5, 2014, 5:16 a.m.)


Review request for cloudstack, Alena Prokharchyk and Sheng Yang.


Changes
-------

Updated the testing done field


Bugs: CLOUDSTACK-7182
    https://issues.apache.org/jira/browse/CLOUDSTACK-7182


Repository: cloudstack-git


Description
-------

- Check to see if network is implemented changed from 'state == Implementing||Implemented' to 'state == Implemented'. The earlier check was a hack to prevent the issue described next.
- At the time of implementing network (using implementNetwork() method), if the VR needs to be deployed then it follows the same path of regular VM deployment. This leads to a nested call to implementNetwork() while preparing VR nics. This flow creates issues in dealing with network state transitions. The original call puts network in "Implementing" state and then the nested call again tries to put it into same state resulting in issues. In order to avoid it, implementNetwork() call for VR is replaced with below code.


Diffs
-----

  engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java cdca839 

Diff: https://reviews.apache.org/r/24152/diff/


Testing (updated)
-------

1. Ran the network related tests under /test/integration/smoke folder using Simulator. Also the other smoke tests didn’t cause any new failures.

2. Also ran the following VPC test using simulator:
a. created vpc, VR gets started as part of this
b. created network in the vpc
c. deployed vm in network created in step#2

3. Manually ran the following tests using simulator (for both regular VR and VPC VR):
a. VR stop/start via API when the network is in Implemented state. Network implement shouldn’t be triggered in this case.
Deployed VM, network gets implemented, stopped/started VR. Repeated this for VPC as well.
b. VR start via API when network is Allocated state. It should trigger network implement process. This test case is a good one to test the chicken-egg problem – VR triggers network implement, and network implement in turn triggers the provider implementation – which is the virtual router itself.
Deployed VM, network gets implemented, destroyed VM, after sometime VR gets stopped and network goes to allocated state. At this point started the VR. Also repeated same for VPC.

4. Ran #3 using XS setup as well


Thanks,

Koushik Das