Adding upstream version 2.1.6.upstream/2.1.6

Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>

1 files changed, 521 insertions, 0 deletions

diff --git a/doc/sphinx/Pacemaker_Development/helpers.rst b/doc/sphinx/Pacemaker_Development/helpers.rst
new file mode 100644
index 0000000..3fcb48d
--- /dev/null
+++ b/doc/sphinx/Pacemaker_Development/helpers.rst
@@ -0,0 +1,521 @@
+C Development Helpers
+---------------------
+
+.. index::
+   single: unit testing
+
+Refactoring
+###########
+
+Pacemaker uses an optional tool called `coccinelle <https://coccinelle.gitlabpages.inria.fr/website/>`_
+to do automatic refactoring.  coccinelle is a very complicated tool that can be
+difficult to understand, and the existing documentation makes it pretty tough
+to get started.  Much of the documentation is either aimed at kernel developers
+or takes the form of grammars.
+
+However, it can apply very complex transformations across an entire source tree.
+This is useful for tasks like code refactoring, changing APIs (number or type of
+arguments, etc.), catching functions that should not be called, and changing
+existing patterns.
+
+coccinelle is driven by input scripts called `semantic patches <https://coccinelle.gitlabpages.inria.fr/website/docs/index.html>`_
+written in its own language.  These scripts bear a passing resemblance to source
+code patches and tell coccinelle how to match and modify a piece of source
+code.  They are stored in ``devel/coccinelle`` and each script either contains
+a single source transformation or several related transformations.  In general,
+we try to keep these as simple as possible.
+
+In Pacemaker development, we use a couple targets in ``devel/Makefile.am`` to
+control coccinelle.  The ``cocci`` target tries to apply each script to every
+Pacemaker source file, printing out any changes it would make to the console.
+The ``cocci-inplace`` target does the same but also makes those changes to the
+source files.  A variety of warnings might also be printed.  If you aren't working
+on a new script, these can usually be ignored.
+
+If you are working on a new coccinelle script, it can be useful (and faster) to
+skip everything else and only run the new script.  The ``COCCI_FILES`` variable
+can be used for this:
+
+.. code-block:: none
+
+   $ make -C devel COCCI_FILES=coccinelle/new-file.cocci cocci
+
+This variable is also used for preventing some coccinelle scripts in the Pacemaker
+source tree from running.  Some scripts are disabled because they are not currently
+fully working or because they are there as templates.  When adding a new script,
+remember to add it to this variable if it should always be run.
+
+One complication when writing coccinelle scripts is that certain Pacemaker source
+files may not use private functions (those whose name starts with ``pcmk__``).
+Handling this requires work in both the Makefile and in the coccinelle scripts.
+
+The Makefile deals with this by maintaining two lists of source files: those that
+may use private functions and those that may not.  For those that may, a special
+argument (``-D internal``) is added to the coccinelle command line.  This creates
+a virtual dependency named ``internal``.
+
+In the coccinelle scripts, those transformations that modify source code to use
+a private function also have a dependency on ``internal``.  If that dependency
+was given on the command line, the transformation will be run.  Otherwise, it will
+be skipped.
+
+This means that not all instances of an older style of code will be changed after
+running a given transformation.  Some developer intervention is still necessary
+to know whether a source code block should have been changed or not.
+
+Probably the easiest way to learn how to use coccinelle is by following other
+people's scripts.  In addition to the ones in the Pacemaker source directory,
+there's several others on the `coccinelle website <https://coccinelle.gitlabpages.inria.fr/website/rules/>`_.
+
+Sanitizers
+##########
+
+gcc supports a variety of run-time checks called sanitizers.  These can be used to
+catch programming errors with memory, race conditions, various undefined behavior
+conditions, and more.  Because these are run-time checks, they should only be used
+during development and not in compiled packages or production code.
+
+Certain sanitizers cannot be combined with others because their run-time checks
+cause interfere.  Instead of trying to figure out which combinations work, it is
+simplest to just enable one at a time.
+
+Each supported sanitizer requires an installed libray.  In addition to just
+enabling the sanitizer, their use can be configured with environment variables.
+For example:
+
+.. code-block:: none
+
+   $ ASAN_OPTIONS=verbosity=1:replace_str=true crm_mon -1R
+
+Pacemaker supports the following subset of gcc's sanitizers:
+
++--------------------+-------------------------+----------+----------------------+
+| Sanitizer          | Configure Option        | Library  | Environment Variable |
++====================+=========================+==========+======================+
+| Address            | --with-sanitizers=asan  | libasan  | ASAN_OPTIONS         |
++--------------------+-------------------------+----------+----------------------+
+| Threads            | --with-sanitizers=tsan  | libtsan  | TSAN_OPTIONS         |
++--------------------+-------------------------+----------+----------------------+
+| Undefined behavior | --with-sanitizers=ubsan | libubsan | UBSAN_OPTIONS        |
++--------------------+-------------------------+----------+----------------------+
+
+The undefined behavior sanitizer further supports suboptions that need to be
+given as CFLAGS when configuring pacemaker:
+
+.. code-block:: none
+
+   $ CFLAGS=-fsanitize=integer-divide-by-zero ./configure --with-sanitizers=ubsan
+
+For more information, see the `gcc documentation <https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html>`_
+which also provides links to more information on each sanitizer.
+
+Unit Testing
+############
+
+Where possible, changes to the C side of Pacemaker should be accompanied by unit
+tests.  Much of Pacemaker cannot effectively be unit tested (and there are other
+testing systems used for those parts), but the ``lib`` subdirectory is pretty easy
+to write tests for.
+
+Pacemaker uses the `cmocka unit testing framework <https://cmocka.org/>`_ which looks
+a lot like other unit testing frameworks for C and should be fairly familiar.  In
+addition to regular unit tests, cmocka also gives us the ability to use
+`mock functions <https://en.wikipedia.org/wiki/Mock_object>`_ for unit testing
+functions that would otherwise be difficult to test.
+
+Organization
+____________
+
+Pay close attention to the organization and naming of test cases to ensure the
+unit tests continue to work as they should.
+
+Tests are spread throughout the source tree, alongside the source code they test.
+For instance, all the tests for the source code in ``lib/common/`` are in the
+``lib/common/tests`` directory.  If there is no ``tests`` subdirectory, there are no
+tests for that library yet.
+
+Under that directory, there is a ``Makefile.am`` and additional subdirectories.  Each
+subdirectory contains the tests for a single library source file.  For instance,
+all the tests for ``lib/common/strings.c`` are in the ``lib/common/tests/strings``
+directory.  Note that the test subdirectory does not have a ``.c`` suffix.  If there
+is no test subdirectory, there are no tests for that file yet.
+
+Finally, under that directory, there is a ``Makefile.am`` and then various source
+files.  Each of these source files tests the single function that it is named
+after.  For instance, ``lib/common/tests/strings/pcmk__btoa_test.c`` tests the
+``pcmk__btoa()`` function in ``lib/common/strings.c``.  If there is no test
+source file, there are no tests for that function yet.
+
+The ``_test`` suffix on the test source file is important.  All tests have this
+suffix, which means all the compiled test cases will also end with this suffix.
+That lets us ignore all the compiled tests with a single line in ``.gitignore``:
+
+.. code-block:: none
+
+   /lib/*/tests/*/*_test
+
+Adding a test
+_____________
+
+Testing a new function in an already testable source file
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Follow these steps if you want to test a function in a source file where there
+are already other tested functions.  For the purposes of this example, we will
+add a test for the ``pcmk__scan_port()`` function in ``lib/common/strings.c``.  As
+you can see, there are already tests for other functions in this same file in
+the ``lib/common/tests/strings`` directory.
+
+* cd into ``lib/common/tests/strings``
+* Add the new file to the the ``check_PROGRAMS`` variable in ``Makefile.am``,
+  making it something like this:
+
+  .. code-block:: none
+
+      check_PROGRAMS = \
+             pcmk__add_word_test             \
+             pcmk__btoa_test                 \
+             pcmk__scan_port_test
+
+* Create a new ``pcmk__scan_port_test.c`` file, copying the copyright and include
+  boilerplate from another file in the same directory.
+* Continue with the steps in `Writing the test`_.
+
+Testing a function in a source file without tests
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Follow these steps if you want to test a function in a source file where there
+are not already other tested functions, but there are tests for other files in
+the same library.  For the purposes of this example, we will add a test for the
+``pcmk_acl_required()`` function in ``lib/common/acls.c``.  At the time of this
+documentation being written, no tests existed for that source file, so there
+is no ``lib/common/tests/acls`` directory.
+
+* Add to ``AC_CONFIG_FILES`` in the top-level ``configure.ac`` file so the build
+  process knows to use directory we're about to create.  That variable would
+  now look something like:
+
+  .. code-block:: none
+
+     dnl Other files we output
+     AC_CONFIG_FILES(Makefile                                            \
+                     ...
+                     lib/common/tests/Makefile                           \
+                     lib/common/tests/acls/Makefile                      \
+                     lib/common/tests/agents/Makefile                    \
+                     ...
+     )
+
+* cd into ``lib/common/tests``
+* Add to the ``SUBDIRS`` variable in ``Makefile.am``, making it something like:
+
+  .. code-block:: none
+
+     SUBDIRS = agents acls cmdline flags operations strings utils xpath results
+
+* Create a new ``acls`` directory, copying the ``Makefile.am`` from some other
+  directory.  At this time, each ``Makefile.am`` is largely boilerplate with
+  very little that needs to change from directory to directory.
+* cd into ``acls``
+* Get rid of any existing values for ``check_PROGRAMS`` and set it to
+  ``pcmk_acl_required_test`` like so:
+
+  .. code-block:: none
+
+     check_PROGRAMS = pcmk_acl_required_test
+
+* Double check that ``$(top_srcdir)/mk/tap.mk`` and ``$(top_srcdir)/mk/unittest.mk``
+  are included in the ``Makefile.am``.  These files contain all the flags necessary
+  for most unit tests.  If necessary, individual settings can be overridden like so:
+
+  .. code-block:: none
+
+     AM_CPPFLAGS += -I$(top_srcdir)
+     LDADD += $(top_builddir)/lib/pengine/libpe_status_test.la
+
+* Follow the steps in `Testing a new function in an already testable source file`_
+  to create the new ``pcmk_acl_required_test.c`` file.
+
+Testing a function in a library without tests
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Adding a test case for a function in a library that doesn't have any test cases
+to begin with is only slightly more complicated.  In general, the steps are the
+same as for the previous section, except with an additional layer of directory
+creation.
+
+For the purposes of this example, we will add a test case for the
+``lrmd_send_resource_alert()`` function in ``lib/lrmd/lrmd_alerts.c``.  Note that this
+may not be a very good function or even library to write actual unit tests for.
+
+* Add to ``AC_CONFIG_FILES`` in the top-level ``configure.ac`` file so the build
+  process knows to use directory we're about to create.  That variable would
+  now look something like:
+
+  .. code-block:: none
+
+     dnl Other files we output
+     AC_CONFIG_FILES(Makefile                                            \
+                     ...
+                     lib/lrmd/Makefile                                   \
+                     lib/lrmd/tests/Makefile                             \
+                     lib/services/Makefile                               \
+                     ...
+     )
+
+* cd into ``lib/lrmd``
+* Create a ``SUBDIRS`` variable in ``Makefile.am`` if it doesn't already exist.
+  Most libraries should not have this variable already.
+
+  .. code-block:: none
+
+     SUBDIRS = tests
+
+* Create a new ``tests`` directory and add a ``Makefile.am`` with the following
+  contents:
+
+  .. code-block:: none
+
+     SUBDIRS = lrmd_alerts
+
+* Follow the steps in `Testing a function in a source file without tests`_ to create
+  the rest of the new directory structure.
+
+* Follow the steps in `Testing a new function in an already testable source file`_
+  to create the new ``lrmd_send_resource_alert_test.c`` file.
+
+Adding to an existing test case
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If all you need to do is add additional test cases to an existing file, none of
+the above work is necessary.  All you need to do is find the test source file
+with the name matching your function and add to it and then follow the
+instructions in `Writing the test`_.
+
+Writing the test
+________________
+
+A test case file contains a fair amount of boilerplate.  For this reason, it's
+usually easiest to just copy an existing file and adapt it to your needs.  However,
+here's the basic structure:
+
+.. code-block:: c
+
+   /*
+    * Copyright 2021 the Pacemaker project contributors
+    *
+    * The version control history for this file may have further details.
+    *
+    * This source code is licensed under the GNU Lesser General Public License
+    * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
+    */
+
+   #include <crm_internal.h>
+
+   #include <crm/common/unittest_internal.h>
+
+   /* Put your test-specific includes here */
+
+   /* Put your test functions here */
+
+   PCMK__UNIT_TEST(NULL, NULL,
+                   /* Register your test functions here */)
+
+Each test-specific function should test one aspect of the library function,
+though it can include many assertions if there are many ways of testing that
+one aspect.  For instance, there might be multiple ways of testing regular
+expression matching:
+
+.. code-block:: c
+
+   static void
+   regex(void **state) {
+       const char *s1 = "abcd";
+       const char *s2 = "ABCD";
+
+       assert_true(pcmk__strcmp(NULL, "a..d", pcmk__str_regex) < 0);
+       assert_true(pcmk__strcmp(s1, NULL, pcmk__str_regex) > 0);
+       assert_int_equal(pcmk__strcmp(s1, "a..d", pcmk__str_regex), 0);
+   }
+
+Each test-specific function must also be registered or it will not be called.
+This is done with ``cmocka_unit_test()`` in the ``PCMK__UNIT_TEST`` macro:
+
+.. code-block:: c
+
+   PCMK__UNIT_TEST(NULL, NULL,
+                   cmocka_unit_test(regex))
+
+Most unit tests do not require a setup and teardown function to be executed
+around the entire group of tests.  On occassion, this may be necessary.  Simply
+pass those functions in as the first two parameters to ``PCMK__UNIT_TEST``
+instead of using NULL.
+
+Assertions
+__________
+
+In addition to the `assertions provided by <https://api.cmocka.org/group__cmocka__asserts.html>`_,
+``unittest_internal.h`` also provides ``pcmk__assert_asserts``.  This macro takes an
+expression and verifies that the expression aborts due to a failed call to
+``CRM_ASSERT`` or some other similar function.  It can be used like so:
+
+.. code-block:: c
+
+   static void
+   null_input_variables(void **state)
+   {
+       long long start, end;
+
+       pcmk__assert_asserts(pcmk__parse_ll_range("1234", NULL, &end));
+       pcmk__assert_asserts(pcmk__parse_ll_range("1234", &start, NULL));
+   }
+
+Here, ``pcmk__parse_ll_range`` expects non-NULL for its second and third
+arguments.  If one of those arguments is NULL, ``CRM_ASSERT`` will fail and
+the program will abort.  ``pcmk__assert_asserts`` checks that the code would
+abort and the test passes.  If the code does not abort, the test fails.
+
+
+Running
+_______
+
+If you had to create any new files or directories, you will first need to run
+``./configure`` from the top level of the source directory.  This will regenerate
+the Makefiles throughout the tree.  If you skip this step, your changes will be
+skipped and you'll be left wondering why the output doesn't match what you
+expected.
+
+To run the tests, simply run ``make check`` after previously building the source
+with ``make``.  The test cases in each directory will be built and then run.
+This should not take long.  If all the tests succeed, you will be back at the
+prompt.  Scrolling back through the history, you should see lines like the
+following:
+
+.. code-block:: none
+
+    PASS: pcmk__strcmp_test 1 - same_pointer
+    PASS: pcmk__strcmp_test 2 - one_is_null
+    PASS: pcmk__strcmp_test 3 - case_matters
+    PASS: pcmk__strcmp_test 4 - case_insensitive
+    PASS: pcmk__strcmp_test 5 - regex
+    ============================================================================
+    Testsuite summary for pacemaker 2.1.0
+    ============================================================================
+    # TOTAL: 33
+    # PASS:  33
+    # SKIP:  0
+    # XFAIL: 0
+    # FAIL:  0
+    # XPASS: 0
+    # ERROR: 0
+    ============================================================================
+    make[7]: Leaving directory '/home/clumens/src/pacemaker/lib/common/tests/strings'
+
+The testing process will quit on the first failed test, and you will see lines
+like these:
+
+.. code-block:: none
+
+   PASS: pcmk__scan_double_test 3 - trailing_chars
+   FAIL: pcmk__scan_double_test 4 - typical_case
+   PASS: pcmk__scan_double_test 5 - double_overflow
+   PASS: pcmk__scan_double_test 6 - double_underflow
+   ERROR: pcmk__scan_double_test - exited with status 1
+   PASS: pcmk__starts_with_test 1 - bad_input
+   ============================================================================
+   Testsuite summary for pacemaker 2.1.0
+   ============================================================================
+   # TOTAL: 56
+   # PASS:  54
+   # SKIP:  0
+   # XFAIL: 0
+   # FAIL:  1
+   # XPASS: 0
+   # ERROR: 1
+   ============================================================================
+   See lib/common/tests/strings/test-suite.log
+   Please report to users@clusterlabs.org
+   ============================================================================
+   make[7]: *** [Makefile:1218: test-suite.log] Error 1
+   make[7]: Leaving directory '/home/clumens/src/pacemaker/lib/common/tests/strings'
+
+The failure is in ``lib/common/tests/strings/test-suite.log``:
+
+.. code-block:: none
+
+   ERROR: pcmk__scan_double_test
+   =============================
+
+   1..6
+   ok 1 - empty_input_string
+   PASS: pcmk__scan_double_test 1 - empty_input_string
+   ok 2 - bad_input_string
+   PASS: pcmk__scan_double_test 2 - bad_input_string
+   ok 3 - trailing_chars
+   PASS: pcmk__scan_double_test 3 - trailing_chars
+   not ok 4 - typical_case
+   FAIL: pcmk__scan_double_test 4 - typical_case
+   # 0.000000 != 3.000000
+   # pcmk__scan_double_test.c:80: error: Failure!
+   ok 5 - double_overflow
+   PASS: pcmk__scan_double_test 5 - double_overflow
+   ok 6 - double_underflow
+   PASS: pcmk__scan_double_test 6 - double_underflow
+   # not ok - tests
+   ERROR: pcmk__scan_double_test - exited with status 1
+
+At this point, you need to determine whether your test case is incorrect or
+whether the code being tested is incorrect.  Fix whichever is wrong and continue.
+
+
+Code Coverage
+#############
+
+Figuring out what needs unit tests written is the purpose of a code coverage tool.
+The Pacemaker build process uses ``lcov`` and special make targets to generate
+an HTML coverage report that can be inspected with any web browser.
+
+To start, you'll need to install the ``lcov`` package which is included in most
+distributions.  Next, reconfigure and rebuild the source tree:
+
+.. code-block:: none
+
+   $ ./configure --with-coverage
+   $ make
+
+Then simply run ``make coverage``.  This will do the same thing as ``make check``,
+but will generate a bunch of intermediate files as part of the compiler's output.
+Essentially, the coverage tools run all the unit tests and make a note if a given
+line if code is executed as a part of some test program.  This will include not
+just things run as part of the tests but anything in the setup and teardown
+functions as well.
+
+Afterwards, the HTML report will be in ``coverage/index.html``.  You can drill down
+into individual source files to see exactly which lines are covered and which are
+not, which makes it easy to target new unit tests.  Note that sometimes, it is
+impossible to achieve 100% coverage for a source file.  For instance, how do you
+test a function with a return type of void that simply returns on some condition?
+
+Note that Pacemaker's overall code coverage numbers are very low at the moment.
+One reason for this is the large amount of code in the ``daemons`` directory that
+will be very difficult to write unit tests for.  For now, it is best to focus
+efforts on increasing the coverage on individual libraries.
+
+Additionally, there is a ``coverage-cts`` target that does the same thing but
+instead of testing ``make check``, it tests ``cts/cts-cli``.  The idea behind this
+target is to see what parts of our command line tools are covered by our regression
+tests.  It is probably best to clean and rebuild the source tree when switching
+between these various targets.
+
+
+Debugging
+#########
+
+gdb
+___
+
+If you use ``gdb`` for debugging, some helper functions are defined in
+``devel/gdbhelpers``, which can be given to ``gdb`` using the ``-x`` option.
+
+From within the debugger, you can then invoke the ``pcmk`` command that
+will describe the helper functions available.