On Mon, Oct 7, 2013 at 2:30 PM, Helga Velroyen <[email protected]> wrote:
> The up/downgrade procedure so far did not consider the > changes in the 'dev_type' attribute of disks. This patch > adds relevant checks and updates. A related problem > was that the logical IDs of disks were adjusted depending > on the dev_type, which made the order of updates not > arbitrary anymore. To make this more robust, the upgrade > procedure also considers the old versions of dev_types > (at least for 2.9). > > Signed-off-by: Helga Velroyen <[email protected]> > --- > test/py/cfgupgrade_unittest.py | 1 + > tools/cfgupgrade | 22 +++++++++++++++++++++- > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/test/py/cfgupgrade_unittest.py > b/test/py/cfgupgrade_unittest.py > index c94920b..7d84ef3 100755 > --- a/test/py/cfgupgrade_unittest.py > +++ b/test/py/cfgupgrade_unittest.py > @@ -386,6 +386,7 @@ class TestCfgupgrade(unittest.TestCase): > """Test for upgrade + downgrade combination.""" > # This test can work only with the previous version of a > configuration! > oldconfname = "cluster_config_2.8.json" > + self.maxDiff = None > I'm not sure if everybody always want's to see the full diff... > self._TestUpgradeFromFile(oldconfname, False) > _RunUpgrade(self.tmpdir, False, True, downgrade=True) > oldconf = self._LoadTestDataConfig(oldconfname) > diff --git a/tools/cfgupgrade b/tools/cfgupgrade > index 554877f..404aaef 100755 > --- a/tools/cfgupgrade > +++ b/tools/cfgupgrade > @@ -174,6 +174,12 @@ def GetExclusiveStorageValue(config_data): > > > def UpgradeInstances(config_data): > + """Upgrades the instances' configuration.""" > + > + # map of legacy device types (mapping differing old LD_* constants to > new DT_* > + # constants) > + LEG_DEV_TYPE_MAP = {"lvm": constants.DT_PLAIN, "drbd8": > constants.DT_DRBD8} > + > network2uuid = dict((n["name"], n["uuid"]) > for n in config_data["networks"].values()) > if "instances" not in config_data: > @@ -201,6 +207,10 @@ def UpgradeInstances(config_data): > " from '%s' to '%s'", > instance, idx, current, expected) > dobj["iv_name"] = expected > + > + if dobj["dev_type"] in LEG_DEV_TYPE_MAP: > + dobj["dev_type"] = LEG_DEV_TYPE_MAP[dobj["dev_type"]] > + > I think this should be done recursively for child disks as well. Especially because the child disks of a DRBD disk are PLAIN AFAIK. > if not "spindles" in dobj: > missing_spindles = True > > @@ -288,7 +298,10 @@ def GetNewNodeIndex(nodes_by_old_key, old_key, > new_key_field): > > def ChangeNodeIndices(config_data, old_key_field, new_key_field): > def ChangeDiskNodeIndices(disk): > - if disk["dev_type"] in constants.LDS_DRBD: > + # FIXME(2.10): 'drbd8' can be removed as soon as no updates from 2.8 > + # to 2.9 are supported anymore > Correct me if I'm wrong, but I thought that upgrades should work from any version > 2.0 to the current version, whereas downgrades should only work one version back. So the TODO would be misleading in this case. > + drbd_disk_types = set(["drbd8"]) | constants.DTS_DRBD > + if disk["dev_type"] in drbd_disk_types: > for i in range(0, 2): > disk["logical_id"][i] = GetNewNodeIndex(nodes_by_old_key, > disk["logical_id"][i], > @@ -351,6 +364,10 @@ def UpgradeAll(config_data): > > > def DowngradeDisks(disks, owner): > + # map of legacy device types (mapping differing old LD_* constants to > new DT_* > + # constants) > + LEG_DEV_TYPE_MAP = {constants.DT_PLAIN: "lvm", constants.DT_DRBD8: > "drbd8"} > + > for disk in disks: > # Remove spindles to downgrade to 2.8 > if "spindles" in disk: > @@ -358,6 +375,9 @@ def DowngradeDisks(disks, owner): > " instance %s", > disk["spindles"], disk["iv_name"], disk["uuid"], > owner) > del disk["spindles"] > + if disk["dev_type"] in LEG_DEV_TYPE_MAP: > + old_val = disk["dev_type"] > + disk["dev_type"] = LEG_DEV_TYPE_MAP[old_val] > Same as above, this should updated child disks recursively. > > > def DowngradeInstances(config_data): > -- > 1.8.4 > > -- 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
