Sun, May 29, 2016 at 11:03:11AM CEST, jprochaz@redhat.com wrote:
>This patch implements version check between Controller and Slaves. Check
>is done after hello messages are exchanged. Controller extracts Slave
>version slave_desc dict and compares with its own version.
>
>These situations might happen:
>1) Versions match - everything continues as normally
>2) Versions mismatch:
>  a) Both Ctl and Slave are run from git repo - warning message is shown,
>   but execution continues
>  b) Ctl/Slave is run from git repo but the second one is run from RPM
>   installation - exception is raised and execution is stopped
>
>Signed-off-by: Jiri Prochazka <jprochaz@redhat.com>
>---
> lnst/Controller/Machine.py | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
>diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py
>index c5d9191..258b1b2 100644
>--- a/lnst/Controller/Machine.py
>+++ b/lnst/Controller/Machine.py
>@@ -247,6 +247,20 @@ class Machine(object):
>                   "to machine %s, handshake failed!" % hostname
>             raise MachineError(msg)
>
>+        slave_version = slave_desc["lnst_version"]
>+        ctl_version = lnst_config.version
>+        ctl_is_git, slave_is_git = self._compare_versions(ctl_version,
>+                                                          slave_version)
>+        if slave_version != ctl_version:
>+            if ctl_is_git and slave_is_git:
>+                msg = "Controller and Slave '%s' git versions are different"\
>+                                                                    % hostname
>+                logging.warning(msg)
>+            else:
>+                msg = "Controller and Slave '%s' versions are not compatible!"\
>+                                                                    % hostname
>+                raise MachineError(msg)
>+
>         self._slave_desc = slave_desc
>
>         devices = self._rpc_call("get_devices")
>@@ -258,6 +272,19 @@ class Machine(object):
>
>         self._configured = True
>
>+    def _compare_versions(self, ctl_version, slave_version):

I don't like the function name. It should compare two versions. The
return value should be greater, less, equal.

IMO, I'd completely remove the check if it's git or major version.
Simple check if the versions are equal or different is ok.

-Jan

>+        try:
>+            int(ctl_version)
>+            ctl_is_git = False
>+        except ValueError:
>+            ctl_is_git = True
>+        try:
>+            int(slave_version)
>+            slave_is_git = False
>+        except ValueError:
>+            slave_is_git = True
>+        return (ctl_is_git, slave_is_git)
>+
>     def is_configured(self):
>         """ Test if the machine was configured """
>
>--
>2.4.11
>_______________________________________________
>LNST-developers mailing list
>lnst-developers@lists.fedorahosted.org
>https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedorahosted.org