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.


> +                  " 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.

Reply via email to