[PATCH v2 00/26] [RFC] PyRecipes draft

[Jiri Prochazka]

Hello everyone,

first working draft implementation is completed. Please, read this cover
letter, check the patches and let me know what you like, what you don't like,
etc. Please, note, that this is not final implementation.

NOTE - patches are not rebased to current HEAD, if you want to try it,
please, reset your branch to commit 2398f93.

What doesn't work
=================
multi_match - didn't have time and resources to check physical machines,
on virt setup it works, but ends with exception due to no res value returned
loopbacks - can't be created
netem support - not implemented yet

Description of new mechanics
============================
Since we need to run TaskAPI methods to get machine requirements from PyRecipe,
methods like provision_machines, prepare_network are run after the python recipe
is executed. Methods add_host and add_interface create machine requirements dict
in ControllerAPI object. When lnst.match() is called, it runs match algorithm
with mreq dict and if match is found, it prepares network and binds Machine
and Interface objects with their HostAPI and InterfaceAPI counterparts.
When everyting is prepared, execution returns to PyRecipe, where task phase
of PyRecipe is executed. Task phase is the same like before, only new method
is breakpoint() which will be useful for debugging, as it allows user to pause
the execution in whatever part of test.
TaskAPI is now used right from lnst module, all callable methods are exported
via __init__.py file from lnst/ dir.
Folder pyrecipes/ contains few working examples you can try.

TODO
====
* multimatch support
* config_only mode should be renamed and polished
* TaskAPI create_ovs() method
* support for loopbacks
* NetEm support
* polishing of the code

Jiri Prochazka (26):
  __init__.py: draft for PyRecipes
  lnst-ctl: draft for PyRecipes
  NetTestController: remove RecipeParser import and related method calls
  NetTestController: add run_mode attribute
  NetTestController: remove obsolete methods
  NetTestController: use mreq dict from PyRecipe
  NetTestController: split provision_machines() in two methods
  NetTestController: cleanup _preapre_interface()
  NetTestController: remove resource sync from _prepare_machine()
  NetTestController: add multi_match attribute ti NetTestController
  NetTestController: don't call abs_path on _recipe_path in __init__
  NetTestController: rework alias handling
  NetTestController: add prepare_test_env()
  NetTestController: rework match_setup(), config_only_recipe(),
    run_recipe(), _run_python_task()
  NetTestController: add init_taskapi()
  NetTestController: use Task name instead of module
  Task: remove deprecated methods
  Task: add default param for get_alias()
  Task: define TaskAPI methods on global level
  Task: add breakpoint()
  Task: add add_host() and init_hosts()
  Task: add interface handling TaskAPI methods
  Task: HostAPI get_id now returns its generated id
  Task: add HostAPI methods required by PyRecipes
  Task: add match() and minor rework of ControllerAPI
  PyRecipes: add example PyRecipes

 lnst-ctl                             |  16 +-
 lnst/Controller/NetTestController.py | 398 ++++++++++-------------------------
 lnst/Controller/Task.py              | 240 +++++++++++----------
 lnst/__init__.py                     |   1 +
 pyrecipes/3_vlans.py                 |  34 +++
 pyrecipes/example.py                 |  33 +++
 pyrecipes/ping_flood.py              |  48 +++++
 7 files changed, 363 insertions(+), 407 deletions(-)
 create mode 100644 pyrecipes/3_vlans.py
 create mode 100644 pyrecipes/example.py
 create mode 100644 pyrecipes/ping_flood.py

-- 
2.4.11

[PATCH v2 01/26] __init__.py: draft for PyRecipes

[Jiri Prochazka]

exports TaskAPI methods to lnst module
New TaskAPI will be used in this format:

import lnst

m1 = lnst.add_host()

m1.add_interface(label="tnet")

while lnst.match():
    mod = lnst.get_module(...)
    lnst.wait(1)
    ...

Thus we need to export methods available via TaskAPI to this __init__.py file

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/__init__.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lnst/__init__.py b/lnst/__init__.py
index e69de29..924a8ff 100644
--- a/lnst/__init__.py
+++ b/lnst/__init__.py
@@ -0,0 +1 @@
+from lnst.Controller.Task import match, add_host, wait, get_alias, get_module,
breakpoint
-- 
2.4.11

[PATCH v2 02/26] lnst-ctl: draft for PyRecipes

[Jiri Prochazka]

Changes:
* add multi_match argument for NetTestController __init__
* add init of TaskAPI before executing the test
* when folder user in recipe_path, use .py files instead of .xml

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst-ctl | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lnst-ctl b/lnst-ctl
index 6d656f8..ef70cb1 100755
--- a/lnst-ctl
+++ b/lnst-ctl
@@ -112,7 +112,8 @@ def get_recipe_result(action, file_path, log_ctl, res_serializer,
                                        defined_aliases=defined_aliases,
                                        overriden_aliases=overriden_aliases,
                                        reduce_sync=reduce_sync,
-                                       restrict_pools=pools)
+                                       restrict_pools=pools,
+                                       multi_match=multi_match)
     except XmlProcessingError as err:
         log_exc_traceback()
         logging.error(err)
@@ -126,9 +127,13 @@ def get_recipe_result(action, file_path, log_ctl, res_serializer,
         res = {}
         if matches == 1:
             try:
-                nettestctl.provision_machines()
-                nettestctl.print_match_description()
+                # init TaskAPI.Ctl
+                nettestctl.init_taskapi()
                 res = exec_action(action, nettestctl)
+            except NoMatchError as err:
+                no_match = True
+                log_ctl.unset_recipe()
+                logging.warning("Match %d not possible." % matches)
             except Exception as err:
                 no_match = True
                 log_exc_traceback()
@@ -138,11 +143,10 @@ def get_recipe_result(action, file_path, log_ctl, res_serializer,
                 retval = RETVAL_ERR
         elif matches > 1:
             try:
-                nettestctl.provision_machines()
+                nettestctl.init_taskapi()
                 log_ctl.set_recipe(file_path, expand="match_%d" % matches)
                 recipe_head_log_entry(file_path, log_dir, matches)
                 res_serializer.add_recipe(file_path, matches)
-                nettestctl.print_match_description()
                 res = exec_action(action, nettestctl)
             except NoMatchError as err:
                 no_match = True
@@ -337,7 +341,7 @@ def main():
             all_files.sort()
             for f in all_files:
                 recipe_file = os.path.join(recipe_path, f)
-                if re.match(r'^.*\.xml$', recipe_file):
+                if re.match(r'^.*\.py$', recipe_file):
                     recipe_files.append(recipe_file)
         else:
             recipe_files.append(recipe_path)
-- 
2.4.11

[PATCH v2 03/26] NetTestController: remove RecipeParser import and related method calls

[Jiri Prochazka]

RecipeParser is no longer used for recipe parsing, thus we can delete it's import
and methods related with it

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index 8fdd43e..39e2950 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -24,7 +24,7 @@ from lnst.Common.NetUtils import MacPool
 from lnst.Common.Utils import md5sum, dir_md5sum
 from lnst.Common.Utils import check_process_running, bool_it, get_module_tools
 from lnst.Common.NetTestCommand import str_command, CommandException
-from lnst.Controller.RecipeParser import RecipeParser, RecipeError
+from lnst.Controller.RecipeParser import RecipeError
 from lnst.Controller.SlavePool import SlavePool
 from lnst.Controller.Machine import MachineError, VirtualInterface
 from lnst.Controller.Machine import StaticInterface
@@ -62,7 +62,6 @@ class NetTestController:
         self._msg_dispatcher = MessageDispatcher(log_ctl)
         self._packet_capture = packet_capture
         self._reduce_sync = reduce_sync
-        self._parser = RecipeParser(recipe_path)
 
         self.remove_saved_machine_config()
 
@@ -73,10 +72,6 @@ class NetTestController:
         mac_pool_range = lnst_config.get_option('environment',
'mac_pool_range')
         self._mac_pool = MacPool(mac_pool_range[0], mac_pool_range[1])
 
-        self._parser.set_machines(self._machines)
-        self._parser.set_aliases(defined_aliases, overriden_aliases)
-        self._recipe = self._parser.parse()
-
         conf_pools = lnst_config.get_pools()
         pools = {}
         if len(restrict_pools) > 0:
-- 
2.4.11

[PATCH v2 04/26] NetTestController: add run_mode attribute

[Jiri Prochazka]

this attribute is set from lnst-ctl and is used in
TaskAPI match() where it stops executing
task phase in match_setup run mode

also it's used in TaskAPI breakpoint(),
because lnst will break only in config_only run_mode
and in run mode it's ignored

it was added because all run modes need to execute python
recipe for which they all use _run_python_task() and this
seemd like best approach

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index 39e2950..3661f77 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -65,6 +65,8 @@ class NetTestController:
 
         self.remove_saved_machine_config()
 
+        self.run_mode = "run"
+
         self._machines = {}
         self._network_bridges = {}
         self._tasks = []
@@ -98,6 +100,9 @@ class NetTestController:
         self._resource_table["module"] = self._load_test_modules(modules_dirs)
         self._resource_table["tools"] = self._load_test_tools(tools_dirs)
 
+    def _get_run_mode(self):
+        return self.run_mode
+
     def _get_machineinfo(self, machine_id):
         try:
             info = self._recipe["machines"][machine_id]["params"]
@@ -608,9 +613,11 @@ class NetTestController:
             os.remove("/tmp/.lnst_machine_conf")
 
     def match_setup(self):
+        self.run_mode = "match_setup"
         return {"passed": True}
 
     def config_only_recipe(self):
+        self.run_mode = "config_only"
         try:
             self._prepare_network(resource_sync=False)
         except (KeyboardInterrupt, Exception) as exc:
-- 
2.4.11

[PATCH v2 05/26] NetTestController: remove obsolete methods

[Jiri Prochazka]

methods removed:
_get_machine_requirements - XML recipe no longer used, meaning
we can use machine requirement dict straight from network
definition in PyRecipe

_prepare_task - XML recipe is no longer used and task defined for them
are now obsolete

_run_task - same as ^

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 127 -----------------------------------
 1 file changed, 127 deletions(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index 3661f77..fdaf532 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -118,76 +118,6 @@ class NetTestController:
         msg = "SSH session terminated with status %s" % status
         raise NetTestError(msg)
 
-    def _get_machine_requirements(self):
-        recipe = self._recipe
-
-        # There must be some machines specified in the recipe
-        if "machines" not in recipe or \
-          ("machines" in recipe and len(recipe["machines"]) == 0):
-            msg = "No hosts specified in the recipe. At least two " \
-                  "hosts are required to perform a network test."
-            raise RecipeError(msg, recipe)
-
-        # machine requirements
-        mreq = {}
-        for machine in recipe["machines"]:
-            m_id = machine["id"]
-
-            if m_id in mreq:
-                msg = "Machine with id='%s' already exists." % m_id
-                raise RecipeError(msg, machine)
-
-            params = {}
-            if "params" in machine:
-                for p in machine["params"]:
-                    if p["name"] in params:
-                        msg = "Parameter '%s' of host %s was specified
" \
-                              "multiple times. Overriding the previous value."
\
-                              % (p["name"], m_id)
-                        logging.warn(RecipeError(msg, p))
-                    name = p["name"]
-                    val = p["value"]
-                    params[name] = val
-
-            # Each machine must have at least one interface
-            if "interfaces" not in machine or \
-              ("interfaces" in machine and len(machine["interfaces"])
== 0):
-                msg = "Host '%s' has no interfaces specified." % m_id
-                raise RecipeError(msg, machine)
-
-            ifaces = {}
-            for iface in machine["interfaces"]:
-                if_id = iface["id"]
-                if if_id in ifaces:
-                    msg = "Interface with id='%s' already exists on host
" \
-                          "'%s'." % (if_id, m_id)
-
-                iface_type = iface["type"]
-                if iface_type != "eth":
-                    continue
-
-                iface_params = {}
-                if "params" in iface:
-                    for i in iface["params"]:
-                        if i["name"] in iface_params:
-                            msg = "Parameter '%s' of interface %s of "
\
-                                  "host %s was defined multiple times. " \
-                                  "Overriding the previous value." \
-                                  % (i["name"], if_id, m_id)
-                            logging.warn(RecipeError(msg, p))
-                        name = i["name"]
-                        val = i["value"]
-                        iface_params[name] = val
-
-                ifaces[if_id] = {
-                    "network": iface["network"],
-                    "params": iface_params
-                }
-
-            mreq[m_id] = {"params": params, "interfaces": ifaces}
-
-        return mreq
-
     def _prepare_network(self, resource_sync=True):
         recipe = self._recipe
 
@@ -350,55 +280,11 @@ class NetTestController:
         if "peer" in iface_xml_data:
             iface.set_peer(iface_xml_data["peer"])
 
-    def _prepare_tasks(self):
-        self._tasks = []
-        for task_data in self._recipe["tasks"]:
-            task = {}
-            task["quit_on_fail"] = False
-            if "quit_on_fail" in task_data:
-                task["quit_on_fail"] =
bool_it(task_data["quit_on_fail"])
-
-            if "module_dir" in task_data:
-                task["module_dir"] = task_data["module_dir"]
-
-            if "tools_dir" in task_data:
-                task["tools_dir"] = task_data["tools_dir"]
-
-            if "python" in task_data:
-                root = Path(None, self._recipe_path).get_root()
-                path = Path(root, task_data["python"])
 
-                task["python"] = path
-                if not path.exists():
-                    msg = "Task file '%s' not found." % path.to_str()
-                    raise RecipeError(msg, task_data)
 
-                self._tasks.append(task)
-                continue
-
-            task["commands"] = task_data["commands"]
-            task["skeleton"] = []
-            for cmd_data in task["commands"]:
-                cmd = {"type": cmd_data["type"]}
-
-                if "host" in cmd_data:
-                    cmd["host"] = cmd_data["host"]
-                    if cmd["host"] not in self._machines:
-                        msg = "Invalid host id '%s'." %
cmd["host"]
-                        raise RecipeError(msg, cmd_data)
-
-                if cmd["type"] in ["test", "exec"]:
-                    if "bg_id" in cmd_data:
-                        cmd["bg_id"] = cmd_data["bg_id"]
-                elif cmd["type"] in ["wait", "intr",
"kill"]:
-                    cmd["proc_id"] = cmd_data["bg_id"]
 
-                task["skeleton"].append(cmd)
 
-            if self._check_task(task):
-                raise RecipeError("Incorrect command sequence.", task_data)
 
-            self._tasks.append(task)
 
     def _prepare_command(self, cmd_data):
         cmd = {"type": cmd_data["type"]}
@@ -710,19 +596,6 @@ class NetTestController:
 
         return module.ctl._result
 
-    def _run_task(self, task):
-        if "python" in task:
-            return self._run_python_task(task)
-
-        seq_passed = True
-        for cmd_data in task["commands"]:
-            cmd = self._prepare_command(cmd_data)
-            cmd_res = self._run_command(cmd)
-            if not cmd_res["passed"]:
-                seq_passed = False
-
-        return seq_passed
-
     def _run_command(self, command):
         logging.info("Executing command: [%s]", str_command(command))
 
-- 
2.4.11

[PATCH v2 06/26] NetTestController: use mreq dict from PyRecipe

[Jiri Prochazka]

setting of machine requirements is now moved so it's
run after executing setup phase of PyRecipe and has
been removed from NTC __init__()

in _prepare_network() mreq dict is used instead of
parsed xml recipe and method's been reworked a bit

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index fdaf532..ae5559a 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -90,9 +90,6 @@ class NetTestController:
         sp = SlavePool(pools, pool_checks)
         self._slave_pool = sp
 
-        mreq = self._get_machine_requirements()
-        sp.set_machine_requirements(mreq)
-
         modules_dirs = lnst_config.get_option('environment',
'module_dirs')
         tools_dirs = lnst_config.get_option('environment', 'tool_dirs')
 
@@ -119,21 +116,21 @@ class NetTestController:
         raise NetTestError(msg)
 
     def _prepare_network(self, resource_sync=True):
-        recipe = self._recipe
+        mreq = Task.get_mreq()
 
         machines = self._machines
         for m_id in machines.keys():
             self._prepare_machine(m_id, resource_sync)
 
-        for machine_xml_data in recipe["machines"]:
-            m_id = machine_xml_data["id"]
+        for machine_id, machine_data in mreq.iteritems():
+            m_id = machine_id
             m = machines[m_id]
             namespaces = set()
-            for iface_xml_data in machine_xml_data["interfaces"]:
-                self._prepare_interface(m_id, iface_xml_data)
+            for if_id, iface_data in machine_data["interfaces"].iteritems():
+                self._prepare_interface(m_id, if_id, iface_data)
 
-                if iface_xml_data["netns"] != None:
-                    namespaces.add(iface_xml_data["netns"])
+                if iface_data["netns"] != None:
+                    namespaces.add(iface_data["netns"])
 
             if len(namespaces) > 0:
                 m.disable_nm()
-- 
2.4.11

[PATCH v2 07/26] NetTestController: split provision_machines() in two methods

[Jiri Prochazka]

set_machine_requirements() is now used to set mreq to SlavePool
object and to reset match state and is executed once on first run of
PyRecipe

provision_machines() stayed the same apart from extracted part from
set_machine_requirements()
Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index ae5559a..a4b48f0 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -152,9 +152,15 @@ class NetTestController:
 
             m.wait_interface_init()
 
-    def provision_machines(self):
+
+    def set_machine_requirements(self):
+        mreq = Task.get_mreq()
         sp = self._slave_pool
+        sp.set_machine_requirements(mreq)
+
+    def provision_machines(self):
         machines = self._machines
+        sp = self._slave_pool
         if not sp.provision_machines(machines):
             msg = "This setup cannot be provisioned with the current pool."
             raise NoMatchError(msg)
-- 
2.4.11

[PATCH v2 08/26] NetTestController: cleanup _preapre_interface()

[Jiri Prochazka]

support for loopback devices has been temporarily removed
support for netem has been temporarily removed

we should consider if we want to define IP address of eth
interface in setup phase or if we want to keep it in task

_prepare_interface() now works only with eth ifaces

soft interfaces are created dynamically in task phase so
if statements can be removed from this method

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 89 +++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 48 deletions(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index a4b48f0..fd54a09 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -239,55 +239,48 @@ class NetTestController:
                             raise RecipeError(msg, cmd)
         machine.sync_resources(sync_table)
 
-    def _prepare_interface(self, m_id, iface_xml_data):
+    def _prepare_interface(self, m_id, if_id, iface_data):
         machine = self._machines[m_id]
-        if_id = iface_xml_data["id"]
-        if_type = iface_xml_data["type"]
-
-        try:
-            iface = machine.get_interface(if_id)
-        except MachineError:
-            if if_type == 'lo':
-                iface = machine.new_loopback_interface(if_id)
-            else:
-                iface = machine.new_soft_interface(if_id, if_type)
-
-        if "slaves" in iface_xml_data:
-            for slave in iface_xml_data["slaves"]:
-                slave_id = slave["id"]
-                iface.add_slave(machine.get_interface(slave_id))
-
-                # Some soft devices (such as team) use per-slave options
-                if "options" in slave:
-                    for opt in slave["options"]:
-                        iface.set_slave_option(slave_id, opt["name"],
-                                               opt["value"])
-
-        if "addresses" in iface_xml_data:
-            for addr in iface_xml_data["addresses"]:
-                iface.add_address(addr)
-
-        if "options" in iface_xml_data:
-            for opt in iface_xml_data["options"]:
-                iface.set_option(opt["name"], opt["value"])
-
-        if "netem" in iface_xml_data:
-            iface.set_netem(iface_xml_data["netem"].to_dict())
-
-        if "ovs_conf" in iface_xml_data:
-            iface.set_ovs_conf(iface_xml_data["ovs_conf"].to_dict())
-
-        if iface_xml_data["netns"] != None:
-            iface.set_netns(iface_xml_data["netns"])
-
-        if "peer" in iface_xml_data:
-            iface.set_peer(iface_xml_data["peer"])
-
-
-
-
-
-
+        #if_type = iface_data["type"]
+
+        #try:
+        iface = machine.get_interface(if_id)
+        #except MachineError:
+            #if if_type == 'lo':
+            #    iface = machine.new_loopback_interface(if_id)
+            #else:
+            #    iface = machine.new_soft_interface(if_id, if_type)
+
+        #if "slaves" in iface_data:
+        #    for slave in iface_data["slaves"]:
+        #        slave_id = slave["id"]
+        #        iface.add_slave(machine.get_interface(slave_id))
+
+        #        # Some soft devices (such as team) use per-slave options
+        #        if "options" in slave:
+        #            for opt in slave["options"]:
+        #                iface.set_slave_option(slave_id, opt["name"],
+        #                                       opt["value"])
+
+        #if "addresses" in iface_data:
+        #    for addr in iface_data["addresses"]:
+        #        iface.add_address(addr)
+
+        #if "options" in iface_data:
+        #    for opt in iface_data["options"]:
+        #        iface.set_option(opt["name"], opt["value"])
+
+        #if "netem" in iface_data:
+        #    iface.set_netem(iface_data["netem"].to_dict())
+
+        #if "ovs_conf" in iface_data:
+        #    iface.set_ovs_conf(iface_data["ovs_conf"].to_dict())
+
+        if iface_data["netns"] != None:
+            iface.set_netns(iface_data["netns"])
+
+        #if "peer" in iface_data:
+        #    iface.set_peer(iface_data["peer"])
 
     def _prepare_command(self, cmd_data):
         cmd = {"type": cmd_data["type"]}
-- 
2.4.11

[PATCH v2 09/26] NetTestController: remove resource sync from _prepare_machine()

[Jiri Prochazka]

modules and tools as were used in XML recipe are not used in
PyRecipes, so they can be removed

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 46 ------------------------------------
 1 file changed, 46 deletions(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index fd54a09..827a943 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -193,52 +193,6 @@ class NetTestController:
         recipe_name = os.path.basename(self._recipe_path)
         machine.configure(recipe_name)
 
-        sync_table = {'module': {}, 'tools': {}}
-        if resource_sync:
-            for task in self._recipe['tasks']:
-                res_table = copy.deepcopy(self._resource_table)
-                if 'module_dir' in task:
-                    modules = self._load_test_modules([task['module_dir']])
-                    res_table['module'].update(modules)
-                if 'tools_dir' in task:
-                    tools = self._load_test_tools([task['tools_dir']])
-                    res_table['tools'].update(tools)
-
-                if 'commands' not in task:
-                    if not self._reduce_sync:
-                        sync_table = res_table
-                        break
-                    else:
-                        continue
-                for cmd in task['commands']:
-                    if 'host' not in cmd or cmd['host'] != m_id:
-                        continue
-                    if cmd['type'] == 'test':
-                        mod = cmd['module']
-                        if mod in res_table['module']:
-                            sync_table['module'][mod] =
res_table['module'][mod]
-                            # check if test module uses some test tools
-                            mod_path =
res_table['module'][mod]["path"]
-                            mod_tools = get_module_tools(mod_path)
-                            for t in mod_tools:
-                                if t in sync_table['tools']:
-                                    continue
-                                logging.debug("Adding '%s' tool as "\
-                                    "dependency of %s test module" % (t, mod))
-                                sync_table['tools'][t] =
res_table['tools'][t]
-                        else:
-                            msg = "Module '%s' not found on the
controller"\
-                                    % mod
-                            raise RecipeError(msg, cmd)
-                    if cmd['type'] == 'exec' and 'from' in cmd:
-                        tool = cmd['from']
-                        if tool in res_table['tools']:
-                            sync_table['tools'][tool] =
res_table['tools'][tool]
-                        else:
-                            msg = "Tool '%s' not found on the
controller" % tool
-                            raise RecipeError(msg, cmd)
-        machine.sync_resources(sync_table)
-
     def _prepare_interface(self, m_id, if_id, iface_data):
         machine = self._machines[m_id]
         #if_type = iface_data["type"]
-- 
2.4.11

[PATCH v2 10/26] NetTestController: add multi_match attribute ti NetTestController

[Jiri Prochazka]

used by TaskAPI in match() - it decides whether it should continue
with more run when multi_match is enabled via command line arsg

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index 827a943..51bd424 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -54,7 +54,7 @@ class NetTestController:
                  res_serializer=None, pool_checks=True,
                  packet_capture=False,
                  defined_aliases=None, overriden_aliases=None,
-                 reduce_sync=False, restrict_pools=[]):
+                 reduce_sync=False, restrict_pools=[], multi_match=False):
         self._res_serializer = res_serializer
         self._remote_capture_files = {}
         self._log_ctl = log_ctl
@@ -62,6 +62,7 @@ class NetTestController:
         self._msg_dispatcher = MessageDispatcher(log_ctl)
         self._packet_capture = packet_capture
         self._reduce_sync = reduce_sync
+        self._multi_match = multi_match
 
         self.remove_saved_machine_config()
 
-- 
2.4.11

[PATCH v2 11/26] NetTestController: don't call abs_path on _recipe_path in __init__

[Jiri Prochazka]

made more sense, because we will need to resolve Path object in
_run_python_task() and abs_path method transforms it to str object

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index 51bd424..04851e1 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -58,7 +58,7 @@ class NetTestController:
         self._res_serializer = res_serializer
         self._remote_capture_files = {}
         self._log_ctl = log_ctl
-        self._recipe_path = Path(None, recipe_path).abs_path()
+        self._recipe_path = Path(None, recipe_path)
         self._msg_dispatcher = MessageDispatcher(log_ctl)
         self._packet_capture = packet_capture
         self._reduce_sync = reduce_sync
@@ -191,7 +191,7 @@ class NetTestController:
         machine.set_mac_pool(self._mac_pool)
         machine.set_network_bridges(self._network_bridges)
 
-        recipe_name = os.path.basename(self._recipe_path)
+        recipe_name = os.path.basename(self._recipe_path.abs_path())
         machine.configure(recipe_name)
 
     def _prepare_interface(self, m_id, if_id, iface_data):
-- 
2.4.11

[PATCH v2 12/26] NetTestController: rework alias handling

[Jiri Prochazka]

aliases will now only be taken from command
line -a (or -A) argument, so we can remove
_overriden_aliases and rework get methods
to look in dict object where they are stored

in TaskAPI in get_alias method, default argument
will be introduced as replacament for alias definition
in <define> part of XML recipe

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index 04851e1..8c1d24f 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -53,8 +53,8 @@ class NetTestController:
     def __init__(self, recipe_path, log_ctl,
                  res_serializer=None, pool_checks=True,
                  packet_capture=False,
-                 defined_aliases=None, overriden_aliases=None,
-                 reduce_sync=False, restrict_pools=[], multi_match=False):
+                 defined_aliases=None, reduce_sync=False,
+                 restrict_pools=[], multi_match=False):
         self._res_serializer = res_serializer
         self._remote_capture_files = {}
         self._log_ctl = log_ctl
@@ -62,6 +62,7 @@ class NetTestController:
         self._msg_dispatcher = MessageDispatcher(log_ctl)
         self._packet_capture = packet_capture
         self._reduce_sync = reduce_sync
+        self._defined_aliases = defined_aliases
         self._multi_match = multi_match
 
         self.remove_saved_machine_config()
@@ -674,12 +675,11 @@ class NetTestController:
         return packages
 
     def _get_alias(self, alias):
-        templates = self._parser._template_proc
-        return templates._find_definition(alias)
+        if alias in self._defined_aliases:
+            return self._defined_aliases[alias]
 
     def _get_aliases(self):
-        templates = self._parser._template_proc
-        return templates._dump_definitions()
+        return self._defined_aliases
 
 class MessageDispatcher(ConnectionHandler):
     def __init__(self, log_ctl):
-- 
2.4.11

[PATCH v2 13/26] NetTestController: add prepare_test_env()

[Jiri Prochazka]

this method is run from Task match(), it runs
matching algorithm, prints match desc, when
match_setup run mode is used it ends here

otherwise it prepares network, binds Machine objects
to its HostAPI counterparts and Device objects to
its InterfaceAPI counterparts

summary - it returns True if it found match and if
run or config_only mode is used, it also prepares network
and initializes HostAPI and InterfaceAPI objects created
in setup phase

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index 8c1d24f..3e7eb69 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -496,6 +496,25 @@ class NetTestController:
 
         return res
 
+    def prepare_test_env(self):
+        try:
+            self.provision_machines()
+            self.print_match_description()
+            if self.run_mode == "match_setup":
+                return True
+            self._prepare_network()
+            Task.ctl.init_hosts(self._machines)
+            return True
+        except (NoMatchError) as exc:
+            self._cleanup_slaves()
+            raise exc
+            return False
+        except (KeyboardInterrupt, Exception) as exc:
+            msg = "Exception raised during configuration."
+            logging.error(msg)
+            self._cleanup_slaves()
+            raise
+
     def _run_recipe(self):
         overall_res = {"passed": True}
 
-- 
2.4.11

[PATCH v2 14/26] NetTestController: rework match_setup(), config_only_recipe(), run_recipe(), _run_python_task()

[Jiri Prochazka]

match_setup() - now requires to run _run_python_task() in order to get mreq dict
for matching algorithm

config_only_recipe() - this method and run mode should be renamed to something like debug
mode,
because now it's same as run mode, the only difference is, that it stops executing of
recipe
on breakpoint()s until user presses Enter. Although this behaviour should be discussed
and
it will be done in next phase of PyRecipes implementation

_run_python_recipe() - modules and tools are not used now, so they cannot be loaded and
TaskAPI is initialized earlier because it's needed for setup phase for all run modes

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 74 +++++++++++-------------------------
 1 file changed, 23 insertions(+), 51 deletions(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index 3e7eb69..80708e5 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -452,37 +452,22 @@ class NetTestController:
 
     def match_setup(self):
         self.run_mode = "match_setup"
+        res = self._run_python_task()
         return {"passed": True}
 
     def config_only_recipe(self):
         self.run_mode = "config_only"
         try:
-            self._prepare_network(resource_sync=False)
-        except (KeyboardInterrupt, Exception) as exc:
-            msg = "Exception raised during configuration."
-            logging.error(msg)
-            self._cleanup_slaves()
+            res = self._run_recipe()
+        except Exception as exc:
+            logging.error("Recipe execution terminated by unexpected
exception")
             raise
-
-        self._save_machine_config()
-
-        self._cleanup_slaves(deconfigure=False)
-        return {"passed": True}
-
-    def run_recipe(self):
-        try:
-            self._prepare_tasks()
-            self._prepare_network()
-        except (KeyboardInterrupt, Exception) as exc:
-            msg = "Exception raised during configuration."
-            logging.error(msg)
+        finally:
             self._cleanup_slaves()
-            raise
 
-        if self._packet_capture:
-            self._start_packet_capture()
+        return res
 
-        err = None
+    def run_recipe(self):
         try:
             res = self._run_recipe()
         except Exception as exc:
@@ -518,43 +503,30 @@ class NetTestController:
     def _run_recipe(self):
         overall_res = {"passed": True}
 
-        for task in self._tasks:
+        try:
             self._res_serializer.add_task()
-            try:
-                res = self._run_task(task)
-            except CommandException as exc:
-                logging.debug(exc)
-                overall_res["passed"] = False
-                overall_res["err_msg"] = "Command exception raised."
-                break
-
-            for machine in self._machines.itervalues():
-                machine.restore_system_config()
-
-            # task failed, check if we should quit_on_fail
-            if not res:
-                overall_res["passed"] = False
-                overall_res["err_msg"] = "At least one command
failed."
-                if task["quit_on_fail"]:
-                    break
+            res = self._run_python_task()
+        except CommandException as exc:
+            logging.debug(exc)
+            overall_res["passed"] = False
+            overall_res["err_msg"] = "Command exception raised."
+
+        for machine in self._machines.itervalues():
+            machine.restore_system_config()
+
+        # task failed
+        if not res:
+            overall_res["passed"] = False
+            overall_res["err_msg"] = "At least one command failed."
 
         return overall_res
 
-    def _run_python_task(self, task):
+    def _run_python_task(self):
         #backup of resource table
         res_table_bkp = copy.deepcopy(self._resource_table)
-        if 'module_dir' in task:
-            modules = self._load_test_modules([task['module_dir']])
-            self._resource_table['module'].update(modules)
-        if 'tools_dir' in task:
-            tools = self._load_test_tools([task['tools_dir']])
-            self._resource_table['tools'].update(tools)
-
-        # Initialize the API handle
-        Task.ctl = Task.ControllerAPI(self, self._machines)
 
         cwd = os.getcwd()
-        task_path = task["python"]
+        task_path = self._recipe_path
         name = os.path.basename(task_path.abs_path()).split(".")[0]
         sys.path.append(os.path.dirname(task_path.resolve()))
         os.chdir(os.path.dirname(task_path.resolve()))
-- 
2.4.11

[PATCH v2 15/26] NetTestController: add init_taskapi()

[Jiri Prochazka]

this initializes ControllerAPI object used in TaskAPI

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index 80708e5..c921cc7 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -521,6 +521,9 @@ class NetTestController:
 
         return overall_res
 
+    def init_taskapi(self):
+        Task.ctl = Task.ControllerAPI(self)
+
     def _run_python_task(self):
         #backup of resource table
         res_table_bkp = copy.deepcopy(self._resource_table)
-- 
2.4.11

[PATCH v2 16/26] NetTestController: use Task name instead of module

[Jiri Prochazka]

it's more readable this way

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/NetTestController.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
index c921cc7..149fe66 100644
--- a/lnst/Controller/NetTestController.py
+++ b/lnst/Controller/NetTestController.py
@@ -540,7 +540,7 @@ class NetTestController:
         #restore resource table
         self._resource_table = res_table_bkp
 
-        return module.ctl._result
+        return Task.ctl._result
 
     def _run_command(self, command):
         logging.info("Executing command: [%s]", str_command(command))
-- 
2.4.11

[PATCH v2 17/26] Task: remove deprecated methods

[Jiri Prochazka]

removes methods marked with deprecated decorator
also get_host and get_hosts methods, since ID's are now
not user defined, thus these methods are obsolete

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Task.py | 102 ------------------------------------------------
 1 file changed, 102 deletions(-)

diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
index 5462d44..8db51d4 100644
--- a/lnst/Controller/Task.py
+++ b/lnst/Controller/Task.py
@@ -58,27 +58,6 @@ class ControllerAPI(object):
         self._result = self._result and res["passed"]
         return res
 
-    def get_host(self, host_id):
-        """
-            Get an API handle for the host from the recipe spec with
-            a specific id.
-
-            :param host_id: id of the host as defined in the recipe
-            :type host_id: string
-
-            :return: The host handle.
-            :rtype: HostAPI
-
-            :raises TaskError: If there is no host with such id.
-        """
-        if host_id not in self._hosts:
-            raise TaskError("Host '%s' not found." % host_id)
-
-        return self._hosts[host_id]
-
-    def get_hosts(self):
-        return self._hosts
-
     def get_module(self, name, options={}):
         """
             Initialize a module to be run on a host.
@@ -287,75 +266,6 @@ class HostAPI(object):
         cmd_res = self._ctl._run_command(cmd)
         return ProcessAPI(self._ctl, self._id, cmd_res, bg_id, cmd["netns"])
 
-    def get_interfaces(self):
-        return self._ifaces
-
-    def get_interface(self, if_id):
-        return self._ifaces[if_id]
-
-    @deprecated
-    def get_devname(self, if_id):
-        """
-            Returns devname of the interface.
-
-            :param if_id: which interface
-            :type if_id: string
-
-            :return: Device name (e.g., eth0).
-            :rtype: str
-        """
-        iface = self._ifaces[if_id]
-        return iface.get_devname()
-
-    @deprecated
-    def get_hwaddr(self, if_id):
-        """
-            Returns hwaddr of the interface.
-
-            :param if_id: which interface
-            :type if_id: string
-
-            :return: HW address (e.g., 00:11:22:33:44:55:FF).
-            :rtype: str
-        """
-        iface = self._ifaces[if_id]
-        return iface.get_hwaddr()
-
-    @deprecated
-    def get_ip(self, if_id, addr_number=0):
-        """
-            Returns an IP address of the interface.
-
-            :param if_id: which interface
-            :type if_id: string
-
-            :param addr_number: which address
-            :type addr_number: int
-
-            :return: IP address (e.g., 192.168.1.10).
-            :rtype: str
-        """
-        iface = self._ifaces[if_id]
-        return iface.get_ip_addr(addr_number)
-
-    @deprecated
-    def get_prefix(self, if_id, addr_number=0):
-        """
-            Returns an IP address prefix (netmask)
-            of the interface.
-
-            :param if_id: which interface
-            :type if_id: string
-
-            :param addr_number: which address
-            :type addr_number: int
-
-            :return: netmask (e.g., 24).
-            :rtype: str
-        """
-        iface = self._ifaces[if_id]
-        return iface.get_ip_prefix(addr_number)
-
     def sync_resources(self, modules=[], tools=[]):
         res_table = self._ctl._ctl._resource_table
         sync_table = {'module': {}, 'tools': {}}
@@ -494,21 +404,9 @@ class InterfaceAPI(object):
     def get_ips(self):
         return VolatileValue(self._if.get_addresses)
 
-    @deprecated
-    def get_ip_addr(self, ip_index=0):
-        return self.get_ip(ip_index)
-
-    @deprecated
-    def get_ip_addrs(self):
-        return self.get_ips()
-
     def get_prefix(self, ip_index=0):
         return VolatileValue(self._if.get_prefix, ip_index)
 
-    @deprecated
-    def get_ip_prefix(self, ip_index=0):
-        return self.get_prefix(ip_index)
-
     def get_mtu(self):
         return VolatileValue(self._if.get_mtu)
 
-- 
2.4.11

[PATCH v2 18/26] Task: add default param for get_alias()

[Jiri Prochazka]

since default value now won't be available in XML recipe,
we can move it to Python with optional default argument

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Task.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
index 8db51d4..8a4be60 100644
--- a/lnst/Controller/Task.py
+++ b/lnst/Controller/Task.py
@@ -83,7 +83,7 @@ class ControllerAPI(object):
         cmd = {"type": "ctl_wait", "seconds":
int(seconds)}
         return self._ctl._run_command(cmd)
 
-    def get_alias(self, alias):
+    def get_alias(self, alias, default=None):
         """
             Get the value of user defined alias.
 
@@ -94,9 +94,13 @@ class ControllerAPI(object):
             :rtype: string
         """
         try:
-            return self._ctl._get_alias(alias)
+            val =  self._ctl._get_alias(alias)
+            if val is None:
+                return default
+            else:
+                return val
         except XmlTemplateError:
-            return None
+            return default
 
     def get_aliases(self):
         """
-- 
2.4.11

[PATCH v2 19/26] Task: define TaskAPI methods on global level

[Jiri Prochazka]

in order to export TaskAPI methods to __init__.py,
we moved it to global level

currently supported - get_alias(), get_mreq(), wait(), get_module()

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Task.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
index 8a4be60..faca548 100644
--- a/lnst/Controller/Task.py
+++ b/lnst/Controller/Task.py
@@ -32,6 +32,18 @@ except:
 # The handle to be imported from each task
 ctl = None
 
+def get_alias(alias, default=None):
+    return ctl.get_alias(alias, default)
+
+def get_mreq():
+    return ctl.get_mreq()
+
+def wait(seconds):
+    return ctl.wait(seconds)
+
+def get_module(name, options={}):
+    return ctl.get_module(name, options)
+
 class TaskError(Exception): pass
 
 class ControllerAPI(object):
@@ -40,10 +52,14 @@ class ControllerAPI(object):
     def __init__(self, ctl, hosts):
         self._ctl = ctl
         self._result = True
+        self.mreq = {}
 
         self._perf_repo_api = PerfRepoAPI()
 
         self._hosts = {}
+
+    def get_mreq(self):
+        return self.mreq
         for host_id, host in hosts.iteritems():
             self._hosts[host_id] = HostAPI(self, host_id, host)
 
-- 
2.4.11

[PATCH v2 20/26] Task: add breakpoint()

[Jiri Prochazka]

this is new method for debugging, it pauses execution
of the test until and resumes after pressing enter

this functionality will be enriched in the future,
when we come up with full design

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Task.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
index faca548..cb52921 100644
--- a/lnst/Controller/Task.py
+++ b/lnst/Controller/Task.py
@@ -44,6 +44,11 @@ def wait(seconds):
 def get_module(name, options={}):
     return ctl.get_module(name, options)
 
+def breakpoint():
+    if ctl.get_run_mode != "config_only":
+        return
+    raw_input("Press enter to continue: ")
+
 class TaskError(Exception): pass
 
 class ControllerAPI(object):
-- 
2.4.11

[PATCH v2 21/26] Task: add add_host() and init_hosts()

[Jiri Prochazka]

add_host() is used in PyRecipe in setup phase, returns
HostAPI object and adds entry to mreq dictionary

id is generated by sequentially get_m_id()

init_hosts() is used to bind HostAPI objects with Machine objects,
because HostAPI objects are created sooner than Machine objects
(it was the other way around in XML recipes)

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Task.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
index cb52921..32e0db9 100644
--- a/lnst/Controller/Task.py
+++ b/lnst/Controller/Task.py
@@ -49,6 +49,13 @@ def breakpoint():
         return
     raw_input("Press enter to continue: ")
 
+def add_host(params=None):
+    m_id = ctl.gen_m_id()
+    ctl.mreq[m_id] = {'interfaces' : {}, 'params' : {}}
+    handle =  HostAPI(ctl, m_id)
+    ctl.add_host(m_id, handle)
+    return handle
+
 class TaskError(Exception): pass
 
 class ControllerAPI(object):
@@ -57,6 +64,7 @@ class ControllerAPI(object):
     def __init__(self, ctl, hosts):
         self._ctl = ctl
         self._result = True
+        self._m_id_seq = 0
         self.mreq = {}
 
         self._perf_repo_api = PerfRepoAPI()
@@ -65,8 +73,17 @@ class ControllerAPI(object):
 
     def get_mreq(self):
         return self.mreq
+
+    def gen_m_id(self):
+        self._m_id_seq += 1
+        return "m_id_%s" % self._m_id_seq
+
+    def add_host(self, host_id, handle):
+        self._hosts[host_id] = handle
+
+    def init_hosts(self, hosts):
         for host_id, host in hosts.iteritems():
-            self._hosts[host_id] = HostAPI(self, host_id, host)
+            self._hosts[host_id].init_host(host)
 
     def _run_command(self, command):
         """
-- 
2.4.11

[PATCH v2 22/26] Task: add interface handling TaskAPI methods

[Jiri Prochazka]

__init__() was changed as it now no longer requires Machine object

if_id_seq attribute was added for generating unique iface ids

_gen_if_id() generates unique if_id (host scope)

init_host() binds Machine object to HostAPI object and initializes interfaces

init_ifaces() binds Device objects to InterfaceAPI objects

add_interface() is setup phase TaskAPI method for adding eth interface to a host

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Task.py | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
index 32e0db9..d98e46a 100644
--- a/lnst/Controller/Task.py
+++ b/lnst/Controller/Task.py
@@ -198,18 +198,44 @@ class ControllerAPI(object):
 class HostAPI(object):
     """ An API class representing a host machine. """
 
-    def __init__(self, ctl, host_id, host):
+    def __init__(self, ctl, host_id):
         self._ctl = ctl
         self._id = host_id
-        self._m = host
+        self._m = None
 
         self._ifaces = {}
+        self._if_id_seq = 0
+        self._bg_id_seq = 0
+
+    def _gen_if_id(self):
+        self._if_id_seq += 1
+        return "if_id_%s" % self._if_id_seq
+
+    def init_host(self, host):
+        self._m = host
+        self.init_ifaces()
+
+    def init_ifaces(self):
         for interface in self._m.get_interfaces():
             if interface.get_id() is None:
                 continue
-            self._ifaces[interface.get_id()] = InterfaceAPI(interface, self)
+            self._ifaces[interface.get_id()].init_iface(interface)
 
-        self._bg_id_seq = 0
+    def add_interface(self, label, netns=None, params=None):
+        m_id = self.get_id()
+        if_id = self._gen_if_id()
+
+        self._ctl.mreq[m_id]['interfaces'][if_id] = {}
+        self._ctl.mreq[m_id]['interfaces'][if_id]['network'] = label
+        self._ctl.mreq[m_id]['interfaces'][if_id]['netns'] = netns
+
+        if params:
+            self._ctl.mreq[m_id]['interfaces'][if_id]['params'] = params
+        else:
+            self._ctl.mreq[m_id]['interfaces'][if_id]['params'] = {}
+
+        self._ifaces[if_id] = InterfaceAPI(None, self)
+        return self._ifaces[if_id]
 
     def get_id(self):
         return self._m.get_id()
@@ -422,6 +448,9 @@ class InterfaceAPI(object):
         self._if = interface
         self._host = host
 
+    def init_iface(self, interface):
+        self._if = interface
+
     def get_id(self):
         return self._if.get_id()
 
-- 
2.4.11

[PATCH v2 23/26] Task: HostAPI get_id now returns its generated id

[Jiri Prochazka]

machine object is not initialized in the time we need the ids

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Task.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
index d98e46a..c201bc0 100644
--- a/lnst/Controller/Task.py
+++ b/lnst/Controller/Task.py
@@ -238,7 +238,7 @@ class HostAPI(object):
         return self._ifaces[if_id]
 
     def get_id(self):
-        return self._m.get_id()
+        return self._id
 
     def config(self, option, value, persistent=False, netns=None):
         """
-- 
2.4.11

[PATCH v2 24/26] Task: add HostAPI methods required by PyRecipes

[Jiri Prochazka]

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Task.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
index c201bc0..4f9e23a 100644
--- a/lnst/Controller/Task.py
+++ b/lnst/Controller/Task.py
@@ -74,6 +74,24 @@ class ControllerAPI(object):
     def get_mreq(self):
         return self.mreq
 
+    def cleanup_slaves(self):
+       self._ctl._cleanup_slaves()
+
+    def get_run_mode(self):
+        return self._run_mode
+
+    def set_machine_requirements(self):
+        self._ctl.set_machine_requirements()
+
+    def packet_capture(self):
+        return self._ctl._packet_capture
+
+    def start_packet_capture(self):
+        self._ctl._start_packet_capture()
+
+    def prepare_test_env(self):
+        return self._ctl.prepare_test_env()
+
     def gen_m_id(self):
         self._m_id_seq += 1
         return "m_id_%s" % self._m_id_seq
-- 
2.4.11

[PATCH v2 25/26] Task: add match() and minor rework of ControllerAPI

[Jiri Prochazka]

ControllerAPI doesn't need hosts for __init__ now

new method match() - removes previous configuration from slaves,
if the run has first_run flag, then set_machine requirements

then it prepares the test environment meaning it runs the matching
algorithm and prepares network, hosts and interfaces for test run

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Controller/Task.py | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
index 4f9e23a..af9c201 100644
--- a/lnst/Controller/Task.py
+++ b/lnst/Controller/Task.py
@@ -56,14 +56,43 @@ def add_host(params=None):
     ctl.add_host(m_id, handle)
     return handle
 
+def match():
+    ctl.cleanup_slaves()
+
+    if ctl.first_run:
+        ctl.first_run = False
+        ctl.set_machine_requirements()
+
+        if ctl.prepare_test_env():
+            if ctl.get_run_mode == "match_setup":
+                return False
+            if ctl.packet_capture():
+                ctl.start_packet_capture()
+            return True
+    else:
+        if ctl._ctl._multi_match:
+            if ctl.prepare_test_env():
+                if ctl.get_run_mode == "match_setup":
+                    return False
+                if ctl.packet_capture():
+                    ctl.start_packet_capture()
+                return True
+            else:
+                return False
+        else:
+            return False
+
+
 class TaskError(Exception): pass
 
 class ControllerAPI(object):
     """ An API class representing the controller. """
 
-    def __init__(self, ctl, hosts):
+    def __init__(self, ctl):
         self._ctl = ctl
+        self._run_mode = ctl._get_run_mode()
         self._result = True
+        self.first_run = True
         self._m_id_seq = 0
         self.mreq = {}
 
-- 
2.4.11

[PATCH v2 26/26] PyRecipes: add example PyRecipes

[Jiri Prochazka]

Description:
  3_vlans.py:
    2 hosts with 1 eth iface
    3 VLAN subifaces on both eth ifaces
    ping from m1_eth.10 to m2_eth.10

  example.py:
    2 hosts, one with 2 eth ifaces, one with 1 eth iface
    team created on host
    ping from m1_team to m2_eth

  ping_flood.py:
    recipe/regression-tests/phase1/ping_flood.* like

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 pyrecipes/3_vlans.py    | 34 ++++++++++++++++++++++++++++++++++
 pyrecipes/example.py    | 33 +++++++++++++++++++++++++++++++++
 pyrecipes/ping_flood.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+)
 create mode 100644 pyrecipes/3_vlans.py
 create mode 100644 pyrecipes/example.py
 create mode 100644 pyrecipes/ping_flood.py

diff --git a/pyrecipes/3_vlans.py b/pyrecipes/3_vlans.py
new file mode 100644
index 0000000..067fd33
--- /dev/null
+++ b/pyrecipes/3_vlans.py
@@ -0,0 +1,34 @@
+import lnst
+
+def ping_mod_init(if1, if2):
+    ping_mod = lnst.get_module("IcmpPing",
+                               options={
+                                  "addr": if2.get_ip(0),
+                                  "count": 10,
+                                  "interval": 1,
+                                  "iface" : if1.get_devname()})
+    return ping_mod
+
+m1 = lnst.add_host()
+m2 = lnst.add_host()
+
+m1_eth1 = m1.add_interface(label="tnet")
+m2_eth1 = m2.add_interface(label="tnet")
+
+while lnst.match():
+    m1.sync_resources(modules=["IcmpPing"])
+    m2.sync_resources(modules=["IcmpPing"])
+
+    m1_vlan10 = m1.create_vlan(realdev_iface=m1_eth1, vlan_tci="10",
ip="192.168.10.1/24")
+    m1_vlan20 = m1.create_vlan(realdev_iface=m1_eth1, vlan_tci="20",
ip="192.168.20.1/24")
+    m1_vlan30 = m1.create_vlan(realdev_iface=m1_eth1, vlan_tci="30",
ip="192.168.30.1/24")
+
+    m2_vlan10 = m2.create_vlan(realdev_iface=m2_eth1, vlan_tci="10",
ip="192.168.10.2/24")
+    m2_vlan20 = m2.create_vlan(realdev_iface=m2_eth1, vlan_tci="20",
ip="192.168.20.2/24")
+    m2_vlan30 = m2.create_vlan(realdev_iface=m2_eth1, vlan_tci="30",
ip="192.168.30.2/24")
+
+    ping_mod = ping_mod_init(m1_vlan10, m2_vlan10)
+    ping_mod_bad = ping_mod_init(m1_vlan10, m2_vlan20)
+
+    m1.run(ping_mod)
+    #m1.run(ping_mod_bad)
diff --git a/pyrecipes/example.py b/pyrecipes/example.py
new file mode 100644
index 0000000..32c1b08
--- /dev/null
+++ b/pyrecipes/example.py
@@ -0,0 +1,33 @@
+import lnst
+
+# if1 ... src
+# if2 ... dst
+def ping_mod_init(if1, if2):
+    ping_mod = lnst.get_module("IcmpPing",
+                               options={
+                                  "addr": if2.get_ip(0),
+                                  "count": 10,
+                                  "interval": 1,
+                                  "iface" : if1.get_devname()})
+    return ping_mod
+
+m1 = lnst.add_host()
+m2 = lnst.add_host()
+
+m1_eth1 = m1.add_interface(label="tnet")
+m1_eth2 = m1.add_interface(label="tnet")
+m2_eth1 = m2.add_interface(label="tnet")
+
+while lnst.match():
+    m1.sync_resources(modules=["IcmpPing"])
+    m2.sync_resources(modules=["IcmpPing"])
+    m2_eth1.reset(ip="192.168.0.2/24")
+    ping = ping_mod_init(m1_eth1, m2_eth1)
+    #lnst.breakpoint()
+    team_if = m1.create_team(slaves=[m1_eth1, m1_eth2])
+    #lnst.breakpoint()
+    team_if.reset(ip="192.168.0.1/24")
+    #lnst.breakpoint()
+    ping = ping_mod_init(team_if, m2_eth1)
+    #lnst.breakpoint()
+    m1.run(ping)
diff --git a/pyrecipes/ping_flood.py b/pyrecipes/ping_flood.py
new file mode 100644
index 0000000..e65879d
--- /dev/null
+++ b/pyrecipes/ping_flood.py
@@ -0,0 +1,48 @@
+import lnst
+
+m1 = lnst.add_host()
+m2 = lnst.add_host()
+
+m1_eth1 = m1.add_interface(label="tnet")
+m2_eth1 = m2.add_interface(label="tnet")
+
+while lnst.match():
+    m1.sync_resources(modules=["Icmp6Ping", "IcmpPing"])
+    m2.sync_resources(modules=["Icmp6Ping", "IcmpPing"])
+
+    lnst.wait(15)
+
+    ipv = lnst.get_alias("ipv", default="ipv4")
+    print "ipv"
+    print ipv
+    mtu = lnst.get_alias("mtu", default="1500")
+    print "mtu"
+    print mtu
+
+    m1_eth1.reset(ip=["192.168.101.10/24", "fc00:0:0:0::1/64"])
+    m2_eth1.reset(ip=["192.168.101.11/24", "fc00:0:0:0::2/64"])
+       
+    ping_mod = lnst.get_module("IcmpPing",
+                               options={
+                                  "addr": m2_eth1.get_ip(0),
+                                  "count": 10,
+                                  "interval": 0.1,
+                                  "iface" : m1_eth1.get_devname(),
+                                  "limit_rate": 90})
+
+    ping_mod6 = lnst.get_module("Icmp6Ping",
+                               options={
+                                  "addr": m2_eth1.get_ip(1),
+                                  "count": 10,
+                                  "interval": 0.1,
+                                  "iface" : m1_eth1.get_devname(),
+                                  "limit_rate": 90})
+ 
+    m1_eth1.set_mtu(mtu)
+    m2_eth1.set_mtu(mtu)
+
+    if ipv in [ 'ipv6', 'both' ]:
+        m1.run(ping_mod6)
+
+    if ipv in ['ipv4', 'both' ]:
+        m1.run(ping_mod)
-- 
2.4.11

[Ido Schimmel]

Hi Jiri,

Fri, Apr 22, 2016 at 11:51:58AM IDT, jprochaz(a)redhat.com wrote:
> Hello everyone,
>
> first working draft implementation is completed. Please, read this cover
> letter, check the patches and let me know what you like, what you don't like,
> etc. Please, note, that this is not final implementation.
>
> NOTE - patches are not rebased to current HEAD, if you want to try it,
> please, reset your branch to commit 2398f93.
>
> What doesn't work
> =================
> multi_match - didn't have time and resources to check physical machines,
> on virt setup it works, but ends with exception due to no res value returned
> loopbacks - can't be created
> netem support - not implemented yet 
Good to see that it's coming along :) I'm unclear about something
though. When applied, will this patchset break functionality of existing
recipes in recipes/ or will this just add another way to construct
recipes?

Thanks!

[Jiri Prochazka]

Hi Ido,
please note, that this is only a draft and not a final product.

But PyRecipes implementation will unfortunately break old XML recipes.
This should be discussed on LNST-dev meeting, my idea is, that when
implementation is ready, we push it to new branch and rework all tests
in recipes/ dir to this implementation, as well as regression-tests.

Then, when everything will be up and running and tested thoroughly,
we can merge the branch with upstream.

Have a nice day,
Jiri

2016-04-23 18:25 GMT+02:00 Ido Schimmel <idosch(a)mellanox.com&gt;:
> Hi Jiri,
>
> Fri, Apr 22, 2016 at 11:51:58AM IDT, jprochaz(a)redhat.com wrote:
>>Hello everyone,
>>
>>first working draft implementation is completed. Please, read this cover
>>letter, check the patches and let me know what you like, what you don't like,
>>etc. Please, note, that this is not final implementation.
>>
>>NOTE - patches are not rebased to current HEAD, if you want to try it,
>>please, reset your branch to commit 2398f93.
>>
>>What doesn't work
>>=================
>>multi_match - didn't have time and resources to check physical machines,
>>on virt setup it works, but ends with exception due to no res value returned
>>loopbacks - can't be created
>>netem support - not implemented yet
>
> Good to see that it's coming along :) I'm unclear about something
> though. When applied, will this patchset break functionality of existing
> recipes in recipes/ or will this just add another way to construct
> recipes?
>
> Thanks!

[PATCH v2 04/26] NetTestController: add run_mode attribute

[Ondrej Lichtner]

On Fri, Apr 22, 2016 at 10:52:02AM +0200, Jiri Prochazka wrote:
>  this attribute is set from lnst-ctl and is used in
>  TaskAPI match() where it stops executing
>  task phase in match_setup run mode
>  
>  also it's used in TaskAPI breakpoint(),
>  because lnst will break only in config_only run_mode
>  and in run mode it's ignored
>  
>  it was added because all run modes need to execute python
>  recipe for which they all use _run_python_task() and this
>  seemd like best approach
>  
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  ---
>   lnst/Controller/NetTestController.py | 7 +++++++
>   1 file changed, 7 insertions(+)
>  
>  diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
>  index 39e2950..3661f77 100644
>  --- a/lnst/Controller/NetTestController.py
>  +++ b/lnst/Controller/NetTestController.py
>  @@ -65,6 +65,8 @@ class NetTestController:
>   
>           self.remove_saved_machine_config()
>   
>  +        self.run_mode = "run"
>  +
>           self._machines = {}
>           self._network_bridges = {}
>           self._tasks = []
>  @@ -98,6 +100,9 @@ class NetTestController:
>           self._resource_table["module"] =
> self._load_test_modules(modules_dirs)
>           self._resource_table["tools"] = self._load_test_tools(tools_dirs)
>   
>  +    def _get_run_mode(self):
>  +        return self.run_mode
>  + 
Why make a private get method? Inside the class you can just access the
attribute... from other objects/classes you shouldn't use private
methods... And the reverse goes for the attribute, make it private...

>      def _get_machineinfo(self, machine_id):
>          try:
>              info =
self._recipe["machines"][machine_id]["params"]
> @@ -608,9 +613,11 @@ class NetTestController:
>              os.remove("/tmp/.lnst_machine_conf")
>  
>      def match_setup(self):
> +        self.run_mode = "match_setup"
>          return {"passed": True}
>  
>      def config_only_recipe(self):
> +        self.run_mode = "config_only"
>          try:
>              self._prepare_network(resource_sync=False)
>          except (KeyboardInterrupt, Exception) as exc:
> -- 
> 2.4.11
> _______________________________________________
> LNST-developers mailing list
> lnst-developers(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[PATCH v2 08/26] NetTestController: cleanup _preapre_interface()

[Ondrej Lichtner]

On Fri, Apr 22, 2016 at 10:52:06AM +0200, Jiri Prochazka wrote:
>  support for loopback devices has been temporarily removed
>  support for netem has been temporarily removed
>  
>  we should consider if we want to define IP address of eth
>  interface in setup phase or if we want to keep it in task
>  
>  _prepare_interface() now works only with eth ifaces
>  
>  soft interfaces are created dynamically in task phase so
>  if statements can be removed from this method
>  
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  ---
>   lnst/Controller/NetTestController.py | 89 +++++++++++++++++-------------------
>   1 file changed, 41 insertions(+), 48 deletions(-)
>  
>  diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
>  index a4b48f0..fd54a09 100644
>  --- a/lnst/Controller/NetTestController.py
>  +++ b/lnst/Controller/NetTestController.py
>  @@ -239,55 +239,48 @@ class NetTestController:
>                               raise RecipeError(msg, cmd)
>           machine.sync_resources(sync_table)
>   
>  -    def _prepare_interface(self, m_id, iface_xml_data):
>  +    def _prepare_interface(self, m_id, if_id, iface_data):
>           machine = self._machines[m_id]
>  -        if_id = iface_xml_data["id"]
>  -        if_type = iface_xml_data["type"]
>  -
>  -        try:
>  -            iface = machine.get_interface(if_id)
>  -        except MachineError:
>  -            if if_type == 'lo':
>  -                iface = machine.new_loopback_interface(if_id)
>  -            else:
>  -                iface = machine.new_soft_interface(if_id, if_type)
>  -
>  -        if "slaves" in iface_xml_data:
>  -            for slave in iface_xml_data["slaves"]:
>  -                slave_id = slave["id"]
>  -                iface.add_slave(machine.get_interface(slave_id))
>  -
>  -                # Some soft devices (such as team) use per-slave options
>  -                if "options" in slave:
>  -                    for opt in slave["options"]:
>  -                        iface.set_slave_option(slave_id, opt["name"],
>  -                                               opt["value"])
>  -
>  -        if "addresses" in iface_xml_data:
>  -            for addr in iface_xml_data["addresses"]:
>  -                iface.add_address(addr)
>  -
>  -        if "options" in iface_xml_data:
>  -            for opt in iface_xml_data["options"]:
>  -                iface.set_option(opt["name"], opt["value"])
>  -
>  -        if "netem" in iface_xml_data:
>  -            iface.set_netem(iface_xml_data["netem"].to_dict())
>  -
>  -        if "ovs_conf" in iface_xml_data:
>  -            iface.set_ovs_conf(iface_xml_data["ovs_conf"].to_dict())
>  -
>  -        if iface_xml_data["netns"] != None:
>  -            iface.set_netns(iface_xml_data["netns"])
>  -
>  -        if "peer" in iface_xml_data:
>  -            iface.set_peer(iface_xml_data["peer"])
>  -
>  -
>  -
>  -
>  -
>  -
>  +        #if_type = iface_data["type"]
>  +
>  +        #try:
>  +        iface = machine.get_interface(if_id)
>  +        #except MachineError:
>  +            #if if_type == 'lo':
>  +            #    iface = machine.new_loopback_interface(if_id)
>  +            #else:
>  +            #    iface = machine.new_soft_interface(if_id, if_type)
>  +
>  +        #if "slaves" in iface_data:
>  +        #    for slave in iface_data["slaves"]:
>  +        #        slave_id = slave["id"]
>  +        #        iface.add_slave(machine.get_interface(slave_id))
>  +
>  +        #        # Some soft devices (such as team) use per-slave options
>  +        #        if "options" in slave:
>  +        #            for opt in slave["options"]:
>  +        #                iface.set_slave_option(slave_id, opt["name"],
>  +        #                                       opt["value"])
>  +
>  +        #if "addresses" in iface_data:
>  +        #    for addr in iface_data["addresses"]:
>  +        #        iface.add_address(addr)
>  +
>  +        #if "options" in iface_data:
>  +        #    for opt in iface_data["options"]:
>  +        #        iface.set_option(opt["name"], opt["value"])
>  +
>  +        #if "netem" in iface_data:
>  +        #    iface.set_netem(iface_data["netem"].to_dict())
>  +
>  +        #if "ovs_conf" in iface_data:
>  +        #    iface.set_ovs_conf(iface_data["ovs_conf"].to_dict())
>  +
>  +        if iface_data["netns"] != None:
>  +            iface.set_netns(iface_data["netns"])
>  +
>  +        #if "peer" in iface_data:
>  +        #    iface.set_peer(iface_data["peer"]) 
Why do we want all of this commented code here?

>  
>      def _prepare_command(self, cmd_data):
>          cmd = {"type": cmd_data["type"]}
> -- 
> 2.4.11
> _______________________________________________
> LNST-developers mailing list
> lnst-developers(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[PATCH v2 19/26] Task: define TaskAPI methods on global level

[Ondrej Lichtner]

On Fri, Apr 22, 2016 at 10:52:17AM +0200, Jiri Prochazka wrote:
>  in order to export TaskAPI methods to __init__.py,
>  we moved it to global level
>  
>  currently supported - get_alias(), get_mreq(), wait(), get_module()
>  
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  ---
>   lnst/Controller/Task.py | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>  
>  diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
>  index 8a4be60..faca548 100644
>  --- a/lnst/Controller/Task.py
>  +++ b/lnst/Controller/Task.py
>  @@ -32,6 +32,18 @@ except:
>   # The handle to be imported from each task
>   ctl = None
>   
>  +def get_alias(alias, default=None):
>  +    return ctl.get_alias(alias, default)
>  +
>  +def get_mreq():
>  +    return ctl.get_mreq()
>  +
>  +def wait(seconds):
>  +    return ctl.wait(seconds)
>  +
>  +def get_module(name, options={}):
>  +    return ctl.get_module(name, options)
>  +
>   class TaskError(Exception): pass
>   
>   class ControllerAPI(object):
>  @@ -40,10 +52,14 @@ class ControllerAPI(object):
>       def __init__(self, ctl, hosts):
>           self._ctl = ctl
>           self._result = True
>  +        self.mreq = {}
>   
>           self._perf_repo_api = PerfRepoAPI()
>   
>           self._hosts = {}
>  +
>  +    def get_mreq(self):
>  +        return self.mreq 
You've placed this get method in the middle of the __init__ method...
that's not gonna work...

>          for host_id, host in hosts.iteritems():
>              self._hosts[host_id] = HostAPI(self, host_id, host)
>  
> -- 
> 2.4.11
> _______________________________________________
> LNST-developers mailing list
> lnst-developers(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[PATCH v2 13/26] NetTestController: add prepare_test_env()

[Ondrej Lichtner]

On Fri, Apr 22, 2016 at 10:52:11AM +0200, Jiri Prochazka wrote:
>  this method is run from Task match(), it runs
>  matching algorithm, prints match desc, when
>  match_setup run mode is used it ends here
>  
>  otherwise it prepares network, binds Machine objects
>  to its HostAPI counterparts and Device objects to
>  its InterfaceAPI counterparts
>  
>  summary - it returns True if it found match and if
>  run or config_only mode is used, it also prepares network
>  and initializes HostAPI and InterfaceAPI objects created
>  in setup phase
>  
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  ---
>   lnst/Controller/NetTestController.py | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
>  
>  diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
>  index 8c1d24f..3e7eb69 100644
>  --- a/lnst/Controller/NetTestController.py
>  +++ b/lnst/Controller/NetTestController.py
>  @@ -496,6 +496,25 @@ class NetTestController:
>   
>           return res
>   
>  +    def prepare_test_env(self):
>  +        try:
>  +            self.provision_machines()
>  +            self.print_match_description()
>  +            if self.run_mode == "match_setup":
>  +                return True
>  +            self._prepare_network()
>  +            Task.ctl.init_hosts(self._machines)
>  +            return True
>  +        except (NoMatchError) as exc:
>  +            self._cleanup_slaves()
>  +            raise exc
>  +            return False 
You reraise the exception here and the try to return something after
that... I'm surprised that pylint didn't complain about that...

> +        except (KeyboardInterrupt, Exception) as exc:
> +            msg = "Exception raised during configuration."
> +            logging.error(msg)
> +            self._cleanup_slaves()
> +            raise
> +
>      def _run_recipe(self):
>          overall_res = {"passed": True}
>  
> -- 
> 2.4.11
> _______________________________________________
> LNST-developers mailing list
> lnst-developers(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[Ondrej Lichtner]

On Fri, Apr 22, 2016 at 10:51:58AM +0200, Jiri Prochazka wrote:
>  Hello everyone,
>  
>  first working draft implementation is completed. Please, read this cover
>  letter, check the patches and let me know what you like, what you don't like,
>  etc. Please, note, that this is not final implementation.
>  
>  NOTE - patches are not rebased to current HEAD, if you want to try it,
>  please, reset your branch to commit 2398f93.
>  
>  What doesn't work
>  =================
>  multi_match - didn't have time and resources to check physical machines,
>  on virt setup it works, but ends with exception due to no res value returned
>  loopbacks - can't be created
>  netem support - not implemented yet
>  
>  Description of new mechanics
>  ============================
>  Since we need to run TaskAPI methods to get machine requirements from PyRecipe,
>  methods like provision_machines, prepare_network are run after the python recipe
>  is executed. Methods add_host and add_interface create machine requirements dict
>  in ControllerAPI object. When lnst.match() is called, it runs match algorithm
>  with mreq dict and if match is found, it prepares network and binds Machine
>  and Interface objects with their HostAPI and InterfaceAPI counterparts.
>  When everyting is prepared, execution returns to PyRecipe, where task phase
>  of PyRecipe is executed. Task phase is the same like before, only new method
>  is breakpoint() which will be useful for debugging, as it allows user to pause
>  the execution in whatever part of test.
>  TaskAPI is now used right from lnst module, all callable methods are exported
>  via __init__.py file from lnst/ dir.
>  Folder pyrecipes/ contains few working examples you can try.
>  
>  TODO
>  ====
>  * multimatch support
>  * config_only mode should be renamed and polished
>  * TaskAPI create_ovs() method
>  * support for loopbacks
>  * NetEm support
>  * polishing of the code
>  
>  Jiri Prochazka (26):
>    __init__.py: draft for PyRecipes
>    lnst-ctl: draft for PyRecipes
>    NetTestController: remove RecipeParser import and related method calls
>    NetTestController: add run_mode attribute
>    NetTestController: remove obsolete methods
>    NetTestController: use mreq dict from PyRecipe
>    NetTestController: split provision_machines() in two methods
>    NetTestController: cleanup _preapre_interface()
>    NetTestController: remove resource sync from _prepare_machine()
>    NetTestController: add multi_match attribute ti NetTestController
>    NetTestController: don't call abs_path on _recipe_path in __init__
>    NetTestController: rework alias handling
>    NetTestController: add prepare_test_env()
>    NetTestController: rework match_setup(), config_only_recipe(),
>      run_recipe(), _run_python_task()
>    NetTestController: add init_taskapi()
>    NetTestController: use Task name instead of module
>    Task: remove deprecated methods
>    Task: add default param for get_alias()
>    Task: define TaskAPI methods on global level
>    Task: add breakpoint()
>    Task: add add_host() and init_hosts()
>    Task: add interface handling TaskAPI methods
>    Task: HostAPI get_id now returns its generated id
>    Task: add HostAPI methods required by PyRecipes
>    Task: add match() and minor rework of ControllerAPI
>    PyRecipes: add example PyRecipes
>  
>   lnst-ctl                             |  16 +-
>   lnst/Controller/NetTestController.py | 398 ++++++++++-------------------------
>   lnst/Controller/Task.py              | 240 +++++++++++----------
>   lnst/__init__.py                     |   1 +
>   pyrecipes/3_vlans.py                 |  34 +++
>   pyrecipes/example.py                 |  33 +++
>   pyrecipes/ping_flood.py              |  48 +++++
>   7 files changed, 363 insertions(+), 407 deletions(-)
>   create mode 100644 pyrecipes/3_vlans.py
>   create mode 100644 pyrecipes/example.py
>   create mode 100644 pyrecipes/ping_flood.py
>  
>  -- 
>  2.4.11
>  _______________________________________________
>  LNST-developers mailing list
>  lnst-developers(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/admin/lists/[e-mail]... 

Overall, in addition to the small comments that I've sent for the
obvious errors I want to add that I don't like how this patchset is
organized.

It first starts adding some references to code that doesn't exist yet.
The it removes the old code while the new one isn't there yet. And only
starts adding the rest later, still introducing references to other
still nonexistant parts of the new code.

There's two problems with this approach:
* It makes the patchset very hard to follow and pretty confusing.
* It breaks git bisect functionality

I realize that this is a big feature change and we still need to discuss
how we're going to move forward with it since it completely breaks
backwards compatibility...

But it's exactly because it's such a big change that I think that the
patchset should be organized in such a way that it's as easy as possible to
understand/follow.

-Ondrej

[PATCH v2 08/26] NetTestController: cleanup _preapre_interface()

[Jiri Prochazka]

2016-05-16 8:37 GMT+02:00 Ondrej Lichtner <olichtne(a)redhat.com&gt;:
>  On Fri, Apr 22, 2016 at 10:52:06AM +0200, Jiri Prochazka wrote:
> > support for loopback devices has been temporarily removed
> > support for netem has been temporarily removed
> >
> > we should consider if we want to define IP address of eth
> > interface in setup phase or if we want to keep it in task
> >
> > _prepare_interface() now works only with eth ifaces
> >
> > soft interfaces are created dynamically in task phase so
> > if statements can be removed from this method
> >
> > Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> > ---
> >  lnst/Controller/NetTestController.py | 89 +++++++++++++++++-------------------
> >  1 file changed, 41 insertions(+), 48 deletions(-)
> >
> > diff --git a/lnst/Controller/NetTestController.py
> b/lnst/Controller/NetTestController.py
> > index a4b48f0..fd54a09 100644
> > --- a/lnst/Controller/NetTestController.py
> > +++ b/lnst/Controller/NetTestController.py
> > @@ -239,55 +239,48 @@ class NetTestController:
> >                              raise RecipeError(msg, cmd)
> >          machine.sync_resources(sync_table)
> >
> > -    def _prepare_interface(self, m_id, iface_xml_data):
> > +    def _prepare_interface(self, m_id, if_id, iface_data):
> >          machine = self._machines[m_id]
> > -        if_id = iface_xml_data["id"]
> > -        if_type = iface_xml_data["type"]
> > -
> > -        try:
> > -            iface = machine.get_interface(if_id)
> > -        except MachineError:
> > -            if if_type == 'lo':
> > -                iface = machine.new_loopback_interface(if_id)
> > -            else:
> > -                iface = machine.new_soft_interface(if_id, if_type)
> > -
> > -        if "slaves" in iface_xml_data:
> > -            for slave in iface_xml_data["slaves"]:
> > -                slave_id = slave["id"]
> > -                iface.add_slave(machine.get_interface(slave_id))
> > -
> > -                # Some soft devices (such as team) use per-slave options
> > -                if "options" in slave:
> > -                    for opt in slave["options"]:
> > -                        iface.set_slave_option(slave_id, opt["name"],
> > -                                               opt["value"])
> > -
> > -        if "addresses" in iface_xml_data:
> > -            for addr in iface_xml_data["addresses"]:
> > -                iface.add_address(addr)
> > -
> > -        if "options" in iface_xml_data:
> > -            for opt in iface_xml_data["options"]:
> > -                iface.set_option(opt["name"], opt["value"])
> > -
> > -        if "netem" in iface_xml_data:
> > -            iface.set_netem(iface_xml_data["netem"].to_dict())
> > -
> > -        if "ovs_conf" in iface_xml_data:
> > -            iface.set_ovs_conf(iface_xml_data["ovs_conf"].to_dict())
> > -
> > -        if iface_xml_data["netns"] != None:
> > -            iface.set_netns(iface_xml_data["netns"])
> > -
> > -        if "peer" in iface_xml_data:
> > -            iface.set_peer(iface_xml_data["peer"])
> > -
> > -
> > -
> > -
> > -
> > -
> > +        #if_type = iface_data["type"]
> > +
> > +        #try:
> > +        iface = machine.get_interface(if_id)
> > +        #except MachineError:
> > +            #if if_type == 'lo':
> > +            #    iface = machine.new_loopback_interface(if_id)
> > +            #else:
> > +            #    iface = machine.new_soft_interface(if_id, if_type)
> > +
> > +        #if "slaves" in iface_data:
> > +        #    for slave in iface_data["slaves"]:
> > +        #        slave_id = slave["id"]
> > +        #        iface.add_slave(machine.get_interface(slave_id))
> > +
> > +        #        # Some soft devices (such as team) use per-slave options
> > +        #        if "options" in slave:
> > +        #            for opt in slave["options"]:
> > +        #                iface.set_slave_option(slave_id, opt["name"],
> > +        #                                       opt["value"])
> > +
> > +        #if "addresses" in iface_data:
> > +        #    for addr in iface_data["addresses"]:
> > +        #        iface.add_address(addr)
> > +
> > +        #if "options" in iface_data:
> > +        #    for opt in iface_data["options"]:
> > +        #        iface.set_option(opt["name"], opt["value"])
> > +
> > +        #if "netem" in iface_data:
> > +        #    iface.set_netem(iface_data["netem"].to_dict())
> > +
> > +        #if "ovs_conf" in iface_data:
> > +        #    iface.set_ovs_conf(iface_data["ovs_conf"].to_dict())
> > +
> > +        if iface_data["netns"] != None:
> > +            iface.set_netns(iface_data["netns"])
> > +
> > +        #if "peer" in iface_data:
> > +        #    iface.set_peer(iface_data["peer"])
>
>  Why do we want all of this commented code here? 
It's just for me so I don't forget about stuff like netem and loopback
interfaces support, I should've erase it, sorry for confusion

>>
>>      def _prepare_command(self, cmd_data):
>>          cmd = {"type": cmd_data["type"]}
>> --
>> 2.4.11
>> _______________________________________________
>> LNST-developers mailing list
>> lnst-developers(a)lists.fedorahosted.org
>> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[PATCH v2 04/26] NetTestController: add run_mode attribute

[Jiri Prochazka]

2016-05-16 8:35 GMT+02:00 Ondrej Lichtner <olichtne(a)redhat.com&gt;:
>  On Fri, Apr 22, 2016 at 10:52:02AM +0200, Jiri Prochazka wrote:
> > this attribute is set from lnst-ctl and is used in
> > TaskAPI match() where it stops executing
> > task phase in match_setup run mode
> >
> > also it's used in TaskAPI breakpoint(),
> > because lnst will break only in config_only run_mode
> > and in run mode it's ignored
> >
> > it was added because all run modes need to execute python
> > recipe for which they all use _run_python_task() and this
> > seemd like best approach
> >
> > Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> > ---
> >  lnst/Controller/NetTestController.py | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/lnst/Controller/NetTestController.py
> b/lnst/Controller/NetTestController.py
> > index 39e2950..3661f77 100644
> > --- a/lnst/Controller/NetTestController.py
> > +++ b/lnst/Controller/NetTestController.py
> > @@ -65,6 +65,8 @@ class NetTestController:
> >
> >          self.remove_saved_machine_config()
> >
> > +        self.run_mode = "run"
> > +
> >          self._machines = {}
> >          self._network_bridges = {}
> >          self._tasks = []
> > @@ -98,6 +100,9 @@ class NetTestController:
> >          self._resource_table["module"] =
> self._load_test_modules(modules_dirs)
> >          self._resource_table["tools"] = self._load_test_tools(tools_dirs)
> >
> > +    def _get_run_mode(self):
> > +        return self.run_mode
> > +
>
>  Why make a private get method? Inside the class you can just access the
>  attribute... from other objects/classes you shouldn't use private
>  methods... And the reverse goes for the attribute, make it private... 
Acknowledged, thanks for remark

>>      def _get_machineinfo(self, machine_id):
>>          try:
>>              info =
self._recipe["machines"][machine_id]["params"]
>> @@ -608,9 +613,11 @@ class NetTestController:
>>              os.remove("/tmp/.lnst_machine_conf")
>>
>>      def match_setup(self):
>> +        self.run_mode = "match_setup"
>>          return {"passed": True}
>>
>>      def config_only_recipe(self):
>> +        self.run_mode = "config_only"
>>          try:
>>              self._prepare_network(resource_sync=False)
>>          except (KeyboardInterrupt, Exception) as exc:
>> --
>> 2.4.11
>> _______________________________________________
>> LNST-developers mailing list
>> lnst-developers(a)lists.fedorahosted.org
>> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[PATCH v2 19/26] Task: define TaskAPI methods on global level

[Jiri Prochazka]

2016-05-16 8:39 GMT+02:00 Ondrej Lichtner <olichtne(a)redhat.com&gt;:
>  On Fri, Apr 22, 2016 at 10:52:17AM +0200, Jiri Prochazka wrote:
> > in order to export TaskAPI methods to __init__.py,
> > we moved it to global level
> >
> > currently supported - get_alias(), get_mreq(), wait(), get_module()
> >
> > Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> > ---
> >  lnst/Controller/Task.py | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
> > index 8a4be60..faca548 100644
> > --- a/lnst/Controller/Task.py
> > +++ b/lnst/Controller/Task.py
> > @@ -32,6 +32,18 @@ except:
> >  # The handle to be imported from each task
> >  ctl = None
> >
> > +def get_alias(alias, default=None):
> > +    return ctl.get_alias(alias, default)
> > +
> > +def get_mreq():
> > +    return ctl.get_mreq()
> > +
> > +def wait(seconds):
> > +    return ctl.wait(seconds)
> > +
> > +def get_module(name, options={}):
> > +    return ctl.get_module(name, options)
> > +
> >  class TaskError(Exception): pass
> >
> >  class ControllerAPI(object):
> > @@ -40,10 +52,14 @@ class ControllerAPI(object):
> >      def __init__(self, ctl, hosts):
> >          self._ctl = ctl
> >          self._result = True
> > +        self.mreq = {}
> >
> >          self._perf_repo_api = PerfRepoAPI()
> >
> >          self._hosts = {}
> > +
> > +    def get_mreq(self):
> > +        return self.mreq
>
>  You've placed this get method in the middle of the __init__ method...
>  that's not gonna work... 
It's fixed in following patches, must've missed that while rebasing
the commits, thanks for catching it!

>
>>          for host_id, host in hosts.iteritems():
>>              self._hosts[host_id] = HostAPI(self, host_id, host)
>>
>> --
>> 2.4.11
>> _______________________________________________
>> LNST-developers mailing list
>> lnst-developers(a)lists.fedorahosted.org
>> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[PATCH v2 13/26] NetTestController: add prepare_test_env()

[Jiri Prochazka]

2016-05-16 8:42 GMT+02:00 Ondrej Lichtner <olichtne(a)redhat.com&gt;:
>  On Fri, Apr 22, 2016 at 10:52:11AM +0200, Jiri Prochazka wrote:
> > this method is run from Task match(), it runs
> > matching algorithm, prints match desc, when
> > match_setup run mode is used it ends here
> >
> > otherwise it prepares network, binds Machine objects
> > to its HostAPI counterparts and Device objects to
> > its InterfaceAPI counterparts
> >
> > summary - it returns True if it found match and if
> > run or config_only mode is used, it also prepares network
> > and initializes HostAPI and InterfaceAPI objects created
> > in setup phase
> >
> > Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> > ---
> >  lnst/Controller/NetTestController.py | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/lnst/Controller/NetTestController.py
> b/lnst/Controller/NetTestController.py
> > index 8c1d24f..3e7eb69 100644
> > --- a/lnst/Controller/NetTestController.py
> > +++ b/lnst/Controller/NetTestController.py
> > @@ -496,6 +496,25 @@ class NetTestController:
> >
> >          return res
> >
> > +    def prepare_test_env(self):
> > +        try:
> > +            self.provision_machines()
> > +            self.print_match_description()
> > +            if self.run_mode == "match_setup":
> > +                return True
> > +            self._prepare_network()
> > +            Task.ctl.init_hosts(self._machines)
> > +            return True
> > +        except (NoMatchError) as exc:
> > +            self._cleanup_slaves()
> > +            raise exc
> > +            return False
>
>  You reraise the exception here and the try to return something after
>  that... I'm surprised that pylint didn't complain about that... 
Forgot it there from debugging, sorry for that

>
>> +        except (KeyboardInterrupt, Exception) as exc:
>> +            msg = "Exception raised during configuration."
>> +            logging.error(msg)
>> +            self._cleanup_slaves()
>> +            raise
>> +
>>      def _run_recipe(self):
>>          overall_res = {"passed": True}
>>
>> --
>> 2.4.11
>> _______________________________________________
>> LNST-developers mailing list
>> lnst-developers(a)lists.fedorahosted.org
>> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[Jiri Prochazka]

2016-05-16 8:47 GMT+02:00 Ondrej Lichtner <olichtne(a)redhat.com&gt;:
>  On Fri, Apr 22, 2016 at 10:51:58AM +0200, Jiri Prochazka wrote:
> > Hello everyone,
> >
> > first working draft implementation is completed. Please, read this cover
> > letter, check the patches and let me know what you like, what you don't like,
> > etc. Please, note, that this is not final implementation.
> >
> > NOTE - patches are not rebased to current HEAD, if you want to try it,
> > please, reset your branch to commit 2398f93.
> >
> > What doesn't work
> > =================
> > multi_match - didn't have time and resources to check physical machines,
> > on virt setup it works, but ends with exception due to no res value returned
> > loopbacks - can't be created
> > netem support - not implemented yet
> >
> > Description of new mechanics
> > ============================
> > Since we need to run TaskAPI methods to get machine requirements from PyRecipe,
> > methods like provision_machines, prepare_network are run after the python recipe
> > is executed. Methods add_host and add_interface create machine requirements dict
> > in ControllerAPI object. When lnst.match() is called, it runs match algorithm
> > with mreq dict and if match is found, it prepares network and binds Machine
> > and Interface objects with their HostAPI and InterfaceAPI counterparts.
> > When everyting is prepared, execution returns to PyRecipe, where task phase
> > of PyRecipe is executed. Task phase is the same like before, only new method
> > is breakpoint() which will be useful for debugging, as it allows user to pause
> > the execution in whatever part of test.
> > TaskAPI is now used right from lnst module, all callable methods are exported
> > via __init__.py file from lnst/ dir.
> > Folder pyrecipes/ contains few working examples you can try.
> >
> > TODO
> > ====
> > * multimatch support
> > * config_only mode should be renamed and polished
> > * TaskAPI create_ovs() method
> > * support for loopbacks
> > * NetEm support
> > * polishing of the code
> >
> > Jiri Prochazka (26):
> >   __init__.py: draft for PyRecipes
> >   lnst-ctl: draft for PyRecipes
> >   NetTestController: remove RecipeParser import and related method calls
> >   NetTestController: add run_mode attribute
> >   NetTestController: remove obsolete methods
> >   NetTestController: use mreq dict from PyRecipe
> >   NetTestController: split provision_machines() in two methods
> >   NetTestController: cleanup _preapre_interface()
> >   NetTestController: remove resource sync from _prepare_machine()
> >   NetTestController: add multi_match attribute ti NetTestController
> >   NetTestController: don't call abs_path on _recipe_path in __init__
> >   NetTestController: rework alias handling
> >   NetTestController: add prepare_test_env()
> >   NetTestController: rework match_setup(), config_only_recipe(),
> >     run_recipe(), _run_python_task()
> >   NetTestController: add init_taskapi()
> >   NetTestController: use Task name instead of module
> >   Task: remove deprecated methods
> >   Task: add default param for get_alias()
> >   Task: define TaskAPI methods on global level
> >   Task: add breakpoint()
> >   Task: add add_host() and init_hosts()
> >   Task: add interface handling TaskAPI methods
> >   Task: HostAPI get_id now returns its generated id
> >   Task: add HostAPI methods required by PyRecipes
> >   Task: add match() and minor rework of ControllerAPI
> >   PyRecipes: add example PyRecipes
> >
> >  lnst-ctl                             |  16 +-
> >  lnst/Controller/NetTestController.py | 398 ++++++++++-------------------------
> >  lnst/Controller/Task.py              | 240 +++++++++++----------
> >  lnst/__init__.py                     |   1 +
> >  pyrecipes/3_vlans.py                 |  34 +++
> >  pyrecipes/example.py                 |  33 +++
> >  pyrecipes/ping_flood.py              |  48 +++++
> >  7 files changed, 363 insertions(+), 407 deletions(-)
> >  create mode 100644 pyrecipes/3_vlans.py
> >  create mode 100644 pyrecipes/example.py
> >  create mode 100644 pyrecipes/ping_flood.py
> >
> > --
> > 2.4.11
> > _______________________________________________
> > LNST-developers mailing list
> > lnst-developers(a)lists.fedorahosted.org
> > https://lists.fedorahosted.org/admin/lists/[e-mail]...
>
>
>  Overall, in addition to the small comments that I've sent for the
>  obvious errors I want to add that I don't like how this patchset is
>  organized. 
Thanks for the remarks Ondrej, appreciated!

>  It first starts adding some references to code that doesn't exist
> yet.
>  The it removes the old code while the new one isn't there yet. And only
>  starts adding the rest later, still introducing references to other
>  still nonexistant parts of the new code.
>
>  There's two problems with this approach:
>  * It makes the patchset very hard to follow and pretty confusing.
>  * It breaks git bisect functionality
>
>  I realize that this is a big feature change and we still need to discuss
>  how we're going to move forward with it since it completely breaks
>  backwards compatibility... 
I'd propose that we create a new branch in official git repo for
testing purposes,
and when the implementation will be ready, we can merge it to master and release
new version.

>  But it's exactly because it's such a big change that I think
> that the
>  patchset should be organized in such a way that it's as easy as possible to
>  understand/follow. 
I agree, I'll try to make new version in more organized manner (I was in a hurry
when creating this patchset). I'll wait for comments from jtluka and if someone
from Mellanox wants to review and comment the patches, I'll be more that glad.

> -Ondrej

[PATCH v2 11/26] NetTestController: don't call abs_path on _recipe_path in __init__

[Jan Tluka]

Fri, Apr 22, 2016 at 10:52:09AM CEST, jprochaz(a)redhat.com wrote:
> made more sense, because we will need to resolve Path object in
> _run_python_task() and abs_path method transforms it to str object
>
> Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> ---
>  lnst/Controller/NetTestController.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
> index 51bd424..04851e1 100644
> --- a/lnst/Controller/NetTestController.py
> +++ b/lnst/Controller/NetTestController.py
> @@ -58,7 +58,7 @@ class NetTestController:
>          self._res_serializer = res_serializer
>          self._remote_capture_files = {}
>          self._log_ctl = log_ctl
> -        self._recipe_path = Path(None, recipe_path).abs_path()
> +        self._recipe_path = Path(None, recipe_path)
>          self._msg_dispatcher = MessageDispatcher(log_ctl)
>          self._packet_capture = packet_capture
>          self._reduce_sync = reduce_sync
> @@ -191,7 +191,7 @@ class NetTestController:
>          machine.set_mac_pool(self._mac_pool)
>          machine.set_network_bridges(self._network_bridges)
>  
> -        recipe_name = os.path.basename(self._recipe_path)
> +        recipe_name = os.path.basename(self._recipe_path.abs_path())
>          machine.configure(recipe_name)
>  
>      def _prepare_interface(self, m_id, if_id, iface_data): 
Please make sure that this does not break specifying recipe as http url.

-Jan

[PATCH v2 14/26] NetTestController: rework match_setup(), config_only_recipe(), run_recipe(), _run_python_task()

[Jan Tluka]

Fri, Apr 22, 2016 at 10:52:12AM CEST, jprochaz(a)redhat.com wrote:
> match_setup() - now requires to run _run_python_task() in order to get
> mreq dict
> for matching algorithm
>
> config_only_recipe() - this method and run mode should be renamed to something like debug
> mode,
> because now it's same as run mode, the only difference is, that it stops executing of
> recipe
> on breakpoint()s until user presses Enter. Although this behaviour should be discussed
> and
> it will be done in next phase of PyRecipes implementation
>
> _run_python_recipe() - modules and tools are not used now, so they cannot be loaded and
> TaskAPI is initialized earlier because it's needed for setup phase for all run modes
>
> Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> ---
>  lnst/Controller/NetTestController.py | 74 +++++++++++-------------------------
>  1 file changed, 23 insertions(+), 51 deletions(-)
>
> diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
> index 3e7eb69..80708e5 100644
> --- a/lnst/Controller/NetTestController.py
> +++ b/lnst/Controller/NetTestController.py
> @@ -452,37 +452,22 @@ class NetTestController:
>  
>      def match_setup(self):
>          self.run_mode = "match_setup"
> +        res = self._run_python_task()
>          return {"passed": True} 
This is a bit weird. The method name says that it matches setup but
along with it it also runs the python task. I see your comment about
this in commit message but it's very hard to understand.

In order to just match the setup user would need to wait for the whole
task to finish.

> 
>     def config_only_recipe(self):
>         self.run_mode = "config_only"
>         try:
>-            self._prepare_network(resource_sync=False)
>-        except (KeyboardInterrupt, Exception) as exc:
>-            msg = "Exception raised during configuration."
>-            logging.error(msg)
>-            self._cleanup_slaves()
>+            res = self._run_recipe()
>+        except Exception as exc:
>+            logging.error("Recipe execution terminated by unexpected
exception")
>             raise
>-
>-        self._save_machine_config()
>-
>-        self._cleanup_slaves(deconfigure=False)
>-        return {"passed": True}
>-
>-    def run_recipe(self):
>-        try:
>-            self._prepare_tasks()
>-            self._prepare_network()
>-        except (KeyboardInterrupt, Exception) as exc:
>-            msg = "Exception raised during configuration."
>-            logging.error(msg)
>+        finally:
>             self._cleanup_slaves()
>-            raise
> 
>-        if self._packet_capture:
>-            self._start_packet_capture()
>+        return res
> 
>-        err = None
>+    def run_recipe(self):
>         try:
>             res = self._run_recipe()
>         except Exception as exc:
>@@ -518,43 +503,30 @@ class NetTestController:
>     def _run_recipe(self):
>         overall_res = {"passed": True}
> 
>-        for task in self._tasks:
>+        try:
>             self._res_serializer.add_task()
>-            try:
>-                res = self._run_task(task)
>-            except CommandException as exc:
>-                logging.debug(exc)
>-                overall_res["passed"] = False
>-                overall_res["err_msg"] = "Command exception
raised."
>-                break
>-
>-            for machine in self._machines.itervalues():
>-                machine.restore_system_config()
>-
>-            # task failed, check if we should quit_on_fail
>-            if not res:
>-                overall_res["passed"] = False
>-                overall_res["err_msg"] = "At least one command
failed."
>-                if task["quit_on_fail"]:
>-                    break
>+            res = self._run_python_task()
>+        except CommandException as exc:
>+            logging.debug(exc)
>+            overall_res["passed"] = False
>+            overall_res["err_msg"] = "Command exception raised."
>+
>+        for machine in self._machines.itervalues():
>+            machine.restore_system_config()
>+
>+        # task failed
>+        if not res:
>+            overall_res["passed"] = False
>+            overall_res["err_msg"] = "At least one command
failed."
> 
>         return overall_res
> 
>-    def _run_python_task(self, task):
>+    def _run_python_task(self):
>         #backup of resource table
>         res_table_bkp = copy.deepcopy(self._resource_table)
>-        if 'module_dir' in task:
>-            modules = self._load_test_modules([task['module_dir']])
>-            self._resource_table['module'].update(modules)
>-        if 'tools_dir' in task:
>-            tools = self._load_test_tools([task['tools_dir']])
>-            self._resource_table['tools'].update(tools)
>-
>-        # Initialize the API handle
>-        Task.ctl = Task.ControllerAPI(self, self._machines)
> 
>         cwd = os.getcwd()
>-        task_path = task["python"]
>+        task_path = self._recipe_path
>         name = os.path.basename(task_path.abs_path()).split(".")[0]
>         sys.path.append(os.path.dirname(task_path.resolve()))
>         os.chdir(os.path.dirname(task_path.resolve()))
>-- 
>2.4.11
>_______________________________________________
>LNST-developers mailing list
&gt;lnst-developers(a)lists.fedorahosted.org
>https://lists.fedorahosted.org/admin/lists/[e-mail]

[PATCH v2 16/26] NetTestController: use Task name instead of module

[Jan Tluka]

Fri, Apr 22, 2016 at 10:52:14AM CEST, jprochaz(a)redhat.com wrote:
> it's more readable this way
>
> Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> ---
>  lnst/Controller/NetTestController.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
> index c921cc7..149fe66 100644
> --- a/lnst/Controller/NetTestController.py
> +++ b/lnst/Controller/NetTestController.py
> @@ -540,7 +540,7 @@ class NetTestController:
>          #restore resource table
>          self._resource_table = res_table_bkp
>  
> -        return module.ctl._result
> +        return Task.ctl._result
>  
>      def _run_command(self, command):
>          logging.info("Executing command: [%s]", str_command(command)) 
IMO it's not. module variable represents the loaded python task and we return
the result of this particular task. I understand that both are correct
but this at least keeps the idea that we return the result of the loaded
task.

[PATCH v2 20/26] Task: add breakpoint()

[Jan Tluka]

Fri, Apr 22, 2016 at 10:52:18AM CEST, jprochaz(a)redhat.com wrote:
> this is new method for debugging, it pauses execution
> of the test until and resumes after pressing enter
>
> this functionality will be enriched in the future,
> when we come up with full design
>
> Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> ---
>  lnst/Controller/Task.py | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
> index faca548..cb52921 100644
> --- a/lnst/Controller/Task.py
> +++ b/lnst/Controller/Task.py
> @@ -44,6 +44,11 @@ def wait(seconds):
>  def get_module(name, options={}):
>      return ctl.get_module(name, options)
>  
> +def breakpoint():
> +    if ctl.get_run_mode != "config_only":
> +        return
> +    raw_input("Press enter to continue: ")
> +
>  class TaskError(Exception): pass
>  
>  class ControllerAPI(object): 
Print also what's happening :-)

Something like "Breakpoint reached. Press enter to continue."

[Jan Tluka]

Fri, Apr 22, 2016 at 10:51:58AM CEST, jprochaz(a)redhat.com wrote:
> Hello everyone,
>
> first working draft implementation is completed. Please, read this cover
> letter, check the patches and let me know what you like, what you don't like,
> etc. Please, note, that this is not final implementation.
>
> NOTE - patches are not rebased to current HEAD, if you want to try it,
> please, reset your branch to commit 2398f93.
>
> What doesn't work
> =================
> multi_match - didn't have time and resources to check physical machines,
> on virt setup it works, but ends with exception due to no res value returned
> loopbacks - can't be created
> netem support - not implemented yet
>
> Description of new mechanics
> ============================
> Since we need to run TaskAPI methods to get machine requirements from PyRecipe,
> methods like provision_machines, prepare_network are run after the python recipe
> is executed. Methods add_host and add_interface create machine requirements dict
> in ControllerAPI object. When lnst.match() is called, it runs match algorithm
> with mreq dict and if match is found, it prepares network and binds Machine
> and Interface objects with their HostAPI and InterfaceAPI counterparts.
> When everyting is prepared, execution returns to PyRecipe, where task phase
> of PyRecipe is executed. Task phase is the same like before, only new method
> is breakpoint() which will be useful for debugging, as it allows user to pause
> the execution in whatever part of test.
> TaskAPI is now used right from lnst module, all callable methods are exported
> via __init__.py file from lnst/ dir.
> Folder pyrecipes/ contains few working examples you can try.
>
> TODO
> ====
> * multimatch support
> * config_only mode should be renamed and polished
> * TaskAPI create_ovs() method
> * support for loopbacks
> * NetEm support
> * polishing of the code
>
> Jiri Prochazka (26):
>   __init__.py: draft for PyRecipes
>   lnst-ctl: draft for PyRecipes
>   NetTestController: remove RecipeParser import and related method calls
>   NetTestController: add run_mode attribute
>   NetTestController: remove obsolete methods
>   NetTestController: use mreq dict from PyRecipe
>   NetTestController: split provision_machines() in two methods
>   NetTestController: cleanup _preapre_interface()
>   NetTestController: remove resource sync from _prepare_machine()
>   NetTestController: add multi_match attribute ti NetTestController
>   NetTestController: don't call abs_path on _recipe_path in __init__
>   NetTestController: rework alias handling
>   NetTestController: add prepare_test_env()
>   NetTestController: rework match_setup(), config_only_recipe(),
>     run_recipe(), _run_python_task()
>   NetTestController: add init_taskapi()
>   NetTestController: use Task name instead of module
>   Task: remove deprecated methods
>   Task: add default param for get_alias()
>   Task: define TaskAPI methods on global level
>   Task: add breakpoint()
>   Task: add add_host() and init_hosts()
>   Task: add interface handling TaskAPI methods
>   Task: HostAPI get_id now returns its generated id
>   Task: add HostAPI methods required by PyRecipes
>   Task: add match() and minor rework of ControllerAPI
>   PyRecipes: add example PyRecipes
>
>  lnst-ctl                             |  16 +-
>  lnst/Controller/NetTestController.py | 398 ++++++++++-------------------------
>  lnst/Controller/Task.py              | 240 +++++++++++----------
>  lnst/__init__.py                     |   1 +
>  pyrecipes/3_vlans.py                 |  34 +++
>  pyrecipes/example.py                 |  33 +++
>  pyrecipes/ping_flood.py              |  48 +++++
>  7 files changed, 363 insertions(+), 407 deletions(-)
>  create mode 100644 pyrecipes/3_vlans.py
>  create mode 100644 pyrecipes/example.py
>  create mode 100644 pyrecipes/ping_flood.py 
Besides the comments to individual patches I'd like to ask you to
consider the format of commit descriptions.

1. Start sentences with capital letters.
2. Why do you do line break at 40 characters?
3. Please do not start sentence in commit message and continue in
message description. No one does that except of you.

Please keep the commit log pretty!

Thanks
-Jan

[Jan Tluka]

Tue, May 17, 2016 at 12:13:09PM CEST, jtluka(a)redhat.com wrote:
> 3. Please do not start sentence in commit message and continue in
> message description. No one does that except of you. 
Please ignore this. Nothing like this is in the patches.

-Jan

[Jan Tluka]

Fri, Apr 22, 2016 at 10:51:58AM CEST, jprochaz(a)redhat.com wrote:
> Hello everyone,
>
> first working draft implementation is completed. Please, read this cover
> letter, check the patches and let me know what you like, what you don't like,
> etc. Please, note, that this is not final implementation.
>
> NOTE - patches are not rebased to current HEAD, if you want to try it,
> please, reset your branch to commit 2398f93.
>
> What doesn't work
> =================
> multi_match - didn't have time and resources to check physical machines,
> on virt setup it works, but ends with exception due to no res value returned
> loopbacks - can't be created
> netem support - not implemented yet
>
> Description of new mechanics
> ============================
> Since we need to run TaskAPI methods to get machine requirements from PyRecipe,
> methods like provision_machines, prepare_network are run after the python recipe
> is executed. 
Just to confirm, is this because of keeping compatibility with xml recipes?
Is this going to be removed completely?

>Methods add_host and add_interface create machine requirements dict
>in ControllerAPI object. When lnst.match() is called, it runs match algorithm
>with mreq dict and if match is found, it prepares network and binds Machine
>and Interface objects with their HostAPI and InterfaceAPI counterparts.
>When everyting is prepared, execution returns to PyRecipe, where task phase
>of PyRecipe is executed. Task phase is the same like before, only new method
>is breakpoint() which will be useful for debugging, as it allows user to pause
>the execution in whatever part of test.
>TaskAPI is now used right from lnst module, all callable methods are exported
>via __init__.py file from lnst/ dir.
>Folder pyrecipes/ contains few working examples you can try.
>
>TODO
>====
>* multimatch support
>* config_only mode should be renamed and polished
>* TaskAPI create_ovs() method
>* support for loopbacks
>* NetEm support
>* polishing of the code
>
>Jiri Prochazka (26):
>  __init__.py: draft for PyRecipes
>  lnst-ctl: draft for PyRecipes
>  NetTestController: remove RecipeParser import and related method calls
>  NetTestController: add run_mode attribute
>  NetTestController: remove obsolete methods
>  NetTestController: use mreq dict from PyRecipe
>  NetTestController: split provision_machines() in two methods
>  NetTestController: cleanup _preapre_interface()
>  NetTestController: remove resource sync from _prepare_machine()
>  NetTestController: add multi_match attribute ti NetTestController
>  NetTestController: don't call abs_path on _recipe_path in __init__
>  NetTestController: rework alias handling
>  NetTestController: add prepare_test_env()
>  NetTestController: rework match_setup(), config_only_recipe(),
>    run_recipe(), _run_python_task()
>  NetTestController: add init_taskapi()
>  NetTestController: use Task name instead of module
>  Task: remove deprecated methods
>  Task: add default param for get_alias()
>  Task: define TaskAPI methods on global level
>  Task: add breakpoint()
>  Task: add add_host() and init_hosts()
>  Task: add interface handling TaskAPI methods
>  Task: HostAPI get_id now returns its generated id
>  Task: add HostAPI methods required by PyRecipes
>  Task: add match() and minor rework of ControllerAPI
>  PyRecipes: add example PyRecipes
>
> lnst-ctl                             |  16 +-
> lnst/Controller/NetTestController.py | 398 ++++++++++-------------------------
> lnst/Controller/Task.py              | 240 +++++++++++----------
> lnst/__init__.py                     |   1 +
> pyrecipes/3_vlans.py                 |  34 +++
> pyrecipes/example.py                 |  33 +++
> pyrecipes/ping_flood.py              |  48 +++++
> 7 files changed, 363 insertions(+), 407 deletions(-)
> create mode 100644 pyrecipes/3_vlans.py
> create mode 100644 pyrecipes/example.py
> create mode 100644 pyrecipes/ping_flood.py
>
>-- 
>2.4.11
>_______________________________________________
>LNST-developers mailing list
>lnst-developers(a)lists.fedorahosted.org
>https://lists.fedorahosted.org/admin/lists/[e-mail]

[Jiri Prochazka]

2016-05-17 13:27 GMT+02:00 Jan Tluka <jtluka(a)redhat.com&gt;:

>  Fri, Apr 22, 2016 at 10:51:58AM CEST, jprochaz(a)redhat.com wrote:
>  >Hello everyone,
>  >
>  >first working draft implementation is completed. Please, read this cover
>  >letter, check the patches and let me know what you like, what you don't
>  like,
>  >etc. Please, note, that this is not final implementation.
>  >
>  >NOTE - patches are not rebased to current HEAD, if you want to try it,
>  >please, reset your branch to commit 2398f93.
>  >
>  >What doesn't work
>  >=================
>  >multi_match - didn't have time and resources to check physical machines,
>  >on virt setup it works, but ends with exception due to no res value
>  returned
>  >loopbacks - can't be created
>  >netem support - not implemented yet
>  >
>  >Description of new mechanics
>  >============================
>  >Since we need to run TaskAPI methods to get machine requirements from
>  PyRecipe,
>  >methods like provision_machines, prepare_network are run after the python
>  recipe
>  >is executed.
>
>  Just to confirm, is this because of keeping compatibility with xml recipes?
>  Is this going to be removed completely?
>
>  
This implementation will not be compatible with XML recipes, we will have to
throw them away completely​.


>  >Methods add_host and add_interface create machine requirements
> dict
>  >in ControllerAPI object. When lnst.match() is called, it runs match
>  algorithm
>  >with mreq dict and if match is found, it prepares network and binds
>  Machine
>  >and Interface objects with their HostAPI and InterfaceAPI counterparts.
>  >When everyting is prepared, execution returns to PyRecipe, where task
>  phase
>  >of PyRecipe is executed. Task phase is the same like before, only new
>  method
>  >is breakpoint() which will be useful for debugging, as it allows user to
>  pause
>  >the execution in whatever part of test.
>  >TaskAPI is now used right from lnst module, all callable methods are
>  exported
>  >via __init__.py file from lnst/ dir.
>  >Folder pyrecipes/ contains few working examples you can try.
>  >
>  >TODO
>  >====
>  >* multimatch support
>  >* config_only mode should be renamed and polished
>  >* TaskAPI create_ovs() method
>  >* support for loopbacks
>  >* NetEm support
>  >* polishing of the code
>  >
>  >Jiri Prochazka (26):
>  >  __init__.py: draft for PyRecipes
>  >  lnst-ctl: draft for PyRecipes
>  >  NetTestController: remove RecipeParser import and related method calls
>  >  NetTestController: add run_mode attribute
>  >  NetTestController: remove obsolete methods
>  >  NetTestController: use mreq dict from PyRecipe
>  >  NetTestController: split provision_machines() in two methods
>  >  NetTestController: cleanup _preapre_interface()
>  >  NetTestController: remove resource sync from _prepare_machine()
>  >  NetTestController: add multi_match attribute ti NetTestController
>  >  NetTestController: don't call abs_path on _recipe_path in __init__
>  >  NetTestController: rework alias handling
>  >  NetTestController: add prepare_test_env()
>  >  NetTestController: rework match_setup(), config_only_recipe(),
>  >    run_recipe(), _run_python_task()
>  >  NetTestController: add init_taskapi()
>  >  NetTestController: use Task name instead of module
>  >  Task: remove deprecated methods
>  >  Task: add default param for get_alias()
>  >  Task: define TaskAPI methods on global level
>  >  Task: add breakpoint()
>  >  Task: add add_host() and init_hosts()
>  >  Task: add interface handling TaskAPI methods
>  >  Task: HostAPI get_id now returns its generated id
>  >  Task: add HostAPI methods required by PyRecipes
>  >  Task: add match() and minor rework of ControllerAPI
>  >  PyRecipes: add example PyRecipes
>  >
>  > lnst-ctl                             |  16 +-
>  > lnst/Controller/NetTestController.py | 398
>  ++++++++++-------------------------
>  > lnst/Controller/Task.py              | 240 +++++++++++----------
>  > lnst/__init__.py                     |   1 +
>  > pyrecipes/3_vlans.py                 |  34 +++
>  > pyrecipes/example.py                 |  33 +++
>  > pyrecipes/ping_flood.py              |  48 +++++
>  > 7 files changed, 363 insertions(+), 407 deletions(-)
>  > create mode 100644 pyrecipes/3_vlans.py
>  > create mode 100644 pyrecipes/example.py
>  > create mode 100644 pyrecipes/ping_flood.py
>  >
>  >--
>  >2.4.11
>  >_______________________________________________
>  >LNST-developers mailing list
>  &gt;lnst-developers(a)lists.fedorahosted.org
>  >
>  https://lists.fedorahosted.org/admin/lists/[e-mail]...
>

[Jiri Prochazka]

2016-05-17 12:13 GMT+02:00 Jan Tluka <jtluka(a)redhat.com&gt;:

>  Fri, Apr 22, 2016 at 10:51:58AM CEST, jprochaz(a)redhat.com wrote:
>  >Hello everyone,
>  >
>  >first working draft implementation is completed. Please, read this cover
>  >letter, check the patches and let me know what you like, what you don't
>  like,
>  >etc. Please, note, that this is not final implementation.
>  >
>  >NOTE - patches are not rebased to current HEAD, if you want to try it,
>  >please, reset your branch to commit 2398f93.
>  >
>  >What doesn't work
>  >=================
>  >multi_match - didn't have time and resources to check physical machines,
>  >on virt setup it works, but ends with exception due to no res value
>  returned
>  >loopbacks - can't be created
>  >netem support - not implemented yet
>  >
>  >Description of new mechanics
>  >============================
>  >Since we need to run TaskAPI methods to get machine requirements from
>  PyRecipe,
>  >methods like provision_machines, prepare_network are run after the python
>  recipe
>  >is executed. Methods add_host and add_interface create machine
>  requirements dict
>  >in ControllerAPI object. When lnst.match() is called, it runs match
>  algorithm
>  >with mreq dict and if match is found, it prepares network and binds
>  Machine
>  >and Interface objects with their HostAPI and InterfaceAPI counterparts.
>  >When everyting is prepared, execution returns to PyRecipe, where task
>  phase
>  >of PyRecipe is executed. Task phase is the same like before, only new
>  method
>  >is breakpoint() which will be useful for debugging, as it allows user to
>  pause
>  >the execution in whatever part of test.
>  >TaskAPI is now used right from lnst module, all callable methods are
>  exported
>  >via __init__.py file from lnst/ dir.
>  >Folder pyrecipes/ contains few working examples you can try.
>  >
>  >TODO
>  >====
>  >* multimatch support
>  >* config_only mode should be renamed and polished
>  >* TaskAPI create_ovs() method
>  >* support for loopbacks
>  >* NetEm support
>  >* polishing of the code
>  >
>  >Jiri Prochazka (26):
>  >  __init__.py: draft for PyRecipes
>  >  lnst-ctl: draft for PyRecipes
>  >  NetTestController: remove RecipeParser import and related method calls
>  >  NetTestController: add run_mode attribute
>  >  NetTestController: remove obsolete methods
>  >  NetTestController: use mreq dict from PyRecipe
>  >  NetTestController: split provision_machines() in two methods
>  >  NetTestController: cleanup _preapre_interface()
>  >  NetTestController: remove resource sync from _prepare_machine()
>  >  NetTestController: add multi_match attribute ti NetTestController
>  >  NetTestController: don't call abs_path on _recipe_path in __init__
>  >  NetTestController: rework alias handling
>  >  NetTestController: add prepare_test_env()
>  >  NetTestController: rework match_setup(), config_only_recipe(),
>  >    run_recipe(), _run_python_task()
>  >  NetTestController: add init_taskapi()
>  >  NetTestController: use Task name instead of module
>  >  Task: remove deprecated methods
>  >  Task: add default param for get_alias()
>  >  Task: define TaskAPI methods on global level
>  >  Task: add breakpoint()
>  >  Task: add add_host() and init_hosts()
>  >  Task: add interface handling TaskAPI methods
>  >  Task: HostAPI get_id now returns its generated id
>  >  Task: add HostAPI methods required by PyRecipes
>  >  Task: add match() and minor rework of ControllerAPI
>  >  PyRecipes: add example PyRecipes
>  >
>  > lnst-ctl                             |  16 +-
>  > lnst/Controller/NetTestController.py | 398
>  ++++++++++-------------------------
>  > lnst/Controller/Task.py              | 240 +++++++++++----------
>  > lnst/__init__.py                     |   1 +
>  > pyrecipes/3_vlans.py                 |  34 +++
>  > pyrecipes/example.py                 |  33 +++
>  > pyrecipes/ping_flood.py              |  48 +++++
>  > 7 files changed, 363 insertions(+), 407 deletions(-)
>  > create mode 100644 pyrecipes/3_vlans.py
>  > create mode 100644 pyrecipes/example.py
>  > create mode 100644 pyrecipes/ping_flood.py
>
>  Besides the comments to individual patches I'd like to ask you to
>  consider the format of commit descriptions.
>
>  1. Start sentences with capital letters.
>  
​Will do from now on.​



>  2. Why do you do line break at 40 characters?
>  
​Bad vim configuration, will fix it now​


>  3. Please do not start sentence in commit message and continue in
>  message description. No one does that except of you.
>
>  Please keep the commit log pretty!
>
>  Thanks
>  -Jan
>  
​Thanks for feedback Jan, much appreciated!​

[PATCH v2 16/26] NetTestController: use Task name instead of module

[Jiri Prochazka]

2016-05-17 12:05 GMT+02:00 Jan Tluka <jtluka(a)redhat.com&gt;:
>  Fri, Apr 22, 2016 at 10:52:14AM CEST, jprochaz(a)redhat.com wrote:
> >it's more readable this way
> >
> >Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> >---
> > lnst/Controller/NetTestController.py | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/lnst/Controller/NetTestController.py
> b/lnst/Controller/NetTestController.py
> >index c921cc7..149fe66 100644
> >--- a/lnst/Controller/NetTestController.py
> >+++ b/lnst/Controller/NetTestController.py
> >@@ -540,7 +540,7 @@ class NetTestController:
> >         #restore resource table
> >         self._resource_table = res_table_bkp
> >
> >-        return module.ctl._result
> >+        return Task.ctl._result
> >
> >     def _run_command(self, command):
> >         logging.info("Executing command: [%s]", str_command(command))
>
>  IMO it's not. module variable represents the loaded python task and we return
>  the result of this particular task. I understand that both are correct
>  but this at least keeps the idea that we return the result of the loaded
>  task. 
Please, Ondrej, what do you think?

[PATCH v2 16/26] NetTestController: use Task name instead of module

[Ondrej Lichtner]

On Tue, May 17, 2016 at 02:09:01PM +0200, Jiri Prochazka wrote:
>  2016-05-17 12:05 GMT+02:00 Jan Tluka <jtluka(a)redhat.com&gt;:
>  > Fri, Apr 22, 2016 at 10:52:14AM CEST, jprochaz(a)redhat.com wrote:
>  >>it's more readable this way
>  >>
>  >>Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  >>---
>  >> lnst/Controller/NetTestController.py | 2 +-
>  >> 1 file changed, 1 insertion(+), 1 deletion(-)
>  >>
>  >>diff --git a/lnst/Controller/NetTestController.py
> b/lnst/Controller/NetTestController.py
>  >>index c921cc7..149fe66 100644
>  >>--- a/lnst/Controller/NetTestController.py
>  >>+++ b/lnst/Controller/NetTestController.py
>  >>@@ -540,7 +540,7 @@ class NetTestController:
>  >>         #restore resource table
>  >>         self._resource_table = res_table_bkp
>  >>
>  >>-        return module.ctl._result
>  >>+        return Task.ctl._result
>  >>
>  >>     def _run_command(self, command):
>  >>         logging.info("Executing command: [%s]", str_command(command))
>  >
>  > IMO it's not. module variable represents the loaded python task and we return
>  > the result of this particular task. I understand that both are correct
>  > but this at least keeps the idea that we return the result of the loaded
>  > task.
>  
>  Please, Ondrej, what do you think? 
I agree with Jan.

-Ondrej

[PATCH v2 16/26] NetTestController: use Task name instead of module

[Jiri Prochazka]

2016-05-17 14:28 GMT+02:00 Ondrej Lichtner <olichtne(a)redhat.com&gt;:
>  On Tue, May 17, 2016 at 02:09:01PM +0200, Jiri Prochazka wrote:
> > 2016-05-17 12:05 GMT+02:00 Jan Tluka <jtluka(a)redhat.com&gt;:
> > > Fri, Apr 22, 2016 at 10:52:14AM CEST, jprochaz(a)redhat.com wrote:
> > >>it's more readable this way
> > >>
> > >>Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> > >>---
> > >> lnst/Controller/NetTestController.py | 2 +-
> > >> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >>diff --git a/lnst/Controller/NetTestController.py
> b/lnst/Controller/NetTestController.py
> > >>index c921cc7..149fe66 100644
> > >>--- a/lnst/Controller/NetTestController.py
> > >>+++ b/lnst/Controller/NetTestController.py
> > >>@@ -540,7 +540,7 @@ class NetTestController:
> > >>         #restore resource table
> > >>         self._resource_table = res_table_bkp
> > >>
> > >>-        return module.ctl._result
> > >>+        return Task.ctl._result
> > >>
> > >>     def _run_command(self, command):
> > >>         logging.info("Executing command: [%s]",
> str_command(command))
> > >
> > > IMO it's not. module variable represents the loaded python task and we
> return
> > > the result of this particular task. I understand that both are correct
> > > but this at least keeps the idea that we return the result of the loaded
> > > task.
> >
> > Please, Ondrej, what do you think?
>
>  I agree with Jan.
>
>  -Ondrej 
Ok then, this will be removed from the next patchset, thanks for your opinions.