Fix: libpe_status: Make cluster_status() idempotent

Previously, if cluster_status() were called twice without resetting the
scheduler between calls, most of the unpacked data would be duplicated
in the scheduler object's data structures. For example, the nodes in
scheduler->input would be stored twice in the scheduler's node list.

This changes the behavior of a public API function, but the previous
behavior resulted in incorrect status, so that seems acceptable.

This is not a perfect fix: scheduler->flags is publicly accessible, so a
caller could clear the pcmk__sched_have_status flag without resetting
the scheduler. That would not be wise to do, however, so we can ignore
that possibility for now and plan to deprecate cluster_status() later.

An alternative approach would be to explicitly free each scheduler data
structure before we unpack into it. Then cluster_status() could be
called multiple times with different inputs, getting the new status each
time. However, there is no internal use case for that, and it seems
prone to unforeseen corner cases.

Signed-off-by: Reid Wahl <[email protected]>

lib/pengine/status.c

@@ -1,5 +1,5 @@
 /*
- * Copyright 2004-2024 the Pacemaker project contributors
+ * Copyright 2004-2025 the Pacemaker project contributors
  *
  * The version control history for this file may have further details.
  *
@@ -59,13 +59,25 @@ check_for_deprecated_rules(pcmk_scheduler_t *scheduler)
 gboolean
 cluster_status(pcmk_scheduler_t * scheduler)
 {
+    // @TODO Deprecate, replacing with a safer public alternative if necessary
     const char *new_version = NULL;
     xmlNode *section = NULL;
 
     if ((scheduler == NULL) || (scheduler->input == NULL)) {
         return FALSE;
     }
 
+    if (pcmk_is_set(scheduler->flags, pcmk__sched_have_status)) {
+        /* cluster_status() has already been called since the last time the
+         * scheduler was reset. Unpacking the input CIB again would cause
+         * duplication within the scheduler object's data structures.
+         *
+         * The correct return code here is not obvious. Nothing internal checks
+         * the code, however.
+         */
+        return TRUE;
+    }
+
     new_version = crm_element_value(scheduler->input, PCMK_XA_CRM_FEATURE_SET);
 
     if (pcmk__check_feature_set(new_version) != pcmk_rc_ok) {
@@ -76,9 +88,7 @@ cluster_status(pcmk_scheduler_t * scheduler)
 
     crm_trace("Beginning unpack");
 
-    if (scheduler->priv->failed != NULL) {
-        pcmk__xml_free(scheduler->priv->failed);
-    }
+    pcmk__xml_free(scheduler->priv->failed);
     scheduler->priv->failed = pcmk__xe_create(NULL, "failed-ops");
 
     if (scheduler->priv->now == NULL) {