From: Ondrej Lichtner olichtne@redhat.com
LNSTVersion is now a class instantiated as a module object lnst_version. This object is then used by both the Slave and Controller for comparison.
This commit also adds a new module lnst.Common.GitVersion which will only be present in the git repository (will not be installed by setup.py and shouldn't be included in packages). This module defines a method that detects a git instalation and returns the current revision hash which is appended to the version number. If the module is present, LNST runs as a development version - failed version comparison only warns about the different versions. In case something goes wrong during the git version detection (e.g. git isn't installed) LNST won't start and return an exception.
If the module isn't present it's assumed that LNST is installed as a stable version with the version number defined in lnst.Common.Version.LNSTVersion._version and a failed version check will report an error and the recipe execution won't continue.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Common/Config.py | 25 ------------------------- lnst/Common/GitVersion.py | 35 +++++++++++++++++++++++++++++++++++ lnst/Common/Version.py | 36 +++++++++++++++++++++++++++++++++++- lnst/Controller/Machine.py | 19 ++++++++++--------- lnst/Slave/NetTestSlave.py | 3 ++- 5 files changed, 82 insertions(+), 36 deletions(-) create mode 100644 lnst/Common/GitVersion.py
diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py index 3745094..2edf854 100644 --- a/lnst/Common/Config.py +++ b/lnst/Common/Config.py @@ -17,7 +17,6 @@ import subprocess from lnst.Common.Utils import bool_it from lnst.Common.NetUtils import verify_mac_address from lnst.Common.Colours import get_preset_conf -from lnst.Common.Version import LNSTMajorVersion from lnst.Common.LnstError import LnstError
DefaultRPCPort = 9999 @@ -31,35 +30,11 @@ class Config():
def __init__(self): self._options = dict() - self.version = self._get_version() self._init_options()
def _init_options(self): raise NotImplementedError()
- def _get_version(self): - # Check if I'm in git - cwd = os.getcwd() - abspath = os.path.abspath(__file__) - dname = os.path.dirname(abspath) - os.chdir(dname) - with open(os.devnull, 'w') as null: - cmd = ['git', 'rev-parse', 'HEAD'] - try: - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=null) - data = proc.communicate() - except: - os.chdir(cwd) - return LNSTMajorVersion - # git command passed - if data[0] != '': - version = data[0].strip() - # git command failed - else: - version = LNSTMajorVersion - os.chdir(cwd) - return version - def colours_scheme(self): self._options['colours'] = dict() self._options['colours']["disable_colours"] = {\ diff --git a/lnst/Common/GitVersion.py b/lnst/Common/GitVersion.py new file mode 100644 index 0000000..b24962a --- /dev/null +++ b/lnst/Common/GitVersion.py @@ -0,0 +1,35 @@ +""" +Module that calculates the LNST version based on the currently checked-out git +commit. Overrides the default LNST version that is reported when LNST is +installed on the machine with setup.py which skips this module. + +Copyright 2017 Red Hat, Inc. +Licensed under the GNU General Public License, version 2 as +published by the Free Software Foundation; see COPYING for details. +""" + +__autor__ = """ +olichtne@redhat.com (Ondrej Lichtner) +""" + +import os +import subprocess +from lnst.Common.LnstError import LnstError +from lnst.Common.Utils import is_installed + +def git_version(): + if not is_installed("git"): + raise LnstError("git not installed, can't check for version") + + with open(os.devnull, 'w') as null: + cwd = os.getcwd() + abspath = os.path.abspath(__file__) + dname = os.path.dirname(abspath) + os.chdir(dname) + + cmd = ['git', 'rev-parse', 'HEAD'] + try: + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=null) + return proc.communicate()[0].strip() + finally: + os.chdir(cwd) diff --git a/lnst/Common/Version.py b/lnst/Common/Version.py index bafbb83..b2da2d2 100644 --- a/lnst/Common/Version.py +++ b/lnst/Common/Version.py @@ -11,4 +11,38 @@ __autor__ = """ olichtne@redhat.com (Ondrej Lichtner) """
-LNSTMajorVersion = '11' +try: + from lnst.Common.GitVersion import git_version +except ImportError: + git_version = None + +class LNSTVersion(object): + def __init__(self): + self._version = "14" + + if git_version: + self._version += "-" + git_version() + + @property + def version(self): + return self._version + + @property + def is_git_version(self): + return git_version is not None + + def __str__(self): + return str(self.version) + + def __eq__(self, other): + if isinstance(other, self.__class__): + return str(self) == str(other) + return NotImplemented + + def __ne__(self, other): + res = self == other + if res is not NotImplemented: + return not res + return NotImplemented + +lnst_version = LNSTVersion() diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index 38a3646..7730d79 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -18,6 +18,7 @@ import signal from lnst.Common.Utils import sha256sum from lnst.Common.Utils import check_process_running from lnst.Common.TestModule import BaseTestModule +from lnst.Common.Version import lnst_version from lnst.Controller.Common import ControllerError from lnst.Controller.CtlSecSocket import CtlSecSocket from lnst.Devices import device_classes @@ -216,19 +217,19 @@ class Machine(object): raise MachineError(msg)
slave_version = slave_desc["lnst_version"] - slave_is_git = self.is_git_version(slave_version) - ctl_version = self._ctl_config.version - ctl_is_git = self.is_git_version(ctl_version) - if slave_version != ctl_version: - if ctl_is_git and slave_is_git: - msg = "Controller and Slave '%s' git versions are different"\ - % hostname + + if lnst_version != slave_version: + if lnst_version.is_git_version: + msg = ("Controller ({}) and Slave '{}' ({}) versions " + "are different".format(lnst_version, hostname, + slave_version)) logging.warning(len(msg)*"=") logging.warning(msg) logging.warning(len(msg)*"=") else: - msg = "Controller and Slave '%s' versions are not compatible!"\ - % hostname + msg = ("Controller ({}) and Slave '{}' ({}) versions " + "are not compatible!".format(lnst_version, hostname, + slave_version)) raise MachineError(msg)
self._slave_desc = slave_desc diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index 942a45c..9c5486c 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -44,6 +44,7 @@ from lnst.Common.DeviceError import DeviceConfigValueError from lnst.Common.TestModule import BaseTestModule from lnst.Common.Parameters import Parameters, DeviceParam from lnst.Common.IpAddress import ipaddress +from lnst.Common.Version import lnst_version from lnst.Slave.Job import Job, JobContext from lnst.Slave.InterfaceManager import InterfaceManager from lnst.Slave.BridgeTool import BridgeTool @@ -104,7 +105,7 @@ class SlaveMethods: r_release, _ = exec_cmd("cat /etc/redhat-release", False, False, False) slave_desc["kernel_release"] = k_release.strip() slave_desc["redhat_release"] = r_release.strip() - slave_desc["lnst_version"] = self._slave_config.version + slave_desc["lnst_version"] = lnst_version
return ("hello", slave_desc)
Mon, Dec 11, 2017 at 09:41:46AM CET, olichtne@redhat.com wrote:
From: Ondrej Lichtner olichtne@redhat.com
LNSTVersion is now a class instantiated as a module object lnst_version. This object is then used by both the Slave and Controller for comparison.
This commit also adds a new module lnst.Common.GitVersion which will only be present in the git repository (will not be installed by setup.py and shouldn't be included in packages). This module defines a method that detects a git instalation and returns the current revision hash which is appended to the version number. If the module is present, LNST runs as a development version - failed version comparison only warns about the different versions. In case something goes wrong during the git version detection (e.g. git isn't installed) LNST won't start and return an exception.
If the module isn't present it's assumed that LNST is installed as a stable version with the version number defined in lnst.Common.Version.LNSTVersion._version and a failed version check will report an error and the recipe execution won't continue.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com
I like this. Thanks. Some comments inline. Other than those, Acked-by: Jan Tluka jtluka@redhat.com
lnst/Common/Config.py | 25 ------------------------- lnst/Common/GitVersion.py | 35 +++++++++++++++++++++++++++++++++++ lnst/Common/Version.py | 36 +++++++++++++++++++++++++++++++++++- lnst/Controller/Machine.py | 19 ++++++++++--------- lnst/Slave/NetTestSlave.py | 3 ++- 5 files changed, 82 insertions(+), 36 deletions(-) create mode 100644 lnst/Common/GitVersion.py
diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py index 3745094..2edf854 100644 --- a/lnst/Common/Config.py +++ b/lnst/Common/Config.py @@ -17,7 +17,6 @@ import subprocess from lnst.Common.Utils import bool_it from lnst.Common.NetUtils import verify_mac_address from lnst.Common.Colours import get_preset_conf -from lnst.Common.Version import LNSTMajorVersion from lnst.Common.LnstError import LnstError
DefaultRPCPort = 9999 @@ -31,35 +30,11 @@ class Config():
def __init__(self): self._options = dict()
self.version = self._get_version() self._init_options()def _init_options(self): raise NotImplementedError()
def _get_version(self):
# Check if I'm in gitcwd = os.getcwd()abspath = os.path.abspath(__file__)dname = os.path.dirname(abspath)os.chdir(dname)with open(os.devnull, 'w') as null:cmd = ['git', 'rev-parse', 'HEAD']try:proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=null)data = proc.communicate()except:os.chdir(cwd)return LNSTMajorVersion# git command passedif data[0] != '':version = data[0].strip()# git command failedelse:version = LNSTMajorVersionos.chdir(cwd)return versiondef colours_scheme(self): self._options['colours'] = dict() self._options['colours']["disable_colours"] = {\
diff --git a/lnst/Common/GitVersion.py b/lnst/Common/GitVersion.py new file mode 100644 index 0000000..b24962a --- /dev/null +++ b/lnst/Common/GitVersion.py @@ -0,0 +1,35 @@ +""" +Module that calculates the LNST version based on the currently checked-out git +commit. Overrides the default LNST version that is reported when LNST is +installed on the machine with setup.py which skips this module.
+Copyright 2017 Red Hat, Inc. +Licensed under the GNU General Public License, version 2 as +published by the Free Software Foundation; see COPYING for details. +"""
+__autor__ = """
^ autHor ?
+olichtne@redhat.com (Ondrej Lichtner) +"""
+import os +import subprocess +from lnst.Common.LnstError import LnstError +from lnst.Common.Utils import is_installed
+def git_version():
- if not is_installed("git"):
raise LnstError("git not installed, can't check for version")- with open(os.devnull, 'w') as null:
cwd = os.getcwd()abspath = os.path.abspath(__file__)dname = os.path.dirname(abspath)os.chdir(dname)cmd = ['git', 'rev-parse', 'HEAD']try:proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=null)return proc.communicate()[0].strip()finally:os.chdir(cwd)diff --git a/lnst/Common/Version.py b/lnst/Common/Version.py index bafbb83..b2da2d2 100644 --- a/lnst/Common/Version.py +++ b/lnst/Common/Version.py @@ -11,4 +11,38 @@ __autor__ = """ olichtne@redhat.com (Ondrej Lichtner) """
-LNSTMajorVersion = '11' +try:
- from lnst.Common.GitVersion import git_version
+except ImportError:
- git_version = None
+class LNSTVersion(object):
- def __init__(self):
self._version = "14"if git_version:self._version += "-" + git_version()- @property
- def version(self):
return self._version- @property
- def is_git_version(self):
return git_version is not None
I'm not an expert on @property use, but can a method be a property? What's the purpose here?
- def __str__(self):
return str(self.version)- def __eq__(self, other):
if isinstance(other, self.__class__):return str(self) == str(other)return NotImplemented- def __ne__(self, other):
res = self == otherif res is not NotImplemented:return not resreturn NotImplemented+lnst_version = LNSTVersion() diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index 38a3646..7730d79 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -18,6 +18,7 @@ import signal from lnst.Common.Utils import sha256sum from lnst.Common.Utils import check_process_running from lnst.Common.TestModule import BaseTestModule +from lnst.Common.Version import lnst_version from lnst.Controller.Common import ControllerError from lnst.Controller.CtlSecSocket import CtlSecSocket from lnst.Devices import device_classes @@ -216,19 +217,19 @@ class Machine(object): raise MachineError(msg)
slave_version = slave_desc["lnst_version"]
slave_is_git = self.is_git_version(slave_version)ctl_version = self._ctl_config.versionctl_is_git = self.is_git_version(ctl_version)if slave_version != ctl_version:if ctl_is_git and slave_is_git:msg = "Controller and Slave '%s' git versions are different"\% hostname
if lnst_version != slave_version:if lnst_version.is_git_version:msg = ("Controller ({}) and Slave '{}' ({}) versions ""are different".format(lnst_version, hostname,slave_version)) logging.warning(len(msg)*"=") logging.warning(msg) logging.warning(len(msg)*"=") else:
msg = "Controller and Slave '%s' versions are not compatible!"\% hostname
msg = ("Controller ({}) and Slave '{}' ({}) versions ""are not compatible!".format(lnst_version, hostname,slave_version)) raise MachineError(msg) self._slave_desc = slave_descdiff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index 942a45c..9c5486c 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -44,6 +44,7 @@ from lnst.Common.DeviceError import DeviceConfigValueError from lnst.Common.TestModule import BaseTestModule from lnst.Common.Parameters import Parameters, DeviceParam from lnst.Common.IpAddress import ipaddress +from lnst.Common.Version import lnst_version from lnst.Slave.Job import Job, JobContext from lnst.Slave.InterfaceManager import InterfaceManager from lnst.Slave.BridgeTool import BridgeTool @@ -104,7 +105,7 @@ class SlaveMethods: r_release, _ = exec_cmd("cat /etc/redhat-release", False, False, False) slave_desc["kernel_release"] = k_release.strip() slave_desc["redhat_release"] = r_release.strip()
slave_desc["lnst_version"] = self._slave_config.version
slave_desc["lnst_version"] = lnst_version return ("hello", slave_desc)-- 2.15.1 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org
On Mon, Dec 11, 2017 at 10:14:51AM +0100, Jan Tluka wrote:
Mon, Dec 11, 2017 at 09:41:46AM CET, olichtne@redhat.com wrote:
+""" +Module that calculates the LNST version based on the currently checked-out git +commit. Overrides the default LNST version that is reported when LNST is +installed on the machine with setup.py which skips this module.
+Copyright 2017 Red Hat, Inc. +Licensed under the GNU General Public License, version 2 as +published by the Free Software Foundation; see COPYING for details. +"""
+__autor__ = """
^ autHor ?
good catch... I normally just copy this from an existing module and edit the description - I checked and this error is present in several Common modules, I'll send a separate patch that fixes these...
- @property
- def is_git_version(self):
return git_version is not NoneI'm not an expert on @property use, but can a method be a property? What's the purpose here?
yeah, @property is actually intended to work only with methods - instead of having a private atttribute and a getter that somehow calculates it's value (more than just return a single value) you define the method and use the @property decorator and therefore define the "attribute" and it's getter all in one.
Thanks for the review.
-Ondrej
lnst-developers@lists.fedorahosted.org
-
Jan Tluka -
olichtne＠redhat.com -
Ondrej Lichtner