fix(k3s_server): skip k3s-init log collection when the destination is unset (#721)

* fix(k3s_server): skip k3s-init log collection when the destination is unset

The guard on the k3s-init diagnostics include used a boolean default for
the ANSIBLE_K3S_LOG_DIR lookup so that the when conditional would be a real
boolean. That is unnecessary: `log_destination | default('') != ''` is a
comparison and always evaluates to a bool, so ansible-core 2.19 never sees
a non-boolean conditional here.

The boolean default also defeats the guard. With ANSIBLE_K3S_LOG_DIR unset,
log_destination is the bool False, and `False != ''` is True in Python
because False == 0. The include therefore runs on every provisioning run,
executing the journalctl, systemctl, ip, ss, iptables, service_facts, ping
and wait_for diagnostics in fetch_k3s_init_logs.yml, then creating a
directory literally named "False" in the operator's working directory.

Use an empty-string default so the unset case renders "" and the guard
skips. CI did not catch this because .github/workflows/test.yml always
exports ANSIBLE_K3S_LOG_DIR.

Add a regression test that parses the role, renders the real log_destination
expression for both the unset and set cases, and asserts that no boolean
default has been reintroduced.

* chore(site): wrap the over-long kubeconfig fetch conditional

.github/scripts/test-k3s-init-log-destination.py

@@ -0,0 +1,112 @@
+#!/usr/bin/env python3
+"""Assert the k3s-init log destination guard skips when unset.
+
+``roles/k3s_server/tasks/main.yml`` fetches k3s-init diagnostics only when
+``ANSIBLE_K3S_LOG_DIR`` is set. The lookup default must be an empty string:
+Jinja stringifies the boolean ``False`` to ``'False'``, which is non-empty, so
+a boolean default makes the guard pass and the role creates a directory
+literally named ``False`` in the operator's working directory on every run.
+
+This test extracts the real ``log_destination`` expression from the role and
+renders it for both the unset and set cases.
+"""
+
+from __future__ import print_function
+
+import os
+import re
+import subprocess
+
+import yaml
+from jinja2 import Environment
+
+ROLE = os.path.join("roles", "k3s_server", "tasks", "main.yml")
+
+
+def repo_root():
+    return subprocess.check_output(
+        ["git", "rev-parse", "--show-toplevel"], text=True
+    ).strip()
+
+
+def fail(message):
+    raise SystemExit("k3s-init log destination test failed: " + message)
+
+
+def find_log_task(root):
+    """Return the 'Save logs of k3s-init.service' task from the role."""
+    with open(os.path.join(root, ROLE), "r") as handle:
+        tasks = yaml.safe_load(handle)
+
+    def walk(entries):
+        for entry in entries or []:
+            if not isinstance(entry, dict):
+                continue
+            if "Save logs of k3s-init.service" in str(entry.get("name", "")):
+                return entry
+            for key in ("block", "always", "rescue"):
+                found = walk(entry.get(key))
+                if found:
+                    return found
+        return None
+
+    task = walk(tasks)
+    if task is None:
+        fail("could not find the 'Save logs of k3s-init.service' task in " + ROLE)
+    return task
+
+
+def env_lookup(name, default=None, **kwargs):
+    """Stand-in for the env lookup: unset names fall back to the default."""
+    return {"ANSIBLE_K3S_LOG_DIR": None}.get(name, default)
+
+
+def render(task, env, value):
+    """Render log_destination the way Ansible folds a '>-' scalar."""
+    expression = str(task["vars"]["log_destination"]).strip()
+    template = env.from_string(expression)
+
+    def lookup(*args, **kwargs):
+        name = args[1] if len(args) > 1 else kwargs.get("name")
+        if name != "ANSIBLE_K3S_LOG_DIR":
+            return kwargs.get("default")
+        return kwargs["default"] if value is None else value
+
+    return template.render(lookup=lookup).strip()
+
+
+def main():
+    root = repo_root()
+    task = find_log_task(root)
+    env = Environment()
+
+    unset = render(task, env, None)
+    if unset != "":
+        fail(
+            "log_destination resolved to %r when ANSIBLE_K3S_LOG_DIR is unset; "
+            "it must be an empty string so the guard skips" % unset
+        )
+    if task["when"].strip() == "":
+        fail("the guard on the log task is empty, so it would always run")
+
+    set_path = render(task, env, "/var/log/k3s-init")
+    if set_path != "/var/log/k3s-init":
+        fail(
+            "log_destination resolved to %r when ANSIBLE_K3S_LOG_DIR is set; "
+            "expected the path to be passed through unchanged" % set_path
+        )
+
+    # A stray default= would leave the boolean in place even though the
+    # expression still renders empty, so assert the source no longer carries
+    # it. This is the exact regression that produced the False/ directory.
+    if re.search(r"default\s*=\s*False", yaml.safe_dump(task)):
+        fail(
+            "log_destination still uses a boolean default; Jinja would "
+            "stringify it to 'False' and create a directory named False"
+        )
+
+    print("k3s-init log destination regression test passed")
+
+
+if __name__ == "__main__":
+    main()

.pre-commit-config.yaml

@@ -64,6 +64,12 @@ repos:
         language: system
         pass_filenames: false
         files: ^roles/k3s_server/tasks/(main|join_master)\.yml$|^\.github/scripts/test-k3s-server-bootstrap\.sh$
+      - id: k3s-init-log-destination-test
+        name: K3s init log destination test
+        entry: .github/scripts/test-k3s-init-log-destination.py
+        language: system
+        pass_filenames: false
+        files: ^roles/k3s_server/tasks/main\.yml$|^\.github/scripts/test-k3s-init-log-destination\.py$
       - id: unique-hostname-precheck-test
         name: Unique hostname precheck test
         entry: .github/scripts/test-unique-hostname-precheck.sh

roles/k3s_server/tasks/main.yml

@@ -124,13 +124,18 @@
   always:
     - name: Save logs of k3s-init.service
       ansible.builtin.include_tasks: fetch_k3s_init_logs.yml
-      # ANSIBLE_K3S_LOG_DIR is a path string when set; evaluate it as a boolean
-      # so the conditional is a real boolean (ansible-core 2.19 rejects string
-      # conditionals derived from env vars).
+      # The env lookup default must be an empty string, not the boolean
+      # False. The `!= ''` comparison below always evaluates to a bool, so
+      # ansible-core 2.19 never sees a non-boolean conditional and no boolean
+      # default is needed. With a boolean default, an unset ANSIBLE_K3S_LOG_DIR
+      # leaves log_destination as the bool False, and `False != ''` is True
+      # because False == 0. The guard would then always pass, running the
+      # diagnostics on every provisioning run and creating a directory
+      # literally named 'False' in the operator's working directory.
       when: log_destination | default('') != ''
       vars:
         log_destination: >-
-          {{ lookup('ansible.builtin.env', 'ANSIBLE_K3S_LOG_DIR', default=False) }}
+          {{ lookup('ansible.builtin.env', 'ANSIBLE_K3S_LOG_DIR', default='') }}
     - name: Kill the temporary service used for initialization
       ansible.builtin.systemd:
         name: k3s-init

site.yml

@@ -85,5 +85,5 @@
         dest: ./kubeconfig
         flat: true
       when: >-
-        ansible_facts.hostname == hostvars[groups[group_name_master | default('master')][0]]
-        ['ansible_facts']['hostname']
+        ansible_facts.hostname ==
+        hostvars[groups[group_name_master | default('master')][0]]['ansible_facts']['hostname']