LGTM, thanks
On Thu, 19 Nov 2015 at 16:01 Hrvoje Ribicic <[email protected]> wrote:
> Another lint interdiff!
>
> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
> index ec0d367..2199d00 100644
> --- a/qa/qa_cluster.py
> +++ b/qa/qa_cluster.py
> @@ -1232,7 +1232,8 @@ def _TestSSHKeyChanges(master_node):
> _RenewWithParams(["--ssh-key-type=dsa"])
> _CheckLoginWithKey("/root/.ssh/id_dsa")
> # Stash the key for now
> - old_key_backup = qa_utils.BackupFile(master_node.primary,
> "/root/.ssh/id_dsa")
> + old_key_backup = qa_utils.BackupFile(master_node.primary,
> + "/root/.ssh/id_dsa")
>
> try:
> _RenewWithParams(["--ssh-key-type=rsa"])
>
>
> On Thu, Nov 19, 2015 at 2:45 PM, Helga Velroyen <[email protected]> wrote:
>
>> 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.
>>
>>
> 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.
