From 28566d6832274c59f27bb7b2f1f54420a3f3d822 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Thu, 9 May 2019 20:26:08 -0500
Subject: [PATCH 1/2] Refactor: libpe_status: functionize unfencing digest code
more
... for readability, reusability, and avoiding unnecessary function calls or
memory allocation.
---
lib/pengine/utils.c | 159 ++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 118 insertions(+), 41 deletions(-)
diff --git a/lib/pengine/utils.c b/lib/pengine/utils.c
index 2f4dc1e..f80f8d4 100644
--- a/lib/pengine/utils.c
+++ b/lib/pengine/utils.c
@@ -2080,57 +2080,134 @@ rsc_action_digest_cmp(resource_t * rsc, xmlNode * xml_op, node_t * node,
return data;
}
+/*!
+ * \internal
+ * \brief Create an unfencing summary for use in special node attribute
+ *
+ * Create a string combining a fence device's resource ID, agent type, and
+ * parameter digest (whether for all parameters or just non-private parameters).
+ * This can be stored in a special node attribute, allowing us to detect changes
+ * in either the agent type or parameters, to know whether unfencing must be
+ * redone or can be safely skipped when the device's history is cleaned.
+ *
+ * \param[in] rsc_id Fence device resource ID
+ * \param[in] agent_type Fence device agent
+ * \param[in] param_digest Fence device parameter digest
+ *
+ * \return Newly allocated string with unfencing digest
+ * \note The caller is responsible for freeing the result.
+ */
+static inline char *
+create_unfencing_summary(const char *rsc_id, const char *agent_type,
+ const char *param_digest)
+{
+ return crm_strdup_printf("%s:%s:%s", rsc_id, agent_type, param_digest);
+}
+
+/*!
+ * \internal
+ * \brief Check whether a node can skip unfencing
+ *
+ * Check whether a fence device's current definition matches a node's
+ * stored summary of when it was last unfenced by the device.
+ *
+ * \param[in] rsc_id Fence device's resource ID
+ * \param[in] agent Fence device's agent type
+ * \param[in] digest_calc Fence device's current parameter digest
+ * \param[in] node_summary Value of node's special unfencing node attribute
+ * (a comma-separated list of unfencing summaries for
+ * all devices that have unfenced this node)
+ *
+ * \return TRUE if digest matches, FALSE otherwise
+ */
+static bool
+unfencing_digest_matches(const char *rsc_id, const char *agent,
+ const char *digest_calc, const char *node_summary)
+{
+ bool matches = FALSE;
+
+ if (rsc_id && agent && digest_calc && node_summary) {
+ char *search_secure = create_unfencing_summary(rsc_id, agent,
+ digest_calc);
+
+ /* The digest was calculated including the device ID and agent,
+ * so there is no risk of collision using strstr().
+ */
+ matches = (strstr(node_summary, search_secure) != NULL);
+ crm_trace("Calculated unfencing digest '%s' %sfound in '%s'",
+ search_secure, matches? "" : "not ", node_summary);
+ free(search_secure);
+ }
+ return matches;
+}
+
+/* Magic string to use as action name for digest cache entries used for
+ * unfencing checks. This is not a real action name (i.e. "on"), so
+ * check_action_definition() won't confuse these entries with real actions.
+ */
#define STONITH_DIGEST_TASK "stonith-on"
+/*!
+ * \internal
+ * \brief Calculate fence device digests and digest comparison result
+ *
+ * \param[in] rsc Fence device resource
+ * \param[in] agent Fence device's agent type
+ * \param[in] node Node with digest cache to use
+ * \param[in] data_set Cluster working set
+ *
+ * \return Node's digest cache entry
+ */
static op_digest_cache_t *
-fencing_action_digest_cmp(resource_t * rsc, node_t * node, pe_working_set_t * data_set)
+fencing_action_digest_cmp(pe_resource_t *rsc, const char *agent,
+ pe_node_t *node, pe_working_set_t *data_set)
{
- char *key = generate_op_key(rsc->id, STONITH_DIGEST_TASK, 0);
- op_digest_cache_t *data = rsc_action_digest(rsc, STONITH_DIGEST_TASK, key, node, NULL, data_set);
+ const char *node_summary = NULL;
- const char *digest_all = pe_node_attribute_raw(node, CRM_ATTR_DIGESTS_ALL);
- const char *digest_secure = pe_node_attribute_raw(node, CRM_ATTR_DIGESTS_SECURE);
+ // Calculate device's current parameter digests
+ char *key = generate_op_key(rsc->id, STONITH_DIGEST_TASK, 0);
+ op_digest_cache_t *data = rsc_action_digest(rsc, STONITH_DIGEST_TASK, key,
+ node, NULL, data_set);
- /* No 'reloads' for fencing device changes
- *
- * We use the resource id + agent + digest so that we can detect
- * changes to the agent and/or the parameters used
- */
- char *search_all = crm_strdup_printf("%s:%s:%s", rsc->id, (const char*)g_hash_table_lookup(rsc->meta, XML_ATTR_TYPE), data->digest_all_calc);
- char *search_secure = crm_strdup_printf("%s:%s:%s", rsc->id, (const char*)g_hash_table_lookup(rsc->meta, XML_ATTR_TYPE), data->digest_secure_calc);
+ free(key);
- data->rc = RSC_DIGEST_ALL;
- if (digest_all == NULL) {
- /* it is unknown what the previous op digest was */
+ // Check whether node has special unfencing summary node attribute
+ node_summary = pe_node_attribute_raw(node, CRM_ATTR_DIGESTS_ALL);
+ if (node_summary == NULL) {
data->rc = RSC_DIGEST_UNKNOWN;
+ return data;
+ }
- } else if (strstr(digest_all, search_all)) {
+ // Check whether full parameter digest matches
+ if (unfencing_digest_matches(rsc->id, agent, data->digest_all_calc,
+ node_summary)) {
data->rc = RSC_DIGEST_MATCH;
+ return data;
+ }
- } else if(digest_secure && data->digest_secure_calc) {
- if(strstr(digest_secure, search_secure)) {
- if (is_set(data_set->flags, pe_flag_stdout)) {
- printf("Only 'private' parameters to %s for unfencing %s changed\n",
- rsc->id, node->details->uname);
- }
- data->rc = RSC_DIGEST_MATCH;
+ // Check whether secure parameter digest matches
+ node_summary = pe_node_attribute_raw(node, CRM_ATTR_DIGESTS_SECURE);
+ if (unfencing_digest_matches(rsc->id, agent, data->digest_secure_calc,
+ node_summary)) {
+ data->rc = RSC_DIGEST_MATCH;
+ if (is_set(data_set->flags, pe_flag_stdout)) {
+ printf("Only 'private' parameters to %s for unfencing %s changed\n",
+ rsc->id, node->details->uname);
}
+ return data;
}
- if (is_set(data_set->flags, pe_flag_sanitized)
- && is_set(data_set->flags, pe_flag_stdout)
- && (data->rc == RSC_DIGEST_ALL)
+ // Parameters don't match
+ data->rc = RSC_DIGEST_ALL;
+ if (is_set(data_set->flags, (pe_flag_sanitized|pe_flag_stdout))
&& data->digest_secure_calc) {
- printf("Parameters to %s for unfencing %s changed, try '%s:%s:%s'\n",
- rsc->id, node->details->uname, rsc->id,
- (const char *) g_hash_table_lookup(rsc->meta, XML_ATTR_TYPE),
- data->digest_secure_calc);
- }
-
- free(key);
- free(search_all);
- free(search_secure);
+ char *digest = create_unfencing_summary(rsc->id, agent,
+ data->digest_secure_calc);
+ printf("Parameters to %s for unfencing %s changed, try '%s'\n",
+ rsc->id, node->details->uname, digest);
+ free(digest);
+ }
return data;
}
@@ -2218,9 +2295,6 @@ pe_fence_op(node_t * node, const char *op, bool optional, const char *reason, pe
*
* We may do this for all nodes in the future, but for now
* the check_action_definition() based stuff works fine.
- *
- * Use "stonith-on" to avoid creating cache entries for
- * operations check_action_definition() would look for.
*/
long max = 1024;
long digests_all_offset = 0;
@@ -2232,8 +2306,11 @@ pe_fence_op(node_t * node, const char *op, bool optional, const char *reason, pe
for (GListPtr gIter = matches; gIter != NULL; gIter = gIter->next) {
resource_t *match = gIter->data;
- op_digest_cache_t *data = fencing_action_digest_cmp(match, node, data_set);
+ const char *agent = g_hash_table_lookup(match->meta,
+ XML_ATTR_TYPE);
+ op_digest_cache_t *data = NULL;
+ data = fencing_action_digest_cmp(match, agent, node, data_set);
if(data->rc == RSC_DIGEST_ALL) {
optional = FALSE;
crm_notice("Unfencing %s (remote): because the definition of %s changed", node->details->uname, match->id);
@@ -2244,11 +2321,11 @@ pe_fence_op(node_t * node, const char *op, bool optional, const char *reason, pe
digests_all_offset += snprintf(
digests_all+digests_all_offset, max-digests_all_offset,
- "%s:%s:%s,", match->id, (const char*)g_hash_table_lookup(match->meta, XML_ATTR_TYPE), data->digest_all_calc);
+ "%s:%s:%s,", match->id, agent, data->digest_all_calc);
digests_secure_offset += snprintf(
digests_secure+digests_secure_offset, max-digests_secure_offset,
- "%s:%s:%s,", match->id, (const char*)g_hash_table_lookup(match->meta, XML_ATTR_TYPE), data->digest_secure_calc);
+ "%s:%s:%s,", match->id, agent, data->digest_secure_calc);
}
g_hash_table_insert(stonith_op->meta,
strdup(XML_OP_ATTR_DIGESTS_ALL),
--
1.8.3.1
From fd6e06ff419c95f4423202163d2d4dca3f03a4c5 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 10 May 2019 11:57:31 -0500
Subject: [PATCH 2/2] Fix: libpe_status: calculate secure digests for unfencing
ops
The calculation of digests for detection of when unfencing is needed reused
rsc_action_digest(). However that would only add secure digests when the
pe_flag_sanitized flag was set, which is only set by crm_simulate, so secure
digests would never be added in normal cluster operation. This led to
node attributes like name="#digests-secure"
value="stonith-fence_compute-fence-nova:fence_compute:(null),".
Now, rsc_action_digest() takes a new argument to select whether secure digests
are added, which is always set to TRUE when calculating unfencing digests.
---
lib/pengine/utils.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/lib/pengine/utils.c b/lib/pengine/utils.c
index f80f8d4..5b893f7 100644
--- a/lib/pengine/utils.c
+++ b/lib/pengine/utils.c
@@ -1936,9 +1936,24 @@ append_versioned_params(xmlNode *versioned_params, const char *ra_version, xmlNo
}
#endif
+/*!
+ * \internal
+ * \brief Calculate action digests and store in node's digest cache
+ *
+ * \param[in] rsc Resource that action was for
+ * \param[in] task Name of action performed
+ * \param[in] key Action's task key
+ * \param[in] node Node action was performed on
+ * \param[in] xml_op XML of operation in CIB status (if available)
+ * \param[in] calc_secure Whether to calculate secure digest
+ * \param[in] data_set Cluster working set
+ *
+ * \return Pointer to node's digest cache entry
+ */
static op_digest_cache_t *
-rsc_action_digest(resource_t * rsc, const char *task, const char *key,
- node_t * node, xmlNode * xml_op, pe_working_set_t * data_set)
+rsc_action_digest(pe_resource_t *rsc, const char *task, const char *key,
+ pe_node_t *node, xmlNode *xml_op, bool calc_secure,
+ pe_working_set_t *data_set)
{
op_digest_cache_t *data = NULL;
@@ -2007,7 +2022,7 @@ rsc_action_digest(resource_t * rsc, const char *task, const char *key,
data->digest_all_calc = calculate_operation_digest(data->params_all, op_version);
- if (is_set(data_set->flags, pe_flag_sanitized)) {
+ if (calc_secure) {
data->params_secure = copy_xml(data->params_all);
if(secure_list) {
filter_parameters(data->params_secure, secure_list, FALSE);
@@ -2053,7 +2068,9 @@ rsc_action_digest_cmp(resource_t * rsc, xmlNode * xml_op, node_t * node,
interval_ms = crm_parse_ms(interval_ms_s);
key = generate_op_key(rsc->id, task, interval_ms);
- data = rsc_action_digest(rsc, task, key, node, xml_op, data_set);
+ data = rsc_action_digest(rsc, task, key, node, xml_op,
+ is_set(data_set->flags, pe_flag_sanitized),
+ data_set);
data->rc = RSC_DIGEST_MATCH;
if (digest_restart && data->digest_restart_calc && strcmp(data->digest_restart_calc, digest_restart) != 0) {
@@ -2167,7 +2184,7 @@ fencing_action_digest_cmp(pe_resource_t *rsc, const char *agent,
// Calculate device's current parameter digests
char *key = generate_op_key(rsc->id, STONITH_DIGEST_TASK, 0);
op_digest_cache_t *data = rsc_action_digest(rsc, STONITH_DIGEST_TASK, key,
- node, NULL, data_set);
+ node, NULL, TRUE, data_set);
free(key);
--
1.8.3.1