Blob Blame History Raw
From f9ad81dfee9780a783ea769e22af89045ef673c1 Mon Sep 17 00:00:00 2001
From: Andrew Beekhof <andrew@beekhof.net>
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 <andrew@beekhof.net>
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 <andrew@beekhof.net>
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 <andrew@beekhof.net>
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 <andrew@beekhof.net>
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 <andrew@beekhof.net>
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 <andrew@beekhof.net>
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 <kgaillot@redhat.com>
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