On Wed, Oct 2, 2013 at 12:02 PM, Thomas Thrainer <[email protected]>wrote:
> While cluster upgrades will work this way, other operations will almost > certainly still fail. > As mentioned in the bug, disk.dev_type changed to use the DT_* constants > instead of the LD_* constants. So in various places throughout the code we > compare disk.dev_type with DT_DRBD8, which happens to be "drbd". So all > those checks would stop to work. > > The real solution would be to update disk.dev_type to constants.DT_DRBD8 > if it was "drbd8" during the upgrade (and change it back during the > downgrade), and verify that LDS_DRBD is no longer used in the codebase (as > it should have been removed with all the other LD_* constants). > The online-update should do exactly this. Could be that I missed this spot though :/ > > Ideally this change of dev_type would not happen in ChangeNodeIndices, but > in another function which runs prior to this one (otherwise the change is a > bit hidden in cfgupgrade). > > Cheers, > Thomas > > > On Wed, Oct 2, 2013 at 11:55 AM, Apollon Oikonomopoulos <[email protected] > > wrote: > >> Commit cd3b4ff4 replaced "drbd8" with "drbd" as the dev_type for DRBD >> disks. >> Commit 73d6b4a7 further introduced config upgrade for disks to replace >> "drbd8" >> with "drbd". >> >> However, when cfgupgrade is run against a 2.8 config, the disks have still >> dev_type "drbd8" and are not considered for upgrade of their logical IDs >> during >> ChangeNodeIndices(). As a result, upgrading an existing config eventually >> fails, >> as later stages expect DRBD disk logical IDs to contain node UUIDs and >> not node >> names: >> >> Traceback (most recent call last): >> File "/usr/share/ganeti/cfgupgrade", line 582, in <module> >> main() >> File "/usr/share/ganeti/cfgupgrade", line 555, in main >> offline=True) >> File "/usr/share/ganeti/ganeti/config.py", line 205, in __init__ >> self._OpenConfig(accept_foreign) >> File "/usr/share/ganeti/ganeti/config.py", line 2347, in _OpenConfig >> self._UpgradeConfig() >> File "/usr/share/ganeti/ganeti/config.py", line 2391, in _UpgradeConfig >> self._WriteConfig() >> File "/usr/share/ganeti/ganeti/config.py", line 2455, in _WriteConfig >> config_errors = self._UnlockedVerifyConfig() >> File "/usr/share/ganeti/ganeti/config.py", line 818, in >> _UnlockedVerifyConfig >> _, duplicates = self._UnlockedComputeDRBDMap() >> File "/usr/share/ganeti/ganeti/config.py", line 1006, in >> _UnlockedComputeDRBDMap >> instance, disk, my_dict)) >> File "/usr/share/ganeti/ganeti/config.py", line 989, in >> _AppendUsedMinors >> (get_node_name_fn(node_uuid), instance.name)) >> File "/usr/share/ganeti/ganeti/config.py", line 2073, in >> _UnlockedGetNodeName >> raise errors.OpExecError("Unknown node: %s" % node_spec) >> ganeti.errors.OpExecError: Unknown node: node1.example.com >> >> There are three possible solutions to this: >> >> 1. Modify cfgupgrade to explicitly consider "drbd8" in addition to >> LDS_DRBD. >> 2. Include "drbd8" in constants.LDS_DRBD, which is only used for >> membership >> checks anyway. >> 3. Move LEG_DEV_TYPE_MAP introduced in objects.Disk with commit >> 73d6b4a7 to >> constants.py and make information about legacy device types globally >> available. >> >> Since it's a one-time change and constants.py is mostly free from legacy >> stuff, I chose to go with 1. Also, in order not to rely on LDS_DRBD >> providing a >> .union() method, an additional check against "drbd8" is performed. >> >> This fixes issue #594. >> >> Signed-off-by: Apollon Oikonomopoulos <[email protected]> >> --- >> tools/cfgupgrade | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/cfgupgrade b/tools/cfgupgrade >> index 554877f..9e2ef22 100755 >> --- a/tools/cfgupgrade >> +++ b/tools/cfgupgrade >> @@ -288,7 +288,7 @@ 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: >> + if disk["dev_type"] in constants.LDS_DRBD or disk["dev_type"] == >> "drbd8": >> for i in range(0, 2): >> disk["logical_id"][i] = GetNewNodeIndex(nodes_by_old_key, >> disk["logical_id"][i], >> -- >> 1.7.10.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 > -- -- Helga Velroyen | 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
