fix: findings from the setup_basic lab rounds

CHANGELOG.md

@@ -63,6 +63,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+* LFOps requires community.general 7.0.0 or newer (still below 9.0.0), which `ansible-galaxy collection install linuxfabrik.lfops` pulls in, while a manually maintained collection list has to be raised.
+* **plugin:bitwarden_item**: The lookup syncs the Bitwarden vault once per Ansible run instead of every 60 seconds, which makes runs with many lookups considerably faster, since each sync makes `bw serve` list the whole vault. Before it creates a missing item, it syncs again, so an item created elsewhere during the run is not created a second time.
 * **role:icingaweb2_module_generictts**: Downloads the module from Linuxfabrik, who maintain it since Icinga archived the original repository. The tarballs of v2.1.0 are identical.
 * **role:duplicity**: `/var/lib/aide` is backed up by default, so that the AIDE database can be compared with a copy outside the host. Hosts without AIDE are not affected.
 * **role:monitoring_plugins**: The source install removes plugins that an earlier run deployed and the checked-out version no longer carries.
@@ -86,6 +88,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+* **plugin:bitwarden_item, module:bitwarden_item**: A failed sync of the Bitwarden vault, such as an "HTTP Error 400: Bad Request" or a timeout of `bw serve`, is tried again after 10, 30 and 60 seconds instead of aborting the run right away.
+* **role:aide**: Before it creates the database, the role also waits for running `dnf-automatic` jobs on the Red Hat family and for the update jobs of the system_update role on every platform, not only for the apt jobs on Debian and Ubuntu. An update during the initialisation left files in the database that the first check then reported.
+* **playbook:setup_basic**: With `setup_basic__skip_duplicity` or `setup_basic__skip_glances`, the playbook no longer builds the Python venv of the skipped role, which could abort the run with a pip error on hosts that do not back up with duplicity.
+* **role:kernel_settings**: `sunrpc.*` settings, such as the `sunrpc.tcp_slot_table_entries` the mariadb_server role sets, survive a reboot. Until now the `sunrpc` module was not loaded again after a reboot on hosts without NFS, so TuneD could not apply the setting and the next run of the role failed in `tuned-adm verify`.
+* **role:chrony**: The role aborts if neither `chrony__ntp_pools` nor `chrony__ntp_servers` is set, instead of leaving the host without a time source.
+* **role:repo_baseos**: The Rocky Linux `security` repository works on Rocky 8 releases before 8.5, where dnf failed to download its metadata.
+* **role:monitoring_plugins**: The source install deploys the OID lists and MIBs of the `snmp` plugin, which until now failed with "No such file or directory" on every host installed this way.
 * **role:bind**: A secondary zone with `type: 'slave'` is saved to its file again, so the secondary answers it after a restart without waiting for the primary.
 * **role:bind**: Reverse lookups for private and special-use addresses, such as `10.0.0.0/8` or `fd00::/8`, are answered locally, as BIND does by default, instead of waiting for the forwarders, which also no longer see the internal addressing.
 * **role:kernel_settings**: The role works with fedora.linux_system_roles 2.5.0 and later, which a fresh installation of LFOps pulls in. Until now the run aborted with "kernel_settings_transparent_hugepages must be null, one of always, madvise, never" unless `kernel_settings__transparent_hugepages__*_var` and `kernel_settings__transparent_hugepages_defrag__*_var` were set.

COMPATIBILITY.md

@@ -35,7 +35,7 @@ Which Ansible role is proven to run on which OS?
 | dnf_makecache                         |        |        |   x    |   x    |    x    |           |           |           |                                              |
 | dnf_versionlock                       |        |        |   x    |   x    |   (x)   |           |           |           |                                              |
 | docker                                |        |        |   x    |  (x)   |   (x)   |           |           |           |                                              |
-| duplicity                             |   x    |   x    |   x    |   x    |    x    |     x     |     x     |     x     | Fedora 35                                    |
+| duplicity                             |   x    |   x    |   x    |   x    |    x    |     x     |     x     |     x     |                                              |
 | elastic_agent                         |  (x)   |  (x)   |  (x)   |   x    |   (x)   |    (x)    |     x     |    (x)    |                                              |
 | elastic_agent_fleet_server            |  (x)   |  (x)   |  (x)   |   x    |   (x)   |    (x)    |     x     |    (x)    |                                              |
 | elasticsearch                         |  (x)   |  (x)   |   x    |   x    |   (x)   |    (x)    |     x     |    (x)    |                                              |

CONTRIBUTING.md

@@ -980,11 +980,11 @@ __mariadb_server__python__modules__dependent_var:
     - name: 'python3-PyMySQL'
 mariadb_server__python__modules__dependent_var: '{{
     __mariadb_server__python__modules__dependent_var
-    | linuxfabrik.lfops.platform_select(ansible_facts)
+    | linuxfabrik.lfops.platform_select(ansible_facts, default=[])
   }}'
 ```
 
-`vars/main.yml` is auto-loaded at play parse, visible to every role in the play, and non-overridable from inventory like other `vars/`. Jinja evaluation is lazy, so the filter only runs when a consumer actually references the public variable.
+`vars/main.yml` is auto-loaded at play parse, visible to every role in the play, and non-overridable from inventory like other `vars/`. The filter only runs when a consumer templates the public variable, but see below for what counts as that.
 
 Consumers stay simple - they reference the public variable directly, with no awareness of the selection mechanism:
 
@@ -995,7 +995,13 @@ Consumers stay simple - they reference the public variable directly, with no awa
 - role: 'linuxfabrik.lfops.mariadb_server'
 ```
 
-The filter mirrors the precedence of `shared/tasks/platform-variables.yml` (least to most specific: `os_family`, `os_family + distribution_major_version`, `os_family + distribution_version`, `distribution`, `distribution + distribution_major_version`, `distribution + distribution_version`) and returns the value of the most specific present key. Pass `default=[]` (or whatever the consumer expects) when the value is optional on platforms not listed in the dict; otherwise an unmatched call raises an error.
+The filter mirrors the precedence of `shared/tasks/platform-variables.yml` (least to most specific: `os_family`, `os_family + distribution_major_version`, `os_family + distribution_version`, `distribution`, `distribution + distribution_major_version`, `distribution + distribution_version`) and returns the value of the most specific present key. Without a `default`, a platform that matches no key raises an error.
+
+A published `__dependent_var` has to give a valid value on every host, whether or not its own role runs there. ansible-core up to 2.18 resolves every variable name of an expression before it evaluates it, so a consumer templates the value even in the untaken branch of a `ternary()` or an inline `if`. Gating the injection on a skip variable in the playbook therefore does not keep an error out. ansible-core 2.19 evaluates these lazily, but LFOps still has to run on older releases. In addition, `skip_injections: false` (see "`skip_role` Variables in Playbooks") uses the injection of a skipped role on purpose, on a host that role does not run on. So:
+
+* Always pass `default=[]` (or the empty value the consumer expects) to `platform_select` in a `__dependent_var`.
+* Guard a value that depends on runtime state, as `roles/nextcloud/vars/main.yml` does below.
+* If the publishing role cannot work without the value, it asserts its supported platforms in its own validation block, tagged `always`. The run then aborts only where that role runs, with a message that names it, instead of in the parameters of some other role. `roles/duplicity` is the reference.
 
 A `__dependent_var` has to be computable before the consuming role starts. Do not derive one from a variable that the consuming role itself only sets at runtime. A consuming role with a `meta/argument_specs.yml` templates every declared role parameter at role entry, before any of its own tasks run, and an undefined value anywhere inside the platform-keyed dictionary collapses the whole dictionary, so `platform_select` aborts the play with `input must be a dict keyed by platform identifier, got AnsibleUndefined`.
 

extensions/molecule/kernel_settings/sysctl/inventory/group_vars/systems_under_test.yml

@@ -1,4 +1,6 @@
 kernel_settings__sysctl__group_var:
+  - name: 'sunrpc.tcp_slot_table_entries'
+    value: 128
   - name: 'vm.overcommit_memory'
     value: 1
 

extensions/molecule/kernel_settings/sysctl/verify.yml

@@ -24,3 +24,29 @@
     - name: 'Assert that transparent hugepages are set to madvise'
       ansible.builtin.assert:
         that: '"[madvise]" in (__molecule__transparent_hugepage_enabled_result["content"] | ansible.builtin.b64decode)'
+
+
+# The sunrpc sysctls only exist while the sunrpc module is loaded. Nothing else loads it on a host
+# without NFS, so check after a reboot that the module comes back on its own and TuneD applies the
+# value again.
+- name: 'Verify sunrpc.tcp_slot_table_entries survives a reboot'
+  hosts: 'systems_under_test'
+  gather_facts: false
+  tasks:
+    - name: 'systemctl reboot'
+      ansible.builtin.reboot:  # yamllint disable-line rule:empty-values
+      become: true
+
+    - name: 'Read sysctl sunrpc.tcp_slot_table_entries from procfs'
+      ansible.builtin.slurp:
+        src: '/proc/sys/sunrpc/tcp_slot_table_entries'
+      register: '__molecule__sysctl_sunrpc_tcp_slot_table_entries_result'
+
+    - name: 'Assert that sunrpc.tcp_slot_table_entries is set to 128'
+      ansible.builtin.assert:
+        that: '__molecule__sysctl_sunrpc_tcp_slot_table_entries_result["content"] | ansible.builtin.b64decode | int == 128'
+
+    - name: 'tuned-adm verify --ignore-missing'
+      ansible.builtin.command: 'tuned-adm verify --ignore-missing'
+      become: true
+      changed_when: false

extensions/molecule/monitoring_plugins_source/install/verify.yml

@@ -83,6 +83,28 @@
       loop_control:
         label: '{{ item["item"] }}'
 
+    # The snmp plugin reads its OID lists and MIBs from directories next to itself. Without
+    # them every call ends in an I/O error before any SNMP request is sent. The manifest has to
+    # list each file, so that a removal takes them away and leaves the device definitions of the
+    # admin in place.
+    - name: 'Stat the default OID list of the snmp plugin'
+      ansible.builtin.stat:
+        path: '/usr/lib64/nagios/plugins/device-oids/any-any-any.csv'
+      register: '__molecule__snmp_device_oids'
+
+    - name: 'Read the install manifest'
+      ansible.builtin.slurp:
+        src: '/usr/lib64/linuxfabrik-monitoring-plugins/install-manifest.txt'
+      register: '__molecule__manifest'
+
+    - name: 'Assert the OID list is deployed, readable and listed in the manifest'
+      ansible.builtin.assert:
+        that:
+          - '__molecule__snmp_device_oids["stat"]["exists"]'
+          - '__molecule__snmp_device_oids["stat"]["mode"] == "0644"'
+          - '__molecule__snmp_device_oids["stat"]["pw_name"] == "root"'
+          - '"/usr/lib64/nagios/plugins/device-oids/any-any-any.csv" in (__molecule__manifest["content"] | b64decode).splitlines()'
+
     # Only the modules and the LICENSE of the library are deployed, the same set the release zip
     # bundles; the documentation and development files of the repository have no business on a
     # monitored host. `__pycache__` appears as soon as a plugin has run.

extensions/molecule/monitoring_plugins_source/remove/verify.yml

@@ -89,6 +89,7 @@
         - '/usr/lib64/nagios/plugins/about-me'
         - '/usr/lib64/nagios/plugins/assets'
         - '/usr/lib64/nagios/plugins/cpu-usage'
+        - '/usr/lib64/nagios/plugins/device-oids'
       register: '__molecule__leftovers'
 
     - name: 'Assert none of it is left'

extensions/molecule/setup_basic/verify.yml

@@ -221,3 +221,14 @@
 
       when:
         - 'ansible_facts["os_family"] == "RedHat"'
+
+    # The inventory skips duplicity. Its venv must not be built anyway: python_venv used to get the
+    # duplicity venv regardless of setup_basic__skip_duplicity, and its pip install can fail.
+    - name: 'stat /opt/python-venv/duplicity'
+      ansible.builtin.stat:
+        path: '/opt/python-venv/duplicity'
+      register: '__molecule__duplicity_venv_stat_result'
+
+    - name: 'Assert that the duplicity venv is absent while duplicity is skipped'
+      ansible.builtin.assert:
+        that: 'not __molecule__duplicity_venv_stat_result["stat"]["exists"]'

galaxy.yml

@@ -59,7 +59,7 @@ dependencies:
   'ansible.utils': '*'
   'ansible.windows': '*'
   'community.crypto': '<3.0.0' # we need python 3.6 support to run against rhel8
-  'community.general': '<9.0.0' # we need python 3.6 support to run against rhel8
+  'community.general': '>=7.0.0,<9.0.0' # >=7.0.0: modprobe `persistent`; <9.0.0: we need python 3.6 support to run against rhel8
   'community.grafana': '*'
   'community.libvirt': '*'
   'community.mongodb': '*'

playbooks/setup_basic.yml

@@ -174,8 +174,8 @@
     # venvs (further down in the Backups section).
     - role: 'linuxfabrik.lfops.python_venv'
       python_venv__venvs__dependent_var: '{{
-          duplicity__python_venv__venvs__dependent_var +
-          glances__python_venv__venvs__dependent_var
+          (not setup_basic__skip_duplicity | d(false)) | ternary(duplicity__python_venv__venvs__dependent_var, []) +
+          (not setup_basic__skip_glances | d(false)) | ternary(glances__python_venv__venvs__dependent_var, [])
         }}'
       when:
         - 'not setup_basic__skip_python_venv | d(false)'

plugins/lookup/bitwarden_item.py

@@ -23,9 +23,10 @@
     - On success, the plugin returns the full Bitwarden item object. C(username) and C(password) are additionally lifted to the top level so they can be addressed without going through the C(login) sub-dictionary.
     - When I(name) is omitted, a title is generated automatically as C(hostname - purpose) (e.g. C(dbserver - MariaDB)) or just C(hostname) when no purpose is given.
     - Generated passwords use Python's C(secrets) module (cryptographically strong RNG), not the Bitwarden generator. This lifts the 128-character limit and allows arbitrary character sets, including hex.
-    - Items are read from a local on-disk cache backed by C(bw serve). A cached C(bw sync) is performed at most every 60 seconds, so consecutive lookups in the same play do not hammer the API.
+    - Items are read from a local on-disk cache backed by C(bw serve). The cache is synced once per Ansible run, since listing all items takes C(bw serve) up to half a minute; where the run cannot be identified (no C(/proc) on the controller), at most every 60 seconds. An item missing from the cache is looked up again after a fresh sync before it is created, so an item created elsewhere during a long run is not created twice.
+    - A failed sync is tried again after 10, 30 and 60 seconds before the lookup fails.
     - Lookups on the same controller run one at a time, so hosts that are processed in parallel and need the same missing item create it only once.
-    - Right after a sync, C(bw serve) can report an empty vault for a few seconds (U(https://github.com/bitwarden/clients/issues/23283)). The plugin then asks again for about ten seconds and fails rather than treat every item as missing. A vault that really is empty needs one item created by hand first.
+    - Right after a sync, C(bw serve) can report an empty vault for a few seconds (U(https://github.com/bitwarden/clients/issues/23283)). The plugin then asks again for about ten seconds, counts a vault that stays empty as a failed sync, and fails after the last sync attempt rather than treat every item as missing. A vault that really is empty needs one item created by hand first.
 
 notes:
     - Lookups are evaluated by the templating engine on the controller and have no notion of check mode, so a run with C(--check) creates a missing item for real. Set I(create) to C(false) to turn that into a failure.
@@ -298,6 +299,8 @@
     sample: 'root'
 """
 
+import os
+
 from ansible.errors import AnsibleError
 from ansible.plugins.lookup import LookupBase
 from ansible.utils.display import Display
@@ -311,6 +314,36 @@
 # inspired by the lookup plugins lastpass (same topic) and redis (more modern)
 
 
+def _read_proc(pid, name):
+    with open(f'/proc/{pid}/{name}', 'rb') as f:
+        return f.read()
+
+
+def get_run_id():
+    """Identify the Ansible run this lookup is evaluated in, or return None.
+
+    Ansible forks its workers from the process of `ansible-playbook` without exec, so
+    they carry the same command line. Walking up the parents while the command line
+    stays the same ends at that process; its PID and start time identify the run, the
+    start time guards against a reused PID. Verified with ansible-core 2.16 on Fedora
+    44, with the linear and the mitogen_linear (Mitogen 0.3.44) strategies: all workers
+    of a run got the same ID, and every run a different one. Without /proc, for example
+    on a macOS controller, there is no ID, and the caller falls back to a sync interval.
+    """
+    try:
+        pid = os.getpid()
+        cmdline = _read_proc(pid, 'cmdline')
+        while True:
+            # the fields after the command name, which itself may contain ") "
+            fields = _read_proc(pid, 'stat').rsplit(b')', 1)[1].split()
+            ppid = int(fields[1])
+            if ppid <= 1 or _read_proc(ppid, 'cmdline') != cmdline:
+                return f'{pid}:{int(fields[19])}'
+            pid = ppid
+    except (OSError, ValueError, IndexError):
+        return None
+
+
 class LookupModule(LookupBase):
     def run(self, terms, variables=None, **kwargs):
         self.set_options(var_options=variables, direct=kwargs)
@@ -328,7 +361,8 @@ def _run_under_mutex(self, bw, terms):
             raise AnsibleError(bw.get_not_unlocked_message(status))
         display.vvv('lfbwlp - run - bitwarden vault is unlocked')
 
-        bw.sync()
+        run_id = get_run_id()
+        synced = bw.sync(run_id=run_id)
 
         ret = []
         for term in terms:
@@ -372,6 +406,16 @@ def _run_under_mutex(self, bw, terms):
             result = bw.get_items(
                 name, username, folder_id, collection_id, organization_id
             )
+            if not result and not synced:
+                # the cache is only synced once per run, so an item created since then
+                # outside of this run would be missing from it and created a second time
+                display.vvv(
+                    'lfbwlp - run - not in the cache, syncing before creating it'
+                )
+                synced = bw.sync(force=True, run_id=run_id)
+                result = bw.get_items(
+                    name, username, folder_id, collection_id, organization_id
+                )
 
             if len(result) > 1:
                 raise AnsibleError(