LGTM with that interdiff. Thanks.
On Thu, 19 Nov 2015 at 13:56 Hrvoje Ribicic <[email protected]> wrote:
> Interdiff to make things work on the vcluster:
>
> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
> index fbb90d0..ec0d367 100644
> --- a/qa/qa_cluster.py
> +++ b/qa/qa_cluster.py
> @@ -1209,6 +1209,15 @@ def _TestSSHKeyChanges(master_node):
> if not fail and verify:
> AssertCommand(["gnt-cluster", "verify"])
>
> + # First test the simplest change
> + _RenewWithParams([])
> +
> + # And stop here if vcluster
> + (vcluster_master, _) = qa_config.GetVclusterSettings()
> + if vcluster_master:
> + print "Skipping further SSH key replacement checks for vcluster"
> + return
> +
> # And the actual tests
> with qa_config.AcquireManyNodesCtx(1, exclude=[master_node]) as nodes:
> node_name = nodes[0].primary
> @@ -1220,17 +1229,14 @@ def _TestSSHKeyChanges(master_node):
> "-F/dev/null", node_name, "true"],
> fail=fail, forward_agent=False)
>
> - # First test the simplest change
> - _RenewWithParams([])
> -
> _RenewWithParams(["--ssh-key-type=dsa"])
> - _CheckLoginWithKey(".ssh/id_dsa")
> + _CheckLoginWithKey("/root/.ssh/id_dsa")
> # Stash the key for now
> - old_key_backup = qa_utils.BackupFile(master_node.primary,
> ".ssh/id_dsa")
> + old_key_backup = qa_utils.BackupFile(master_node.primary,
> "/root/.ssh/id_dsa")
>
> try:
> _RenewWithParams(["--ssh-key-type=rsa"])
> - _CheckLoginWithKey(".ssh/id_rsa")
> + _CheckLoginWithKey("/root/.ssh/id_rsa")
> # And check that we cannot log in with the old key
> _CheckLoginWithKey(old_key_backup, fail=True)
> finally:
>
>
> On Tue, Nov 17, 2015 at 9:31 AM, Helga Velroyen <[email protected]> wrote:
>
>> That makes more sense :) LGTM, thanks
>>
>> On Mon, 16 Nov 2015 at 10:09 Hrvoje Ribicic <[email protected]> wrote:
>>
>>> On Fri, Nov 13, 2015 at 2:41 PM, Helga Velroyen <[email protected]>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Fri, 13 Nov 2015 at 11:18 'Hrvoje Ribicic' via ganeti-devel <
>>>> [email protected]> wrote:
>>>>
>>>>> This patch expands the testing of SSH key renewal by changing the key
>>>>> type existing on a cluster during the QA.
>>>>>
>>>>> Signed-off-by: Hrvoje Ribicic <[email protected]>
>>>>> ---
>>>>> qa/qa_cluster.py | 55
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>>>> 1 file changed, 52 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
>>>>> index 1ad2fb9..941967c 100644
>>>>> --- a/qa/qa_cluster.py
>>>>> +++ b/qa/qa_cluster.py
>>>>> @@ -1195,6 +1195,56 @@ def _AssertSsconfCertFiles():
>>>>> " '%s'." % (node, first_node))
>>>>>
>>>>>
>>>>> +def _TestSSHKeyChanges(master_node):
>>>>> + """Tests a lot of SSH key type- and size- related functionality.
>>>>> +
>>>>> + @type master_node: string
>>>>> + @param master_node: The cluster master.
>>>>>
>>>>
>>>> Nit: I personally would rephrase that to "the *name* of the cluster
>>>> master" since we have way to many places already where one has to think 'is
>>>> this now the UUID or the name?".
>>>>
>>>
>>> Well, good that you pointed this out as this is actually not a string :D
>>> Interdiff:
>>>
>>> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
>>> index 941967c..fbb90d0 100644
>>> --- a/qa/qa_cluster.py
>>> +++ b/qa/qa_cluster.py
>>> @@ -1198,7 +1198,7 @@ def _AssertSsconfCertFiles():
>>> def _TestSSHKeyChanges(master_node):
>>> """Tests a lot of SSH key type- and size- related functionality.
>>>
>>> - @type master_node: string
>>> + @type master_node: L{qa_config._QaNode}
>>> @param master_node: The cluster master.
>>>
>>> """
>>>
>>>
>>>>
>>>>
>>>>> +
>>>>
>>>>
>>>>> + """
>>>>> + # Helper fn to avoid specifying base params too many times
>>>>> + def _RenewWithParams(new_params, verify=True, fail=False):
>>>>> + AssertCommand(["gnt-cluster", "renew-crypto", "--new-ssh-keys",
>>>>> "-f",
>>>>> + "--no-ssh-key-check"] + new_params, fail=fail)
>>>>> + if not fail and verify:
>>>>> + AssertCommand(["gnt-cluster", "verify"])
>>>>> +
>>>>> + # And the actual tests
>>>>> + with qa_config.AcquireManyNodesCtx(1, exclude=[master_node]) as
>>>>> nodes:
>>>>> + node_name = nodes[0].primary
>>>>> +
>>>>> + # Another helper function for checking whether a specific key can
>>>>> log in
>>>>> + def _CheckLoginWithKey(key_path, fail=False):
>>>>> + AssertCommand(["ssh", "-oIdentityFile=%s" % key_path,
>>>>> "-oBatchMode=yes",
>>>>> + "-oStrictHostKeyChecking=no",
>>>>> "-oIdentitiesOnly=yes",
>>>>> + "-F/dev/null", node_name, "true"],
>>>>> + fail=fail, forward_agent=False)
>>>>> +
>>>>>
>>>> Nice to actually test that!
>>>>
>>>>
>>>>> + # First test the simplest change
>>>>> + _RenewWithParams([])
>>>>> +
>>>>> + _RenewWithParams(["--ssh-key-type=dsa"])
>>>>> + _CheckLoginWithKey(".ssh/id_dsa")
>>>>> + # Stash the key for now
>>>>> + old_key_backup = qa_utils.BackupFile(master_node.primary,
>>>>> ".ssh/id_dsa")
>>>>> +
>>>>> + try:
>>>>> + _RenewWithParams(["--ssh-key-type=rsa"])
>>>>> + _CheckLoginWithKey(".ssh/id_rsa")
>>>>> + # And check that we cannot log in with the old key
>>>>> + _CheckLoginWithKey(old_key_backup, fail=True)
>>>>> + finally:
>>>>> + AssertCommand(["rm", "-f", old_key_backup])
>>>>> +
>>>>> + _RenewWithParams(["--ssh-key-bits=4096"])
>>>>> + _RenewWithParams(["--ssh-key-bits=521"], fail=True)
>>>>> +
>>>>> + # Restore the cluster to its pristine state, skipping the verify
>>>>> as we did
>>>>> + # way too many already
>>>>> + _RenewWithParams(["--ssh-key-type=rsa", "--ssh-key-bits=2048"],
>>>>> + verify=False)
>>>>> +
>>>>> +
>>>>> def TestClusterRenewCrypto():
>>>>> """gnt-cluster renew-crypto"""
>>>>> master = qa_config.GetMasterNode()
>>>>> @@ -1266,9 +1316,8 @@ def TestClusterRenewCrypto():
>>>>> _AssertSsconfCertFiles()
>>>>> AssertCommand(["gnt-cluster", "verify"])
>>>>>
>>>>> - # Only renew SSH keys
>>>>> - AssertCommand(["gnt-cluster", "renew-crypto", "--force",
>>>>> - "--new-ssh-keys", "--no-ssh-key-check"])
>>>>> + # Comprehensively test various types of SSH key changes
>>>>> + _TestSSHKeyChanges(master)
>>>>>
>>>>> # Restore RAPI certificate
>>>>> AssertCommand(["gnt-cluster", "renew-crypto", "--force",
>>>>> --
>>>>> 2.6.0.rc2.230.g3dd15c0
>>>>>
>>>>>
>>>> Nice patch, LGTM.
>>>>
>>> --
>>>>
>>>> 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.
>>
>>
> 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.
