Actually, I figured out, that in this case, we should throw a proper exception. I will send another patch. Disregard this one, I will not push it.
On Tue, 24 Mar 2015 at 10:40 Petr Pudlak <[email protected]> wrote: > LGTM, thanks > > On Fri, Mar 20, 2015 at 02:27:21PM +0100, 'Helga Velroyen' via > ganeti-devel wrote: > >When removing SSH keys, it can happen that updating the SSH > >files fails on some nodes. This patch makes sure that we > >collect the failures of the nodes and display them to the > >user instead of breaking as soon as the first occurs. > >This fixes Issue 1039. > > > >Signed-off-by: Helga Velroyen <[email protected]> > >--- > > lib/backend.py | 44 ++++++++++++++++++++++++++++---------------- > > lib/cmdlib/node.py | 4 ++++ > > 2 files changed, 32 insertions(+), 16 deletions(-) > > > >diff --git a/lib/backend.py b/lib/backend.py > >index f908bc5..01c31db 100644 > >--- a/lib/backend.py > >+++ b/lib/backend.py > >@@ -1609,18 +1609,20 @@ def RemoveNodeSshKey(node_uuid, node_name, > > should be cleared on the node whose keys are removed > > @type clear_public_keys: boolean > > @param clear_public_keys: whether to clear the node's > C{ganeti_pub_key} file > >+ @rtype: list of string > >+ @returns: list of feedback messages > > > > """ > >+ result_msgs = [] > >+ > > # Make sure at least one of these flags is true. > >- assert (from_authorized_keys or from_public_keys or > clear_authorized_keys > >- or clear_public_keys) > >+ if not (from_authorized_keys or from_public_keys or > clear_authorized_keys > >+ or clear_public_keys): > >+ result_msgs.append("No removal from any key file was requested.") > > > > if not ssconf_store: > > ssconf_store = ssconf.SimpleStore() > > > >- if not (from_authorized_keys or from_public_keys or > clear_authorized_keys): > >- raise errors.SshUpdateError("No removal from any key file was > requested.") > >- > > master_node = ssconf_store.GetMasterNode() > > > > if from_authorized_keys or from_public_keys: > >@@ -1676,16 +1678,24 @@ def RemoveNodeSshKey(node_uuid, node_name, > > " node '%s', map: %s." % > > (node, ssh_port_map)) > > if node in potential_master_candidates: > >- run_cmd_fn(cluster_name, node, pathutils.SSH_UPDATE, > >- ssh_port, pot_mc_data, > >- debug=False, verbose=False, use_cluster_key=False, > >- ask_key=False, strict_host_check=False) > >- else: > >- if from_authorized_keys: > >+ try: > > run_cmd_fn(cluster_name, node, pathutils.SSH_UPDATE, > >- ssh_port, base_data, > >+ ssh_port, pot_mc_data, > > debug=False, verbose=False, use_cluster_key=False, > > ask_key=False, strict_host_check=False) > >+ except errors.OpExecError as e: > >+ result_msgs.append("Warning: the SSH setup of node '%s' > could not" > >+ " be adjusted." % node) > >+ else: > >+ if from_authorized_keys: > >+ try: > >+ run_cmd_fn(cluster_name, node, pathutils.SSH_UPDATE, > >+ ssh_port, base_data, > >+ debug=False, verbose=False, > use_cluster_key=False, > >+ ask_key=False, strict_host_check=False) > >+ except errors.OpExecError as e: > >+ result_msgs.append("Warning: the SSH setup of node '%s' > could" > >+ " not be adjusted." % node) > > > > if clear_authorized_keys or from_public_keys or clear_public_keys: > > data = {} > >@@ -1728,10 +1738,12 @@ def RemoveNodeSshKey(node_uuid, node_name, > > ssh_port, data, > > debug=False, verbose=False, use_cluster_key=False, > > ask_key=False, strict_host_check=False) > >- except errors.OpExecError, e: > >- logging.info("Removing SSH keys from node '%s' failed. This can > happen" > >- " when the node is already unreachable. Error: %s", > >- node_name, e) > >+ except errors.OpExecError as e: > >+ result_msgs.append("Removing SSH keys from node '%s' failed. This > can" > >+ " happen when the node is already unreachable." > >+ " Error: %s" % (node_name, e)) > >+ > >+ return result_msgs > > > > > > def _GenerateNodeSshKey(node_uuid, node_name, ssh_port_map, > >diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py > >index e3d4b1f..4f533b2 100644 > >--- a/lib/cmdlib/node.py > >+++ b/lib/cmdlib/node.py > >@@ -877,9 +877,13 @@ class LUNodeSetParams(LogicalUnit): > > False, # currently, all nodes are potential master candidates > > False, # do not clear node's 'authorized_keys' > > False) # do not clear node's 'ganeti_pub_keys' > >+ if not ssh_result[master_node].fail_msg: > >+ for message in ssh_result[master_node].payload: > >+ feedback_fn(message) > > ssh_result[master_node].Raise( > > "Could not adjust the SSH setup after demoting node '%s'" > > " (UUID: %s)." % (node.name, node.uuid)) > >+ > > if self.new_role == self._ROLE_CANDIDATE: > > ssh_result = self.rpc.call_node_ssh_key_add( > > [master_node], node.uuid, node.name, > >-- > >2.2.0.rc0.207.ga3a616c > > >
