Merge lp:~raharper/curtin/trunk.curtin-announce into lp:~curtin-dev/curtin/trunk

[Ryan Harper]

curtin: add version module and display in output and logs

It's difficult to determine what curtin version is running
from the collected logs in a maas deployment which uses
curtin by sending it to the target system. Resolve this by
having curtin announce itself.

- Introduce a new version module which does the following:
  - search for a .version file in the curtin PYTHONPATH
    if present and use that file contents for version
  - if no .version file is found, test if we're in-tree
    and use the existing curtin.__version__ value plus the
    bzr revno
  - if neither are found, just use the curtin.__version__ value
- Modify curtin.command.main to print the version string in argparse
  output
- Update curtin logging setup to emit a version string as soon as logging
  is configured

curtin: add version module and display in output and logs

It's difficult to determine what curtin version is running
from the collected logs in a maas deployment which uses
curtin by sending it to the target system. Resolve this by
having curtin announce itself.

- Introduce a new version module which does the following:
  - search for a .version file in the curtin PYTHONPATH
    if present and use that file contents for version
  - if no .version file is found, test if we're in-tree
    and use the existing curtin.__version__ value plus the
    bzr revno
  - if neither are found, just use the curtin.__version__ value
- Modify curtin.command.main to print the version string in argparse
  output
- Update curtin logging setup to emit a version string as soon as logging
  is configured

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:427
https://jenkins.ubuntu.com/server/job/curtin-ci/266/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=amd64/266
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=ppc64el/266
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=s390x/266

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/266/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

It seems like a generally nice idea. a couple of things though

I think it'd be good to get the dpkg info if it looks like it is installed.
possibly if the __file__ here is in some set of expected install paths... the problem with that is that dpkg -S 'filename' is heavy, and requires dpkg.

Alternatively, and maybe better, we could have the package install a file that states its version.
and then just read that.

[Ryan Harper (raharper)]

On Thu, Jan 19, 2017 at 12:30 PM, Scott Moser <email address hidden> wrote:

> It seems like a generally nice idea. a couple of things though
>
> I think it'd be good to get the dpkg info if it looks like it is installed.
> possibly if the __file__ here is in some set of expected install paths...
> the problem with that is that dpkg -S 'filename' is heavy, and requires
> dpkg.
>
> Alternatively, and maybe better, we could have the package install a file
> that states its version.
> and then just read that.
>
> Diff comments:
>
> >
> > === added file 'curtin/version.py'
> > --- curtin/version.py 1970-01-01 00:00:00 +0000
> > +++ curtin/version.py 2016-09-21 16:08:03 +0000
> > @@ -0,0 +1,40 @@
> > +from curtin import __version__ as old_version
> > +import os
> > +import subprocess
> > +
> > +
> > +def version_string():
> > + """ Extract a version string from curtin source or version file"""
> > + def _find_path(pathfile):
> > + """ Check for file existance and return dirpath
> > + Search PYTHONPATH, as curtin is typically
> > + launched with PYTHONPATH set, as with curtin pack
> > + executables.
> > + """
> > + if os.path.exists(pathfile):
> > + return os.getcwd()
> > + elif 'PYTHONPATH' in os.environ:
> > + for path in os.environ['PYTHONPATH'].split(":"):
> > + curpath = os.path.join(path, pathfile)
> > + if os.path.exists(curpath):
> > + return os.path.dirname(curpath)
> > +
> > + return None
> > +
>
> any reason not to use __file__ ? rather than PYTHONPATH ? python path just
> seems like it might go wrong.
>

I didn't know of __file__; fixing.

>
> > + dotversion = '.version'
> > + dotpath = _find_path(dotversion)
> > + if dotpath:
> > + return open(os.path.join(dotpath, dotversion),
> 'r').read().strip()
> > +
> > + bzrdir = _find_path('.bzr')
> > + revno = None
> > + if bzrdir:
> > + os.chdir(bzrdir)
> > + out = subprocess.check_output(['bzr', 'revno'])
> > + revno = "bzr%s" % out.decode('ascii').strip()
>
> any reason for ascii ?
>

Not sure why. I can 'utf-8' instead.

> Again here, i think better to just use the path that we know. .bzr will
> be 2 dirs up from this file name .
>

If I use __file__ above, then we get that.

>
> and we need to catch an exception here if there is a bzr dir but no bzr
> command.
>

Are there really .bzr dirs in a orig tarball? Otherwise, I don't see how
we'd have curtin source without bzr?

>
> > +
> > + version = old_version
> > + if revno:
> > + version += "~%s" % revno
> > +
> > + return version
>
>
> --
> https://code.launchpad.net/~raharper/curtin/trunk.curtin-
> announce/+merge/306358
> You are the owner of lp:~raharper/curtin/trunk.curtin-announce.
>

[lp:~raharper/curtin/trunk.curtin-announce]

428. By Ryan Harper on 2017-01-19

Use __file__, catch subprocess exception; export LANG into install log

429. By Ryan Harper on 2017-01-19

merge from trunk

430. By Ryan Harper on 2017-01-23

merge from lp:~smoser/curtin/trunk.curtin-announce

431. By Ryan Harper on 2017-01-23

pep fix for merge

432. By Ryan Harper on 2017-01-24

version: drop .version file; add dpkg-version, unittests

433. By Ryan Harper on 2017-01-27

merge from trunk

434. By Ryan Harper on 2017-02-06

merge from trunk

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:434
https://jenkins.ubuntu.com/server/job/curtin-ci/317/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/317
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/317
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/317
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-amd64/317
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/317

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/317/rebuild

review: Approve (continuous-integration)

=== modified file 'curtin/__init__.py'
--- curtin/__init__.py	2016-08-18 16:02:27 +0000
+++ curtin/__init__.py	2017-02-06 22:02:13 +0000
@@ -35,6 +35,8 @@
     'SUBCOMMAND_SYSTEM_UPGRADE',
     # supports new format of apt configuration
     'APT_CONFIG_V1',
+    # has version module
+    'HAS_VERSION_MODULE',
 ]
 
 __version__ = "0.1.0"

=== modified file 'curtin/commands/main.py'
--- curtin/commands/main.py	2016-08-10 21:44:19 +0000
+++ curtin/commands/main.py	2017-02-06 22:02:13 +0000
@@ -24,6 +24,9 @@
 from .. import log
 from .. import util
 from ..deps import install_deps
+import curtin.version
+
+VERSIONSTR = curtin.version.version_string()
 
 SUB_COMMAND_MODULES = [
     'apply_net', 'block-info', 'block-meta', 'block-wipe', 'curthooks',
@@ -57,7 +60,7 @@
 
 def get_main_parser(stacktrace=False, verbosity=0,
                     parser_class=argparse.ArgumentParser):
-    parser = parser_class(prog='curtin')
+    parser = parser_class(prog='curtin', epilog='Version %s' % VERSIONSTR)
     parser.add_argument('--showtrace', action='store_true', default=stacktrace)
     parser.add_argument('-v', '--verbose', action='count', default=verbosity,
                         dest='verbosity')
@@ -180,6 +183,8 @@
         sys.exit(1)
 
     log.basicConfig(stream=args.log_file, verbosity=verbosity)
+    log.LOG.error('curtin v. %s started' % VERSIONSTR)
+    log.LOG.error('LANG=%s', os.environ.get('LANG'))
 
     paths = util.get_paths()
 

=== modified file 'curtin/pack.py'
--- curtin/pack.py	2015-12-17 16:19:56 +0000
+++ curtin/pack.py	2017-02-06 22:02:13 +0000
@@ -140,6 +140,7 @@
         shutil.copytree(paths['helpers'], os.path.join(exdir, "helpers"))
         shutil.copytree(paths['lib'], os.path.join(exdir, "curtin"),
                         ignore=not_dot_py)
+
         write_exe_wrapper(entrypoint='curtin.commands.main',
                           path=os.path.join(bindir, 'curtin'),
                           deps_check_entry="curtin.deps.check")

=== added file 'curtin/version.py'
--- curtin/version.py	1970-01-01 00:00:00 +0000
+++ curtin/version.py	2017-02-06 22:02:13 +0000
@@ -0,0 +1,52 @@
+from curtin import __version__ as old_version
+import os
+import subprocess
+
+_PACKAGED_VERSION = '@@PACKAGED_VERSION@@'
+
+
+def version_string():
+    """ Extract a version string from curtin source or version file"""
+    def _find_path(pathfile):
+        """ Check for file existance and return dirpath
+            Search PYTHONPATH, as curtin is typically
+            launched with PYTHONPATH set, as with curtin pack
+            executables.
+        """
+        if os.path.exists(pathfile):
+            return os.getcwd()
+        else:
+            path = os.path.abspath(os.path.join(
+                                   __file__, '..', '..'))
+            curpath = os.path.join(path, pathfile)
+            if os.path.exists(curpath):
+                return os.path.dirname(curpath)
+
+        return None
+
+    if not _PACKAGED_VERSION.startswith('@@'):
+        return _PACKAGED_VERSION
+
+    bzrdir = _find_path('.bzr')
+    revno = dpkg_version = None
+    if bzrdir:
+        try:
+            out = subprocess.check_output(['bzr', 'revno'], cwd=bzrdir)
+            revno = "bzr%s" % out.decode('utf-8').strip()
+        except subprocess.CalledProcessError:
+            pass
+    else:
+        try:
+            out = subprocess.check_output(['dpkg-query', '--show',
+                                           '--showformat', '${Version}',
+                                           'curtin-common'])
+            dpkg_version = out.decode('utf-8').strip()
+        except subprocess.CalledProcessError:
+            pass
+    version = old_version
+    if revno:
+        version += "~%s" % revno
+    if dpkg_version:
+        version = dpkg_version
+
+    return version

=== modified file 'debian/rules'
--- debian/rules	2013-09-20 20:55:05 +0000
+++ debian/rules	2017-02-06 22:02:13 +0000
@@ -3,6 +3,10 @@
 PYVERS := $(shell pyversions -r)
 PY3VERS := $(shell py3versions -r)
 
+DEB_VERSION := $(shell dpkg-parsechangelog --show-field=Version)
+UPSTREAM_VERSION := $(shell x="$(DEB_VERSION)"; echo "$${x%-*}")
+PKG_VERSION := $(shell x="$(DEB_VERSION)"; echo "$${x\#\#*-}")
+
 %:
 	dh $@ --with=python2,python3
 
@@ -13,3 +17,5 @@
 		$$python setup.py install --root=$(CURDIR)/debian/tmp --install-layout=deb; \
 	done
 	chmod 755 $(CURDIR)/debian/tmp/usr/lib/curtin/helpers/*
+	find $(CURDIR)/debian/tmp
+	for f in $$(find $(CURDIR)/debian/tmp/usr/lib -type f -name version.py); do [ -f "$$f" ] || continue; sed -i 's,@@PACKAGED_VERSION@@,$(DEB_VERSION),' "$$f"; done

=== added file 'tests/unittests/test_version.py'
--- tests/unittests/test_version.py	1970-01-01 00:00:00 +0000
+++ tests/unittests/test_version.py	2017-02-06 22:02:13 +0000
@@ -0,0 +1,87 @@
+from unittest import TestCase
+import mock
+import subprocess
+import os
+
+from curtin import version
+from curtin import __version__ as old_version
+
+
+class CurtinVersionBase(TestCase):
+    def setUp(self):
+        super(CurtinVersionBase, self).setUp()
+
+    def add_patch(self, target, attr):
+        """Patches specified target object and sets it as attr on test
+        instance also schedules cleanup"""
+        m = mock.patch(target, autospec=True)
+        p = m.start()
+        self.addCleanup(m.stop)
+        setattr(self, attr, p)
+
+
+class TestCurtinVersion(CurtinVersionBase):
+
+    def setUp(self):
+        self.add_patch('subprocess.check_output', 'mock_subp')
+        self.add_patch('os.path', 'mock_path')
+
+    @mock.patch.object(os, 'getcwd')
+    def test_packaged_version(self, mock_getcwd):
+        original_pkg_string = version._PACKAGED_VERSION
+        test_pkg_version = '9.8.7-curtin-0ubuntu12'
+        version._PACKAGED_VERSION = test_pkg_version
+
+        ver_string = version.version_string()
+
+        # check we got the packaged version string set
+        self.assertEqual(test_pkg_version, ver_string)
+        # make sure we didn't take any other path
+        self.assertEqual([], self.mock_path.call_args_list)
+        self.assertEqual([], mock_getcwd.call_args_list)
+        self.assertEqual([], self.mock_subp.call_args_list)
+
+        version._PACKAGED_VERSION = original_pkg_string
+
+    def test_bzr_revno_version(self):
+        self.mock_path.exists.return_value = True
+        bzr_revno = "999"
+        self.mock_subp.return_value = bzr_revno.encode("utf-8")
+
+        ver_string = version.version_string()
+        self.assertEqual(old_version + "~bzr" + bzr_revno, ver_string)
+
+    @mock.patch.object(os, 'getcwd')
+    def test_bzr_revno_version_exception(self, mock_getcwd):
+        self.mock_path.exists.return_value = True
+        mock_getcwd.return_value = "/tmp/foo"
+        self.mock_subp.side_effect = subprocess.CalledProcessError(1, 'foo')
+
+        ver_string = version.version_string()
+        self.assertEqual(old_version, ver_string)
+
+    def test_dpkg_version(self):
+        self.mock_path.exists.return_value = False
+        dpkg_version = "7.12.23_curtin_2ubunt29"
+        self.mock_subp.return_value = dpkg_version.encode("utf-8")
+
+        ver_string = version.version_string()
+        self.assertEqual(dpkg_version, ver_string)
+
+    def test_dpkg_version_exception(self):
+        self.mock_path.exists.return_value = True
+        self.mock_subp.side_effect = subprocess.CalledProcessError(1, '')
+
+        ver_string = version.version_string()
+        self.assertEqual(old_version, ver_string)
+
+    def test_old_version(self):
+        self.mock_path.exists.return_value = False
+        self.mock_subp.return_value = "".encode('utf-8')
+
+        ver_string = version.version_string()
+
+        self.assertEqual(old_version, ver_string)
+
+
+# vi: ts=4 expandtab syntax=python

=== modified file 'tests/vmtests/__init__.py'
--- tests/vmtests/__init__.py	2017-02-06 20:29:33 +0000
+++ tests/vmtests/__init__.py	2017-02-06 22:02:13 +0000
@@ -13,6 +13,7 @@
 import yaml
 import curtin.net as curtin_net
 import curtin.util as util
+import curtin.version
 
 from curtin.commands.install import INSTALL_PASS_MSG
 
@@ -748,6 +749,23 @@
         with open(os.path.join(self.td.collect, filename), mode) as fp:
             return fp.read()
 
+    def load_log_file(self, filename):
+        with open(filename, 'rb') as l:
+            return l.read().decode('utf-8', errors='replace')
+
+    def get_install_log_curtin_version(self):
+        # "curtin v. 0.1.0~bzr426 started"
+        install_log = self.load_log_file(self.install_log)
+        curtin_vers = [line for line in install_log.splitlines()
+                       if 'curtin v.' in line and line.endswith('started')]
+        for ver in curtin_vers:
+            version = ver.split('curtin v. ')[-1].split(' started')[0]
+            if len(version) > 0:
+                return version
+
+    def get_curtin_version(self):
+        return curtin.version.version_string()
+
     def check_file_strippedline(self, filename, search):
         lines = self.load_collect_file(filename).splitlines()
         self.assertIn(search, [i.strip() for i in lines])

=== modified file 'tests/vmtests/test_basic.py'
--- tests/vmtests/test_basic.py	2016-08-25 23:32:11 +0000
+++ tests/vmtests/test_basic.py	2017-02-06 22:02:13 +0000
@@ -122,6 +122,13 @@
             # no proxy, so the output of apt-config dump should be empty
             self.assertEqual("", apt_proxy_found)
 
+    def test_curtin_install_version(self):
+        installed_version = self.get_install_log_curtin_version()
+        print('Install log version: %s' % installed_version)
+        source_version = self.get_curtin_version()
+        print('Source repo version: %s' % source_version)
+        self.assertEqual(source_version, installed_version)
+
 
 class PreciseTestBasic(relbase.precise, TestBasicAbs):
     __test__ = True