From f9ad81dfee9780a783ea769e22af89045ef673c1 Mon Sep 17 00:00:00 2001 From: Andrew Beekhof Date: Fri, 2 Jun 2017 12:02:28 +1000 Subject: [PATCH 1/8] PE: Bundles: Implement colocation of primitives and groups with bundles --- pengine/container.c | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/pengine/container.c b/pengine/container.c index 7ec390e..3dc1b35 100644 --- a/pengine/container.c +++ b/pengine/container.c @@ -115,6 +115,7 @@ container_color(resource_t * rsc, node_t * prefer, pe_working_set_t * data_set) } clear_bit(rsc->flags, pe_rsc_allocating); + clear_bit(rsc->flags, pe_rsc_provisional); return NULL; } @@ -205,7 +206,46 @@ container_rsc_colocation_lh(resource_t * rsc_lh, resource_t * rsc_rh, rsc_coloca void container_rsc_colocation_rh(resource_t * rsc_lh, resource_t * rsc_rh, rsc_colocation_t * constraint) { - pe_err("Container %s cannot be colocated with anything", rsc_rh->id); + GListPtr allocated_rhs = NULL; + container_variant_data_t *container_data = NULL; + + CRM_CHECK(constraint != NULL, return); + CRM_CHECK(rsc_lh != NULL, pe_err("rsc_lh was NULL for %s", constraint->id); return); + CRM_CHECK(rsc_rh != NULL, pe_err("rsc_rh was NULL for %s", constraint->id); return); + + if (is_set(rsc_rh->flags, pe_rsc_provisional)) { + pe_rsc_trace(rsc_rh, "%s is still provisional", rsc_rh->id); + return; + + } else if(rsc_lh->variant > pe_group) { + pe_err("Only basic resources and groups can be colocated with %s", rsc_rh->id); + return; + } + + get_container_variant_data(container_data, constraint->rsc_rh); + pe_rsc_trace(rsc_rh, "Processing constraint %s: %s -> %s %d", + constraint->id, rsc_lh->id, rsc_rh->id, constraint->score); + + for (GListPtr gIter = container_data->tuples; gIter != NULL; gIter = gIter->next) { + container_grouping_t *tuple = (container_grouping_t *)gIter->data; + + if (constraint->score < INFINITY) { + tuple->docker->cmds->rsc_colocation_rh(rsc_lh, tuple->docker, constraint); + + } else { + node_t *chosen = tuple->docker->fns->location(tuple->docker, NULL, FALSE); + + if (chosen != NULL && is_set_recursive(tuple->docker, pe_rsc_block, TRUE) == FALSE) { + pe_rsc_trace(rsc_rh, "Allowing %s: %s %d", constraint->id, chosen->details->uname, chosen->weight); + allocated_rhs = g_list_prepend(allocated_rhs, chosen); + } + } + } + + if (constraint->score >= INFINITY) { + node_list_exclude(rsc_lh->allowed_nodes, allocated_rhs, FALSE); + } + g_list_free(allocated_rhs); } enum pe_action_flags -- 1.8.3.1 From 12abb14f516cf53f526b0bbdade1fda9ad612091 Mon Sep 17 00:00:00 2001 From: Andrew Beekhof Date: Fri, 2 Jun 2017 12:40:34 +1000 Subject: [PATCH 2/8] PE: Bundles: Allow clones to be colocated with bundles --- pengine/clone.c | 61 +++++++++++++++++++--------------- pengine/container.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++------- pengine/utils.h | 2 ++ 3 files changed, 120 insertions(+), 38 deletions(-) diff --git a/pengine/clone.c b/pengine/clone.c index 2b332b1..f8c7503 100644 --- a/pengine/clone.c +++ b/pengine/clone.c @@ -925,23 +925,22 @@ clone_internal_constraints(resource_t * rsc, pe_working_set_t * data_set) } } -static bool +bool assign_node(resource_t * rsc, node_t * node, gboolean force) { bool changed = FALSE; if (rsc->children) { - GListPtr gIter = rsc->children; - - for (; gIter != NULL; gIter = gIter->next) { + for (GListPtr gIter = rsc->children; gIter != NULL; gIter = gIter->next) { resource_t *child_rsc = (resource_t *) gIter->data; - changed |= native_assign_node(child_rsc, NULL, node, force); + changed |= assign_node(child_rsc, node, force); } return changed; } + if (rsc->allocated_to != NULL) { changed = true; } @@ -954,7 +953,6 @@ static resource_t * find_compatible_child_by_node(resource_t * local_child, node_t * local_node, resource_t * rsc, enum rsc_role_e filter, gboolean current) { - node_t *node = NULL; GListPtr gIter = NULL; if (local_node == NULL) { @@ -968,29 +966,11 @@ find_compatible_child_by_node(resource_t * local_child, node_t * local_node, res gIter = rsc->children; for (; gIter != NULL; gIter = gIter->next) { resource_t *child_rsc = (resource_t *) gIter->data; - enum rsc_role_e next_role = child_rsc->fns->state(child_rsc, current); - if (is_set_recursive(child_rsc, pe_rsc_block, TRUE) == FALSE) { - /* We only want instances that haven't failed */ - node = child_rsc->fns->location(child_rsc, NULL, current); - } - - if (filter != RSC_ROLE_UNKNOWN && next_role != filter) { - crm_trace("Filtered %s", child_rsc->id); - continue; - } - - if (node && local_node && node->details == local_node->details) { + if(is_child_compatible(child_rsc, local_node, filter, current)) { crm_trace("Pairing %s with %s on %s", - local_child->id, child_rsc->id, node->details->uname); + local_child->id, child_rsc->id, local_node->details->uname); return child_rsc; - - } else if (node) { - crm_trace("%s - %s vs %s", child_rsc->id, node->details->uname, - local_node->details->uname); - - } else { - crm_trace("%s - not allocated %d", child_rsc->id, current); } } @@ -998,6 +978,35 @@ find_compatible_child_by_node(resource_t * local_child, node_t * local_node, res return NULL; } +gboolean +is_child_compatible(resource_t *child_rsc, node_t * local_node, enum rsc_role_e filter, gboolean current) +{ + node_t *node = NULL; + enum rsc_role_e next_role = child_rsc->fns->state(child_rsc, current); + + if (is_set_recursive(child_rsc, pe_rsc_block, TRUE) == FALSE) { + /* We only want instances that haven't failed */ + node = child_rsc->fns->location(child_rsc, NULL, current); + } + + if (filter != RSC_ROLE_UNKNOWN && next_role != filter) { + crm_trace("Filtered %s", child_rsc->id); + return FALSE; + } + + if (node && local_node && node->details == local_node->details) { + return TRUE; + + } else if (node) { + crm_trace("%s - %s vs %s", child_rsc->id, node->details->uname, + local_node->details->uname); + + } else { + crm_trace("%s - not allocated %d", child_rsc->id, current); + } + return FALSE; +} + resource_t * find_compatible_child(resource_t * local_child, resource_t * rsc, enum rsc_role_e filter, gboolean current) diff --git a/pengine/container.c b/pengine/container.c index 3dc1b35..67b0052 100644 --- a/pengine/container.c +++ b/pengine/container.c @@ -197,34 +197,105 @@ container_internal_constraints(resource_t * rsc, pe_working_set_t * data_set) } } + +static resource_t * +find_compatible_tuple_by_node(resource_t * rsc_lh, node_t * candidate, resource_t * rsc, + enum rsc_role_e filter, gboolean current) +{ + container_variant_data_t *container_data = NULL; + + CRM_CHECK(candidate != NULL, return NULL); + get_container_variant_data(container_data, rsc); + + crm_trace("Looking for compatible child from %s for %s on %s", + rsc_lh->id, rsc->id, candidate->details->uname); + + for (GListPtr gIter = container_data->tuples; gIter != NULL; gIter = gIter->next) { + container_grouping_t *tuple = (container_grouping_t *)gIter->data; + + if(is_child_compatible(tuple->docker, candidate, filter, current)) { + crm_trace("Pairing %s with %s on %s", + rsc_lh->id, tuple->docker->id, candidate->details->uname); + return tuple->docker; + } + } + + crm_trace("Can't pair %s with %s", rsc_lh->id, rsc->id); + return NULL; +} + +static resource_t * +find_compatible_tuple(resource_t *rsc_lh, resource_t * rsc, enum rsc_role_e filter, + gboolean current) +{ + GListPtr scratch = NULL; + resource_t *pair = NULL; + node_t *active_node_lh = NULL; + + active_node_lh = rsc_lh->fns->location(rsc_lh, NULL, current); + if (active_node_lh) { + return find_compatible_tuple_by_node(rsc_lh, active_node_lh, rsc, filter, current); + } + + scratch = g_hash_table_get_values(rsc_lh->allowed_nodes); + scratch = g_list_sort_with_data(scratch, sort_node_weight, NULL); + + for (GListPtr gIter = scratch; gIter != NULL; gIter = gIter->next) { + node_t *node = (node_t *) gIter->data; + + pair = find_compatible_tuple_by_node(rsc_lh, node, rsc, filter, current); + if (pair) { + goto done; + } + } + + pe_rsc_debug(rsc, "Can't pair %s with %s", rsc_lh->id, rsc->id); + done: + g_list_free(scratch); + return pair; +} + void -container_rsc_colocation_lh(resource_t * rsc_lh, resource_t * rsc_rh, rsc_colocation_t * constraint) +container_rsc_colocation_lh(resource_t * rsc, resource_t * rsc_rh, rsc_colocation_t * constraint) { - pe_err("Container %s cannot be colocated with anything", rsc_lh->id); + pe_err("Container %s cannot be colocated with anything", rsc->id); } void -container_rsc_colocation_rh(resource_t * rsc_lh, resource_t * rsc_rh, rsc_colocation_t * constraint) +container_rsc_colocation_rh(resource_t * rsc_lh, resource_t * rsc, rsc_colocation_t * constraint) { GListPtr allocated_rhs = NULL; container_variant_data_t *container_data = NULL; CRM_CHECK(constraint != NULL, return); CRM_CHECK(rsc_lh != NULL, pe_err("rsc_lh was NULL for %s", constraint->id); return); - CRM_CHECK(rsc_rh != NULL, pe_err("rsc_rh was NULL for %s", constraint->id); return); + CRM_CHECK(rsc != NULL, pe_err("rsc was NULL for %s", constraint->id); return); - if (is_set(rsc_rh->flags, pe_rsc_provisional)) { - pe_rsc_trace(rsc_rh, "%s is still provisional", rsc_rh->id); + if (is_set(rsc->flags, pe_rsc_provisional)) { + pe_rsc_trace(rsc, "%s is still provisional", rsc->id); return; - } else if(rsc_lh->variant > pe_group) { - pe_err("Only basic resources and groups can be colocated with %s", rsc_rh->id); + } else if(constraint->rsc_lh->variant > pe_group) { + resource_t *rh_child = find_compatible_tuple(rsc_lh, rsc, RSC_ROLE_UNKNOWN, FALSE); + + if (rh_child) { + pe_rsc_debug(rsc, "Pairing %s with %s", rsc_lh->id, rh_child->id); + rsc_lh->cmds->rsc_colocation_lh(rsc_lh, rh_child, constraint); + + } else if (constraint->score >= INFINITY) { + crm_notice("Cannot pair %s with instance of %s", rsc_lh->id, rsc->id); + assign_node(rsc_lh, NULL, TRUE); + + } else { + pe_rsc_debug(rsc, "Cannot pair %s with instance of %s", rsc_lh->id, rsc->id); + } + return; } - get_container_variant_data(container_data, constraint->rsc_rh); - pe_rsc_trace(rsc_rh, "Processing constraint %s: %s -> %s %d", - constraint->id, rsc_lh->id, rsc_rh->id, constraint->score); + get_container_variant_data(container_data, rsc); + pe_rsc_trace(rsc, "Processing constraint %s: %s -> %s %d", + constraint->id, rsc_lh->id, rsc->id, constraint->score); for (GListPtr gIter = container_data->tuples; gIter != NULL; gIter = gIter->next) { container_grouping_t *tuple = (container_grouping_t *)gIter->data; @@ -236,7 +307,7 @@ container_rsc_colocation_rh(resource_t * rsc_lh, resource_t * rsc_rh, rsc_coloca node_t *chosen = tuple->docker->fns->location(tuple->docker, NULL, FALSE); if (chosen != NULL && is_set_recursive(tuple->docker, pe_rsc_block, TRUE) == FALSE) { - pe_rsc_trace(rsc_rh, "Allowing %s: %s %d", constraint->id, chosen->details->uname, chosen->weight); + pe_rsc_trace(rsc, "Allowing %s: %s %d", constraint->id, chosen->details->uname, chosen->weight); allocated_rhs = g_list_prepend(allocated_rhs, chosen); } } diff --git a/pengine/utils.h b/pengine/utils.h index fc503be..79fd33d 100644 --- a/pengine/utils.h +++ b/pengine/utils.h @@ -52,6 +52,8 @@ extern void log_action(unsigned int log_level, const char *pre_text, extern gboolean can_run_any(GHashTable * nodes); extern resource_t *find_compatible_child(resource_t * local_child, resource_t * rsc, enum rsc_role_e filter, gboolean current); +gboolean is_child_compatible(resource_t *child_rsc, node_t * local_node, enum rsc_role_e filter, gboolean current); +bool assign_node(resource_t * rsc, node_t * node, gboolean force); enum filter_colocation_res { -- 1.8.3.1 From 2cc26c662dbbee5a7288c83c71fe172a1d3d4ee1 Mon Sep 17 00:00:00 2001 From: Andrew Beekhof Date: Fri, 2 Jun 2017 12:52:24 +1000 Subject: [PATCH 3/8] Refactor: PE: Simplify the bundle and clone colocation_lh functions --- pengine/clone.c | 15 +-------------- pengine/container.c | 6 +++++- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/pengine/clone.c b/pengine/clone.c index f8c7503..51338d6 100644 --- a/pengine/clone.c +++ b/pengine/clone.c @@ -1047,20 +1047,7 @@ clone_rsc_colocation_lh(resource_t * rsc_lh, resource_t * rsc_rh, rsc_colocation * * Instead we add the colocation constraints to the child and call from there */ - - GListPtr gIter = rsc_lh->children; - - CRM_CHECK(FALSE, crm_err("This functionality is not thought to be used. Please report a bug.")); - CRM_CHECK(rsc_lh, return); - CRM_CHECK(rsc_rh, return); - - for (; gIter != NULL; gIter = gIter->next) { - resource_t *child_rsc = (resource_t *) gIter->data; - - child_rsc->cmds->rsc_colocation_lh(child_rsc, rsc_rh, constraint); - } - - return; + CRM_ASSERT(FALSE); } void diff --git a/pengine/container.c b/pengine/container.c index 67b0052..008b2ec 100644 --- a/pengine/container.c +++ b/pengine/container.c @@ -258,7 +258,11 @@ find_compatible_tuple(resource_t *rsc_lh, resource_t * rsc, enum rsc_role_e filt void container_rsc_colocation_lh(resource_t * rsc, resource_t * rsc_rh, rsc_colocation_t * constraint) { - pe_err("Container %s cannot be colocated with anything", rsc->id); + /* -- Never called -- + * + * Instead we add the colocation constraints to the child and call from there + */ + CRM_ASSERT(FALSE); } void -- 1.8.3.1 From 77a80de9cda4b48ef16f29b42abe5c6d6e7fe179 Mon Sep 17 00:00:00 2001 From: Andrew Beekhof Date: Thu, 8 Jun 2017 21:04:09 +1000 Subject: [PATCH 4/8] Fix: PE: Bundle location constraints should only apply to the IP and docker resources --- pengine/container.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pengine/container.c b/pengine/container.c index 008b2ec..58f6fca 100644 --- a/pengine/container.c +++ b/pengine/container.c @@ -342,16 +342,22 @@ container_update_actions(action_t * first, action_t * then, node_t * node, enum void container_rsc_location(resource_t * rsc, rsc_to_node_t * constraint) { - GListPtr gIter = rsc->children; + container_variant_data_t *container_data = NULL; + get_container_variant_data(container_data, rsc); pe_rsc_trace(rsc, "Processing location constraint %s for %s", constraint->id, rsc->id); native_rsc_location(rsc, constraint); - for (; gIter != NULL; gIter = gIter->next) { - resource_t *child_rsc = (resource_t *) gIter->data; + for (GListPtr gIter = container_data->tuples; gIter != NULL; gIter = gIter->next) { + container_grouping_t *tuple = (container_grouping_t *)gIter->data; - child_rsc->cmds->rsc_location(child_rsc, constraint); + if (tuple->docker) { + tuple->docker->cmds->rsc_location(tuple->docker, constraint); + } + if(tuple->ip) { + tuple->ip->cmds->rsc_location(tuple->ip, constraint); + } } } -- 1.8.3.1 From 499d2d4d300b45984e2165e75a9bbdb33f8e752d Mon Sep 17 00:00:00 2001 From: Andrew Beekhof Date: Wed, 14 Jun 2017 12:24:12 +1000 Subject: [PATCH 5/8] Fix: PE: Do not re-add a node's default score for each location constraint --- pengine/test10/base-score.scores | 2 +- pengine/utils.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pengine/test10/base-score.scores b/pengine/test10/base-score.scores index 1c47a9b..7955241 100644 --- a/pengine/test10/base-score.scores +++ b/pengine/test10/base-score.scores @@ -1,5 +1,5 @@ Allocation scores: -native_color: Dummy allocation score on puma1: 210 +native_color: Dummy allocation score on puma1: 110 native_color: Dummy allocation score on puma2: 0 native_color: Dummy allocation score on puma3: -2000 native_color: Dummy allocation score on puma4: 0 diff --git a/pengine/utils.c b/pengine/utils.c index 755f1c8..a587e58 100644 --- a/pengine/utils.c +++ b/pengine/utils.c @@ -96,7 +96,7 @@ rsc2node_new(const char *id, resource_t * rsc, if (foo_node != NULL) { node_t *copy = node_copy(foo_node); - copy->weight = merge_weights(node_weight, foo_node->weight); + copy->weight = node_weight; new_con->node_list_rh = g_list_prepend(NULL, copy); } -- 1.8.3.1 From 61c6a8acd9c093af333cabe5381c9b7500880c5f Mon Sep 17 00:00:00 2001 From: Andrew Beekhof Date: Thu, 15 Jun 2017 10:40:39 +1000 Subject: [PATCH 6/8] Fix: PE: Correctly implement pe_order_implies_first_printed --- pengine/graph.c | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/pengine/graph.c b/pengine/graph.c index e3bde09..2a0b4b9 100644 --- a/pengine/graph.c +++ b/pengine/graph.c @@ -174,7 +174,8 @@ rsc_expand_action(action_t * action) } static enum pe_graph_flags -graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_action_flags flags, +graph_update_action(action_t * first, action_t * then, node_t * node, + enum pe_action_flags first_flags, enum pe_action_flags then_flags, enum pe_ordering type) { enum pe_graph_flags changed = pe_graph_none; @@ -186,10 +187,10 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; if (then->rsc) { changed |= - then->rsc->cmds->update_actions(first, then, node, flags & pe_action_optional, + then->rsc->cmds->update_actions(first, then, node, first_flags & pe_action_optional, pe_action_optional, pe_order_implies_then); - } else if (is_set(flags, pe_action_optional) == FALSE) { + } else if (is_set(first_flags, pe_action_optional) == FALSE) { if (update_action_flags(then, pe_action_optional | pe_action_clear, __FUNCTION__, __LINE__)) { changed |= pe_graph_updated_then; } @@ -206,7 +207,7 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; changed |= - then->rsc->cmds->update_actions(first, then, node, flags, restart, pe_order_restart); + then->rsc->cmds->update_actions(first, then, node, first_flags, restart, pe_order_restart); if (changed) { pe_rsc_trace(then->rsc, "restart: %s then %s: changed", first->uuid, then->uuid); } else { @@ -218,10 +219,10 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; if (first->rsc) { changed |= - first->rsc->cmds->update_actions(first, then, node, flags, + first->rsc->cmds->update_actions(first, then, node, first_flags, pe_action_optional, pe_order_implies_first); - } else if (is_set(flags, pe_action_optional) == FALSE) { + } else if (is_set(first_flags, pe_action_optional) == FALSE) { pe_rsc_trace(first->rsc, "first unrunnable: %s then %s", first->uuid, then->uuid); if (update_action_flags(first, pe_action_runnable | pe_action_clear, __FUNCTION__, __LINE__)) { changed |= pe_graph_updated_first; @@ -239,7 +240,7 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; if (then->rsc) { changed |= - then->rsc->cmds->update_actions(first, then, node, flags & pe_action_optional, + then->rsc->cmds->update_actions(first, then, node, first_flags & pe_action_optional, pe_action_optional, pe_order_implies_first_master); } @@ -257,10 +258,10 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; if (then->rsc) { changed |= - then->rsc->cmds->update_actions(first, then, node, flags, + then->rsc->cmds->update_actions(first, then, node, first_flags, pe_action_runnable, pe_order_one_or_more); - } else if (is_set(flags, pe_action_runnable)) { + } else if (is_set(first_flags, pe_action_runnable)) { /* alright. a "first" action is considered runnable, incremente * the 'runnable_before' counter */ then->runnable_before++; @@ -285,13 +286,13 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; if (then->rsc) { changed |= - then->rsc->cmds->update_actions(first, then, node, flags, + then->rsc->cmds->update_actions(first, then, node, first_flags, pe_action_runnable, pe_order_runnable_left); - } else if (is_set(flags, pe_action_runnable) == FALSE) { + } else if (is_set(first_flags, pe_action_runnable) == FALSE) { pe_rsc_trace(then->rsc, "then unrunnable: %s then %s", first->uuid, then->uuid); if (update_action_flags(then, pe_action_runnable | pe_action_clear, __FUNCTION__, __LINE__)) { - changed |= pe_graph_updated_then; + changed |= pe_graph_updated_then; } } if (changed) { @@ -305,7 +306,7 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; if (then->rsc) { changed |= - then->rsc->cmds->update_actions(first, then, node, flags, + then->rsc->cmds->update_actions(first, then, node, first_flags, pe_action_optional, pe_order_implies_first_migratable); } if (changed) { @@ -319,7 +320,7 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; if (then->rsc) { changed |= - then->rsc->cmds->update_actions(first, then, node, flags, + then->rsc->cmds->update_actions(first, then, node, first_flags, pe_action_optional, pe_order_pseudo_left); } if (changed) { @@ -333,7 +334,7 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; if (then->rsc) { changed |= - then->rsc->cmds->update_actions(first, then, node, flags, + then->rsc->cmds->update_actions(first, then, node, first_flags, pe_action_runnable, pe_order_optional); } if (changed) { @@ -347,7 +348,7 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac processed = TRUE; if (then->rsc) { changed |= - then->rsc->cmds->update_actions(first, then, node, flags, + then->rsc->cmds->update_actions(first, then, node, first_flags, pe_action_runnable, pe_order_asymmetrical); } @@ -360,13 +361,13 @@ graph_update_action(action_t * first, action_t * then, node_t * node, enum pe_ac } if ((first->flags & pe_action_runnable) && (type & pe_order_implies_then_printed) - && (flags & pe_action_optional) == 0) { + && (first_flags & pe_action_optional) == 0) { processed = TRUE; crm_trace("%s implies %s printed", first->uuid, then->uuid); update_action_flags(then, pe_action_print_always, __FUNCTION__, __LINE__); /* don't care about changed */ } - if ((type & pe_order_implies_first_printed) && (flags & pe_action_optional) == 0) { + if (is_set(type, pe_order_implies_first_printed) && is_set(then_flags, pe_action_optional) == FALSE) { processed = TRUE; crm_trace("%s implies %s printed", then->uuid, first->uuid); update_action_flags(first, pe_action_print_always, __FUNCTION__, __LINE__); /* don't care about changed */ @@ -510,7 +511,6 @@ update_action(action_t * then) crm_trace("Then: Found node %s for %s", then_node->details->uname, then->uuid); } } - /* Disable constraint if it only applies when on same node, but isn't */ if (is_set(other->type, pe_order_same_node) && first_node && then_node && (first_node->details != then_node->details)) { @@ -524,8 +524,7 @@ update_action(action_t * then) clear_bit(changed, pe_graph_updated_first); - if (first->rsc != then->rsc - && first->rsc != NULL && then->rsc != NULL && first->rsc != then->rsc->parent) { + if (first->rsc != then->rsc && is_parent(then->rsc, first->rsc) == FALSE) { first = rsc_expand_action(first); } if (first != other->action) { @@ -584,7 +583,8 @@ update_action(action_t * then) node = first->node; } clear_bit(first_flags, pe_action_pseudo); - changed |= graph_update_action(first, then, node, first_flags, otype); + + changed |= graph_update_action(first, then, node, first_flags, then_flags, otype); /* 'first' was for a complex resource (clone, group, etc), * create a new dependency if necessary -- 1.8.3.1 From 44f2c05dcdcf6dd5a1bff5e441ddd845a3063b6b Mon Sep 17 00:00:00 2001 From: Andrew Beekhof Date: Thu, 15 Jun 2017 10:43:47 +1000 Subject: [PATCH 7/8] Fix: PE: Clones within bundles may also have notifications enabled --- pengine/graph.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/pengine/graph.c b/pengine/graph.c index 2a0b4b9..0d80e79 100644 --- a/pengine/graph.c +++ b/pengine/graph.c @@ -146,25 +146,32 @@ convert_non_atomic_uuid(char *old_uuid, resource_t * rsc, gboolean allow_notify, static action_t * rsc_expand_action(action_t * action) { + gboolean notify = FALSE; action_t *result = action; + resource_t *rsc = action->rsc; - if (action->rsc && action->rsc->variant >= pe_group) { + if (rsc == NULL) { + return action; + } + + if(pe_rsc_is_clone(rsc) || rsc->parent == NULL) { + /* Only outermost resources have notification actions. + * The exception is those in bundles. + */ + notify = is_set(rsc->flags, pe_rsc_notify); + } + + if (rsc->variant >= pe_group) { /* Expand 'start' -> 'started' */ char *uuid = NULL; - gboolean notify = FALSE; - - if (action->rsc->parent == NULL) { - /* Only outermost resources have notification actions */ - notify = is_set(action->rsc->flags, pe_rsc_notify); - } - uuid = convert_non_atomic_uuid(action->uuid, action->rsc, notify, FALSE); + uuid = convert_non_atomic_uuid(action->uuid, rsc, notify, FALSE); if (uuid) { - pe_rsc_trace(action->rsc, "Converting %s to %s %d", action->uuid, uuid, - is_set(action->rsc->flags, pe_rsc_notify)); - result = find_first_action(action->rsc->actions, uuid, NULL, NULL); + pe_rsc_trace(rsc, "Converting %s to %s %d", action->uuid, uuid, + is_set(rsc->flags, pe_rsc_notify)); + result = find_first_action(rsc->actions, uuid, NULL, NULL); if (result == NULL) { - crm_err("Couldn't expand %s", action->uuid); + crm_err("Couldn't expand %s to %s in %s", action->uuid, uuid, rsc->id); result = action; } free(uuid); -- 1.8.3.1 From f822f38b15e99005f4ace270b6450443d53f6043 Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Mon, 19 Jun 2017 10:47:24 -0500 Subject: [PATCH 8/8] Low: pengine: make checks a little safer --- pengine/graph.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pengine/graph.c b/pengine/graph.c index 0d80e79..c93745b 100644 --- a/pengine/graph.c +++ b/pengine/graph.c @@ -154,7 +154,8 @@ rsc_expand_action(action_t * action) return action; } - if(pe_rsc_is_clone(rsc) || rsc->parent == NULL) { + if ((rsc->parent == NULL) + || (pe_rsc_is_clone(rsc) && (rsc->parent->variant == pe_container))) { /* Only outermost resources have notification actions. * The exception is those in bundles. */ @@ -531,7 +532,9 @@ update_action(action_t * then) clear_bit(changed, pe_graph_updated_first); - if (first->rsc != then->rsc && is_parent(then->rsc, first->rsc) == FALSE) { + if (first->rsc && then->rsc && (first->rsc != then->rsc) + && (is_parent(then->rsc, first->rsc) == FALSE)) { + first = rsc_expand_action(first); } if (first != other->action) { -- 1.8.3.1