test(molecule): fix false passes in host-env/schema and shared staging in validation

ansible/molecule/host-env/side_effect.yml

@@ -1,7 +1,8 @@
 ---
 # Edit the host-provisioned env file OUT OF BAND, exactly as an operator rotating
 # an RPC key would, and record the daemon's current MainPID. molecule.yml runs a
-# SECOND converge after this, and verify.yml then asserts the PID changed.
+# SECOND converge after this, and verify.yml then asserts the PID changed without
+# systemd having auto-restarted the unit.
 #
 # This is the only test of the feature's headline behaviour. Every other assertion
 # about /etc/decdn/.decdn.env.sha256 checks a precondition (it exists, it is 0600
@@ -14,31 +15,38 @@
   vars:
     decdn_etc: /etc/decdn
   tasks:
-    # pgrep, not `systemctl show --property=MainPID`: ansible-lint's
-    # command-instead-of-module rule rejects any `systemctl` invocation, and there
-    # is no module that returns a unit's PID. This queries the process table
-    # instead, which is a different job from managing the service.
-    - name: Record the daemon's PID before the edit
-      ansible.builtin.command:
-        cmd: pgrep -f '/usr/local/bin/decdn-node run'
-      register: decdn_pid_before
-      changed_when: false
-      failed_when: false
+    # A read-only `systemctl show`: the module fills `status` before it acts, and
+    # check_mode stops it acting. `state` is only there because the module rejects
+    # a bare `name` — it never starts anything here. (A `systemctl` command would
+    # trip ansible-lint's command-instead-of-module.)
+    - name: Read the daemon's unit state before the edit
+      ansible.builtin.systemd_service:
+        name: decdn-node
+        state: started
+      check_mode: true
+      register: decdn_unit_before
 
-    - name: Assert the daemon was actually running before the edit
+    # NRestarts counts only restarts systemd made under Restart=always, never the
+    # role's handler. It must start at 0 so verify.yml can require it to still be 0:
+    # that is what tells a handler restart apart from a crash that systemd revived.
+    - name: Assert the daemon was running cleanly before the edit
       ansible.builtin.assert:
         that:
-          - decdn_pid_before.stdout_lines | length == 1
-          - decdn_pid_before.stdout | trim | int > 0
+          - decdn_unit_before.status.ActiveState == 'active'
+          - decdn_unit_before.status.MainPID | int > 0
+          - decdn_unit_before.status.NRestarts | int == 0
         fail_msg: >-
-          Expected exactly one running decdn-node before the out-of-band edit
-          (got {{ decdn_pid_before.stdout_lines | default([]) }}), so a PID change
-          after it would prove nothing.
+          Expected decdn-node active, with a MainPID and no automatic restarts,
+          before the out-of-band edit (got ActiveState={{ decdn_unit_before.status.ActiveState }},
+          MainPID={{ decdn_unit_before.status.MainPID }},
+          NRestarts={{ decdn_unit_before.status.NRestarts }}). A daemon that is
+          already crash-looping makes a PID change after the edit prove nothing —
+          check: journalctl -u decdn-node -e
 
     - name: Save the pre-edit MainPID for verify.yml
       ansible.builtin.copy:
         dest: "{{ decdn_etc }}/molecule-pid-before"
-        content: "{{ decdn_pid_before.stdout | trim }}\n"
+        content: "{{ decdn_unit_before.status.MainPID }}\n"
         mode: "0644"
 
     # Append/replace in place rather than rewriting the file: this must look like

ansible/molecule/host-env/verify.yml

@@ -138,31 +138,48 @@
     # still serving with the old endpoint behind a green deploy. Comparing MainPID
     # is what makes this behavioural — the checksum assertions above all remain
     # true even if the restart signal is deleted outright.
+    #
+    # A changed PID alone is only the EFFECT. The unit is Restart=always, so a
+    # daemon that crashed (or was OOM-killed under parallel molecule load) between
+    # side_effect and the re-converge also comes back with a new PID. NRestarts
+    # pins the CAUSE: it counts only systemd's automatic restarts, so a new PID
+    # with NRestarts still 0 can only have come from the role's handler.
     - name: Read the pre-edit MainPID recorded by side_effect
       ansible.builtin.slurp:
         src: "{{ decdn_etc }}/molecule-pid-before"
       register: decdn_pid_before
 
-    # pgrep rather than `systemctl show` — see the note in side_effect.yml.
-    - name: Read the daemon's PID after the re-converge
-      ansible.builtin.command:
-        cmd: pgrep -f '/usr/local/bin/decdn-node run'
-      register: decdn_pid_after
-      changed_when: false
-      failed_when: false
+    # Read-only `systemctl show` via check_mode — see the note in side_effect.yml.
+    - name: Read the daemon's unit state after the re-converge
+      ansible.builtin.systemd_service:
+        name: decdn-node
+        state: started
+      check_mode: true
+      register: decdn_unit_after
 
     - name: Assert the out-of-band edit restarted decdn-node
       ansible.builtin.assert:
         that:
-          - decdn_pid_after.stdout_lines | length == 1
-          - decdn_pid_after.stdout | trim | int > 0
-          - (decdn_pid_after.stdout | trim) != (decdn_pid_before.content | b64decode | trim)
+          - decdn_unit_after.status.MainPID | int > 0
+          - (decdn_unit_after.status.MainPID | string) != (decdn_pid_before.content | b64decode | trim)
         fail_msg: >-
-          decdn-node still runs as PID {{ decdn_pid_after.stdout | trim }} after
+          decdn-node still runs as PID {{ decdn_unit_after.status.MainPID }} after
           {{ decdn_etc }}/decdn.env was edited out of band and the role re-converged
           — the daemon is on the PRE-rotation secret behind a green deploy. The
           env-file checksum record did not drive the restart handler.
 
+    - name: Assert the restart came from the role, not from systemd
+      ansible.builtin.assert:
+        that:
+          - decdn_unit_after.status.NRestarts | int == 0
+        fail_msg: >-
+          systemd auto-restarted decdn-node {{ decdn_unit_after.status.NRestarts }}
+          time(s) (Restart=always) — the daemon died on its own at some point
+          between side_effect and verify, so the PID change above cannot prove the
+          role's handler restarted it. Find out why it died
+          (journalctl -u decdn-node -e; an OOM kill under `make molecule` load shows
+          in dmesg) rather than relaxing this check.
+
     # Without EnvironmentFile the daemon never receives the host-provisioned
     # secret at all, and every other assertion in this scenario would still pass.
     - name: Read the rendered systemd unit
@@ -190,3 +207,23 @@
             'decdn-node.service' in ansible_facts.services
             and ansible_facts.services['decdn-node.service'].state == 'running'
         fail_msg: "decdn-node is not running — check: journalctl -u decdn-node -e"
+
+    - name: Collect listening TCP sockets
+      ansible.builtin.command:
+        cmd: ss -ltn
+      register: listeners
+      changed_when: false
+
+    # The role's /metrics readiness probe only WARNS on timeout, so without this a
+    # daemon that never binds metrics would still leave this scenario green.
+    # Same check as ../default/verify.yml: bound on loopback, and on no wildcard.
+    - name: Assert metrics are up and bound to loopback only
+      ansible.builtin.assert:
+        that:
+          - "'127.0.0.1:9090' in listeners.stdout"
+          - "'0.0.0.0:9090' not in listeners.stdout"
+          - "'[::]:9090' not in listeners.stdout"
+          - "'*:9090' not in listeners.stdout"
+        fail_msg: >-
+          metrics are not up on 127.0.0.1:9090 only (check: journalctl -u decdn-node -e):
+          {{ listeners.stdout }}

ansible/molecule/schema/verify.yml

@@ -141,3 +141,24 @@
           fi
           echo "breadth OK: $keys keys, $(echo "$tables" | grep -c '') tables"
       changed_when: false
+
+    - name: Collect listening TCP sockets
+      ansible.builtin.command:
+        cmd: ss -ltn
+      register: listeners
+      changed_when: false
+
+    # The role's /metrics readiness probe only WARNS on timeout, so without this a
+    # daemon that never binds metrics would still leave this scenario green.
+    # Same check as ../default/verify.yml: bound on loopback, and on no wildcard.
+    # It checks the daemon the LAST converge play (resolve-only) left running.
+    - name: Assert metrics are up and bound to loopback only
+      ansible.builtin.assert:
+        that:
+          - "'127.0.0.1:9090' in listeners.stdout"
+          - "'0.0.0.0:9090' not in listeners.stdout"
+          - "'[::]:9090' not in listeners.stdout"
+          - "'*:9090' not in listeners.stdout"
+        fail_msg: >-
+          metrics are not up on 127.0.0.1:9090 only (check: journalctl -u decdn-node -e):
+          {{ listeners.stdout }}

ansible/molecule/validation/converge.yml

@@ -70,11 +70,21 @@
     # format assert rejects it — the "built a macOS Mach-O and shipped it" guard.
     # Staged + validated on the controller (delegate_to: localhost), so it aborts
     # before any host mutation, like the other assert-only cases.
+    # The staging dir is a fresh tempdir, not a fixed path: a fixed one is shared
+    # by every run on the controller (MOLECULE_EPHEMERAL_DIRECTORY is too — it is
+    # keyed on the scenario, not the run), so one run's `always:` cleanup could
+    # delete another's pair mid-case.
     - name: "Case binary-format — dir-mode resolves a non-ELF binary"
       vars:
-        _decdn_badbin_dir: >-
-          {{ lookup('ansible.builtin.env', 'MOLECULE_PROJECT_DIRECTORY') }}/.molecule-badbin/target/release
+        _decdn_badbin_dir: "{{ _decdn_badbin_tmp.path }}/target/release"
       block:
+        - name: Create a per-run staging root on the control machine
+          ansible.builtin.tempfile:
+            state: directory
+            prefix: decdn-molecule-badbin.
+          register: _decdn_badbin_tmp
+          delegate_to: localhost
+          become: false
         - name: Create the staging directory on the control machine
           ansible.builtin.file:
             path: "{{ _decdn_badbin_dir }}"
@@ -103,10 +113,13 @@
             decdn_rejected: "{{ decdn_rejected + ['binary-format'] }}"
           when: ansible_failed_task.name is match('^Require both binaries to be ELF')
       always:
+        # Guarded: if tempfile itself failed there is nothing to remove, and an
+        # undefined-variable error here would bury the real failure.
         - name: Remove the staged non-ELF binaries
           ansible.builtin.file:
-            path: "{{ lookup('ansible.builtin.env', 'MOLECULE_PROJECT_DIRECTORY') }}/.molecule-badbin"
+            path: "{{ _decdn_badbin_tmp.path }}"
             state: absent
+          when: _decdn_badbin_tmp.path is defined
           delegate_to: localhost
           become: false
         # The role's derive-paths set_fact left decdn_node_manual_bin_src pointing at
@@ -121,11 +134,19 @@
 
     # A read-only `file -b` probe must still execute under --check. Before this
     # regression guard it was skipped and the following ELF assert saw stdout="".
+    # Per-run tempdir for the same reason as binary-format above.
     - name: "Case binary-check-mode — dir-mode format probe runs during dry-run"
       vars:
-        _decdn_check_dir: >-
-          {{ lookup('ansible.builtin.env', 'MOLECULE_PROJECT_DIRECTORY') }}/.molecule-checkbin/target/release
+        _decdn_check_dir: "{{ _decdn_checkbin_tmp.path }}/target/release"
       block:
+        - name: Create a per-run check-mode root on the control machine
+          ansible.builtin.tempfile:
+            state: directory
+            prefix: decdn-molecule-checkbin.
+          register: _decdn_checkbin_tmp
+          delegate_to: localhost
+          become: false
+
         - name: Create the check-mode binary directory on the control machine
           ansible.builtin.file:
             path: "{{ _decdn_check_dir }}"
@@ -166,8 +187,9 @@
       always:
         - name: Remove the check-mode binary fixture
           ansible.builtin.file:
-            path: "{{ lookup('ansible.builtin.env', 'MOLECULE_PROJECT_DIRECTORY') }}/.molecule-checkbin"
+            path: "{{ _decdn_checkbin_tmp.path }}"
             state: absent
+          when: _decdn_checkbin_tmp.path is defined
           delegate_to: localhost
           become: false
         - name: Restore binary-source facts after the check-mode probe