You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Alex Brett <al...@citrix.com> on 2014/08/29 17:12:36 UTC

Review Request 25187: test_delete_account and test_releaseIP failing in advanced zone

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

Review request for cloudstack, Ashutosh Kelkar and Santhosh Edukulla.


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


Repository: cloudstack-git


Description
-------

CLOUDSTACK-4840 changed test_data.py to make the lbrule publicport be 22,
instead of 2222. In doing so, this caused the following tests to fail, as they
hit a problem where they tried to use port 22 for both the lbrule and for other
purposes:
integration.smoke.test_network.TestDeleteAccount.test_delete_account
integration.smoke.test_network.TestReleaseIP.test_releaseIP

The reason the change appears to have been made was that in
test_lb_secondary_ip.py, despite setting up the load balancer using lbrule, the
tests then used the SSH port from natrule to try and access the VM. By changing
lbrule to use port 22 (the same as natrule) this avoided the problem.

This patch updates test_lb_secondary_ip.py to use the SSH port in lbrule where
necessary to access the VMs, and reverts the change to test_data.py


Diffs
-----

  test/integration/component/test_lb_secondary_ip.py f54caa6 
  tools/marvin/marvin/config/test_data.py fca2442 

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


Testing
-------

Ran tests in test_lb_secondary_ip.py with changes in place, verified results equivalent to those seen on regular regression runs.
Ran test_delete_account and test_releaseIP to verify these now pass as expected.


Thanks,

Alex Brett


Re: Review Request 25187: test_delete_account and test_releaseIP failing in advanced zone

Posted by Chandan Purushothama <Ch...@citrix.com>.

> On Sept. 4, 2014, 8:24 p.m., Chandan Purushothama wrote:
> > Ship It!

I reviewed the patch. I read the entire test suite to understand the Use Cases. The Suite does refer to the wrong dictionary "nat_rule" instead of "lb_rule". Alex's patch should fix the issue.


- Chandan


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


On Sept. 3, 2014, 3:17 p.m., Alex Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25187/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2014, 3:17 p.m.)
> 
> 
> Review request for cloudstack, Ashutosh Kelkar, Santhosh Edukulla, and SrikanteswaraRao Talluri.
> 
> 
> Bugs: CLOUDSTACK-7448
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7448
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-4840 changed test_data.py to make the lbrule publicport be 22,
> instead of 2222. In doing so, this caused the following tests to fail, as they
> hit a problem where they tried to use port 22 for both the lbrule and for other
> purposes:
> integration.smoke.test_network.TestDeleteAccount.test_delete_account
> integration.smoke.test_network.TestReleaseIP.test_releaseIP
> 
> The reason the change appears to have been made was that in
> test_lb_secondary_ip.py, despite setting up the load balancer using lbrule, the
> tests then used the SSH port from natrule to try and access the VM. By changing
> lbrule to use port 22 (the same as natrule) this avoided the problem.
> 
> This patch updates test_lb_secondary_ip.py to use the SSH port in lbrule where
> necessary to access the VMs, and reverts the change to test_data.py
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_lb_secondary_ip.py f54caa6 
>   tools/marvin/marvin/config/test_data.py fca2442 
> 
> Diff: https://reviews.apache.org/r/25187/diff/
> 
> 
> Testing
> -------
> 
> Ran tests in test_lb_secondary_ip.py with changes in place, verified results equivalent to those seen on regular regression runs.
> Ran test_delete_account and test_releaseIP to verify these now pass as expected.
> 
> 
> Thanks,
> 
> Alex Brett
> 
>


Re: Review Request 25187: test_delete_account and test_releaseIP failing in advanced zone

Posted by Chandan Purushothama <Ch...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25187/#review52348
-----------------------------------------------------------

Ship it!


Ship It!

- Chandan Purushothama


On Sept. 3, 2014, 3:17 p.m., Alex Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25187/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2014, 3:17 p.m.)
> 
> 
> Review request for cloudstack, Ashutosh Kelkar, Santhosh Edukulla, and SrikanteswaraRao Talluri.
> 
> 
> Bugs: CLOUDSTACK-7448
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7448
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-4840 changed test_data.py to make the lbrule publicport be 22,
> instead of 2222. In doing so, this caused the following tests to fail, as they
> hit a problem where they tried to use port 22 for both the lbrule and for other
> purposes:
> integration.smoke.test_network.TestDeleteAccount.test_delete_account
> integration.smoke.test_network.TestReleaseIP.test_releaseIP
> 
> The reason the change appears to have been made was that in
> test_lb_secondary_ip.py, despite setting up the load balancer using lbrule, the
> tests then used the SSH port from natrule to try and access the VM. By changing
> lbrule to use port 22 (the same as natrule) this avoided the problem.
> 
> This patch updates test_lb_secondary_ip.py to use the SSH port in lbrule where
> necessary to access the VMs, and reverts the change to test_data.py
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_lb_secondary_ip.py f54caa6 
>   tools/marvin/marvin/config/test_data.py fca2442 
> 
> Diff: https://reviews.apache.org/r/25187/diff/
> 
> 
> Testing
> -------
> 
> Ran tests in test_lb_secondary_ip.py with changes in place, verified results equivalent to those seen on regular regression runs.
> Ran test_delete_account and test_releaseIP to verify these now pass as expected.
> 
> 
> Thanks,
> 
> Alex Brett
> 
>


Re: Review Request 25187: test_delete_account and test_releaseIP failing in advanced zone

Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25187/#review53510
-----------------------------------------------------------


Commit 06b96d1ccf85455587d5098402de14c2930328f0 in cloudstack's branch refs/heads/master

Please mark this as submitted.

- SrikanteswaraRao Talluri


On Sept. 3, 2014, 3:17 p.m., Alex Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25187/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2014, 3:17 p.m.)
> 
> 
> Review request for cloudstack, Ashutosh Kelkar, Santhosh Edukulla, and SrikanteswaraRao Talluri.
> 
> 
> Bugs: CLOUDSTACK-7448
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7448
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-4840 changed test_data.py to make the lbrule publicport be 22,
> instead of 2222. In doing so, this caused the following tests to fail, as they
> hit a problem where they tried to use port 22 for both the lbrule and for other
> purposes:
> integration.smoke.test_network.TestDeleteAccount.test_delete_account
> integration.smoke.test_network.TestReleaseIP.test_releaseIP
> 
> The reason the change appears to have been made was that in
> test_lb_secondary_ip.py, despite setting up the load balancer using lbrule, the
> tests then used the SSH port from natrule to try and access the VM. By changing
> lbrule to use port 22 (the same as natrule) this avoided the problem.
> 
> This patch updates test_lb_secondary_ip.py to use the SSH port in lbrule where
> necessary to access the VMs, and reverts the change to test_data.py
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_lb_secondary_ip.py f54caa6 
>   tools/marvin/marvin/config/test_data.py fca2442 
> 
> Diff: https://reviews.apache.org/r/25187/diff/
> 
> 
> Testing
> -------
> 
> Ran tests in test_lb_secondary_ip.py with changes in place, verified results equivalent to those seen on regular regression runs.
> Ran test_delete_account and test_releaseIP to verify these now pass as expected.
> 
> 
> Thanks,
> 
> Alex Brett
> 
>


Re: Review Request 25187: test_delete_account and test_releaseIP failing in advanced zone

Posted by Alex Brett <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25187/
-----------------------------------------------------------

(Updated Sept. 3, 2014, 3:17 p.m.)


Review request for cloudstack, Ashutosh Kelkar, Santhosh Edukulla, and SrikanteswaraRao Talluri.


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


Repository: cloudstack-git


Description
-------

CLOUDSTACK-4840 changed test_data.py to make the lbrule publicport be 22,
instead of 2222. In doing so, this caused the following tests to fail, as they
hit a problem where they tried to use port 22 for both the lbrule and for other
purposes:
integration.smoke.test_network.TestDeleteAccount.test_delete_account
integration.smoke.test_network.TestReleaseIP.test_releaseIP

The reason the change appears to have been made was that in
test_lb_secondary_ip.py, despite setting up the load balancer using lbrule, the
tests then used the SSH port from natrule to try and access the VM. By changing
lbrule to use port 22 (the same as natrule) this avoided the problem.

This patch updates test_lb_secondary_ip.py to use the SSH port in lbrule where
necessary to access the VMs, and reverts the change to test_data.py


Diffs
-----

  test/integration/component/test_lb_secondary_ip.py f54caa6 
  tools/marvin/marvin/config/test_data.py fca2442 

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


Testing
-------

Ran tests in test_lb_secondary_ip.py with changes in place, verified results equivalent to those seen on regular regression runs.
Ran test_delete_account and test_releaseIP to verify these now pass as expected.


Thanks,

Alex Brett