From 45f6215642b4a57a82db9596b470d4f82534b253 Mon Sep 17 00:00:00 2001
From: Bernardo Dal Seno <bdalseno@google.com>
Date: Wed, 6 Feb 2013 14:52:30 +0100
Subject: [PATCH] Upgrades made on loading the configuration are always saved
Before, only some upgrades were written back to the configuration file. A
little refactoring of _UpgradeConfig() has been done to write unit tests.
Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
---
lib/config.py | 26 ++++++++++++---------
test/py/ganeti.config_unittest.py | 38 +++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/lib/config.py b/lib/config.py
index 9d87c2b6f..eb37c710f 100644
--- a/lib/config.py
+++ b/lib/config.py
@@ -34,6 +34,7 @@ much memory.
# pylint: disable=R0904
# R0904: Too many public methods
+import copy
import os
import random
import logging
@@ -2033,24 +2034,22 @@ class ConfigWriter:
(data.cluster.master_node, self._my_hostname))
raise errors.ConfigurationError(msg)
- # Upgrade configuration if needed
- data.UpgradeConfig()
-
self._config_data = data
# reset the last serial as -1 so that the next write will cause
# ssconf update
self._last_cluster_serial = -1
- # And finally run our (custom) config upgrade sequence
+ # Upgrade configuration if needed
self._UpgradeConfig()
self._cfg_id = utils.GetFileID(path=self._cfg_file)
def _UpgradeConfig(self):
- """Run upgrade steps that cannot be done purely in the objects.
+ """Run any upgrade steps.
- This is because some data elements need uniqueness across the
- whole configuration, etc.
+ This method performs both in-object upgrades and also update some data
+ elements that need uniqueness across the whole configuration or interact
+ with other objects.
@warning: this function will call L{_WriteConfig()}, but also
L{DropECReservations} so it needs to be called only from a
@@ -2059,26 +2058,31 @@ class ConfigWriter:
created first, to avoid causing deadlock.
"""
- modified = False
+ # Keep a copy of the persistent part of _config_data to check for changes
+ # Serialization doesn't guarantee order in dictionaries
+ oldconf = copy.deepcopy(self._config_data.ToDict())
+
+ # In-object upgrades
+ self._config_data.UpgradeConfig()
+
for item in self._AllUUIDObjects():
if item.uuid is None:
item.uuid = self._GenerateUniqueID(_UPGRADE_CONFIG_JID)
- modified = True
if not self._config_data.nodegroups:
default_nodegroup_name = constants.INITIAL_NODE_GROUP_NAME
default_nodegroup = objects.NodeGroup(name=default_nodegroup_name,
members=[])
self._UnlockedAddNodeGroup(default_nodegroup, _UPGRADE_CONFIG_JID, True)
- modified = True
for node in self._config_data.nodes.values():
if not node.group:
node.group = self.LookupNodeGroup(None)
- modified = True
# This is technically *not* an upgrade, but needs to be done both when
# nodegroups are being added, and upon normally loading the config,
# because the members list of a node group is discarded upon
# serializing/deserializing the object.
self._UnlockedAddNodeToGroup(node.name, node.group)
+
+ modified = (oldconf != self._config_data.ToDict())
if modified:
self._WriteConfig()
# This is ok even if it acquires the internal lock, as _UpgradeConfig is
diff --git a/test/py/ganeti.config_unittest.py b/test/py/ganeti.config_unittest.py
index b06f774c4..112cdf248 100755
--- a/test/py/ganeti.config_unittest.py
+++ b/test/py/ganeti.config_unittest.py
@@ -174,6 +174,44 @@ class TestConfigRunner(unittest.TestCase):
self.failUnlessRaises(errors.ConfigurationError, cfg.Update, fake_instance,
None)
+ def testUpgradeSave(self):
+ """Test that any modification done during upgrading is saved back"""
+ cfg = self._get_object()
+
+ # Remove an element, run upgrade, and check if the element is
+ # back and the file upgraded
+ node = cfg.GetNodeInfo(cfg.GetNodeList()[0])
+ # For a ConfigObject, None is the same as a missing field
+ node.ndparams = None
+ oldsaved = utils.ReadFile(self.cfg_file)
+ cfg._UpgradeConfig()
+ self.assertTrue(node.ndparams is not None)
+ newsaved = utils.ReadFile(self.cfg_file)
+ # We rely on the fact that at least the serial number changes
+ self.assertNotEqual(oldsaved, newsaved)
+
+ # Add something that should not be there this time
+ key = list(constants.NDC_GLOBALS)[0]
+ node.ndparams[key] = constants.NDC_DEFAULTS[key]
+ cfg._WriteConfig(None)
+ oldsaved = utils.ReadFile(self.cfg_file)
+ cfg._UpgradeConfig()
+ self.assertTrue(node.ndparams.get(key) is None)
+ newsaved = utils.ReadFile(self.cfg_file)
+ self.assertNotEqual(oldsaved, newsaved)
+
+ # Do the upgrade again, this time there should be no update
+ oldsaved = newsaved
+ cfg._UpgradeConfig()
+ newsaved = utils.ReadFile(self.cfg_file)
+ self.assertEqual(oldsaved, newsaved)
+
+ # Reload the configuration again: it shouldn't change the file
+ oldsaved = newsaved
+ self._get_object()
+ newsaved = utils.ReadFile(self.cfg_file)
+ self.assertEqual(oldsaved, newsaved)
+
def testNICParameterSyntaxCheck(self):
"""Test the NIC's CheckParameterSyntax function"""
mode = constants.NIC_MODE
--
GitLab