> On Aug. 27, 2013, 6:02 p.m., venkata swamy babu budumuru wrote: > > test/integration/component/test_portable_ip.py, line 1274 > > <https://reviews.apache.org/r/13559/diff/1/?file=341135#file341135line1274> > > > > After delete, can you add a check that verifies the portable IP address > > is now marked as free in DB?
Checked this by verifying that "Listing this IP should throw exception if the IP does not exist" > On Aug. 27, 2013, 6:02 p.m., venkata swamy babu budumuru wrote: > > test/integration/component/test_portable_ip.py, line 318 > > <https://reviews.apache.org/r/13559/diff/1/?file=341135#file341135line318> > > > > In case of have a valid reason for keeping increment_cidr (or for that > > matter all the other helper functions), Is it a good a idea to consider > > moving it to common.py because I see these definitions in all the class of > > this suite. Added "get_portable_ip_range_services" function to common.py > On Aug. 27, 2013, 6:02 p.m., venkata swamy babu budumuru wrote: > > test/integration/component/test_portable_ip.py, line 168 > > <https://reviews.apache.org/r/13559/diff/1/?file=341135#file341135line168> > > > > It would be better if we prepare services dictionary for > > "portable_ip_range" manually on what range it should take. > > > > Current logic just increments the range by 1 and it expects the > > existing physical infra to be configured with that CIDR. This approach many > > not suite for everyone who fires this script. This is mandating the user of > > the scripts to have the next cidr of his public range to be configured and > > available for portable ip range testing. > > Gaurav Aradhye wrote: > will directly using "untagged" vlan from the list of vlans do? Can you > please explain how we can manually specify the range? As the range will be > different for every setup. Please correct me if I'm wrong. > > Thanks. Added common function to read the portable ip range values (startip, endip, netmask ,gateway, vlan) from the config and fill up the dictionary accordingly. This requires the config should be set properly before running the test cases. > On Aug. 27, 2013, 6:02 p.m., venkata swamy babu budumuru wrote: > > test/integration/component/test_portable_ip.py, line 776 > > <https://reviews.apache.org/r/13559/diff/1/?file=341135#file341135line776> > > > > Can you move service_offering, virtualmachine create operations to > > setupClass()? Service offering creation and virtualmachine creation is used in only one test case. Moving it to setupClass or keeping it in test case will have same effect. This code is related to only one particular test case, hence kept in test case. > On Aug. 27, 2013, 6:02 p.m., venkata swamy babu budumuru wrote: > > test/integration/component/test_portable_ip.py, line 185 > > <https://reviews.apache.org/r/13559/diff/1/?file=341135#file341135line185> > > > > There is no param / code that takes VLAN info in case if it has to be > > set. currently it taking/assuming a native vlan environment. we need make > > this flexible enough to consider the VLAN info. if we take the approach of > > filling the service dictionary for portable_ip_range then we don't have to > > worry about this. If vlan is mentioned in the config, then it will be passed, else it will be by default set to untagged. - Gaurav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13559/#review25613 ----------------------------------------------------------- On Sept. 12, 2013, 12:28 p.m., Gaurav Aradhye wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13559/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2013, 12:28 p.m.) > > > Review request for cloudstack, Murali Reddy, venkata swamy babu budumuru, > and Prasanna Santhanam. > > > Repository: cloudstack-git > > > Description > ------- > > Adding new test cases for feature Portable IP > > > Diffs > ----- > > test/integration/component/test_portable_ip.py PRE-CREATION > tools/marvin/marvin/integration/lib/base.py 3016ee4 > tools/marvin/marvin/integration/lib/common.py 7e8d92d > > Diff: https://reviews.apache.org/r/13559/diff/ > > > Testing > ------- > > > Thanks, > > Gaurav Aradhye > >
