Oh, this was meant for a different patch. So I will still push this one. On Tue, 24 Mar 2015 at 11:25 Helga Velroyen <[email protected]> wrote:
> 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 >> > >> >
