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/#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
--- 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
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
--- 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
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
--- 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
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
--- 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
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/#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
--- 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
On July 29, 2013, 12:32 p.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 374 https://reviews.apache.org/r/12934/diff/3/?file=330049#file330049line374 I think you are not clear with the default egress policy. - egress policy true All the guest network traffic to public n/w is allowed by default. Adding egress rule blocks only the rule specific traffic. -egress policy false By default guest network traffic to public n/w is block. if you add egress rule then only egress rule specific traffic is allowed. When egress policy is true, by default the guest network traffic is allow. public network should be reachable As per your last email, and discussion over GTM. I have put in the tests for egress_policy = true. Currently all those tests are skipped from test run. - Ashutosh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review24101 --- 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
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review24101 --- test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47940 I think you are not clear with the default egress policy. - egress policy true All the guest network traffic to public n/w is allowed by default. Adding egress rule blocks only the rule specific traffic. -egress policy false By default guest network traffic to public n/w is block. if you add egress rule then only egress rule specific traffic is allowed. When egress policy is true, by default the guest network traffic is allow. public network should be reachable test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47942 here ping google.com should success. test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47941 Please update all the comments in this file, network offering with egress policy . # deploy vm with network offering egress policy true test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47944 Please understand the basics and update the test cases. ping should be blocked. 100% packet loss test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47945 mention the network offering with egress policy=false test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47946 Here ping is success. I did not get why 100% packet loss ? test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47948 In egress CIDR is not destination/public CIDR. CIDR is guest network/source CIDR. Please change the comments test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47949 if 10.2.2.2 exists then ping will be success. You test case should be like. give CIDR 10.1.1.120/32 and try to access wget from the vm 10.1.1.100. Then ping should fail from the 10.1.1.100 and pass from the vm with ip 10.1.1.120 test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47950 The purpose of the test is not correct. The CIDR is source cidr, so testing public network is not needed. test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47951 wget will be success.Why we get failed here ? test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47953 here egress default policy is true. created egress rule for icmp , the icmp traffic is BLOCKED and other traffic is allowed. test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47954 connection to public should be allowed test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47952 do we need second vm here ? test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47955 here ping should fail here test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47956 ping should success here test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47957 Here you are passing valid CIDR. The comment should be updated to 'create egress rule with other than guest cidr' test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47958 remove this comment test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47959 Remove this comment - Jayapal Reddy On July 29, 2013, 4:57 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 29, 2013, 4:57 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
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 29, 2013, 4:57 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
Re: Review Request 12934: Tests for egress firewall rules for advance zone
On July 26, 2013, 12:28 p.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 370 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line370 Please add network offering details also here. #1. deploy VM using network offering with egress policy true Done. On July 26, 2013, 12:28 p.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 373 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line373 Created network offering with egress policy True. That means by default all the guest traffic is allowed. If you create egress rules (ex: icmp) then the icmp traffic is blocked. So #4. Public Network should be reachable from the VM Done. On July 26, 2013, 12:28 p.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 390 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line390 I gone through the your test cases. I think you bit confused on the egress default policy and rules . Please update you test cases and test case comments as per below. 1. Network offering with egress_policy = true. - By default guest network traffic is allowed. - Egress rules traffic will be blocked and other traffic is allowed Ex: if you create egress rule for icmp traffic then except icmp other traffic is allowed. - Rules with DROP target added. -A FW_EGRESS_RULES -p icmp -j DROP 2. Network offering with egress_policy = false - By default the guest network traffic is blocked. - Egress rule traffic is allowed. If you create egress rule with icmp protocol then except icmp other traffic is blocked. -Rules added with target ACCEPT. -A FW_EGRESS_RULES -p icmp -j ACCPT The CIDR in the egress rules is guest network cidr. The traffic allowed/blocked for guest network CIDR. CIDR is not Public/destination network cidr. Added test scenario for guest network access check. On July 26, 2013, 12:28 p.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 426 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line426 In egress the CIDR is source CIDR (guest network CIDR). If you don't mention the CIDR it will take the default guest network CIDR. Egress compares the source CIDR. Done, added test scenario for guest network access check. - Ashutosh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review23939 --- On July 29, 2013, 4:57 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 29, 2013, 4:57 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
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 26, 2013, 6:27 a.m.) Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam. Changes --- Code review changes: 1) Updated egress_policy in network_offering definition. 2) Added test for egress_policy=true Added test for DB table value check. Added test for RVR. 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
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review23920 --- test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47766 Missing clean up for account and domain. test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47767 Missing tearDown() test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47769 I think we already have alike mechanism for executing inside user VM? Check get_ssh_client. If it doesn't fit, we need some generic mechanism rather than one method for one test case. test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47771 You need to wait a while (e.g. 60 seconds) for mgmt server complete the updating of RvR status. - Sheng Yang On July 26, 2013, 6:27 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 26, 2013, 6:27 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
On July 25, 2013, 6:25 a.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 99 https://reviews.apache.org/r/12934/diff/1/?file=327722#file327722line99 Please check the below configurable option for egress firewall rules. https://issues.apache.org/jira/browse/CLOUDSTACK-1578 For the default offering egress is block. For the newly creating offering the egress default is allow and block the traffic for the created rules. So update the test cases for the above Added Test for this use case. On July 25, 2013, 6:25 a.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 100 https://reviews.apache.org/r/12934/diff/1/?file=327722#file327722line100 create two network offering one with 'egress_policy'= true and other is 'egress_policy'= false Add the test cases for both Updated the service data to cover egress policy. On July 25, 2013, 6:25 a.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 106 https://reviews.apache.org/r/12934/diff/1/?file=327722#file327722line106 Add 'egress_policy' entry in the network offering. Done On July 25, 2013, 6:25 a.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 121 https://reviews.apache.org/r/12934/diff/1/?file=327722#file327722line121 1. create a network offering without redundant router 2. You can have another offering/test case with redundant router Done On July 25, 2013, 6:25 a.m., Jayapal Reddy wrote: test/integration/component/test_egress_fw_rules.py, line 363 https://reviews.apache.org/r/12934/diff/1/?file=327722#file327722line363 Please refer https://cwiki.apache.org/confluence/display/CLOUDSTACK/Egress+firewall+rules+-+Ability+to+change+the+default update the test case accordingly Done - Ashutosh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review23820 --- On July 26, 2013, 6:27 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 26, 2013, 6:27 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
On July 26, 2013, 8:46 a.m., Sheng Yang wrote: test/integration/component/test_egress_fw_rules.py, line 253 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line253 I think we already have alike mechanism for executing inside user VM? Check get_ssh_client. If it doesn't fit, we need some generic mechanism rather than one method for one test case. Yes, that method works when VM is created with mode=advance. Here we don't want to create PF rule and setup public ip for a VM. we just want to test the egress rule and policy. In this case we have to take hopes to Hypervisor Host- Router- VM instance to reach out VM for checking the public network access. On July 26, 2013, 8:46 a.m., Sheng Yang wrote: test/integration/component/test_egress_fw_rules.py, line 619 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line619 You need to wait a while (e.g. 60 seconds) for mgmt server complete the updating of RvR status. VirtualMachine.Create() waits till VM is up and VM comes up only after Network is up leading RVR up. no need to put a wait here. On July 26, 2013, 8:46 a.m., Sheng Yang wrote: test/integration/component/test_egress_fw_rules.py, line 183 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line183 Missing tearDown() No, teamDown is not missing. it's there on line 352. On July 26, 2013, 8:46 a.m., Sheng Yang wrote: test/integration/component/test_egress_fw_rules.py, line 166 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line166 Missing clean up for account and domain. No, cleanup for account and domain is in teamDownClass. - Ashutosh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review23920 --- On July 26, 2013, 6:27 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 26, 2013, 6:27 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
On July 26, 2013, 8:46 a.m., Sheng Yang wrote: test/integration/component/test_egress_fw_rules.py, line 166 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line166 Missing clean up for account and domain. Ashutosh Kelkar wrote: No, cleanup for account and domain is in teamDownClass. I didn't saw adding account or domain last time, but I found it now. I must be tired… On July 26, 2013, 8:46 a.m., Sheng Yang wrote: test/integration/component/test_egress_fw_rules.py, line 183 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line183 Missing tearDown() Ashutosh Kelkar wrote: No, teamDown is not missing. it's there on line 352. Yes. On July 26, 2013, 8:46 a.m., Sheng Yang wrote: test/integration/component/test_egress_fw_rules.py, line 253 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line253 I think we already have alike mechanism for executing inside user VM? Check get_ssh_client. If it doesn't fit, we need some generic mechanism rather than one method for one test case. Ashutosh Kelkar wrote: Yes, that method works when VM is created with mode=advance. Here we don't want to create PF rule and setup public ip for a VM. we just want to test the egress rule and policy. In this case we have to take hopes to Hypervisor Host- Router- VM instance to reach out VM for checking the public network access. Got it. But still it should be a good idea to add it to generic mechanism rather than implement it for each test case. On July 26, 2013, 8:46 a.m., Sheng Yang wrote: test/integration/component/test_egress_fw_rules.py, line 619 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line619 You need to wait a while (e.g. 60 seconds) for mgmt server complete the updating of RvR status. Ashutosh Kelkar wrote: VirtualMachine.Create() waits till VM is up and VM comes up only after Network is up leading RVR up. no need to put a wait here. No, the detection of RvR state is an separate thread, it won't change as soon as the VR is up. So you need to wait for a while. But in fact the VM's start up would likely take more than more minute, so it should be fine. - Sheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review23920 --- On July 26, 2013, 6:27 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 26, 2013, 6:27 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
On July 26, 2013, 8:46 a.m., Sheng Yang wrote: test/integration/component/test_egress_fw_rules.py, line 253 https://reviews.apache.org/r/12934/diff/2/?file=328044#file328044line253 I think we already have alike mechanism for executing inside user VM? Check get_ssh_client. If it doesn't fit, we need some generic mechanism rather than one method for one test case. Ashutosh Kelkar wrote: Yes, that method works when VM is created with mode=advance. Here we don't want to create PF rule and setup public ip for a VM. we just want to test the egress rule and policy. In this case we have to take hopes to Hypervisor Host- Router- VM instance to reach out VM for checking the public network access. Sheng Yang wrote: Got it. But still it should be a good idea to add it to generic mechanism rather than implement it for each test case. Sure, I will add an enhancement ticket and will update the base.py with VirtualMachine.exec_script_on_user_vm(). It will be help full in future test case dev. - Ashutosh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review23920 --- On July 26, 2013, 6:27 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 26, 2013, 6:27 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
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review23939 --- test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47783 Please add network offering details also here. #1. deploy VM using network offering with egress policy true test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47782 Created network offering with egress policy True. That means by default all the guest traffic is allowed. If you create egress rules (ex: icmp) then the icmp traffic is blocked. So #4. Public Network should be reachable from the VM test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47784 I gone through the your test cases. I think you bit confused on the egress default policy and rules . Please update you test cases and test case comments as per below. 1. Network offering with egress_policy = true. - By default guest network traffic is allowed. - Egress rules traffic will be blocked and other traffic is allowed Ex: if you create egress rule for icmp traffic then except icmp other traffic is allowed. - Rules with DROP target added. -A FW_EGRESS_RULES -p icmp -j DROP 2. Network offering with egress_policy = false - By default the guest network traffic is blocked. - Egress rule traffic is allowed. If you create egress rule with icmp protocol then except icmp other traffic is blocked. -Rules added with target ACCEPT. -A FW_EGRESS_RULES -p icmp -j ACCPT The CIDR in the egress rules is guest network cidr. The traffic allowed/blocked for guest network CIDR. CIDR is not Public/destination network cidr. test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47785 In egress the CIDR is source CIDR (guest network CIDR). If you don't mention the CIDR it will take the default guest network CIDR. Egress compares the source CIDR. - Jayapal Reddy On July 26, 2013, 6:27 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 26, 2013, 6:27 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
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review23820 --- test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47659 Please check the below configurable option for egress firewall rules. https://issues.apache.org/jira/browse/CLOUDSTACK-1578 For the default offering egress is block. For the newly creating offering the egress default is allow and block the traffic for the created rules. So update the test cases for the above test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47663 create two network offering one with 'egress_policy'= true and other is 'egress_policy'= false Add the test cases for both test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47660 Add 'egress_policy' entry in the network offering. test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47658 1. create a network offering without redundant router 2. You can have another offering/test case with redundant router test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47661 1. deploy VM should ' deploy vm with network offering offering name test/integration/component/test_egress_fw_rules.py https://reviews.apache.org/r/12934/#comment47662 Please refer https://cwiki.apache.org/confluence/display/CLOUDSTACK/Egress+firewall+rules+-+Ability+to+change+the+default update the test case accordingly - Jayapal Reddy On July 25, 2013, 4:39 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 25, 2013, 4:39 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
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/#review23828 --- Review 12934 PASSED the build test The url of build cloudstack-master-with-patch #33 is : http://jenkins.cloudstack.org/job/cloudstack-master-with-patch/33/ - Jenkins Cloudstack.org On July 25, 2013, 4:39 a.m., Ashutosh Kelkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 25, 2013, 4:39 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
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/ --- Review request for cloudstack, Girish Shilamkar 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
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12934/ --- (Updated July 25, 2013, 4:39 a.m.) Review request for cloudstack, Girish Shilamkar, Jayapal Reddy, and Prasanna Santhanam. Changes --- Jayapal, need your help reviewing this buddy. 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
