You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Ashutosh Kelkar <as...@clogeny.com> on 2013/08/01 08:20:00 UTC

Re: Review Request 12934: Tests for egress firewall rules for advance zone

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

(Updated Aug. 1, 2013, 6:19 a.m.)


Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.


Changes
-------

As per discussion with Jaypal on GTM added following Tests.
1) Tests to cover egress_policy = true/false
2) Skipping tests for egress_policy = true. We can relabel these test once we get confirmed response from QA and Jayapal.


Repository: cloudstack-git


Description
-------

Tests for egress firewall rules for advance zone.


Diffs (updated)
-----

  test/integration/component/test_egress_fw_rules.py PRE-CREATION 

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


Testing
-------


Thanks,

Ashutosh Kelkar


Re: Review Request 12934: Tests for egress firewall rules for advance zone

Posted by Jayapal Reddy <ja...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12934/#review24701
-----------------------------------------------------------

Ship it!


Ship It!

- Jayapal Reddy


On Aug. 6, 2013, 4:38 a.m., Ashutosh Kelkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12934/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2013, 4:38 a.m.)
> 
> 
> Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Tests for egress firewall rules for advance zone.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_egress_fw_rules.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/12934/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashutosh Kelkar
> 
>


Re: Review Request 12934: Tests for egress firewall rules for advance zone

Posted by Prasanna Santhanam <ts...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12934/#review24709
-----------------------------------------------------------

Ship it!


Applied on master and 4.2 and some minor changes.
Please watch the runs for failures/errors from the next run onwards:
http://jenkins.buildacloud.org/view/cloudstack-qa/job/test-regression-matrix/




- Prasanna Santhanam


On Aug. 6, 2013, 4:38 a.m., Ashutosh Kelkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12934/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2013, 4:38 a.m.)
> 
> 
> Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Tests for egress firewall rules for advance zone.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_egress_fw_rules.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/12934/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashutosh Kelkar
> 
>


Re: Review Request 12934: Tests for egress firewall rules for advance zone

Posted by Ashutosh Kelkar <as...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12934/
-----------------------------------------------------------

(Updated Aug. 6, 2013, 4:38 a.m.)


Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.


Changes
-------

Removed hard coded value of VLAN ID from network service data.
White space cleanup.


Repository: cloudstack-git


Description
-------

Tests for egress firewall rules for advance zone.


Diffs (updated)
-----

  test/integration/component/test_egress_fw_rules.py PRE-CREATION 

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


Testing
-------


Thanks,

Ashutosh Kelkar


Re: Review Request 12934: Tests for egress firewall rules for advance zone

Posted by Ashutosh Kelkar <as...@clogeny.com>.

> On Aug. 5, 2013, 9:55 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 27
> > <https://reviews.apache.org/r/12934/diff/5/?file=336941#file336941line27>
> >
> >     remove the white space (red colour).
> >     Please apply the patch in your local and make sure there is no warning.
> >     # git apply patchName.patch

Removed the trailing white space.


> On Aug. 5, 2013, 9:55 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 108
> > <https://reviews.apache.org/r/12934/diff/5/?file=336941#file336941line108>
> >
> >     do we need specify vlan here ?

No need to specify the VLAN ID here.


> On Aug. 5, 2013, 9:55 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 130
> > <https://reviews.apache.org/r/12934/diff/5/?file=336941#file336941line130>
> >
> >     Hard coding vlan may not work for others setups.
> >     If specify vlan is set then query the free vlan id

Removed VLAN ID value from service data.


- Ashutosh


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


On Aug. 1, 2013, 6:19 a.m., Ashutosh Kelkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12934/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2013, 6:19 a.m.)
> 
> 
> Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Tests for egress firewall rules for advance zone.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_egress_fw_rules.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/12934/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashutosh Kelkar
> 
>


Re: Review Request 12934: Tests for egress firewall rules for advance zone

Posted by Jayapal Reddy <ja...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12934/#review24640
-----------------------------------------------------------



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48746>

    remove the white space (red colour).
    Please apply the patch in your local and make sure there is no warning.
    # git apply patchName.patch



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48747>

    do we need specify vlan here ?



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48748>

    Hard coding vlan may not work for others setups.
    If specify vlan is set then query the free vlan id


- Jayapal Reddy


On Aug. 1, 2013, 6:19 a.m., Ashutosh Kelkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12934/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2013, 6:19 a.m.)
> 
> 
> Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Tests for egress firewall rules for advance zone.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_egress_fw_rules.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/12934/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashutosh Kelkar
> 
>


Re: Review Request 12934: Tests for egress firewall rules for advance zone

Posted by Prasanna Santhanam <ts...@apache.org>.

> On Aug. 2, 2013, 10:29 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 55
> > <https://reviews.apache.org/r/12934/diff/4/?file=332296#file332296line55>
> >
> >     I appreciate your effort in correcting the test cases.
> >     There are few comments from my side. Please fix those.
> >     Also the script is failed to run.Also look into this. The below exception is thrown
> >     
> >     
> >     integration.component.test_egress_fw_rules.log_test_exceptions ... ERROR
> >     
> >     ======================================================================
> >     ERROR: integration.component.test_egress_fw_rules.log_test_exceptions
> >     ----------------------------------------------------------------------
> >     Traceback (most recent call last):
> >       File "/Library/Python/2.7/site-packages/nose/case.py", line 197, in runTest
> >         self.test(*self.arg)
> >     TypeError: log_test_exceptions() takes exactly 1 argument (0 given)
> >
> 
> Ashutosh Kelkar wrote:
>     I am able to run the script without any issue.
>     python2.7 -m marvin.deployAndRun -c  $TESTPATH/run.cfg -d $TESTPATH -r /var/log/cs4.2.x-result.log -t /var/log/cs4.2.x-client.log -l &
>     testpid=$!
>     tail --pid $testpid -f /var/log/cs4.2.x-result.log /var/log/cs4.2.x-client.log
>     Let me know if you are still facing issues running the tests.

The reason why this won't run using nosetests is that the testrunner in nose needs the testnames to start with the test_ prefix. When you wrap the decorator over each test the name of the method changes. Check the test_assign_vm.py where I made the fix. Since you are using the logger decorator often why not also move it to the utils/common module?


- Prasanna


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


On Aug. 1, 2013, 6:19 a.m., Ashutosh Kelkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12934/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2013, 6:19 a.m.)
> 
> 
> Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Tests for egress firewall rules for advance zone.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_egress_fw_rules.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/12934/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashutosh Kelkar
> 
>


Re: Review Request 12934: Tests for egress firewall rules for advance zone

Posted by Ashutosh Kelkar <as...@clogeny.com>.

> On Aug. 2, 2013, 10:29 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 676
> > <https://reviews.apache.org/r/12934/diff/4/?file=332296#file332296line676>
> >
> >     change the comment.
> >     Egress policy is true

Done.


> On Aug. 2, 2013, 10:29 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 679
> > <https://reviews.apache.org/r/12934/diff/4/?file=332296#file332296line679>
> >
> >     fix the comment.
> >     access to public network for tcp port 80 is blocked.

Done.


> On Aug. 2, 2013, 10:29 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 785
> > <https://reviews.apache.org/r/12934/diff/4/?file=332296#file332296line785>
> >
> >     Egress policy is not set to false, but comment says false. The above test case is already set to true

Done.


> On Aug. 2, 2013, 10:29 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 819
> > <https://reviews.apache.org/r/12934/diff/4/?file=332296#file332296line819>
> >
> >     Fix the comment, invalid cidr is passed to create the rule

Done.


> On Aug. 2, 2013, 10:29 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 837
> > <https://reviews.apache.org/r/12934/diff/4/?file=332296#file332296line837>
> >
> >     Invalid cidr is not passed to rule. fix the comment

Done.


> On Aug. 2, 2013, 10:29 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 856
> > <https://reviews.apache.org/r/12934/diff/4/?file=332296#file332296line856>
> >
> >     Fix the comment

Done


> On Aug. 2, 2013, 10:29 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 912
> > <https://reviews.apache.org/r/12934/diff/4/?file=332296#file332296line912>
> >
> >     Fix the comment

Done


> On Aug. 2, 2013, 10:29 a.m., Jayapal Reddy wrote:
> > test/integration/component/test_egress_fw_rules.py, line 55
> > <https://reviews.apache.org/r/12934/diff/4/?file=332296#file332296line55>
> >
> >     I appreciate your effort in correcting the test cases.
> >     There are few comments from my side. Please fix those.
> >     Also the script is failed to run.Also look into this. The below exception is thrown
> >     
> >     
> >     integration.component.test_egress_fw_rules.log_test_exceptions ... ERROR
> >     
> >     ======================================================================
> >     ERROR: integration.component.test_egress_fw_rules.log_test_exceptions
> >     ----------------------------------------------------------------------
> >     Traceback (most recent call last):
> >       File "/Library/Python/2.7/site-packages/nose/case.py", line 197, in runTest
> >         self.test(*self.arg)
> >     TypeError: log_test_exceptions() takes exactly 1 argument (0 given)
> >

I am able to run the script without any issue.
python2.7 -m marvin.deployAndRun -c  $TESTPATH/run.cfg -d $TESTPATH -r /var/log/cs4.2.x-result.log -t /var/log/cs4.2.x-client.log -l &
testpid=$!
tail --pid $testpid -f /var/log/cs4.2.x-result.log /var/log/cs4.2.x-client.log
Let me know if you are still facing issues running the tests.


- Ashutosh


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


On Aug. 1, 2013, 6:19 a.m., Ashutosh Kelkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12934/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2013, 6:19 a.m.)
> 
> 
> Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Tests for egress firewall rules for advance zone.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_egress_fw_rules.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/12934/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashutosh Kelkar
> 
>


Re: Review Request 12934: Tests for egress firewall rules for advance zone

Posted by Jayapal Reddy <ja...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12934/#review24517
-----------------------------------------------------------



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48526>

    I appreciate your effort in correcting the test cases.
    There are few comments from my side. Please fix those.
    Also the script is failed to run.Also look into this. The below exception is thrown
    
    
    integration.component.test_egress_fw_rules.log_test_exceptions ... ERROR
    
    ======================================================================
    ERROR: integration.component.test_egress_fw_rules.log_test_exceptions
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/Library/Python/2.7/site-packages/nose/case.py", line 197, in runTest
        self.test(*self.arg)
    TypeError: log_test_exceptions() takes exactly 1 argument (0 given)
    



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48519>

    change the comment.
    Egress policy is true



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48520>

    fix the comment.
    access to public network for tcp port 80 is blocked.



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48521>

    Egress policy is not set to false, but comment says false. The above test case is already set to true



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48522>

    Fix the comment, invalid cidr is passed to create the rule



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48523>

    Invalid cidr is not passed to rule. fix the comment



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48524>

    Fix the comment



test/integration/component/test_egress_fw_rules.py
<https://reviews.apache.org/r/12934/#comment48525>

    Fix the comment


- Jayapal Reddy


On Aug. 1, 2013, 6:19 a.m., Ashutosh Kelkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12934/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2013, 6:19 a.m.)
> 
> 
> Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Tests for egress firewall rules for advance zone.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_egress_fw_rules.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/12934/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashutosh Kelkar
> 
>


Re: Review Request 12934: Tests for egress firewall rules for advance zone

Posted by Ashutosh Kelkar <as...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12934/
-----------------------------------------------------------

(Updated Aug. 5, 2013, 4:33 a.m.)


Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam.


Changes
-------

code review changes.


Repository: cloudstack-git


Description
-------

Tests for egress firewall rules for advance zone.


Diffs (updated)
-----

  test/integration/component/test_egress_fw_rules.py PRE-CREATION 

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


Testing
-------


Thanks,

Ashutosh Kelkar