Comment #1 on issue 1048 by [email protected]: Unittests for
NodeSsh{Add,Remove}Key and renew crypto need improvment
https://code.google.com/p/ganeti/issues/detail?id=1048
Partially fixed for Add/Remove ssh keys:
commit bacd27531526faa60e595b3b4a4498a35b258779
Author: Helga Velroyen <[email protected]>
Date: Fri Apr 17 14:35:06 2015 +0200
NodeSshRemoveKey: Add retries for updating the target node
This patch adds retries to the part of 'NodeSshRemoveKey',
where the target node itself is updated. As this is likely
to fail if the node is for example already offline, we
do not fail the entire operation, but emit a non-disruptive
error which can be showing as feedback by the LU.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Hrvoje Ribicic <[email protected]>
commit 18bd654825339b9e2f9d05c6310b5f20e2d4a35c
Author: Helga Velroyen <[email protected]>
Date: Fri Apr 17 14:18:27 2015 +0200
SSH file manager: properly name assertion function
So far, the (fake) SSH file manager had a couple of check
functions (which return "True" or "False") and a couple of
functions which throw an exception instead of returning
"False" (as in those cases more information for debugging
is neeede). However, it was not obvious from the function
name which behavior to expect. This patch renames all
functions which throw exceptions to "Assert*" and removes
superfluous 'self.assertTrue(*)" calls around them.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Hrvoje Ribicic <[email protected]>
commit aea6bbd2cf4eee079a00401fd29cbc5a94f3188c
Author: Helga Velroyen <[email protected]>
Date: Fri Apr 17 14:06:45 2015 +0200
Unittests: Simplify generating a master candidate node
As by now a new master candidate node is generated quite
often, we add a function for it to remove some duplicate
code.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Hrvoje Ribicic <[email protected]>
commit b54540a37bc97b8c6a1a358e8cfbff519692a2b0
Author: Helga Velroyen <[email protected]>
Date: Fri Apr 17 12:45:03 2015 +0200
RemoveNodeSshKey: use retries when updating other nodes
When removing an SSH key, RemoveNodeSshKey so far did only
try to contact each node once. This made the entire
procedure not very resilient towards temporary network
failures. This patch adds a couple of retries for contacting
each node and only gives up after those are exhausted.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Hrvoje Ribicic <[email protected]>
commit 6ad504bd7696e49e9f68d9e627112a5ad866dc94
Author: Helga Velroyen <[email protected]>
Date: Fri Apr 17 11:47:54 2015 +0200
AddNodeSshKey: retries for all non-master nodes
This patch introduces retries for updating non-master nodes
when a new SSH key is added to the cluster. If an SSH update
fails even after the maximum number of retries is exhausted,
this does not raise an exception, but an error message is
added to the result of the method, which can be displayed
in the LU operations so that the admin sees which nodes
were the problem and need further attention.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Hrvoje Ribicic <[email protected]>
commit 1a30e46a910d772058d081ad2f155da2d2eee81a
Author: Helga Velroyen <[email protected]>
Date: Thu Apr 16 16:55:16 2015 +0200
AddNodeSshKey: retry when target node not reachable
So far, if a SSH key of a node is added to the cluster
and updating the SSH setup of the node itself fails,
the entire operation fails quite disgraceful. This
patch introduces a retry mechanism and a proper cleanup
in case of a failure.
Note that there are more places where retries would
be a good idea. They'll be added in other patches.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Petr Pudlak <[email protected]>
commit e3ccc0ae05a1a7a657ca1e27a4ce810fe93b8c98
Author: Helga Velroyen <[email protected]>
Date: Thu Apr 16 15:30:58 2015 +0200
Remove obsolete constant SSHS_RENAME
This must have been forgotten in some older patches.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Petr Pudlak <[email protected]>
commit f553d8ad8a8cc9f4d097f5878c4b028e17152bc8
Author: Helga Velroyen <[email protected]>
Date: Thu Apr 16 15:06:20 2015 +0200
Simplify testdata setup and teardown
Since now the old SSH unit tests are removed, we can
simplify the test data setup and teardown by adding it
to the general setup and teardown functions.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Petr Pudlak <[email protected]>
commit 49f49f81148ffcde660eee58ccfdafb2010e2329
Author: Helga Velroyen <[email protected]>
Date: Thu Apr 16 15:02:47 2015 +0200
Consider offline nodes when removing SSH keys
Like the previous patch, also NodeSshRemoveKey did not
consider offline nodes so far. This patch fixes it.
This fixes issue 1047.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Petr Pudlak <[email protected]>
commit d95ba4afd730c87198cfd17ae32e5211a2723d64
Author: Helga Velroyen <[email protected]>
Date: Thu Apr 16 15:02:08 2015 +0200
Consider offline nodes in NodeSshKeyAdd
So far the backend function NodeSshKeyAdd naively assumed
that all nodes are online and reachable via SSH. This
patch adds offline status information to the function's
signature to respect the offline status when updating
SSH keys.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Petr Pudlak <[email protected]>
commit ff5e257b30b917605b53d2ebe1b619993592f6e7
Author: Helga Velroyen <[email protected]>
Date: Tue Apr 14 15:17:25 2015 +0200
Use SSH file manager for unittests removing keys
This patch replaces the big and unmaintainable unittest
for removing SSH keys by several small ones, which each
cover one of the aspects of the big test.
Note that a certain amount of code duplication is
intended as there are more refactorings coming.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Petr Pudlak <[email protected]>
commit e0bee917c0f05835a005ac9f5b316f384d6b8540
Author: Helga Velroyen <[email protected]>
Date: Tue Apr 14 11:52:33 2015 +0200
Use SSH file manager in key adding unit tests
This patch changes the backend unittests in these aspects:
- They now use the newly introduced SSH file manager. In
particular, the creating of test data is now done
by the file manager.
- We add four smaller unit test which replace the big
(and hardly maintainable) unit test for adding keys.
On first sight, these four tests seem to duplicate
some code. This is true on the on hand, but that will
be addressed in this patch series as there are more
refactorings ahead. On the other hand, those four
tests differ in subtle ways, and there was not a good
way to consolidate the code without creating one big
test again.
- We remove the now superfluous big unit test for adding
keys again.
- In the course of this refactoring, we simplify some
parts where the original code was dealing with lists
of keys, but where we only actually need one key.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Petr Pudlak <[email protected]>
commit b3465c032ebd80875f379b4e7e469c750ab8abf1
Author: Helga Velroyen <[email protected]>
Date: Tue Apr 14 11:45:21 2015 +0200
Introduce (testutils) SSH file manager
Testing the backend functions which update SSH keys is a
pain and maintaining the tests even more. Therefore, this
patch introduces a manager for all SSH key files of a
clusters (ganeti_pub_keys and authorized_keys). It
emulates all operations on these files for all nodes in
the cluster.
This has the following advantages:
- One can query the state of the entire cluster in a
consisten way, for example "Do all nodes have this
master candidates' key?" instead of tediously evaluating
a history of mock calls.
- The file manager emulates both local changes in the
master nodes' key files and changes on other nodes'
key files using the ssh_update tool. This way, the
state of the cluster ssh files is managed consistently
no matter by what mechanism they were changed.
- The file manager offers a couple of convenience
functions to set up the test data and to query their
state after test operation.
Note that this might look like a lot of code, but it
vastly simplifies the current unit tests and it will
make future tests (for example for invalid calls) much
more easier. As it is a test utility, it is properly
documented to make it maintainable.
Signed-off-by: Helga Velroyen <[email protected]>
Reviewed-by: Petr Pudlak <[email protected]>
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
