Hi,
I forgot to add to the Makefile. This is the interdiff for the patch
that add 'version.py'.
diff --git a/Makefile.am b/Makefile.am
index e610179..5d1ad95 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -483,6 +483,7 @@ utils_PYTHON = \
lib/utils/retry.py \
lib/utils/storage.py \
lib/utils/text.py \
+ lib/utils/version.py \
lib/utils/wrapper.py \
lib/utils/x509.py
On Wed, Sep 25, 2013 at 12:29:02PM +0200, Thomas Thrainer wrote:
> On Wed, Sep 25, 2013 at 12:11 PM, Jose A. Lopes <[email protected]>wrote:
>
> > Functions 'BuildVersion' and 'SplitVersion' are no longer needed by
> > the constants and, given that they are not constants, they should be
> > moved elsewhere. Since they are only used by 'cfgupgrade' and tests,
> > these functions are moved to 'lib/utils/version.py' and references to
> > them updated. Note that in 'lib/server/masterd.py', local variable
> > 'version' is renamed 'ver' to avoid redefining the import
> > 'ganeti.utils.version'.
> >
> > Signed-off-by: Jose A. Lopes <[email protected]>
> > ---
> > lib/constants.py | 43 ------------------------
> > lib/server/masterd.py | 12 ++++---
> > lib/utils/version.py | 64
> > ++++++++++++++++++++++++++++++++++++
> > test/py/cfgupgrade_unittest.py | 52 +++++++++++++++--------------
> > test/py/ganeti.constants_unittest.py | 20 ++++++-----
> > tools/cfgupgrade | 11 ++++---
> > tools/cfgupgrade12 | 4 ++-
> > 7 files changed, 118 insertions(+), 88 deletions(-)
> > create mode 100644 lib/utils/version.py
> >
> > diff --git a/lib/constants.py b/lib/constants.py
> > index 67a1cc5..218e937 100644
> > --- a/lib/constants.py
> > +++ b/lib/constants.py
> > @@ -44,49 +44,6 @@ VCS_VERSION = _vcsversion.VCS_VERSION
> > EXPORT_VERSION = 0
> > RAPI_VERSION = 2
> >
> > -
> > -# Format for CONFIG_VERSION:
> > -# 01 03 0123 = 01030123
> > -# ^^ ^^ ^^^^
> > -# | | + Configuration version/revision
> > -# | + Minor version
> > -# + Major version
> > -#
> > -# It is stored as an integer. Make sure not to write an octal number.
> > -
> > -# BuildVersion and SplitVersion must be in here because we can't import
> > other
> > -# modules. The cfgupgrade tool must be able to read and write version
> > numbers
> > -# and thus requires these functions. To avoid code duplication, they're
> > kept in
> > -# here.
> > -
> > -def BuildVersion(major, minor, revision):
> > - """Calculates int version number from major, minor and revision numbers.
> > -
> > - Returns: int representing version number
> > -
> > - """
> > - assert isinstance(major, int)
> > - assert isinstance(minor, int)
> > - assert isinstance(revision, int)
> > - return (1000000 * major +
> > - 10000 * minor +
> > - 1 * revision)
> > -
> > -
> > -def SplitVersion(version):
> > - """Splits version number stored in an int.
> > -
> > - Returns: tuple; (major, minor, revision)
> > -
> > - """
> > - assert isinstance(version, int)
> > -
> > - (major, remainder) = divmod(version, 1000000)
> > - (minor, revision) = divmod(remainder, 10000)
> > -
> > - return (major, minor, revision)
> > -
> > -
> > CONFIG_MAJOR = _constants.CONFIG_MAJOR
> > CONFIG_MINOR = _constants.CONFIG_MINOR
> > CONFIG_REVISION = _constants.CONFIG_REVISION
> > diff --git a/lib/server/masterd.py b/lib/server/masterd.py
> > index a41b203..dd1f5eb 100644
> > --- a/lib/server/masterd.py
> > +++ b/lib/server/masterd.py
> > @@ -61,6 +61,8 @@ from ganeti import runtime
> > from ganeti import pathutils
> > from ganeti import ht
> >
> > +from ganeti.utils import version
> > +
> >
> > CLIENT_REQUEST_WORKERS = 16
> >
> > @@ -90,7 +92,7 @@ class ClientRequestWorker(workerpool.BaseWorker):
> > client_ops = ClientOps(server)
> >
> > try:
> > - (method, args, version) = luxi.ParseRequest(message)
> > + (method, args, ver) = luxi.ParseRequest(message)
> > except luxi.ProtocolError, err:
> > logging.error("Protocol Error: %s", err)
> > client.close_log()
> > @@ -99,9 +101,9 @@ class ClientRequestWorker(workerpool.BaseWorker):
> > success = False
> > try:
> > # Verify client's version if there was one in the request
> > - if version is not None and version != constants.LUXI_VERSION:
> > + if ver is not None and ver != constants.LUXI_VERSION:
> > raise errors.LuxiError("LUXI version mismatch, server %s, request
> > %s" %
> > - (constants.LUXI_VERSION, version))
> > + (constants.LUXI_VERSION, ver))
> >
> > result = client_ops.handle_request(method, args)
> > success = True
> > @@ -685,8 +687,8 @@ def CheckMasterd(options, args):
> > try:
> > config.ConfigWriter()
> > except errors.ConfigVersionMismatch, err:
> > - v1 = "%s.%s.%s" % constants.SplitVersion(err.args[0])
> > - v2 = "%s.%s.%s" % constants.SplitVersion(err.args[1])
> > + v1 = "%s.%s.%s" % version.SplitVersion(err.args[0])
> > + v2 = "%s.%s.%s" % version.SplitVersion(err.args[1])
> > print >> sys.stderr, \
> > ("Configuration version mismatch. The current Ganeti software"
> > " expects version %s, but the on-disk configuration file has"
> > diff --git a/lib/utils/version.py b/lib/utils/version.py
> > new file mode 100644
> > index 0000000..b291dc8
> > --- /dev/null
> > +++ b/lib/utils/version.py
> > @@ -0,0 +1,64 @@
> > +#!/usr/bin/python
> > +#
> > +
> > +# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Google Inc.
> >
>
> Please add 2013.
>
>
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful, but
> > +# WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > +# General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write to the Free Software
> > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > +# 02110-1301, USA.
> > +
> > +
> > +"""Version utilities."""
> > +
> > +
> > +# Format for CONFIG_VERSION:
> > +# 01 03 0123 = 01030123
> > +# ^^ ^^ ^^^^
> > +# | | + Configuration version/revision
> > +# | + Minor version
> > +# + Major version
> > +#
> > +# It is stored as an integer. Make sure not to write an octal number.
> > +
> > +# BuildVersion and SplitVersion must be in here because we can't import
> > other
> > +# modules. The cfgupgrade tool must be able to read and write version
> > numbers
> > +# and thus requires these functions. To avoid code duplication, they're
> > kept in
> > +# here.
> > +
> > +def BuildVersion(major, minor, revision):
> > + """Calculates int version number from major, minor and revision numbers.
> > +
> > + Returns: int representing version number
> > +
> > + """
> > + assert isinstance(major, int)
> > + assert isinstance(minor, int)
> > + assert isinstance(revision, int)
> > + return (1000000 * major +
> > + 10000 * minor +
> > + 1 * revision)
> > +
> > +
> > +def SplitVersion(version):
> > + """Splits version number stored in an int.
> > +
> > + Returns: tuple; (major, minor, revision)
> > +
> > + """
> > + assert isinstance(version, int)
> > +
> > + (major, remainder) = divmod(version, 1000000)
> > + (minor, revision) = divmod(remainder, 10000)
> > +
> > + return (major, minor, revision)
> > diff --git a/test/py/cfgupgrade_unittest.py
> > b/test/py/cfgupgrade_unittest.py
> > index 2e0aadf..a08b167 100755
> > --- a/test/py/cfgupgrade_unittest.py
> > +++ b/test/py/cfgupgrade_unittest.py
> > @@ -34,6 +34,8 @@ from ganeti import errors
> > from ganeti import serializer
> > from ganeti import netutils
> >
> > +from ganeti.utils import version
> > +
> > import testutils
> >
> >
> > @@ -199,7 +201,7 @@ class TestCfgupgrade(unittest.TestCase):
> >
> > self.assertFalse(os.path.exists(os.path.dirname(self.rapi_users_path)))
> >
> > utils.WriteFile(self.rapi_users_path_pre24, data="some user\n")
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 3, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 3, 0), False)
> >
> > self.assertTrue(os.path.isdir(os.path.dirname(self.rapi_users_path)))
> > self.assert_(os.path.islink(self.rapi_users_path_pre24))
> > @@ -215,7 +217,7 @@ class TestCfgupgrade(unittest.TestCase):
> >
> > os.mkdir(os.path.dirname(self.rapi_users_path))
> > utils.WriteFile(self.rapi_users_path, data="other user\n")
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 3, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 3, 0), False)
> >
> > self.assert_(os.path.islink(self.rapi_users_path_pre24))
> > self.assert_(os.path.isfile(self.rapi_users_path))
> > @@ -232,7 +234,7 @@ class TestCfgupgrade(unittest.TestCase):
> > os.symlink(self.rapi_users_path, self.rapi_users_path_pre24)
> > utils.WriteFile(self.rapi_users_path, data="hello world\n")
> >
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 2, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 2, 0), False)
> >
> > self.assert_(os.path.isfile(self.rapi_users_path) and
> > not os.path.islink(self.rapi_users_path))
> > @@ -251,7 +253,7 @@ class TestCfgupgrade(unittest.TestCase):
> > utils.WriteFile(self.rapi_users_path_pre24, data="hello world\n")
> >
> > self.assertRaises(Exception, self._TestSimpleUpgrade,
> > - constants.BuildVersion(2, 2, 0), False)
> > + version.BuildVersion(2, 2, 0), False)
> >
> > for path in [self.rapi_users_path, self.rapi_users_path_pre24]:
> > self.assert_(os.path.isfile(path) and not os.path.islink(path))
> > @@ -264,7 +266,7 @@ class TestCfgupgrade(unittest.TestCase):
> > self.assertFalse(os.path.exists(self.rapi_users_path_pre24))
> >
> > utils.WriteFile(self.rapi_users_path_pre24, data="some user\n")
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 3, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 3, 0), True)
> >
> > self.assertFalse(os.path.isdir(os.path.dirname(self.rapi_users_path)))
> > self.assertTrue(os.path.isfile(self.rapi_users_path_pre24) and
> > @@ -277,7 +279,7 @@ class TestCfgupgrade(unittest.TestCase):
> >
> > os.mkdir(os.path.dirname(self.rapi_users_path))
> > utils.WriteFile(self.rapi_users_path, data="other user\n")
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 3, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 3, 0), True)
> >
> > self.assertTrue(os.path.isfile(self.rapi_users_path) and
> > not os.path.islink(self.rapi_users_path))
> > @@ -292,7 +294,7 @@ class TestCfgupgrade(unittest.TestCase):
> > os.symlink(self.rapi_users_path, self.rapi_users_path_pre24)
> > utils.WriteFile(self.rapi_users_path, data="hello world\n")
> >
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 2, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 2, 0), True)
> >
> > self.assertTrue(os.path.islink(self.rapi_users_path_pre24))
> > self.assertTrue(os.path.isfile(self.rapi_users_path) and
> > @@ -305,7 +307,7 @@ class TestCfgupgrade(unittest.TestCase):
> > def testFileStoragePathsDryRun(self):
> > self.assertFalse(os.path.exists(self.file_storage_paths))
> >
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 6, 0), True,
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 6, 0), True,
> > file_storage_dir=self.tmpdir,
> > shared_file_storage_dir="/tmp")
> >
> > @@ -314,7 +316,7 @@ class TestCfgupgrade(unittest.TestCase):
> > def testFileStoragePathsBoth(self):
> > self.assertFalse(os.path.exists(self.file_storage_paths))
> >
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 6, 0), False,
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 6, 0), False,
> > file_storage_dir=self.tmpdir,
> > shared_file_storage_dir="/tmp")
> >
> > @@ -330,7 +332,7 @@ class TestCfgupgrade(unittest.TestCase):
> > def testFileStoragePathsSharedOnly(self):
> > self.assertFalse(os.path.exists(self.file_storage_paths))
> >
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 5, 0), False,
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 5, 0), False,
> > file_storage_dir=None,
> > shared_file_storage_dir=self.tmpdir)
> >
> > @@ -341,28 +343,28 @@ class TestCfgupgrade(unittest.TestCase):
> > self.assertFalse(lines)
> >
> > def testUpgradeFrom_2_0(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 0, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 0, 0), False)
> >
> > def testUpgradeFrom_2_1(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 1, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 1, 0), False)
> >
> > def testUpgradeFrom_2_2(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 2, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 2, 0), False)
> >
> > def testUpgradeFrom_2_3(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 3, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 3, 0), False)
> >
> > def testUpgradeFrom_2_4(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 4, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 4, 0), False)
> >
> > def testUpgradeFrom_2_5(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 5, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 5, 0), False)
> >
> > def testUpgradeFrom_2_6(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 6, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 6, 0), False)
> >
> > def testUpgradeFrom_2_7(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 7, 0), False)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 7, 0), False)
> >
> > def testUpgradeFullConfigFrom_2_7(self):
> > self._TestUpgradeFromFile("cluster_config_2.7.json", False)
> > @@ -431,25 +433,25 @@ class TestCfgupgrade(unittest.TestCase):
> > self._RunDowngradeTwice()
> >
> > def testUpgradeDryRunFrom_2_0(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 0, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 0, 0), True)
> >
> > def testUpgradeDryRunFrom_2_1(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 1, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 1, 0), True)
> >
> > def testUpgradeDryRunFrom_2_2(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 2, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 2, 0), True)
> >
> > def testUpgradeDryRunFrom_2_3(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 3, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 3, 0), True)
> >
> > def testUpgradeDryRunFrom_2_4(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 4, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 4, 0), True)
> >
> > def testUpgradeDryRunFrom_2_5(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 5, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 5, 0), True)
> >
> > def testUpgradeDryRunFrom_2_6(self):
> > - self._TestSimpleUpgrade(constants.BuildVersion(2, 6, 0), True)
> > + self._TestSimpleUpgrade(version.BuildVersion(2, 6, 0), True)
> >
> > def testUpgradeCurrentDryRun(self):
> > self._TestSimpleUpgrade(constants.CONFIG_VERSION, True)
> > diff --git a/test/py/ganeti.constants_unittest.py b/test/py/
> > ganeti.constants_unittest.py
> > index cb1cdf1..166b963 100755
> > --- a/test/py/ganeti.constants_unittest.py
> > +++ b/test/py/ganeti.constants_unittest.py
> > @@ -30,6 +30,8 @@ from ganeti import constants
> > from ganeti import locking
> > from ganeti import utils
> >
> > +from ganeti.utils import version
> > +
> > import testutils
> >
> >
> > @@ -46,16 +48,16 @@ class TestConstants(unittest.TestCase):
> > self.failUnless(constants.CONFIG_VERSION >= 0 and
> > constants.CONFIG_VERSION <= 99999999)
> >
> > - self.failUnless(constants.BuildVersion(0, 0, 0) == 0)
> > - self.failUnless(constants.BuildVersion(10, 10, 1010) == 10101010)
> > - self.failUnless(constants.BuildVersion(12, 34, 5678) == 12345678)
> > - self.failUnless(constants.BuildVersion(99, 99, 9999) == 99999999)
> > + self.failUnless(version.BuildVersion(0, 0, 0) == 0)
> > + self.failUnless(version.BuildVersion(10, 10, 1010) == 10101010)
> > + self.failUnless(version.BuildVersion(12, 34, 5678) == 12345678)
> > + self.failUnless(version.BuildVersion(99, 99, 9999) == 99999999)
> >
> > - self.failUnless(constants.SplitVersion(00000000) == (0, 0, 0))
> > - self.failUnless(constants.SplitVersion(10101010) == (10, 10, 1010))
> > - self.failUnless(constants.SplitVersion(12345678) == (12, 34, 5678))
> > - self.failUnless(constants.SplitVersion(99999999) == (99, 99, 9999))
> > - self.failUnless(constants.SplitVersion(constants.CONFIG_VERSION) ==
> > + self.failUnless(version.SplitVersion(00000000) == (0, 0, 0))
> > + self.failUnless(version.SplitVersion(10101010) == (10, 10, 1010))
> > + self.failUnless(version.SplitVersion(12345678) == (12, 34, 5678))
> > + self.failUnless(version.SplitVersion(99999999) == (99, 99, 9999))
> > + self.failUnless(version.SplitVersion(constants.CONFIG_VERSION) ==
> > (constants.CONFIG_MAJOR, constants.CONFIG_MINOR,
> > constants.CONFIG_REVISION))
> >
> > diff --git a/tools/cfgupgrade b/tools/cfgupgrade
> > index 648d732..1b30ea9 100755
> > --- a/tools/cfgupgrade
> > +++ b/tools/cfgupgrade
> > @@ -44,6 +44,8 @@ from ganeti import config
> > from ganeti import netutils
> > from ganeti import pathutils
> >
> > +from ganeti.utils import version
> > +
> >
> > options = None
> > args = None
> > @@ -347,8 +349,7 @@ def UpgradeInstanceIndices(config_data):
> >
> >
> > def UpgradeAll(config_data):
> > - config_data["version"] = constants.BuildVersion(TARGET_MAJOR,
> > - TARGET_MINOR, 0)
> > + config_data["version"] = version.BuildVersion(TARGET_MAJOR,
> > TARGET_MINOR, 0)
> > UpgradeRapiUsers()
> > UpgradeWatcher()
> > UpgradeFileStoragePaths(config_data)
> > @@ -379,8 +380,8 @@ def DowngradeNicParamsVLAN(nics, owner):
> > def DowngradeAll(config_data):
> > # Any code specific to a particular version should be labeled that way,
> > so
> > # it can be removed when updating to the next version.
> > - config_data["version"] = constants.BuildVersion(DOWNGRADE_MAJOR,
> > - DOWNGRADE_MINOR, 0)
> > + config_data["version"] = version.BuildVersion(DOWNGRADE_MAJOR,
> > + DOWNGRADE_MINOR, 0)
> > DowngradeInstances(config_data)
> >
> >
> > @@ -482,7 +483,7 @@ def main():
> > raise Error("Unable to determine configuration version")
> >
> > (config_major, config_minor, config_revision) = \
> > - constants.SplitVersion(config_version)
> > + version.SplitVersion(config_version)
> >
> > logging.info("Found configuration version %s (%d.%d.%d)",
> > config_version, config_major, config_minor,
> > config_revision)
> > diff --git a/tools/cfgupgrade12 b/tools/cfgupgrade12
> > index dd76ce9..f684c9e 100755
> > --- a/tools/cfgupgrade12
> > +++ b/tools/cfgupgrade12
> > @@ -49,6 +49,8 @@ from ganeti import utils
> > from ganeti import cli
> > from ganeti import pathutils
> >
> > +from ganeti.utils import version
> > +
> >
> > options = None
> > args = None
> > @@ -345,7 +347,7 @@ def main():
> > raise Error("Unsupported configuration version: %s" %
> > old_config_version)
> > if "version" not in config_data:
> > - config_data["version"] = constants.BuildVersion(2, 0, 0)
> > + config_data["version"] = version.BuildVersion(2, 0, 0)
> > if F_SERIAL not in config_data:
> > config_data[F_SERIAL] = 1
> >
> > --
> > 1.8.4
> >
> > Rest LGTM, thanks.
>
>
> --
> Thomas Thrainer | Software Engineer | [email protected] |
>
> Google Germany GmbH
> Dienerstr. 12
> 80331 München
>
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
--
Jose Antonio Lopes
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
