fix(web-app-keycloak): keep concurrent kcadm jobs from losing the config lock

In the async pass the role runs kcadm jobs side by side. Every kcadm call in
the Keycloak container shares one config file. kcadm rewrites that file when
it refreshes the access token, about every 55 seconds, takes the file lock
with tryLock() and gives up at once when another kcadm process holds it. One
of two overlapping declared-client jobs then failed with "Failed to get lock
on /opt/keycloak/.keycloak/kcadm.config", and the deploy with it.

keycloak_kcadm_update now repeats a call that lost the lock, up to ten times
one second apart. The call fails while it refreshes its token, before it
sends its own request, so repeating it is safe. Every other error still ends
the call at once.

That covers the module only, so the async jobs are reduced to the ones that
go through it:

- The default-scope assignment was an async shell task. It ran beside the
  shell tasks that follow it, and neither side repeats a lost lock. It runs
  in line now.
- The two mail-settings jobs were async without a register, so nothing
  reaped them and a failure was never seen. They run in line now.

The client job and the declared-client jobs stay async. A test fails when an
async kcadm task does not use the module or is not registered for the reap.

A config file of its own per call was the other option. It does not work by
itself, because the admin session lives only in the shared file.

Closes #805

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

library/keycloak_kcadm_update.py

@@ -14,6 +14,9 @@ from ansible.module_utils.keycloak_scopes import converge_client_scope_lists
 NO_SUCH_CONTAINER_RE = re.compile(r"no such container", re.IGNORECASE)
 DEAD_CID_MAX_RETRIES = 80
 DEAD_CID_RETRY_DELAY = 3
+CONFIG_LOCK_RE = re.compile(r"Failed to get lock on")
+CONFIG_LOCK_MAX_RETRIES = 10
+CONFIG_LOCK_RETRY_DELAY = 1
 
 DOCUMENTATION = r"""
 ---
@@ -119,7 +122,7 @@ result:
 """
 
 
-def _shell(cmd, check):
+def _shell(cmd, check, lock_retries=CONFIG_LOCK_MAX_RETRIES):
     rc = subprocess.run(  # noqa: S602 - host-trusted argv; kcadm wrapper needs shell pipes/&&
         cmd,
         shell=True,
@@ -128,6 +131,9 @@ def _shell(cmd, check):
     )
     stdout = rc.stdout.decode("utf-8", errors="replace").strip()
     stderr = rc.stderr.decode("utf-8", errors="replace").strip()
+    if rc.returncode != 0 and lock_retries and CONFIG_LOCK_RE.search(stderr):
+        time.sleep(CONFIG_LOCK_RETRY_DELAY)
+        return _shell(cmd, check, lock_retries - 1)
     return rc.returncode, stdout, stderr
 
 

roles/web-app-keycloak/tasks/03_manager_ops/update_routines.yml

@@ -61,8 +61,6 @@
 - name: "⏳ Reap the remaining Keycloak update jobs"
   ansible.builtin.include_tasks: utils/async/reap.yml
   vars:
-    async_reap_jobs: >-
-      {{ (kc_rbac_default.results | default([kc_rbac_default] if kc_rbac_default is defined else []))
-         + (kc_declared_client_jobs | default([])) }}
+    async_reap_jobs: "{{ kc_declared_client_jobs | default([]) }}"
     async_reap_label: "Keycloak realm updates"
     async_reap_tolerate: []

roles/web-app-keycloak/tasks/update/02_mail_realm.yml

@@ -10,6 +10,3 @@
     kcadm_exec: "{{ KEYCLOAK_EXEC_KCADM }}"
     assert_mode: "{{ MODE_ASSERT }}"
   no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}"
-  # rationale: async; smtpServer is written here and never read back anywhere
-  async: "{{ ASYNC_TIME if ASYNC_ENABLED | bool else omit }}"
-  poll: "{{ ASYNC_POLL if ASYNC_ENABLED | bool else omit }}"

roles/web-app-keycloak/tasks/update/03_mail_master.yml

@@ -10,6 +10,3 @@
     kcadm_exec: "{{ KEYCLOAK_EXEC_KCADM }}"
     assert_mode: "{{ MODE_ASSERT }}"
   no_log: "{{ MASK_CREDENTIALS_IN_LOGS | bool }}"
-  # rationale: async; writes the master realm smtpServer, which nothing reads back
-  async: "{{ ASYNC_TIME if ASYNC_ENABLED | bool else omit }}"
-  poll: "{{ ASYNC_POLL if ASYNC_ENABLED | bool else omit }}"

roles/web-app-keycloak/tasks/update/04_rbac_client_scope.yml

@@ -92,8 +92,4 @@
     {{ KEYCLOAK_EXEC_KCADM }} update
     clients/{{ app_client_id }}/default-client-scopes/{{ scope_id_rbac }}
     -r {{ KEYCLOAK_REALM }}
-  register: kc_rbac_default
   changed_when: true
-  # rationale: async; no task in the chain reads the scope back, reaped at the end of update_routines.yml
-  async: "{{ ASYNC_TIME if ASYNC_ENABLED | bool else omit }}"
-  poll: "{{ ASYNC_POLL if ASYNC_ENABLED | bool else omit }}"

tests/unit/python/roles/web-app-keycloak/test_async_kcadm_jobs.py

@@ -0,0 +1,57 @@
+from __future__ import annotations
+
+import unittest
+
+from utils.cache.files import PROJECT_ROOT, iter_project_files
+from utils.cache.yaml import load_yaml_any
+
+TASKS_DIR = str(PROJECT_ROOT / "roles/web-app-keycloak/tasks") + "/"
+LOCK_RETRYING_MODULE = "keycloak_kcadm_update"
+
+
+def tasks_of(node):
+    for task in node or []:
+        if not isinstance(task, dict):
+            continue
+        yield task
+        for nested in ("block", "rescue", "always"):
+            yield from tasks_of(task.get(nested))
+
+
+def async_kcadm_jobs():
+    for path in iter_project_files(extensions=(".yml",)):
+        if not path.startswith(TASKS_DIR):
+            continue
+        for task in tasks_of(load_yaml_any(path)):
+            if "async" in task and "kcadm" in str(task).lower():
+                yield f"{path[len(TASKS_DIR) :]}: {task.get('name')}", task
+
+
+class TestAsyncKcadmJobs(unittest.TestCase):
+    def test_the_role_runs_kcadm_jobs_async(self):
+        self.assertTrue(list(async_kcadm_jobs()))
+
+    def test_every_async_kcadm_job_goes_through_the_lock_retrying_module(self):
+        for label, task in async_kcadm_jobs():
+            with self.subTest(task=label):
+                self.assertIn(
+                    LOCK_RETRYING_MODULE,
+                    task,
+                    "kcadm gives up its config lock to a concurrent kcadm at once; "
+                    "only keycloak_kcadm_update repeats such a call, so no other "
+                    "kcadm task may run async",
+                )
+
+    def test_every_async_kcadm_job_is_registered_for_the_reap(self):
+        for label, task in async_kcadm_jobs():
+            with self.subTest(task=label):
+                self.assertIn(
+                    "register",
+                    task,
+                    "an async job without a register cannot be reaped, so nobody "
+                    "sees it fail",
+                )
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/unit/python/roles/web_app_keycloak/test_library_keycloak_kcadm_update.py

@@ -15,6 +15,10 @@ if TYPE_CHECKING:
 ROLE_DIR = PROJECT_ROOT / "roles" / "web-app-keycloak"
 LIB_PATH = PROJECT_ROOT / "library" / "keycloak_kcadm_update.py"
 MODUTILS_PATH = PROJECT_ROOT / "utils" / "kcadm_json.py"
+LOCK_ERROR = (
+    "Failed to refresh access token - "
+    "Failed to get lock on /opt/keycloak/.keycloak/kcadm.config"
+)
 
 
 def _load_py_module(name: str, path: Path):
@@ -214,6 +218,46 @@ class TestKeycloakKcadmUpdate(unittest.TestCase):
         self.assertEqual(rc, 1)
         self.assertEqual(len(calls), 1)
 
+    def test_run_kcadm_repeats_a_call_that_lost_the_config_lock(self):
+        m = self.mod
+        outcomes = [
+            DummyCompleted(1, "", LOCK_ERROR),
+            DummyCompleted(1, "", LOCK_ERROR),
+            DummyCompleted(0, "[]", ""),
+        ]
+        calls = []
+
+        def fake_run(cmd, **kwargs):
+            calls.append(cmd)
+            return outcomes[len(calls) - 1]
+
+        with (
+            patch.object(m.subprocess, "run", side_effect=fake_run),
+            patch.object(m.time, "sleep", return_value=None) as sleep,
+        ):
+            rc, out, _err = m.run_kcadm(DummyModule(), "kcadm get clients")
+
+        self.assertEqual((rc, out), (0, "[]"))
+        self.assertEqual(calls, ["kcadm get clients"] * 3)
+        self.assertEqual(sleep.call_count, 2)
+
+    def test_run_kcadm_gives_up_on_a_config_lock_that_never_frees(self):
+        m = self.mod
+        calls = []
+
+        def fake_run(cmd, **kwargs):
+            calls.append(cmd)
+            return DummyCompleted(1, "", LOCK_ERROR)
+
+        with (
+            patch.object(m.subprocess, "run", side_effect=fake_run),
+            patch.object(m.time, "sleep", return_value=None),
+            self.assertRaises(m.subprocess.CalledProcessError),
+        ):
+            m.run_kcadm(DummyModule(), "kcadm get clients")
+
+        self.assertEqual(len(calls), m.CONFIG_LOCK_MAX_RETRIES + 1)
+
     def test_get_current_object_parses_noisy(self):
         m = self.mod
         dummy = DummyModule()