LGTM, thanks On Mon, 16 Nov 2015 at 10:09 Hrvoje Ribicic <[email protected]> wrote:
> On Fri, Nov 13, 2015 at 2:26 PM, Helga Velroyen <[email protected]> wrote: > >> >> >> On Fri, 13 Nov 2015 at 11:18 'Hrvoje Ribicic' via ganeti-devel < >> [email protected]> wrote: >> >>> When performing an upgrade of an old cluster, it is necessary to set >>> the SSH key parameters to the exact same values earlier versions >>> implicitly used - DSA with 1024 bits. >>> >>> In the other direction, we simply do not permit downgrades if keys >>> other than DSA are being used. Triggering a gnt-cluster renew-crypto >>> might be time-consuming and surprising for the user, so we are simply >>> throwing out an error message, explaining that the downgrade cannot be >>> performed in the current state of the cluster. >>> >>> Signed-off-by: Hrvoje Ribicic <[email protected]> >>> --- >>> lib/tools/cfgupgrade.py | 50 >>> +++++++++++++++++++++++++++++++++++++++++- >>> test/py/cfgupgrade_unittest.py | 2 ++ >>> 2 files changed, 51 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/tools/cfgupgrade.py b/lib/tools/cfgupgrade.py >>> index 43cf2c6..943b9a9 100644 >>> --- a/lib/tools/cfgupgrade.py >>> +++ b/lib/tools/cfgupgrade.py >>> @@ -307,24 +307,33 @@ class CfgUpgrade(object): >>> cluster = self.config_data.get("cluster", None) >>> if cluster is None: >>> raise Error("Cannot find cluster") >>> + >>> ipolicy = cluster.setdefault("ipolicy", None) >>> if ipolicy: >>> self.UpgradeIPolicy(ipolicy, constants.IPOLICY_DEFAULTS, False) >>> ial_params = cluster.get("default_iallocator_params", None) >>> + >>> if not ial_params: >>> cluster["default_iallocator_params"] = {} >>> + >>> if not "candidate_certs" in cluster: >>> cluster["candidate_certs"] = {} >>> + >>> cluster["instance_communication_network"] = \ >>> cluster.get("instance_communication_network", "") >>> + >>> cluster["install_image"] = \ >>> cluster.get("install_image", "") >>> + >>> cluster["zeroing_image"] = \ >>> cluster.get("zeroing_image", "") >>> + >>> cluster["compression_tools"] = \ >>> cluster.get("compression_tools", constants.IEC_DEFAULT_TOOLS) >>> + >>> if "enabled_user_shutdown" not in cluster: >>> cluster["enabled_user_shutdown"] = False >>> + >>> cluster["data_collectors"] = cluster.get("data_collectors", {}) >>> for name in constants.DATA_COLLECTOR_NAMES: >>> cluster["data_collectors"][name] = \ >>> @@ -332,6 +341,14 @@ class CfgUpgrade(object): >>> name, dict(active=True, >>> interval=constants.MOND_TIME_INTERVAL * 1e6)) >>> >>> + # These parameters are set to pre-2.16 default values, which >>> + # differ from post-2.16 default values >>> + if "ssh_key_type" not in cluster: >>> + cluster["ssh_key_type"] = constants.SSHK_DSA >>> + >>> + if "ssh_key_bits" not in cluster: >>> + cluster["ssh_key_bits"] = 1024 >>> + >>> @OrFail("Upgrading groups") >>> def UpgradeGroups(self): >>> cl_ipolicy = self.config_data["cluster"].get("ipolicy") >>> @@ -699,10 +716,41 @@ class CfgUpgrade(object): >>> >>> # DOWNGRADE >>> ------------------------------------------------------------ >>> >>> + @OrFail("Removing SSH parameters") >>> + def DowngradeSshKeyParams(self): >>> + """Removes the SSH key type and bits parameters from the config. >>> + >>> + Also fails if these have been changed from values appropriate in >>> lower >>> + Ganeti versions. >>> + >>> + """ >>> + # pylint: disable=E1103 >>> + # Because config_data is a dictionary which has the get method. >>> + cluster = self.config_data.get("cluster", None) >>> + if cluster is None: >>> + raise Error("Can't find the cluster entry in the configuration") >>> + >>> + def _FetchAndDelete(key): >>> + val = cluster.get(key, None) >>> + if key in cluster: >>> + del cluster[key] >>> + return val >>> + >>> + ssh_key_type = _FetchAndDelete("ssh_key_type") >>> + _FetchAndDelete("ssh_key_bits") >>> + >>> + if ssh_key_type is not None and ssh_key_type != "dsa": >>> + raise Error("The current Ganeti setup is using non-DSA SSH keys, >>> and" >>> + " versions below 2.16 do not support these. To >>> downgrade," >>> + " please perform a gnt-cluster renew-crypto, and >>> generate" >>> >> >> To make it even easier for the user, also mention the correct options and >> save him a detour via the man page to figure those out. >> > > Given the number of options the user may pick, I will mention only the > --new-ssh-keys and --ssh-key-type=dsa options. > Interdiff: > > diff --git a/lib/tools/cfgupgrade.py b/lib/tools/cfgupgrade.py > index 943b9a9..ffa6f3f 100644 > --- a/lib/tools/cfgupgrade.py > +++ b/lib/tools/cfgupgrade.py > @@ -742,7 +742,8 @@ class CfgUpgrade(object): > if ssh_key_type is not None and ssh_key_type != "dsa": > raise Error("The current Ganeti setup is using non-DSA SSH keys, > and" > " versions below 2.16 do not support these. To > downgrade," > - " please perform a gnt-cluster renew-crypto, and > generate" > + " please perform a gnt-cluster renew-crypto using the " > + " --new-ssh-keys and --ssh-key-type=dsa options, > generating" > " DSA keys that older versions can also use.") > > def DowngradeAll(self): > > >> >> >>> + " DSA keys that older versions can also use.") >>> + >>> def DowngradeAll(self): >>> self.config_data["version"] = version.BuildVersion(DOWNGRADE_MAJOR, >>> DOWNGRADE_MINOR, >>> 0) >>> - return True >>> + >>> + self.DowngradeSshKeyParams() >>> + return not self.errors >>> >>> def _ComposePaths(self): >>> # We need to keep filenames locally because they might be renamed >>> between >>> diff --git a/test/py/cfgupgrade_unittest.py >>> b/test/py/cfgupgrade_unittest.py >>> index 9d5d950..3f587662 100755 >>> --- a/test/py/cfgupgrade_unittest.py >>> +++ b/test/py/cfgupgrade_unittest.py >>> @@ -74,6 +74,8 @@ def GetMinimalConfig(): >>> "cpu-avg-load": { "active": True, "interval": 5000000 }, >>> "xen-cpu-avg-load": { "active": True, "interval": 5000000 }, >>> }, >>> + "ssh_key_type": "dsa", >>> + "ssh_key_bits": 1024, >>> }, >>> "instances": {}, >>> "disks": {}, >>> -- >>> 2.6.0.rc2.230.g3dd15c0 >>> >>> >> Rest LGTM, thanks >> -- >> >> Helga Velroyen >> Software Engineer >> [email protected] >> >> Google Germany GmbH >> Dienerstraße 12 >> 80331 München >> >> Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle >> Registergericht und -nummer: Hamburg, HRB 86891 >> Sitz der Gesellschaft: Hamburg >> >> Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, >> leiten Sie diese bitte nicht weiter, informieren Sie den Absender und >> löschen Sie die E-Mail und alle Anhänge. Vielen Dank. >> >> This e-mail is confidential. If you are not the right addressee please do >> not forward it, please inform the sender, and please erase this e-mail >> including any attachments. Thanks. >> >> > Hrvoje Ribicic > Ganeti Engineering > Google Germany GmbH > Dienerstr. 12, 80331, München > > > Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > > Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, > leiten Sie diese bitte nicht weiter, informieren Sie den Absender und > löschen Sie die E-Mail und alle Anhänge. Vielen Dank. > > This e-mail is confidential. If you are not the right addressee please do > not forward it, please inform the sender, and please erase this e-mail > including any attachments. Thanks. > > -- Helga Velroyen Software Engineer [email protected] Google Germany GmbH Dienerstraße 12 80331 München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank. This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks.
