Minor documentation improvements

ChangeLog.md

@@ -1,3 +1,50 @@
+# Pacemaker-3.0.0-rc3 (23 Dec 2024)
+* 33 commits with 9 files changed, 77 insertions(+), 12 deletions(-)
+
+## Features added since Pacemaker-3.0.0-rc2
+
+* Inkscape is no longer a build dependency for Pacemaker documentation
+* The `ocf:pacemaker:controld` agent will now always manage `dlm_controld`
+  (previously, it would try to manage the long-obsolete `gfs_controld` if the
+  resource name started with `gfs`)
+
+## Fixes since Pacemaker-3.0.0-rc2
+
+* **agents:** `ocf:pacemaker:ping` now uses `grep -E` instead of the obsolete
+  `egrep`
+* **tools:** when injecting a fail count with `crm_simulate`, use an `INFINITY`
+  score when the cluster would
+* **tools:** validate `stonith_admin --timeout` value
+
+## Public API changes since Pacemaker-3.0.0-rc2
+
+* **Python:** add `PACEMAKER_CONFIG_DIR` to `BuildOptions`
+
+# Pacemaker-3.0.0-rc2 (11 Dec 2024)
+* 57 commits with 44 files changed, 1487 insertions(+), 633 deletions(-)
+
+## Features added since Pacemaker-3.0.0-rc1
+
+* **Pacemaker Remote and CIB manager:** support X.509 (SSL/TLS) certificates
+  for encrypting Pacemaker Remote connections and remote CIB administration
+
+## Fixes since Pacemaker-3.0.0-rc1
+
+* **libcrmcluster:** restore ability to do rolling upgrades
+  (regression introduced in 3.0.0-rc1)
+* **controller:** avoid memory leak when updating join phase
+  (regression introduced in 3.0.0-rc1)
+* **scheduler:** avoid memory leaks in bundles and when freeing node copies
+  (regression introduced in 3.0.0-rc1)
+* **CIB:** log warnings if CIB upgrade might not preserve behavior exactly
+* **CIB:** ensure ACLs remain valid after CIB upgrades, and warn if upgrade
+  might change ACL effect (regression introduced in 3.0.0-rc1)
+
+## Public API changes since Pacemaker-3.0.0-rc1
+
+* **libcrmcommon:** add `pcmk_common_cleanup()`
+
+
 # Pacemaker-3.0.0-rc1 (14 Nov 2024)
 * 1938 commits with 685 files changed, 26363 insertions(+), 33503 deletions(-)
 

cts/README.md

@@ -112,6 +112,10 @@ CTS includes:
     the log file, then CTS lab will not check for aggregation.
   * CTS lab does not currently detect systemd journal log aggregation.
 
+* Optionally, if the lab nodes use the systemd journal for logs, create
+  /etc/systemd/journald.conf.d/cts-lab.conf on each with
+  `RateLimitIntervalSec=0` or `RateLimitBurst=0`, to avoid issues with log
+  detection.
 
 ### Run
 

cts/cts-exec.in

@@ -304,15 +304,20 @@ class ExecTests(Tests):
 
     def setup_environment(self):
         """Prepare the host before executing any tests."""
+
         if BuildOptions.REMOTE_ENABLED:
+            # @TODO Use systemctl when available, and use the subprocess module
+            # with an argument array instead of os.system()
             os.system("service pacemaker_remote stop")
         self.cleanup_environment()
 
         # @TODO Support the option of using specified existing certificates
         authkey = "%s/authkey" % BuildOptions.PACEMAKER_CONFIG_DIR
         if self.tls and not os.path.isfile(authkey):
             print("Installing %s ..." % authkey)
+            # @TODO Use os.mkdir() instead
             os.system("mkdir -p %s" % BuildOptions.PACEMAKER_CONFIG_DIR)
+            # @TODO Use the subprocess module with an argument array instead
             os.system("dd if=/dev/urandom of=%s bs=4096 count=1" % authkey)
             self._installed_files.append(authkey)
 

daemons/controld/controld_callbacks.c

@@ -106,6 +106,7 @@ node_alive(const pcmk__node_status_t *node)
 
 #define state_text(state) ((state)? (const char *)(state) : "in unknown state")
 
+// @TODO This is insanely long, and some parts should be functionized
 void
 peer_update_callback(enum pcmk__node_update type, pcmk__node_status_t *node,
                      const void *data)
@@ -128,9 +129,8 @@ peer_update_callback(enum pcmk__node_update type, pcmk__node_status_t *node,
         && pcmk_is_set(node->processes, crm_get_cluster_proc())
         && !AM_I_DC
         && !is_remote) {
-        /*
-         * This is a hack until we can send to a nodeid and/or we fix node name lookups
-         * These messages are ignored in crmd_ha_msg_filter()
+        /* relay_message() on the recipient ignores these messages, but
+         * libcrmcluster will have cached the node name by then
          */
         xmlNode *query = pcmk__new_request(pcmk_ipc_controld, CRM_SYSTEM_CRMD,
                                            NULL, CRM_SYSTEM_CRMD, CRM_OP_HELLO,
@@ -140,7 +140,6 @@ peer_update_callback(enum pcmk__node_update type, pcmk__node_status_t *node,
                   "node name",
                   node->cluster_layer_id);
         pcmk__cluster_send_message(node, pcmk_ipc_controld, query);
-
         pcmk__xml_free(query);
     }
 

daemons/controld/controld_execd.c

@@ -572,6 +572,7 @@ build_active_RAs(lrm_state_t * lrm_state, xmlNode * rsc_list)
 xmlNode *
 controld_query_executor_state(void)
 {
+    // @TODO Ensure all callers handle NULL returns
     xmlNode *xml_state = NULL;
     xmlNode *xml_data = NULL;
     xmlNode *rsc_list = NULL;
@@ -597,7 +598,7 @@ controld_query_executor_state(void)
     crm_xml_add(xml_data, PCMK_XA_ID, peer->xml_id);
     rsc_list = pcmk__xe_create(xml_data, PCMK__XE_LRM_RESOURCES);
 
-    /* Build a list of active (not always running) resources */
+    // Build a list of active (not necessarily running) resources
     build_active_RAs(lrm_state, rsc_list);
 
     crm_log_xml_trace(xml_state, "Current executor state");

daemons/controld/controld_execd_state.c

@@ -319,6 +319,9 @@ controld_get_executor_state(const char *node_name, bool create)
     return state;
 }
 
+/* @TODO the lone caller just needs to iterate over the values, so replace this
+ * with a g_hash_table_foreach() wrapper instead
+ */
 GList *
 lrm_state_get_list(void)
 {

daemons/controld/controld_membership.c

@@ -125,6 +125,7 @@ xmlNode *
 create_node_state_update(pcmk__node_status_t *node, int flags,
                          xmlNode *parent, const char *source)
 {
+    // @TODO Ensure all callers handle NULL returns
     const char *value = NULL;
     xmlNode *node_state;
 

daemons/controld/controld_remote_ra.c

@@ -606,6 +606,7 @@ monitor_timeout_cb(gpointer data)
     free_cmd(cmd);
 
     if(lrm_state) {
+        // @TODO Should we move this before reporting the result above?
         lrm_state_disconnect(lrm_state);
     }
     return FALSE;
@@ -950,12 +951,13 @@ handle_remote_ra_exec(gpointer user_data)
         } else if (!strcmp(cmd->action, PCMK_ACTION_STOP)) {
 
             if (pcmk_is_set(ra_data->status, expect_takeover)) {
-                /* briefly wait on stop for the takeover event to occur. If the
-                 * takeover event does not occur during the wait period, that's fine.
-                 * It just means that the remote-node's lrm_status section is going to get
-                 * cleared which will require all the resources running in the remote-node
-                 * to be explicitly re-detected via probe actions.  If the takeover does occur
-                 * successfully, then we can leave the status section intact. */
+                /* Briefly wait on stop for an expected takeover to occur. If
+                 * the takeover does not occur during the wait, that's fine; it
+                 * just means that the remote node's resource history will be
+                 * cleared, which will require probing all resources on the
+                 * remote node. If the takeover does occur successfully, then we
+                 * can leave the status section intact.
+                 */
                 cmd->takeover_timeout_id = pcmk__create_timer((cmd->timeout/2),
                                                               connection_takeover_timeout_cb,
                                                               cmd);

daemons/execd/execd_commands.c

@@ -1516,6 +1516,15 @@ process_lrmd_signon(pcmk__client_t *client, xmlNode *request, int call_id,
             // This is a remote connection from a cluster node's controller
             ipc_proxy_add_provider(client);
 
+            /* @TODO Allowing multiple proxies makes no sense given that clients
+             * have no way to choose between them. Maybe always use the most
+             * recent one and switch any existing IPC connections to use it,
+             * by iterating over ipc_clients here, and if client->id doesn't
+             * match the client's userdata, replace the userdata with the new
+             * ID. After the iteration, call lrmd_remote_client_destroy() on any
+             * of the replaced values in ipc_providers.
+             */
+
             /* If this was a register operation, also ask for new schema files but
              * only if it's supported by the protocol version.
              */

daemons/execd/pacemaker-execd.c

@@ -139,6 +139,10 @@ lrmd_ipc_dispatch(qb_ipcs_connection_t * c, void *data, size_t size)
         return 0;
     }
 
+    /* @TODO functionize some of this to reduce duplication with
+     * lrmd_remote_client_msg()
+     */
+
     if (!client->name) {
         const char *value = crm_element_value(request,
                                               PCMK__XA_LRMD_CLIENTNAME);
@@ -297,6 +301,7 @@ exit_executor(void)
 
     g_hash_table_destroy(rsc_list);
 
+    // @TODO End mainloop instead so all cleanup is done
     crm_exit(CRM_EX_OK);
 }
 

daemons/execd/remoted_proxy.c

@@ -33,7 +33,13 @@ static qb_ipcs_service_t *pacemakerd_ipcs = NULL;
 
 // An IPC provider is a cluster node controller connecting as a client
 static GList *ipc_providers = NULL;
-/* ipc clients == things like cibadmin, crm_resource, connecting locally */
+
+
+/* ipc clients == things like cibadmin, crm_resource, connecting locally
+ *
+ * @TODO This should be unnecessary (pcmk__foreach_ipc_client() should be
+ * sufficient)
+ */
 static GHashTable *ipc_clients = NULL;
 
 /*!

daemons/fenced/fenced_cib.c

@@ -173,6 +173,7 @@ update_stonith_watchdog_timeout_ms(xmlNode *cib)
     xmlNode *stonith_watchdog_xml = NULL;
     const char *value = NULL;
 
+    // @TODO An XPath search can't handle multiple instances or rules
     stonith_watchdog_xml = get_xpath_object(XPATH_WATCHDOG_TIMEOUT, cib,
                                             LOG_NEVER);
     if (stonith_watchdog_xml) {

daemons/fenced/fenced_commands.c

@@ -46,6 +46,7 @@ struct device_search_s {
     /* requested fence action */
     char *action;
     /* timeout to use if a device is queried dynamically for possible targets */
+    // @TODO This name is misleading now, it's the value of stonith-timeout
     int per_device_timeout;
     /* number of registered fencing devices at time of request */
     int replies_needed;
@@ -3323,7 +3324,10 @@ handle_fence_request(pcmk__request_t *request)
             crm_xml_add(request->xml, PCMK__XA_ST_CLIENTID,
                         request->ipc_client->id);
             crm_xml_add(request->xml, PCMK__XA_ST_REMOTE_OP, op->id);
+
+            // @TODO On failure, fail request immediately, or maybe panic
             pcmk__cluster_send_message(node, pcmk_ipc_fenced, request->xml);
+
             pcmk__set_result(&request->result, CRM_EX_OK, PCMK_EXEC_PENDING,
                              NULL);
 

daemons/fenced/pacemaker-fenced.c

@@ -40,7 +40,9 @@
 
 #define SUMMARY "daemon for executing fencing devices in a Pacemaker cluster"
 
+// @TODO This should be guint
 long long stonith_watchdog_timeout_ms = 0;
+
 GList *stonith_watchdog_targets = NULL;
 
 static GMainLoop *mainloop = NULL;

daemons/fenced/pacemaker-fenced.h

@@ -75,6 +75,11 @@ enum st_remap_phase {
 };
 
 typedef struct remote_fencing_op_s {
+    /* @TODO Abstract the overlap with async_command_t (some members have
+     * different names for the same thing), which should allow reducing
+     * duplication in some functions
+     */
+
     /* The unique id associated with this operation */
     char *id;
     /*! The node this operation will fence */

devel/Makefile.am

@@ -250,6 +250,8 @@ coverage-clean: coverage-partial-clean
 #
 # indent cannot cope with all our exceptions and needs heavy manual editing
 #
+# @TODO investigate using clang-format instead
+#
 
 # indent target: Limit indent to these directories
 INDENT_DIRS	?= .

doc/sphinx/Pacemaker_Administration/configuring.rst

@@ -229,9 +229,9 @@ To use TLS certificates, the administrator's machine also needs their
 public/private key pair, signed client certificate, and root CA certificate.
 Those must additionally be specified with the following environment variables:
 
-* :ref:`CIB_ca_file <file>`
-* :ref:`CIB_cert_file <file>`
-* :ref:`CIB_key_file <file>`
+* :ref:`CIB_ca_file <CIB_ca_file>`
+* :ref:`CIB_cert_file <CIB_cert_file>`
+* :ref:`CIB_key_file <CIB_key_file>`
 
 As an example, if **node1** is a cluster node, and the CIB is configured with
 ``remote-tls-port`` set to 1234, the administrator could read the current
@@ -246,6 +246,9 @@ the daemon user's password:
    # export CIB_key_file=/etc/pacemaker/admin.key.pem
    # cibadmin -Q
 
+Optionally, :ref:`CIB_crl_file <CIB_crl_file>` may be set to the location of a
+Certificate Revocation List in PEM format.
+
 .. note::
 
    Pacemaker must have been built with PAM support for remote access to work.

doc/sphinx/Pacemaker_Administration/options.rst

@@ -72,6 +72,60 @@ Pacemaker uses several environment variables set on the client side.
      - The host to connect to. Used with :ref:`CIB_port <CIB_port>` for
        connecting to a remote CIB instance; ignored if
        :ref:`CIB_port <CIB_port>` is not set.
+   * - .. _CIB_ca_file:
+
+       .. index::
+          single: CIB_ca_file
+          single: environment variable; CIB_ca_file
+
+       CIB_ca_file
+     - 
+     - If this, :ref:`CIB_cert_file <CIB_cert_file>`, and
+       :ref:`CIB_key_file <CIB_key_file>` are set, remote CIB administration
+       will be encrypted using X.509 (SSL/TLS) certificates, with this root
+       certificate for the certificate authority. Used with :ref:`CIB_port
+       <CIB_port>` for connecting to a remote CIB instance; ignored if
+       :ref:`CIB_port <CIB_port>` is not set.
+   * - .. _CIB_cert_file:
+
+       .. index::
+          single: CIB_cert_file
+          single: environment variable; CIB_cert_file
+
+       CIB_cert_file
+     - 
+     - If this, :ref:`CIB_ca_file <CIB_ca_file>`, and
+       :ref:`CIB_key_file <CIB_key_file>` are set, remote CIB administration
+       will be encrypted using X.509 (SSL/TLS) certificates, with this
+       certificate for the local host. Used with :ref:`CIB_port <CIB_port>` for
+       connecting to a remote CIB instance; ignored if
+       :ref:`CIB_port <CIB_port>` is not set.
+   * - .. _CIB_key_file:
+
+       .. index::
+          single: CIB_key_file
+          single: environment variable; CIB_key_file
+
+       CIB_key_file
+     - 
+     - If this, :ref:`CIB_ca_file <CIB_ca_file>`, and
+       :ref:`CIB_cert_file <CIB_cert_file>` are set, remote CIB administration
+       will be encrypted using X.509 (SSL/TLS) certificates, with this
+       private key for the local host. Used with :ref:`CIB_port <CIB_port>` for
+       connecting to a remote CIB instance; ignored if
+       :ref:`CIB_port <CIB_port>` is not set.
+   * - .. _CIB_crl_file:
+
+       .. index::
+          single: CIB_crl_file
+          single: environment variable; CIB_crl_file
+
+       CIB_crl_file
+     - 
+     - If this, :ref:`CIB_ca_file <CIB_ca_file>`,
+       :ref:`CIB_cert_file <CIB_cert_file>`, and
+       :ref:`CIB_key_file <CIB_key_file>` are all set, then certificates listed
+       in this PEM-format Certificate Revocation List file will be rejected.
    * - .. _CIB_shadow:
 
        .. index::