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.

Reply via email to