From 540905202bef8a5e0cec2186182a5d67eea4b924 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Thu, 26 Jan 2017 11:34:15 -0600 Subject: [PATCH 1/7] Test: CTS: don't continue with remote tests if setup fails Otherwise CTS aborts with an exception --- cts/CTStests.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/cts/CTStests.py b/cts/CTStests.py index a01658079cb..61051f4b5b1 100644 --- a/cts/CTStests.py +++ b/cts/CTStests.py @@ -3032,11 +3032,12 @@ def start_new_test(self, node): ret = self.startall(None) if not ret: - return self.failure("Setup failed, start all nodes failed.") + return self.failure("setup failed: could not start all nodes") self.setup_env(node) self.start_metal(node) self.add_dummy_rsc(node) + return True def __call__(self, node): return self.failure("This base class is not meant to be called directly.") @@ -3056,7 +3057,9 @@ class RemoteBasic(RemoteDriver): def __call__(self, node): '''Perform the 'RemoteBaremetal' test. ''' - self.start_new_test(node) + if not self.start_new_test(node): + return self.failure(self.fail_string) + self.test_attributes(node) self.cleanup_metal(node) @@ -3074,7 +3077,9 @@ class RemoteStonithd(RemoteDriver): def __call__(self, node): '''Perform the 'RemoteStonithd' test. ''' - self.start_new_test(node) + if not self.start_new_test(node): + return self.failure(self.fail_string) + self.fail_connection(node) self.cleanup_metal(node) @@ -3115,7 +3120,9 @@ class RemoteMigrate(RemoteDriver): def __call__(self, node): '''Perform the 'RemoteMigrate' test. ''' - self.start_new_test(node) + if not self.start_new_test(node): + return self.failure(self.fail_string) + self.migrate_connection(node) self.cleanup_metal(node) @@ -3134,7 +3141,8 @@ class RemoteRscFailure(RemoteDriver): def __call__(self, node): '''Perform the 'RemoteRscFailure' test. ''' - self.start_new_test(node) + if not self.start_new_test(node): + return self.failure(self.fail_string) # This is an important step. We are migrating the connection # before failing the resource. This verifies that the migration From 05592ec909229b00107de4f9c65a342686905217 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Thu, 2 Feb 2017 17:05:54 -0600 Subject: [PATCH 2/7] Test: lrmd: use pacemaker (not heartbeat) Dummy in versioned params test We don't control the heartbeat agent, so we can't rely on its behavior. --- lrmd/regression.py.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lrmd/regression.py.in b/lrmd/regression.py.in index 3fd7b60ecd3..5bf47c7bd00 100755 --- a/lrmd/regression.py.in +++ b/lrmd/regression.py.in @@ -1029,7 +1029,7 @@ if __name__ == "__main__": test.add_sys_cmd("rm", "-f %s %s" % (state_file_expected, state_file_default)) # Register and start the resource. - test.add_cmd("-c register_rsc -r test_rsc -P heartbeat -C ocf -T Dummy " + test.add_cmd("-c register_rsc -r test_rsc -P pacemaker -C ocf -T Dummy " "-l 'NEW_EVENT event_type:register rsc_id:test_rsc action:none rc:ok op_status:complete' " "%s" % (self.action_timeout)) test.add_cmd("-c exec -r test_rsc -a start -t 6000 %s" % cmd_params) From 89dbc7daa0757610f8ed613ed95cab6dbef7743c Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Tue, 24 Jan 2017 10:37:33 -0600 Subject: [PATCH 3/7] Low: crmd: don't clear remote node transient attributes unnecessarily cd10f0b modified crmd to update the CIB directly for remote node attributes when legacy attrd was in use. This included clearing remote node transient attributes when calling attrd_update_delegate() with 'C' (i.e. ATTRD_OP_PEER_REMOVE). However, this is not necessary, because peer removal is only intended to clears caches, not the CIB. The CIB is cleared separately, e.g. in remote_node_up() and remote_node_down(). This also fixes a minor issue where an rc variable was mistyped and then obscured by a redeclaration. --- crmd/attrd.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/crmd/attrd.c b/crmd/attrd.c index ded56a62313..cae1ab5dc46 100644 --- a/crmd/attrd.c +++ b/crmd/attrd.c @@ -31,11 +31,6 @@ update_without_attrd(const char *host_uuid, const char *name, const char *value, call_opt = crmd_cib_smart_opt(); - if (command == 'C') { - erase_status_tag(host_uuid, XML_TAG_TRANSIENT_NODEATTRS, call_opt); - return pcmk_ok; - } - crm_trace("updating status for host_uuid %s, %s=%s", host_uuid, (name? name : ""), (value? value : "")); if (value) { @@ -90,7 +85,7 @@ update_attrd_helper(const char *host, const char *name, const char *value, const char *user_name, gboolean is_remote_node, char command) { - gboolean rc; + int rc; int max = 5; int attrd_opts = attrd_opt_none; @@ -99,16 +94,17 @@ update_attrd_helper(const char *host, const char *name, const char *value, attrd_opts |= attrd_opt_remote; #else /* Talk directly to cib for remote nodes if it's legacy attrd */ - int rc; /* host is required for updating a remote node */ CRM_CHECK(host != NULL, return;); - /* remote node uname and uuid are equal */ - rc = update_without_attrd(host, name, value, user_name, is_remote_node, - command); - if (rc < pcmk_ok) { - log_attrd_error(host, name, value, is_remote_node, command, rc); + if (command != 'C') { + /* remote node uname and uuid are equal */ + rc = update_without_attrd(host, name, value, user_name, + is_remote_node, command); + if (rc < pcmk_ok) { + log_attrd_error(host, name, value, is_remote_node, command, rc); + } } return; #endif From 1e801beccedfce18d3ce26c505911ed28f940225 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Thu, 26 Jan 2017 17:40:02 -0600 Subject: [PATCH 4/7] Refactor: attrd: functionize shared heartbeat/corosync request code Reduces code duplication and makes it easier to make changes. --- attrd/legacy.c | 55 +++++++++++++++++++++----------------------------- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/attrd/legacy.c b/attrd/legacy.c index 592fb0d0c13..ed8ee78fc62 100644 --- a/attrd/legacy.c +++ b/attrd/legacy.c @@ -283,6 +283,27 @@ find_hash_entry(xmlNode * msg) return hash_entry; } +static void +process_xml_request(xmlNode *xml) +{ + attr_hash_entry_t *hash_entry = NULL; + const char *from = crm_element_value(xml, F_ORIG); + const char *op = crm_element_value(xml, F_ATTRD_TASK); + const char *host = crm_element_value(xml, F_ATTRD_HOST); + const char *ignore = crm_element_value(xml, F_ATTRD_IGNORE_LOCALLY); + + if (host && safe_str_eq(host, attrd_uname)) { + crm_info("Update relayed from %s", from); + attrd_local_callback(xml); + + } else if ((ignore == NULL) || safe_str_neq(from, attrd_uname)) { + crm_trace("%s message from %s", op, from); + hash_entry = find_hash_entry(xml); + stop_attrd_timer(hash_entry); + attrd_perform_update(hash_entry); + } +} + #if SUPPORT_HEARTBEAT static void attrd_ha_connection_destroy(gpointer user_data) @@ -305,23 +326,9 @@ attrd_ha_connection_destroy(gpointer user_data) static void attrd_ha_callback(HA_Message * msg, void *private_data) { - attr_hash_entry_t *hash_entry = NULL; xmlNode *xml = convert_ha_message(NULL, msg, __FUNCTION__); - const char *from = crm_element_value(xml, F_ORIG); - const char *op = crm_element_value(xml, F_ATTRD_TASK); - const char *host = crm_element_value(xml, F_ATTRD_HOST); - const char *ignore = crm_element_value(xml, F_ATTRD_IGNORE_LOCALLY); - if (host != NULL && safe_str_eq(host, attrd_uname)) { - crm_info("Update relayed from %s", from); - attrd_local_callback(xml); - - } else if (ignore == NULL || safe_str_neq(from, attrd_uname)) { - crm_info("%s message from %s", op, from); - hash_entry = find_hash_entry(xml); - stop_attrd_timer(hash_entry); - attrd_perform_update(hash_entry); - } + process_xml_request(xml); free_xml(xml); } @@ -349,25 +356,9 @@ attrd_cs_dispatch(cpg_handle_t handle, } if (xml != NULL) { - attr_hash_entry_t *hash_entry = NULL; - const char *op = crm_element_value(xml, F_ATTRD_TASK); - const char *host = crm_element_value(xml, F_ATTRD_HOST); - const char *ignore = crm_element_value(xml, F_ATTRD_IGNORE_LOCALLY); - /* crm_xml_add_int(xml, F_SEQ, wrapper->id); */ crm_xml_add(xml, F_ORIG, from); - - if (host != NULL && safe_str_eq(host, attrd_uname)) { - crm_notice("Update relayed from %s", from); - attrd_local_callback(xml); - - } else if (ignore == NULL || safe_str_neq(from, attrd_uname)) { - crm_trace("%s message from %s", op, from); - hash_entry = find_hash_entry(xml); - stop_attrd_timer(hash_entry); - attrd_perform_update(hash_entry); - } - + process_xml_request(xml); free_xml(xml); } From f6870b1f6e896ba721a9ddcb6cdd75fc42c4ee31 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Fri, 20 Jan 2017 18:19:55 -0600 Subject: [PATCH 5/7] Low: attrd,crmd: implement peer remove requests in legacy attrd This should help prevent any issues with the peer caches being stale. --- attrd/legacy.c | 12 +++++++++++- crmd/attrd.c | 10 ++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/attrd/legacy.c b/attrd/legacy.c index ed8ee78fc62..5a673bb1ea5 100644 --- a/attrd/legacy.c +++ b/attrd/legacy.c @@ -296,6 +296,12 @@ process_xml_request(xmlNode *xml) crm_info("Update relayed from %s", from); attrd_local_callback(xml); + } else if (safe_str_eq(op, ATTRD_OP_PEER_REMOVE)) { + CRM_CHECK(host != NULL, return); + crm_debug("Removing %s from peer caches for %s", host, from); + crm_remote_peer_cache_remove(host); + reap_crm_member(0, host); + } else if ((ignore == NULL) || safe_str_neq(from, attrd_uname)) { crm_trace("%s message from %s", op, from); hash_entry = find_hash_entry(xml); @@ -772,8 +778,12 @@ attrd_local_callback(xmlNode * msg) crm_notice("Sending full refresh (origin=%s)", from); g_hash_table_foreach(attr_hash, update_for_hash_entry, NULL); return; + } else if (safe_str_eq(op, ATTRD_OP_PEER_REMOVE)) { - /* The legacy code didn't understand this command - swallow silently */ + if (host) { + crm_notice("Broadcasting removal of peer %s", host); + send_cluster_message(NULL, crm_msg_attrd, msg, FALSE); + } return; } diff --git a/crmd/attrd.c b/crmd/attrd.c index cae1ab5dc46..e9a5e4be136 100644 --- a/crmd/attrd.c +++ b/crmd/attrd.c @@ -90,10 +90,12 @@ update_attrd_helper(const char *host, const char *name, const char *value, int attrd_opts = attrd_opt_none; if (is_remote_node) { -#if HAVE_ATOMIC_ATTRD attrd_opts |= attrd_opt_remote; -#else - /* Talk directly to cib for remote nodes if it's legacy attrd */ + +#if !HAVE_ATOMIC_ATTRD + /* Legacy attrd can handle remote peer remove ('C') requests, + * otherwise talk directly to cib for remote nodes. + */ /* host is required for updating a remote node */ CRM_CHECK(host != NULL, return;); @@ -105,8 +107,8 @@ update_attrd_helper(const char *host, const char *name, const char *value, if (rc < pcmk_ok) { log_attrd_error(host, name, value, is_remote_node, command, rc); } + return; } - return; #endif } From e5e5065624b6713dc1faff3bb9fc5f347ae8aced Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Fri, 20 Jan 2017 18:35:00 -0600 Subject: [PATCH 6/7] Refactor: attrd: functionize updating a single attribute in legacy attrd This will make it easier to support regular expressions in legacy attrd. --- attrd/legacy.c | 144 ++++++++++++++++++++++++++++++------------------- 1 file changed, 89 insertions(+), 55 deletions(-) diff --git a/attrd/legacy.c b/attrd/legacy.c index 5a673bb1ea5..c5be7f9d6de 100644 --- a/attrd/legacy.c +++ b/attrd/legacy.c @@ -763,40 +763,60 @@ attrd_perform_update(attr_hash_entry_t * hash_entry) return; } -void -attrd_local_callback(xmlNode * msg) +/* strlen("value") */ +#define plus_plus_len (5) + +/*! + * \internal + * \brief Expand attribute values that use "++" or "+=" + * + * \param[in] value Attribute value to expand + * \param[in] old_value Previous value of attribute + * + * \return Newly allocated string with expanded value, or NULL if not expanded + */ +static char * +expand_attr_value(const char *value, const char *old_value) { - static int plus_plus_len = 5; - attr_hash_entry_t *hash_entry = NULL; - const char *from = crm_element_value(msg, F_ORIG); - const char *op = crm_element_value(msg, F_ATTRD_TASK); - const char *attr = crm_element_value(msg, F_ATTRD_ATTRIBUTE); - const char *value = crm_element_value(msg, F_ATTRD_VALUE); - const char *host = crm_element_value(msg, F_ATTRD_HOST); + int value_len = strlen(value); + char *expanded = NULL; - if (safe_str_eq(op, ATTRD_OP_REFRESH)) { - crm_notice("Sending full refresh (origin=%s)", from); - g_hash_table_foreach(attr_hash, update_for_hash_entry, NULL); - return; + if ((value_len >= (plus_plus_len + 2)) + && (value[plus_plus_len] == '+') + && ((value[plus_plus_len + 1] == '+') + || (value[plus_plus_len + 1] == '='))) { - } else if (safe_str_eq(op, ATTRD_OP_PEER_REMOVE)) { - if (host) { - crm_notice("Broadcasting removal of peer %s", host); - send_cluster_message(NULL, crm_msg_attrd, msg, FALSE); + int offset = 1; + int int_value = char2score(old_value); + + if (value[plus_plus_len + 1] != '+') { + const char *offset_s = value + (plus_plus_len + 2); + + offset = char2score(offset_s); } - return; - } + int_value += offset; - if (host != NULL && safe_str_neq(host, attrd_uname)) { - send_cluster_message(crm_get_peer(0, host), crm_msg_attrd, msg, FALSE); - return; - } + if (int_value > INFINITY) { + int_value = INFINITY; + } - crm_debug("%s message from %s: %s=%s", op, from, attr, crm_str(value)); - hash_entry = find_hash_entry(msg); - if (hash_entry == NULL) { - return; + expanded = crm_itoa(int_value); } + return expanded; +} + +/*! + * \internal + * \brief Update a single node attribute for this node + * + * \param[in] msg XML message with update + * \param[in,out] hash_entry Node attribute structure + */ +static void +update_local_attr(xmlNode *msg, attr_hash_entry_t *hash_entry) +{ + const char *value = crm_element_value(msg, F_ATTRD_VALUE); + char *expanded = NULL; if (hash_entry->uuid == NULL) { const char *key = crm_element_value(msg, F_ATTRD_KEY); @@ -815,44 +835,24 @@ attrd_local_callback(xmlNode * msg) return; } else if (value) { - int offset = 1; - int int_value = 0; - int value_len = strlen(value); - - if (value_len < (plus_plus_len + 2) - || value[plus_plus_len] != '+' - || (value[plus_plus_len + 1] != '+' && value[plus_plus_len + 1] != '=')) { - goto set_unexpanded; - } - - int_value = char2score(hash_entry->value); - if (value[plus_plus_len + 1] != '+') { - const char *offset_s = value + (plus_plus_len + 2); - - offset = char2score(offset_s); - } - int_value += offset; - - if (int_value > INFINITY) { - int_value = INFINITY; + expanded = expand_attr_value(value, hash_entry->value); + if (expanded) { + crm_info("Expanded %s=%s to %s", hash_entry->id, value, expanded); + value = expanded; } - - crm_info("Expanded %s=%s to %d", attr, value, int_value); - crm_xml_add_int(msg, F_ATTRD_VALUE, int_value); - value = crm_element_value(msg, F_ATTRD_VALUE); } - set_unexpanded: if (safe_str_eq(value, hash_entry->value) && hash_entry->timer_id) { /* We're already waiting to set this value */ + free(expanded); return; } free(hash_entry->value); hash_entry->value = NULL; if (value != NULL) { - hash_entry->value = strdup(value); - crm_debug("New value of %s is %s", attr, value); + hash_entry->value = (expanded? expanded : strdup(value)); + crm_debug("New value of %s is %s", hash_entry->id, value); } stop_attrd_timer(hash_entry); @@ -862,8 +862,42 @@ attrd_local_callback(xmlNode * msg) } else { attrd_trigger_update(hash_entry); } +} - return; +void +attrd_local_callback(xmlNode * msg) +{ + attr_hash_entry_t *hash_entry = NULL; + const char *from = crm_element_value(msg, F_ORIG); + const char *op = crm_element_value(msg, F_ATTRD_TASK); + const char *attr = crm_element_value(msg, F_ATTRD_ATTRIBUTE); + const char *value = crm_element_value(msg, F_ATTRD_VALUE); + const char *host = crm_element_value(msg, F_ATTRD_HOST); + + if (safe_str_eq(op, ATTRD_OP_REFRESH)) { + crm_notice("Sending full refresh (origin=%s)", from); + g_hash_table_foreach(attr_hash, update_for_hash_entry, NULL); + return; + + } else if (safe_str_eq(op, ATTRD_OP_PEER_REMOVE)) { + if (host) { + crm_notice("Broadcasting removal of peer %s", host); + send_cluster_message(NULL, crm_msg_attrd, msg, FALSE); + } + return; + } + + if (host != NULL && safe_str_neq(host, attrd_uname)) { + send_cluster_message(crm_get_peer(0, host), crm_msg_attrd, msg, FALSE); + return; + } + + crm_debug("%s message from %s: %s=%s", op, from, attr, crm_str(value)); + hash_entry = find_hash_entry(msg); + if (hash_entry == NULL) { + return; + } + update_local_attr(msg, hash_entry); } gboolean From 30052abc2311a4a3584edde2f98bf70f8a2814c0 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Wed, 1 Feb 2017 16:11:49 -0600 Subject: [PATCH 7/7] Low: attrd: ignore unsupported requests in legacy attrd --- attrd/legacy.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/attrd/legacy.c b/attrd/legacy.c index c5be7f9d6de..52ef1a5a2da 100644 --- a/attrd/legacy.c +++ b/attrd/legacy.c @@ -885,6 +885,10 @@ attrd_local_callback(xmlNode * msg) send_cluster_message(NULL, crm_msg_attrd, msg, FALSE); } return; + + } else if (op && safe_str_neq(op, ATTRD_OP_UPDATE)) { + crm_notice("Ignoring unsupported %s request from %s", op, from); + return; } if (host != NULL && safe_str_neq(host, attrd_uname)) {