Hi Dimitris, I suggest, we follow our normal approach, push this to devel (when fixed) and eventually it will get merged into master. I don't expect other cherry-picks here anymore.
Cheers, Helga On Tue, Mar 5, 2013 at 2:13 PM, Dimitris Aragiorgis <[email protected]> wrote: > Hi Helga, > > Good to have you back. OK about the fixes. I have a question. I tried to > port the network qa in master and needs some modifications concerning > AquireInstance() (it returns _QaInstance and not a dict anymore). I > think maybe we should push this to master at once. And by that we do not > have to cherry-pick anything nor merge devel-2.7 again to master. > > What do you think? > > dimara > > * Helga Velroyen <[email protected]> [2013-03-05 13:31:38 +0100]: > > > Sorry, I forgot to mention that in the last mail. While you are at it, > > please also fix the lint errors. > > > > qa/qa_network.py:200:3: E303 too many blank lines (2) > > qa/qa_network.py:246:3: E303 too many blank lines (2) > > qa/qa_network.py:272:4: E123 closing bracket does not match > > indentation of opening bracket's line > > qa/qa_network.py:281:19: E128 continuation line under-indented for > visual indent > > qa/qa_network.py:283:19: E128 continuation line under-indented for > visual indent > > qa/qa_network.py:307:3: E303 too many blank lines (2) > > qa/qa_network.py:321:19: E128 continuation line under-indented for > visual indent > > > > > > thanks! > > > > Helga > > > > > > > > On Tue, Mar 5, 2013 at 1:18 PM, Helga Velroyen <[email protected]> > wrote: > > > > > Hi! > > > > > > thanks, please see my comments inline: > > > > > > > > > On Mon, Mar 4, 2013 at 11:54 AM, Dimitris Aragiorgis <[email protected] > >wrote: > > > > > >> Move GetNonexistentNetwork() in qa_utils. > > >> > > >> Add various corner cases for existing test add/remove and > > >> connect/disconnect. > > >> > > >> Add qa for OpInstanceCreate and OpInstanceSetParams to test > > >> how NICs interact with networks. > > >> > > >> Enable network tests in qa-sample.json. > > >> > > >> Signed-off-by: Dimitris Aragiorgis <[email protected]> > > >> --- > > >> qa/ganeti-qa.py | 3 + > > >> qa/qa-sample.json | 2 +- > > >> qa/qa_network.py | 298 > > >> ++++++++++++++++++++++++++++++++++++++++++++++++----- > > >> qa/qa_utils.py | 10 ++ > > >> 4 files changed, 289 insertions(+), 24 deletions(-) > > >> > > >> diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py > > >> index d06c963..0f0c504 100755 > > >> --- a/qa/ganeti-qa.py > > >> +++ b/qa/ganeti-qa.py > > >> @@ -334,7 +334,10 @@ def RunNetworkTests(): > > >> > > >> """ > > >> RunTestIf("network", qa_network.TestNetworkAddRemove) > > >> + RunTestIf("network", qa_network.TestNetworkSetParams) > > >> RunTestIf("network", qa_network.TestNetworkConnect) > > >> + RunTestIf("network", qa_network.TestInstanceAddAndNetAdd) > > >> + RunTestIf("network", qa_network.TestInstanceNetMod) > > >> > > >> > > >> def RunGroupRwTests(): > > >> diff --git a/qa/qa-sample.json b/qa/qa-sample.json > > >> index c662f2a..e13b4f1 100644 > > >> --- a/qa/qa-sample.json > > >> +++ b/qa/qa-sample.json > > >> @@ -134,7 +134,7 @@ > > >> "group-list": true, > > >> "group-rwops": true, > > >> > > >> - "network": false, > > >> + "network": true, > > >> > > >> "node-list": true, > > >> "node-info": true, > > >> diff --git a/qa/qa_network.py b/qa/qa_network.py > > >> index 5648fd6..ee908c2 100644 > > >> --- a/qa/qa_network.py > > >> +++ b/qa/qa_network.py > > >> @@ -29,53 +29,305 @@ import qa_utils > > >> from qa_utils import AssertCommand > > >> > > >> > > >> -def GetNonexistentNetworks(count): > > >> - """Gets network names which shouldn't exist on the cluster. > > >> +def GetNicParams(): > > >> + default_mode = "bridged" > > >> + default_link = "br0" > > >> + nicparams = qa_config.get("default-nicparams") > > >> + if nicparams: > > >> + mode = nicparams.get("mode", default_mode) > > >> + link = nicparams.get("link", default_link) > > >> + else: > > >> + mode = default_mode > > >> + link = default_link > > >> + > > >> + return mode, link > > >> + > > >> > > >> - @param count: Number of networks to get > > >> - @rtype: integer > > >> +def GetNetOption(idx=-1, action=None, mac=None, ip=None, > network=None, > > >> + mode=None, link=None): > > >> + net = "%d:" % idx > > >> + if action: > > >> + net += action > > >> + if mac: > > >> + net += ",mac=" + mac > > >> + if ip: > > >> + net += ",ip=" + ip > > >> + if network: > > >> + net += ",network=" + network > > >> + if mode: > > >> + net += ",mode=" + mode > > >> + if link: > > >> + net += ",link=" + link > > >> > > >> - """ > > >> - return qa_utils.GetNonexistentEntityNames(count, "networks", > "network") > > >> + return net.replace(":,", ":") > > >> + > > >> + > > >> +def RemoveInstance(instance): > > >> + name = instance["name"] > > >> + AssertCommand(["gnt-instance", "remove", "-f", name]) > > >> + qa_config.ReleaseInstance(instance) > > >> + > > >> + > > >> +def LaunchInstance(instance, mac=None, ip=None, network=None, > > >> + mode=None, link=None, fail=False): > > >> + > > >> + name = instance["name"] > > >> + net = GetNetOption(0, None, mac, ip, network, mode, link) > > >> + AssertCommand(["gnt-instance", "add", "-o", "debootstrap+default", > > >> + "-t", "file", "--disk", "0:size=1G", "--net", net, > > >> + "--no-name-check", "--no-ip-check", "--no-install", > > >> name], > > >> + fail=fail) > > >> + > > >> + > > >> +def ModifyInstance(instance, idx=-1, action="add", mac=None, > > >> + ip=None, network=None, mode=None, link=None, > > >> fail=False): > > >> + > > >> + name = instance["name"] > > >> + net = GetNetOption(idx, action, mac, ip, network, mode, link) > > >> + AssertCommand(["gnt-instance", "modify", "--net", net, name], > > >> fail=fail) > > >> > > >> > > >> def TestNetworkAddRemove(): > > >> """gnt-network add/remove""" > > >> - (network1, network2) = GetNonexistentNetworks(2) > > >> + (network1, network2, network3, ) = > qa_utils.GetNonexistentNetworks(3) > > >> > > >> - # Add some networks of different sizes. > > >> + # Add a network without subnet > > >> + # TODO: make this fail=False once abstract networks are implemented > > >> # Note: Using RFC5737 addresses. > > >> - AssertCommand(["gnt-network", "add", "--network", "192.0.2.0/30", > > >> network1]) > > >> + AssertCommand(["gnt-network", "add", network1], fail=True) > > >> + > > >> + # Check wrong opcode parameters > > >> + AssertCommand(["gnt-network", "add", "--network", "xxxxx", > network1], > > >> + fail=True) > > >> + AssertCommand(["gnt-network", "add", "--network", "192.168.1.0/24 > ", > > >> + "--gateway", "192.168.2.54", network1], > > >> + fail=True) > > >> > > > > > > Please use RFC5737 IP addresses and not 192.168.x.x wherever possible. > > > Those nets could actually been set up on the clusters where one runs > the QA > > > and it might lead to undesired side effects. > > > > > > > > >> + AssertCommand(["gnt-network", "add", "--network", "192.168.1.0/24 > ", > > >> + "--gateway", "192.168.2.54", network1], > > >> + fail=True) > > >> + #AssertCommand(["gnt-network", "add", "--network", "192.168.1.0/24 > ", > > >> + # "--gateway6", "2001:648:2ffc:1201::1", network1], > > >> + # fail=False) > > >> + AssertCommand(["gnt-network", "add", "--network", "192.168.1.0/24 > ", > > >> + "--mac-prefix", "xxxx", network1], > > >> + fail=True) > > >> + > > >> + AssertCommand(["gnt-network", "add", "--network", "192.168.1.0/24 > ", > > >> + "--gateway", "192.168.1.1", "--mac-prefix", > "aa:bb:cc", > > >> + "--add-reserved-ips", "192.168.1.10,192.168.1.110", > > >> + "--network6", "2001:648:2fc:201::/64", > > >> + "--gateway6", "2001:648:2fc:201::1", network1]) > > >> + > > >> + # TODO: add a network that contains the nodes' IPs > > >> + # This should reserve them > > >> AssertCommand(["gnt-network", "add", "--network", "198.51.100.0/24 > ", > > >> network2]) > > >> + > > >> + # This does not reserve master/node IPs > > >> + AssertCommand(["gnt-network", "add", "--network", "198.51.100.0/24 > ", > > >> + "--no-conflicts-check", network3]) > > >> + > > >> # Try to add a network with an existing name. > > >> AssertCommand(["gnt-network", "add", "--network", "203.0.133.0/24 > ", > > >> network2], > > >> fail=True) > > >> > > >> AssertCommand(["gnt-network", "remove", network1]) > > >> AssertCommand(["gnt-network", "remove", network2]) > > >> + AssertCommand(["gnt-network", "remove", network3]) > > >> + > > >> + > > >> +def TestNetworkSetParams(): > > >> + (network1, ) = qa_utils.GetNonexistentNetworks(1) > > >> + > > >> + print network1 > > >> + AssertCommand(["gnt-network", "add", "--network", "192.168.1.0/24 > ", > > >> + "--gateway", "192.168.1.1", "--mac-prefix", > "aa:bb:cc", > > >> + "--add-reserved-ips", "192.168.1.10,192.168.1.110", > > >> + "--network6", "2001:648:2fc:201::/64", > > >> + "--gateway6", "2001:648:2fc:201::1", network1]) > > >> + > > >> + # Cannot modify subnet > > >> + AssertCommand(["gnt-network", "modify", "--network", " > 192.168.1.0/24", > > >> + network1], fail=True) > > >> + > > >> + # Gateway outside network > > >> + AssertCommand(["gnt-network", "modify", > > >> + "--gateway", "192.168.2.1", "--mac-prefix", > "aa:bb:cc", > > >> + "--network6", "2001:648:2fc:201::/64", > > >> + "--gateway6", "2001:648:2fc:301::1", > > >> + network1], fail=True) > > >> + > > >> + # Gateway with reserved ips > > >> + AssertCommand(["gnt-network", "modify", "--gateway", > "192.168.1.60", > > >> + "--add-reserved-ips", "192.168.1.100,192.168.1.99", > > >> + network1], fail=True) > > >> + > > >> + # Edit all > > >> + AssertCommand(["gnt-network", "modify", "--network", " > 192.168.1.0/24", > > >> + "--add-reserved-ips", "192.168.1.70,192.168.1.40", > > >> + "--remove-reserved-ips", > "192.168.99.100,192.168.1.110", > > >> + "--network6", "2001:648:2fc:333::/64", > > >> + "--gateway6", "2001:648:2fc:333::10", > > >> + network1]) > > >> + > > >> + # reset everything > > >> + AssertCommand(["gnt-network", "modify", "--gateway", "none", > > >> + "--network6", "none", "--gateway6", "none", > > >> + "--mac-prefix", "none", > > >> + network1]) > > >> + > > >> + AssertCommand(["gnt-network", "remove", network1]) > > >> > > >> > > >> def TestNetworkConnect(): > > >> """gnt-network connect/disconnect""" > > >> - (group1, ) = qa_utils.GetNonexistentGroups(1) > > >> - (network1, ) = GetNonexistentNetworks(1) > > >> - > > >> - default_mode = "bridged" > > >> - default_link = "xen-br0" > > >> - nicparams = qa_config.get("default-nicparams") > > >> - if nicparams: > > >> - mode = nicparams.get("mode", default_mode) > > >> - link = nicparams.get("link", default_link) > > >> - else: > > >> - mode = default_mode > > >> - link = default_link > > >> + (group1, group2, ) = qa_utils.GetNonexistentGroups(2) > > >> + (network1, network2, ) = qa_utils.GetNonexistentNetworks(2) > > >> + defmode, deflink = GetNicParams() > > >> > > >> AssertCommand(["gnt-group", "add", group1]) > > >> + AssertCommand(["gnt-group", "add", group2]) > > >> AssertCommand(["gnt-network", "add", "--network", "192.0.2.0/24", > > >> network1]) > > >> + AssertCommand(["gnt-network", "add", "--network", "203.0.133.0/24 > ", > > >> network2]) > > >> > > >> - AssertCommand(["gnt-network", "connect", network1, mode, link, > group1]) > > >> - AssertCommand(["gnt-network", "disconnect", network1, group1]) > > >> + AssertCommand(["gnt-network", "connect", network1, > > >> + defmode, deflink, group1]) > > >> + # This should produce a warning. > > >> + AssertCommand(["gnt-network", "connect", network1, > > >> + defmode, deflink, group1, group2]) > > >> > > >> + > > >> + instance1 = qa_config.AcquireInstance() > > >> + # TODO: add conflicting image. > > >> + LaunchInstance(instance1, ip="192.0.2.5") > > >> + # Conflicting IPs > > >> + AssertCommand(["gnt-network", "connect", network1, defmode, > deflink], > > >> + fail=True) > > >> + AssertCommand(["gnt-network", "connect", "--no-conflicts-check", > > >> + network1, defmode, deflink]) > > >> + > > >> + # Network still connected > > >> + AssertCommand(["gnt-network", "remove", network1], fail=True) > > >> + > > >> + instance2 = qa_config.AcquireInstance() > > >> + # Add instance inside the network > > >> + LaunchInstance(instance2, ip="pool", network=network1) > > >> + # Conflicting IP, at least one instance belongs to the network > > >> + AssertCommand(["gnt-network", "disconnect", network1], fail=True) > > >> + RemoveInstance(instance2) > > >> + > > >> + AssertCommand(["gnt-network", "disconnect", network1]) > > >> + # This should only produce a warning. > > >> + AssertCommand(["gnt-network", "disconnect", network1]) > > >> + > > >> + RemoveInstance(instance1) > > >> AssertCommand(["gnt-group", "remove", group1]) > > >> + AssertCommand(["gnt-group", "remove", group2]) > > >> + AssertCommand(["gnt-network", "remove", network1]) > > >> + AssertCommand(["gnt-network", "remove", network2]) > > >> + > > >> + > > >> +def TestInstanceAddAndNetAdd(): > > >> + (network1, network2) = qa_utils.GetNonexistentNetworks(2) > > >> + defmode, deflink = GetNicParams() > > >> + > > >> + AssertCommand(["gnt-network", "add", "--network", "192.168.1.0/24 > ", > > >> + "--gateway", "192.168.1.1", "--mac-prefix", > "aa:bb:cc", > > >> + "--add-reserved-ips", "192.168.1.10,192.168.1.110", > > >> + "--network6", "2001:648:2fc:201::/64", > > >> + "--gateway6", "2001:648:2fc:201::1", network1]) > > >> + AssertCommand(["gnt-network", "connect", network1, defmode, > deflink]) > > >> + > > >> + AssertCommand(["gnt-network", "add", "--network", "203.0.133.0/24 > ", > > >> network2]) > > >> + AssertCommand(["gnt-network", "connect", network2, "routed", > "rt5000"]) > > >> + > > >> + > > >> + # (mac, ip, network, mode, link) > > >> + success_cases = [ > > >> + (None, None, None, None, None), # random mac and default > nicparams > > >> + ("generate", "5.5.5.5", None, "routed", "rt5000"), # given params > > >> + (None, "pool", network1, None, None), # first IP in network given > > >> + # TODO: include this use case with --no-conflicts-check > > >> + # just add an extra field in Launch|ModifyInstance > > >> + #(None, "192.168.1.6", None, None, None), # IP but no net > > >> + (None, None, network1, None, None) # nicparams/mac inherited by > > >> network > > >> + ] > > >> + > > >> + for (mac, ip, network, mode, link) in success_cases: > > >> + instance1 = qa_config.AcquireInstance() > > >> + LaunchInstance(instance1, mac, ip, network, mode, link) > > >> + ModifyInstance(instance1, idx=-1, action="add", mac=mac, > > >> + ip=ip, network=network, mode=mode, link=link) > > >> + ModifyInstance(instance1, idx=1, action="remove") > > >> + RemoveInstance(instance1) > > >> + > > >> + fail_cases = [ > > >> + (None, None, None, "lala", None), > > >> + (None, "lala", None, None, None), > > >> + (None, None, "lala", None, None), > > >> + (None, "203.0.133.5", None, None, None), # conflicting IP > > >> + (None, None, None, "routed", None), # routed with no IP > > >> + (None, "pool", network1, "routed", None), # nicparams along with > > >> network > > >> + (None, "pool", network1, None, deflink) > > >> + ] > > >> + > > >> + instance1 = qa_config.AcquireInstance() > > >> + instance2 = qa_config.AcquireInstance() > > >> + LaunchInstance(instance2) > > >> + for (mac, ip, network, mode, link) in fail_cases: > > >> + LaunchInstance(instance1, mac=mac, ip=ip, network=network, > > >> + mode=mode, link=link, fail=True) > > >> + ModifyInstance(instance2, idx=-1, action="add", mac=mac, > > >> + ip=ip, network=network, mode=mode, link=link, > > >> fail=True) > > >> + ModifyInstance(instance2, idx=0, action="modify", mac=mac, > > >> + ip=ip, network=network, mode=mode, link=link, > > >> fail=True) > > >> + > > >> + RemoveInstance(instance2) > > >> + AssertCommand(["gnt-network", "disconnect", network1]) > > >> AssertCommand(["gnt-network", "remove", network1]) > > >> + AssertCommand(["gnt-network", "disconnect", network2]) > > >> + AssertCommand(["gnt-network", "remove", network2]) > > >> + > > >> + > > >> +def TestInstanceNetMod(): > > >> + (network1, network2) = qa_utils.GetNonexistentNetworks(2) > > >> + defmode, deflink = GetNicParams() > > >> + > > >> + AssertCommand(["gnt-network", "add", "--network", "192.168.1.0/24 > ", > > >> + "--gateway", "192.168.1.1", "--mac-prefix", > "aa:bb:cc", > > >> + "--add-reserved-ips", "192.168.1.10,192.168.1.110", > > >> + "--network6", "2001:648:2fc:201::/64", > > >> + "--gateway6", "2001:648:2fc:201::1", network1]) > > >> + AssertCommand(["gnt-network", "connect", network1, defmode, > deflink]) > > >> + > > >> + AssertCommand(["gnt-network", "add", "--network", "203.0.133.0/24 > ", > > >> network2]) > > >> + AssertCommand(["gnt-network", "connect", network2, "routed", > "rt5000"]) > > >> + > > >> + > > >> + success_cases = [ > > >> + ("generate", "5.5.5.5", None, "routed", "rt5000"), # given params > > >> + (None, "pool", network1, None, None), # first IP in network given > > >> + (None, "none", "none", None, None), # random mac and default > > >> nicparams > > >> + (None, "192.168.1.6", network1, None, None), # IP but no net > > >> + #TODO: include this use case with --no-conflickts-check > > >> + #(None, "192.168.1.6", None, None, None), # IP but no net > > >> + (None, None, network1, None, None) # nicparams/mac inherited by > > >> network > > >> + ] > > >> + > > >> + instance1 = qa_config.AcquireInstance() > > >> + LaunchInstance(instance1) > > >> + for (mac, ip, network, mode, link) in success_cases: > > >> + ModifyInstance(instance1, idx=0, action="modify", mac=mac, > > >> + ip=ip, network=network, mode=mode, link=link) > > >> + # reset to defaults > > >> + ModifyInstance(instance1, idx=0, action="modify", mac="generate", > > >> + ip="none", network="none", mode=defmode, > link=deflink) > > >> + > > >> + ModifyInstance(instance1, idx=0, action="modify", ip="pool", > > >> network=network1) > > >> + ModifyInstance(instance1, idx=0, action="modify", ip="pool", > > >> network=network2) > > >> + > > >> + RemoveInstance(instance1) > > >> + AssertCommand(["gnt-network", "disconnect", network1]) > > >> + AssertCommand(["gnt-network", "remove", network1]) > > >> + AssertCommand(["gnt-network", "disconnect", network2]) > > >> + AssertCommand(["gnt-network", "remove", network2]) > > >> diff --git a/qa/qa_utils.py b/qa/qa_utils.py > > >> index 789c52e..e928c8c 100644 > > >> --- a/qa/qa_utils.py > > >> +++ b/qa/qa_utils.py > > >> @@ -701,6 +701,16 @@ def GetNonexistentGroups(count): > > >> return GetNonexistentEntityNames(count, "groups", "group") > > >> > > >> > > >> +def GetNonexistentNetworks(count): > > >> + """Gets network names which shouldn't exist on the cluster. > > >> + > > >> + @param count: Number of networks to get > > >> + @rtype: integer > > >> + > > >> + """ > > >> + return GetNonexistentEntityNames(count, "networks", "network") > > >> + > > >> + > > >> def GetNonexistentEntityNames(count, name_config, name_prefix): > > >> """Gets entity names which shouldn't exist on the cluster. > > >> > > >> -- > > >> 1.7.10.4 > > >> > > >> > > > Thanks, > > > Helga > > > > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.10 (GNU/Linux) > > iQEcBAEBCAAGBQJRNe98AAoJEMk10sCdtK5jEwsIAK8gg1At9Cb1ZDykzDHR7phq > RAP51R+jrhs/oyuLaAC/ckh3uNZ0XEt8mcix/d3xnZH7wecgotlDXs6wwcG1HqBY > xTUyKlOKNVd31MQ9kdibEytHFO3KjBaKJDdFJEVI8UdPCirWvNMBWrghY2+OnfzB > GbZWyzz3FURoXTXtxG3gdj7xYpD3mrpIyXHUtwHz1G74lHVRiHV8avpGczrm8jom > DEXOm4IizD1sJiz/G2y9/E00JTQsIbE/ii7IP8JebNLsBSAXB4U1dlkwT8UEGowu > 0pH2vSjI5wbNWaankgGSaWhBptuYwgaLKv98e6tWAUjgyQD3pd8Jc+d0BAEGXdw= > =xbpU > -----END PGP SIGNATURE----- > >
