The cfgupgrade tool was designed to be idempotent, that means it could be run several times and still give produce the correct result. Ganeti 2.4 moved the file containing the RAPI users to a separate directory (…/lib/ganeti/rapi/users). If it exists, cfgupgrade would automatically move an existing file from …/lib/ganeti/rapi_users and replace it with a symlink.
Unfortunately one of the checks for this was incorrect and, when run multiple times, replaces the users file at the new location with a symlink created during a previous run. In addition the “--dry-run” parameter to cfgupgrade was not respected. Unittests are updated for all these cases. Signed-off-by: Michael Hanselmann <[email protected]> --- test/cfgupgrade_unittest.py | 74 +++++++++++++++++++++++++++++++++++++++++- tools/cfgupgrade | 30 +++++++++++------ 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/test/cfgupgrade_unittest.py b/test/cfgupgrade_unittest.py index 0819dfa..359a298 100755 --- a/test/cfgupgrade_unittest.py +++ b/test/cfgupgrade_unittest.py @@ -161,12 +161,16 @@ class TestCfgupgrade(unittest.TestCase): def testRapiUsers(self): self.assertFalse(os.path.exists(self.rapi_users_path)) self.assertFalse(os.path.exists(self.rapi_users_path_pre24)) + 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.assertTrue(os.path.isdir(os.path.dirname(self.rapi_users_path))) self.assert_(os.path.islink(self.rapi_users_path_pre24)) self.assert_(os.path.isfile(self.rapi_users_path)) + self.assertEqual(os.readlink(self.rapi_users_path_pre24), + self.rapi_users_path) for path in [self.rapi_users_path, self.rapi_users_path_pre24]: self.assertEqual(utils.ReadFile(path), "some user\n") @@ -180,6 +184,8 @@ class TestCfgupgrade(unittest.TestCase): self.assert_(os.path.islink(self.rapi_users_path_pre24)) self.assert_(os.path.isfile(self.rapi_users_path)) + self.assertEqual(os.readlink(self.rapi_users_path_pre24), + self.rapi_users_path) for path in [self.rapi_users_path, self.rapi_users_path_pre24]: self.assertEqual(utils.ReadFile(path), "other user\n") @@ -187,13 +193,77 @@ class TestCfgupgrade(unittest.TestCase): self.assertFalse(os.path.exists(self.rapi_users_path)) self.assertFalse(os.path.exists(self.rapi_users_path_pre24)) + os.mkdir(os.path.dirname(self.rapi_users_path)) os.symlink(self.rapi_users_path, self.rapi_users_path_pre24) - utils.WriteFile(self.rapi_users_path_pre24, data="hello world\n") + utils.WriteFile(self.rapi_users_path, data="hello world\n") self._TestSimpleUpgrade(constants.BuildVersion(2, 2, 0), False) - self.assert_(os.path.isfile(self.rapi_users_path)) + self.assert_(os.path.isfile(self.rapi_users_path) and + not os.path.islink(self.rapi_users_path)) self.assert_(os.path.islink(self.rapi_users_path_pre24)) + self.assertEqual(os.readlink(self.rapi_users_path_pre24), + self.rapi_users_path) + for path in [self.rapi_users_path, self.rapi_users_path_pre24]: + self.assertEqual(utils.ReadFile(path), "hello world\n") + + def testRapiUsersExistingTarget(self): + self.assertFalse(os.path.exists(self.rapi_users_path)) + self.assertFalse(os.path.exists(self.rapi_users_path_pre24)) + + os.mkdir(os.path.dirname(self.rapi_users_path)) + utils.WriteFile(self.rapi_users_path, data="other user\n") + utils.WriteFile(self.rapi_users_path_pre24, data="hello world\n") + + self.assertRaises(Exception, self._TestSimpleUpgrade, + constants.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)) + self.assertEqual(utils.ReadFile(self.rapi_users_path), "other user\n") + self.assertEqual(utils.ReadFile(self.rapi_users_path_pre24), + "hello world\n") + + def testRapiUsersDryRun(self): + self.assertFalse(os.path.exists(self.rapi_users_path)) + 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.assertFalse(os.path.isdir(os.path.dirname(self.rapi_users_path))) + self.assertTrue(os.path.isfile(self.rapi_users_path_pre24) and + not os.path.islink(self.rapi_users_path_pre24)) + self.assertFalse(os.path.exists(self.rapi_users_path)) + + def testRapiUsers24AndAboveDryRun(self): + self.assertFalse(os.path.exists(self.rapi_users_path)) + self.assertFalse(os.path.exists(self.rapi_users_path_pre24)) + + 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.assertTrue(os.path.isfile(self.rapi_users_path) and + not os.path.islink(self.rapi_users_path)) + self.assertFalse(os.path.exists(self.rapi_users_path_pre24)) + self.assertEqual(utils.ReadFile(self.rapi_users_path), "other user\n") + + def testRapiUsersExistingSymlinkDryRun(self): + self.assertFalse(os.path.exists(self.rapi_users_path)) + self.assertFalse(os.path.exists(self.rapi_users_path_pre24)) + + os.mkdir(os.path.dirname(self.rapi_users_path)) + 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.assertTrue(os.path.islink(self.rapi_users_path_pre24)) + self.assertTrue(os.path.isfile(self.rapi_users_path) and + not os.path.islink(self.rapi_users_path)) + self.assertEqual(os.readlink(self.rapi_users_path_pre24), + self.rapi_users_path) for path in [self.rapi_users_path, self.rapi_users_path_pre24]: self.assertEqual(utils.ReadFile(path), "hello world\n") diff --git a/tools/cfgupgrade b/tools/cfgupgrade index 2222920..e1d5463 100755 --- a/tools/cfgupgrade +++ b/tools/cfgupgrade @@ -185,17 +185,25 @@ def main(): raise Error("Configuration version %d.%d.%d not supported by this tool" % (config_major, config_minor, config_revision)) - if os.path.isfile(options.RAPI_USERS_FILE_PRE24): - logging.info("Found pre-2.4 RAPI users file at %s, renaming to %s", - options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE) - utils.RenameFile(options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE, - mkdir=True, mkdir_mode=0750) - - # Create a symlink for RAPI users file - if not os.path.islink(options.RAPI_USERS_FILE_PRE24): - logging.info("Creating symlink from %s to %s", - options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE) - os.symlink(options.RAPI_USERS_FILE, options.RAPI_USERS_FILE_PRE24) + if not options.dry_run: + if (os.path.isfile(options.RAPI_USERS_FILE_PRE24) and + not os.path.islink(options.RAPI_USERS_FILE_PRE24)): + if os.path.exists(options.RAPI_USERS_FILE): + raise Error("Found pre-2.4 RAPI users file at %s, but another file" + " already exists at %s" % + (options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE)) + logging.info("Found pre-2.4 RAPI users file at %s, renaming to %s", + options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE) + utils.RenameFile(options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE, + mkdir=True, mkdir_mode=0750) + + # Create a symlink for RAPI users file + if (not (os.path.islink(options.RAPI_USERS_FILE_PRE24) or + os.path.isfile(options.RAPI_USERS_FILE_PRE24)) and + os.path.isfile(options.RAPI_USERS_FILE)): + logging.info("Creating symlink from %s to %s", + options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE) + os.symlink(options.RAPI_USERS_FILE, options.RAPI_USERS_FILE_PRE24) try: logging.info("Writing configuration file to %s", options.CONFIG_DATA_PATH) -- 1.7.3.5
