This patch is first from series which implements version control in LNST.
The are two types of versions - LNST Major Version and git HEAD commit hash. LNST Major Version is used when LNST is installed and run from RPM and git commit hash is used when LNST is run from devel git repo.
LNSTMajorVersion variable in Config.py will have to be manully incremented each time new release is made.
Used version is stored in Config.version attribute.
Getting git HEAD commit hash is done in following steps: 1) Save cwd 2) Change cwd to directory where Config.py file is located 3) Try running git rev-parse HEAD, which returns commit hash 4) If git command execution was successful, use hash as version 5) Else use value of LNSTMajorVersion as version 6) Return to original working directory
If we didn't change cwd, we would have problems with running LNST installed from RPM in any git repo folder.
Signed-off-by: Jiri Prochazka jprochaz@redhat.com --- lnst/Common/Config.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py index 31481b2..7b4d17e 100644 --- a/lnst/Common/Config.py +++ b/lnst/Common/Config.py @@ -13,12 +13,15 @@ olichtne@redhat.com (Ondrej Lichtner) import os import sys import re +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
DefaultRPCPort = 9999
+LNSTMajorVersion = '11' + class ConfigError(Exception): pass
@@ -28,6 +31,21 @@ class Config():
def __init__(self): self._options = dict() + self.version = self._get_version() + + def _get_version(self): + # Check if I'm in git + try: + cwd = os.getcwd() + abspath = os.path.abspath(__file__) + dname = os.path.dirname(abspath) + os.chdir(dname) + head = subprocess.check_output(['git', 'rev-parse', 'HEAD']).strip() + return head + except subprocess.CalledProcessError: + return LNSTMajorVersion + finally: + os.chdir(cwd)
def controller_init(self): self._options['environment'] = dict()
This patch just adds LNST version string to slave description dict which is sent to Controller.
Version comparison will be done on Controller.
Signed-off-by: Jiri Prochazka jprochaz@redhat.com --- lnst/Slave/NetTestSlave.py | 1 + 1 file changed, 1 insertion(+)
diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index 9a366f3..8d8fada 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -97,6 +97,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"] = lnst_config.version
return ("hello", slave_desc)
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): + 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 """
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:
- Versions match - everything continues as normally
- 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.versionctl_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"\% hostnamelogging.warning(msg)else:msg = "Controller and Slave '%s' versions are not compatible!"\% hostnameraise 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 = Falseexcept ValueError:ctl_is_git = Truetry:int(slave_version)slave_is_git = Falseexcept ValueError:slave_is_git = Truereturn (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.fedorahoste...
2016-05-30 13:55 GMT+02:00 Jan Tluka jtluka@redhat.com:
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:
- Versions match - everything continues as normally
- 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.versionctl_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 aredifferent"\
%hostname
logging.warning(msg)else:msg = "Controller and Slave '%s' versions are notcompatible!"\
%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.
I might either rename the function or move version comparison from function init_connection() here.
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.
We discussed on meeting, that we don't want to quit test execution when both Ctl and Slave are run from git and hashes mismatch (if I develop feature on Ctl, it doesn't necessarily need to be on Slave as well. For this behaviour we need to distinguish between major version and git version.
-Jan
try:int(ctl_version)ctl_is_git = Falseexcept ValueError:ctl_is_git = Truetry:int(slave_version)slave_is_git = Falseexcept ValueError:slave_is_git = Truereturn (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.fedorahoste...
Mon, May 30, 2016 at 02:26:56PM CEST, jprochaz@redhat.com wrote:
2016-05-30 13:55 GMT+02:00 Jan Tluka jtluka@redhat.com:
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:
- Versions match - everything continues as normally
- 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.versionctl_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 aredifferent"\
%hostname
logging.warning(msg)else:msg = "Controller and Slave '%s' versions are notcompatible!"\
%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.
I might either rename the function or move version comparison from function init_connection() here.
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.
We discussed on meeting, that we don't want to quit test execution when both Ctl and Slave are run from git and hashes mismatch (if I develop feature on Ctl, it doesn't necessarily need to be on Slave as well. For this behaviour we need to distinguish between major version and git version.
I meant just the ctl_is_git, slave_is_git that's in this patch. In other words the whole compare_versions function and just leave: if slave_version != ctl_version:
That's sufficient from my point of view.
-Jan
try:int(ctl_version)ctl_is_git = Falseexcept ValueError:ctl_is_git = Truetry:int(slave_version)slave_is_git = Falseexcept ValueError:slave_is_git = Truereturn (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.fedorahoste...
On Mon, May 30, 2016 at 01:55:53PM +0200, Jan Tluka wrote:
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:
- Versions match - everything continues as normally
- 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.versionctl_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"\% hostnamelogging.warning(msg)else:msg = "Controller and Slave '%s' versions are not compatible!"\% hostnameraise 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
I agree... calling the function "compare_versions" and then not doing any comparison in the function is weird... And doing the actual compression after calling a compare_versions function is even more weird :)
-Ondrej
try:int(ctl_version)ctl_is_git = Falseexcept ValueError:ctl_is_git = Truetry:int(slave_version)slave_is_git = Falseexcept ValueError:slave_is_git = Truereturn (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.fedorahoste...
LNST-developers mailing list lnst-developers@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedorahoste...
On Sun, May 29, 2016 at 11:03:09AM +0200, Jiri Prochazka wrote:
This patch is first from series which implements version control in LNST.
The are two types of versions - LNST Major Version and git HEAD commit hash. LNST Major Version is used when LNST is installed and run from RPM and git commit hash is used when LNST is run from devel git repo.
LNSTMajorVersion variable in Config.py will have to be manully incremented each time new release is made.
Used version is stored in Config.version attribute.
Getting git HEAD commit hash is done in following steps:
- Save cwd
- Change cwd to directory where Config.py file is located
- Try running git rev-parse HEAD, which returns commit hash
- If git command execution was successful, use hash as version
- Else use value of LNSTMajorVersion as version
- Return to original working directory
If we didn't change cwd, we would have problems with running LNST installed from RPM in any git repo folder.
Signed-off-by: Jiri Prochazka jprochaz@redhat.com
Regarding this commit message... I don't think it should contain the "introduction" part of the patchset... that's normaly done via a separate 0/x email, git can create a template for you with the --cover-letter option. The individual commit messages should contain description of just that commit.
Most of the description is fine, only the formulation seems weird to me at the start... maybe that's just me... what do others think?
-Ondrej
lnst/Common/Config.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py index 31481b2..7b4d17e 100644 --- a/lnst/Common/Config.py +++ b/lnst/Common/Config.py @@ -13,12 +13,15 @@ olichtne@redhat.com (Ondrej Lichtner) import os import sys import re +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
DefaultRPCPort = 9999
+LNSTMajorVersion = '11'
class ConfigError(Exception): pass
@@ -28,6 +31,21 @@ class Config():
def __init__(self): self._options = dict()
self.version = self._get_version()def _get_version(self):
# Check if I'm in gittry:cwd = os.getcwd()abspath = os.path.abspath(__file__)dname = os.path.dirname(abspath)os.chdir(dname)head = subprocess.check_output(['git', 'rev-parse', 'HEAD']).strip()return headexcept subprocess.CalledProcessError:return LNSTMajorVersionfinally:os.chdir(cwd)def controller_init(self): self._options['environment'] = dict()
-- 2.4.11 _______________________________________________ LNST-developers mailing list lnst-developers@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedorahoste...
Sun, May 29, 2016 at 11:03:09AM CEST, jprochaz@redhat.com wrote:
This patch is first from series which implements version control in LNST.
The are two types of versions - LNST Major Version and git HEAD commit hash. LNST Major Version is used when LNST is installed and run from RPM and git commit hash is used when LNST is run from devel git repo.
LNSTMajorVersion variable in Config.py will have to be manully incremented each time new release is made.
I somehow missed this and I don't like this. It's addition of garbage commit everytime we need to do a release.
Since we release rpm only can't we rely on function of rpm? For example,
# rpm -qa --queryformat '%{VERSION}' lnst-ctl 11
-Jan
On Mon, May 30, 2016 at 04:30:36PM +0200, Jan Tluka wrote:
Sun, May 29, 2016 at 11:03:09AM CEST, jprochaz@redhat.com wrote:
This patch is first from series which implements version control in LNST.
The are two types of versions - LNST Major Version and git HEAD commit hash. LNST Major Version is used when LNST is installed and run from RPM and git commit hash is used when LNST is run from devel git repo.
LNSTMajorVersion variable in Config.py will have to be manully incremented each time new release is made.
I somehow missed this and I don't like this. It's addition of garbage commit everytime we need to do a release.
Since we release rpm only can't we rely on function of rpm? For example,
# rpm -qa --queryformat '%{VERSION}' lnst-ctl 11
-Jan
Well... we already do this "garbage" commit since the version needs to be updated in the setup.py script... I don't see a problem with both variables being updated in this commit... or maybe we could only have just one variable that is used for both the setup.py and the version comparison with slave?
-Ondrej
LNST-developers mailing list lnst-developers@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedorahoste...
lnst-developers@lists.fedorahosted.org
-
Jan Tluka -
Jiri Prochazka -
Ondrej Lichtner