[PATCH 1/3] lnst.Controller.Job: refactoring

From: Ondrej Lichtner <olichtne(a)redhat.com&gt;

Removing the netns parameter from the __init__ method and renaming the
host parameter to namespace.

The parameter expects a Namespace instance (a Host or a NetNamespace),
the host can be accessed through a new property "host" and the
"netns"
proerty now returns the original Namespace instance the Job was created
with (these two can be identical if the Job is running on the initial
namespace of a slave).

I also moved the id property higher since I think it makes a bit more
sense to have it at the beginning of the class definition....

Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
---
 lnst/Controller/Job.py       | 63 ++++++++++++++++++++++----------------------
 lnst/Controller/Machine.py   |  4 +--
 lnst/Controller/Namespace.py |  3 +--
 3 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/lnst/Controller/Job.py b/lnst/Controller/Job.py
index 1026938..1ed079f 100644
--- a/lnst/Controller/Job.py
+++ b/lnst/Controller/Job.py
@@ -26,13 +26,12 @@ class Job(object):
         job = m1.run("ls ~/")
         print job.stdout
     """
-    def __init__(self, host, what,
-                 expect=True, json=False, netns=None, desc=None):
-        self._host = host
+    def __init__(self, namespace, what,
+                 expect=True, json=False, desc=None):
         self._what = what
         self._expect = expect
         self._json = json
-        self._netns = netns
+        self._netns = namespace
         self._desc = desc
 
         self._res = None
@@ -46,6 +45,32 @@ class Job(object):
 
         self._id = None
 
+    @property
+    def id(self):
+        """id of the job
+
+        Used internally by the Machine class to identify results coming
+        from the slave.
+        TODO make private?
+        """
+        return self._id
+
+    @id.setter
+    def id(self, val):
+        if self._id is not None:
+            raise Exception("Id already set")
+        self._id = val
+
+    @property
+    def host(self):
+        """the initial namespace of the host the job is running
on"""
+        return self._netns.initns
+
+    @property
+    def netns(self):
+        """network namespace the Job is running in"""
+        return self._netns
+
     @property
     def stdout(self):
         """standard output of the Job
@@ -109,30 +134,6 @@ class Job(object):
         else:
             return False
 
-    @property
-    def netns(self):
-        """name of the network namespace the Job is running in
-
-        Not relevant yet as network namespaces aren't supported yet.
-        """
-        return self._netns
-
-    @property
-    def id(self):
-        """id of the job
-
-        Used internally by the Machine class to identify results coming
-        from the slave.
-        TODO make private?
-        """
-        return self._id
-
-    @id.setter
-    def id(self, val):
-        if self._id is not None:
-            raise Exception("Id already set")
-        self._id = val
-
     def wait(self, timeout=0):
         """waits for the Job to finish for the specified amount of time
 
@@ -150,7 +151,7 @@ class Job(object):
             return True
         if timeout < 0:
             raise JobError("Negative timeout value not allowed.")
-        return self._host.wait_for_job(self, timeout)
+        return self._netns._machine.wait_for_job(self, timeout)
 
     def kill(self, signal=signal.SIGKILL):
         """send specified signal to the remotely running Job process
@@ -164,8 +165,8 @@ class Job(object):
             False if an exception was raised while sending the signal.
         """
         logging.info("Sending signal {} to job {} on host {}".format(signal,
-                     self._id, self._host.get_id()))
-        return self._host.kill(self, signal)
+                     self._id, self._netns.hostid))
+        return self._netns._machine.kill(self, signal)
 
     def _to_dict(self):
         d = {"job_id": self._id,
diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py
index 09e7540..67907be 100644
--- a/lnst/Controller/Machine.py
+++ b/lnst/Controller/Machine.py
@@ -366,7 +366,7 @@ class Machine(object):
         if job._desc is not None:
             logging.info("Job description: %s" % job._desc)
 
-        res = self.rpc_call("run_job", job._to_dict(), netns=job.netns)
+        res = self.rpc_call("run_job", job._to_dict(), netns=job.netns.name)
 
         self._recipe.current_run.add_result(JobStartResult(job, res))
         return res
@@ -436,7 +436,7 @@ class Machine(object):
         if job.id not in self._jobs:
             raise MachineError("No job '%s' running on Machine %s" %
                                (job.id(), self._id))
-        return self.rpc_call("kill_job", job.id, signal, netns=job.netns)
+        return self.rpc_call("kill_job", job.id, signal, netns=job.netns.name)
 
     def get_hostname(self):
         """ Get hostname/ip of the machine
diff --git a/lnst/Controller/Namespace.py b/lnst/Controller/Namespace.py
index d84c327..1f9d199 100644
--- a/lnst/Controller/Namespace.py
+++ b/lnst/Controller/Namespace.py
@@ -98,8 +98,7 @@ class Namespace(object):
             the Job object will be automatically updated.
         """
 
-        job = Job(self._machine, what, expect=not fail, json=json,
-                  netns=self.name, desc=desc)
+        job = Job(self, what, expect=not fail, json=json, desc=desc)
 
         try:
             self._machine.run_job(job)
-- 
2.16.1

[PATCH 2/3] lnst.Controller.Namespace: remove ABCMeta as metaclass

From: Ondrej Lichtner <olichtne(a)redhat.com&gt;

I don't think there's a reason for this to be a metaclass it's probably
a copy paste error from when I added the class...

Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
---
 lnst/Controller/Namespace.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lnst/Controller/Namespace.py b/lnst/Controller/Namespace.py
index 1f9d199..6b81d44 100644
--- a/lnst/Controller/Namespace.py
+++ b/lnst/Controller/Namespace.py
@@ -36,8 +36,6 @@ class Namespace(object):
         m1.bond0 = Bond() # to create a new bond device
         m1.run("ip a") # to run a shell command"""
 
-    __metaclass__ = ABCMeta
-
     #TODO add packet capture options
     def __init__(self, machine):
         #storage for mapped objects (Devices, Namespaces...)
-- 
2.16.1

[PATCH 3/3] lnst.Controller.Recipe: move the _set_ctl method to the property

From: Ondrej Lichtner <olichtne(a)redhat.com&gt;

Just moving the method higher so it's next to the getter...

Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
---
 lnst/Controller/Recipe.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lnst/Controller/Recipe.py b/lnst/Controller/Recipe.py
index a10f081..0f3a54e 100644
--- a/lnst/Controller/Recipe.py
+++ b/lnst/Controller/Recipe.py
@@ -111,15 +111,15 @@ class BaseRecipe(object):
     def ctl(self):
         return self._ctl
 
+    def _set_ctl(self, ctl):
+        self._ctl = ctl
+
     @property
     def matched(self):
         if self.ctl is None:
             return None
         return self.ctl._hosts
 
-    def _set_ctl(self, ctl):
-        self._ctl = ctl
-
     def test(self):
         """Method to be implemented by the Tester"""
         raise NotImplementedError("Method test must be defined by a child
class.")
-- 
2.16.1

[Ondrej Lichtner]

On Fri, Mar 02, 2018 at 12:13:20PM +0100, olichtne(a)redhat.com wrote:
>  From: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>  
>  Removing the netns parameter from the __init__ method and renaming the
>  host parameter to namespace.
>  
>  The parameter expects a Namespace instance (a Host or a NetNamespace),
>  the host can be accessed through a new property "host" and the
> "netns"
>  proerty now returns the original Namespace instance the Job was created
>  with (these two can be identical if the Job is running on the initial
>  namespace of a slave).
>  
>  I also moved the id property higher since I think it makes a bit more
>  sense to have it at the beginning of the class definition....
>  
>  Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>  ---
>   lnst/Controller/Job.py       | 63 ++++++++++++++++++++++----------------------
>   lnst/Controller/Machine.py   |  4 +--
>   lnst/Controller/Namespace.py |  3 +--
>   3 files changed, 35 insertions(+), 35 deletions(-)
>  
>  diff --git a/lnst/Controller/Job.py b/lnst/Controller/Job.py
>  index 1026938..1ed079f 100644
>  --- a/lnst/Controller/Job.py
>  +++ b/lnst/Controller/Job.py
>  @@ -26,13 +26,12 @@ class Job(object):
>           job = m1.run("ls ~/")
>           print job.stdout
>       """
>  -    def __init__(self, host, what,
>  -                 expect=True, json=False, netns=None, desc=None):
>  -        self._host = host
>  +    def __init__(self, namespace, what,
>  +                 expect=True, json=False, desc=None):
>           self._what = what
>           self._expect = expect
>           self._json = json
>  -        self._netns = netns
>  +        self._netns = namespace
>           self._desc = desc
>   
>           self._res = None
>  @@ -46,6 +45,32 @@ class Job(object):
>   
>           self._id = None
>   
>  +    @property
>  +    def id(self):
>  +        """id of the job
>  +
>  +        Used internally by the Machine class to identify results coming
>  +        from the slave.
>  +        TODO make private?
>  +        """
>  +        return self._id
>  +
>  +    @id.setter
>  +    def id(self, val):
>  +        if self._id is not None:
>  +            raise Exception("Id already set")
>  +        self._id = val
>  +
>  +    @property
>  +    def host(self):
>  +        """the initial namespace of the host the job is running
> on"""
>  +        return self._netns.initns
>  +
>  +    @property
>  +    def netns(self):
>  +        """network namespace the Job is running in"""
>  +        return self._netns
>  +
>       @property
>       def stdout(self):
>           """standard output of the Job
>  @@ -109,30 +134,6 @@ class Job(object):
>           else:
>               return False
>   
>  -    @property
>  -    def netns(self):
>  -        """name of the network namespace the Job is running in
>  -
>  -        Not relevant yet as network namespaces aren't supported yet.
>  -        """
>  -        return self._netns
>  -
>  -    @property
>  -    def id(self):
>  -        """id of the job
>  -
>  -        Used internally by the Machine class to identify results coming
>  -        from the slave.
>  -        TODO make private?
>  -        """
>  -        return self._id
>  -
>  -    @id.setter
>  -    def id(self, val):
>  -        if self._id is not None:
>  -            raise Exception("Id already set")
>  -        self._id = val
>  -
>       def wait(self, timeout=0):
>           """waits for the Job to finish for the specified amount of time
>   
>  @@ -150,7 +151,7 @@ class Job(object):
>               return True
>           if timeout < 0:
>               raise JobError("Negative timeout value not allowed.")
>  -        return self._host.wait_for_job(self, timeout)
>  +        return self._netns._machine.wait_for_job(self, timeout)
>   
>       def kill(self, signal=signal.SIGKILL):
>           """send specified signal to the remotely running Job process
>  @@ -164,8 +165,8 @@ class Job(object):
>               False if an exception was raised while sending the signal.
>           """
>           logging.info("Sending signal {} to job {} on host {}".format(signal,
>  -                     self._id, self._host.get_id()))
>  -        return self._host.kill(self, signal)
>  +                     self._id, self._netns.hostid))
>  +        return self._netns._machine.kill(self, signal)
>   
>       def _to_dict(self):
>           d = {"job_id": self._id,
>  diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py
>  index 09e7540..67907be 100644
>  --- a/lnst/Controller/Machine.py
>  +++ b/lnst/Controller/Machine.py
>  @@ -366,7 +366,7 @@ class Machine(object):
>           if job._desc is not None:
>               logging.info("Job description: %s" % job._desc)
>   
>  -        res = self.rpc_call("run_job", job._to_dict(), netns=job.netns)
>  +        res = self.rpc_call("run_job", job._to_dict(), netns=job.netns.name)
>   
>           self._recipe.current_run.add_result(JobStartResult(job, res))
>           return res
>  @@ -436,7 +436,7 @@ class Machine(object):
>           if job.id not in self._jobs:
>               raise MachineError("No job '%s' running on Machine %s" %
>                                  (job.id(), self._id))
>  -        return self.rpc_call("kill_job", job.id, signal, netns=job.netns)
>  +        return self.rpc_call("kill_job", job.id, signal,
> netns=job.netns.name)
>   
>       def get_hostname(self):
>           """ Get hostname/ip of the machine
>  diff --git a/lnst/Controller/Namespace.py b/lnst/Controller/Namespace.py
>  index d84c327..1f9d199 100644
>  --- a/lnst/Controller/Namespace.py
>  +++ b/lnst/Controller/Namespace.py
>  @@ -98,8 +98,7 @@ class Namespace(object):
>               the Job object will be automatically updated.
>           """
>   
>  -        job = Job(self._machine, what, expect=not fail, json=json,
>  -                  netns=self.name, desc=desc)
>  +        job = Job(self, what, expect=not fail, json=json, desc=desc)
>   
>           try:
>               self._machine.run_job(job)
>  -- 
>  2.16.1
>   
pushed