Fix systemd oneshot upgrade handoff deadlock

tests/test_backup.py

@@ -541,20 +541,38 @@ QUIESCE_SERVICES=(nc-cron)
             result=self.run_script('app-backup.sh','--check',ok=False,APP_BACKUP_INHERIT_LOCK='1')
             self.assertIn('Inherited lock descriptor',result.stdout)
 
-    def test_detached_package_handoff_finishes_after_starter_exits(self):
-        self.configure_upgrade()
+    def systemd_launcher(self):
+        """Model start-job waiting, detached FDs, and ambiguous submit failures."""
         launcher=self.bin/'systemd-run'
         launcher.write_text('''#!/usr/bin/env python3
 import os,subprocess,sys
 from pathlib import Path
 args=sys.argv[1:]
 assert '--unit=app-backup-upgrade' in args
+assert '--property=Type=oneshot' in args
 command=args[args.index('/bin/bash'):]
 state=Path(command[-1])
+if os.environ.get('TEST_SYSTEMD_SUBMIT') == 'reject':
+ sys.exit('systemd refused submission')
 with open(state/'test-worker.log','w') as log:
- subprocess.Popen(command,stdin=subprocess.DEVNULL,stdout=log,stderr=subprocess.STDOUT,env=os.environ,close_fds=True,start_new_session=True)
+ worker=subprocess.Popen(command,stdin=subprocess.DEVNULL,stdout=log,stderr=subprocess.STDOUT,env=os.environ,close_fds=True,start_new_session=True)
+if os.environ.get('TEST_SYSTEMD_SUBMIT') == 'accepted-error':
+ sys.exit('simulated reply failure after systemd accepted the job')
+# Type=oneshot remains starting until ExecStart exits. Real systemd-run waits
+# for that start job unless --no-block is used; our old stub missed this.
+if '--no-block' not in args:
+ try:
+  sys.exit(worker.wait(timeout=3))
+ except subprocess.TimeoutExpired:
+  worker.terminate()
+  worker.wait(timeout=5)
+  sys.exit('oneshot start job still waiting: launcher/worker lock deadlock')
 ''')
         launcher.chmod(0o755)
+
+    def test_detached_package_handoff_finishes_after_starter_exits(self):
+        self.configure_upgrade()
+        self.systemd_launcher()
         package=self.build_package()
         result=subprocess.run(['bash',str(package)],env=self.env,capture_output=True,text=True,timeout=60)
         self.assertEqual(result.returncode,0,result.stdout+result.stderr)
@@ -566,6 +584,66 @@ with open(state/'test-worker.log','w') as log:
         self.assertTrue(status.startswith('SUCCESS'),status+(state/'test-worker.log').read_text())
         self.assertIn('unabhaengig von SSH',result.stdout)
 
+    def test_systemd_submission_failure_preserves_installation_and_reports_result(self):
+        canonical=self.configure_upgrade()
+        original=canonical.read_bytes()
+        self.systemd_launcher()
+        package=self.build_package()
+        result=subprocess.run(['bash',str(package)],env=dict(self.env,TEST_SYSTEMD_SUBMIT='reject'),capture_output=True,text=True,timeout=60)
+        self.assertNotEqual(result.returncode,0)
+        state=Path(result.stdout.split('Update-Sicherung und Protokoll: ')[1].splitlines()[0])
+        self.assertIn('FAILED: systemd submission failed',(state/'result.txt').read_text())
+        self.assertIn(str(state/'result.txt'),result.stdout)
+        self.assertEqual(canonical.read_bytes(),original)
+        self.assertEqual(json.loads(self.state.read_text())['services']['app-backup.timer'],'active')
+        self.assertNotIn('systemctl stop',self.events.read_text())
+
+    def test_failed_submission_revokes_even_an_already_queued_worker(self):
+        canonical=self.configure_upgrade()
+        original=canonical.read_bytes()
+        self.systemd_launcher()
+        package=self.build_package()
+        result=subprocess.run(['bash',str(package)],env=dict(self.env,TEST_SYSTEMD_SUBMIT='accepted-error'),capture_output=True,text=True,timeout=60)
+        self.assertNotEqual(result.returncode,0)
+        state=Path(result.stdout.split('Update-Sicherung und Protokoll: ')[1].splitlines()[0])
+        for _ in range(100):
+            log=(state/'test-worker.log').read_text()
+            if 'Installation not started' in log: break
+            time.sleep(.1)
+        self.assertIn('submission was cancelled',log)
+        self.assertIn('Installation not started',log)
+        self.assertTrue((state/'result.txt').read_text().startswith('FAILED'))
+        self.assertEqual(canonical.read_bytes(),original)
+        self.assertEqual(json.loads(self.state.read_text())['services']['app-backup.timer'],'active')
+        self.assertNotIn('systemctl stop',self.events.read_text())
+        self.assertFalse((state/'package-rollback').exists())
+
+    def test_worker_lock_timeout_is_persisted_before_installation(self):
+        import fcntl
+        canonical=self.configure_upgrade()
+        original=canonical.read_bytes()
+        self.systemd_launcher()
+        package=self.build_package()
+        # Obtain a complete prepared state without starting an actual worker.
+        result=subprocess.run(['bash',str(package)],env=dict(self.env,TEST_SYSTEMD_SUBMIT='reject'),capture_output=True,text=True,timeout=60)
+        self.assertNotEqual(result.returncode,0)
+        state=Path(result.stdout.split('Update-Sicherung und Protokoll: ')[1].splitlines()[0])
+        (state/'result.txt').write_text('QUEUED\n')
+        wrapper=self.bin/'flock'
+        # Keep real flock contention; shorten only the 60s timeout in this test.
+        wrapper.write_text('#!/usr/bin/env bash\nif [[ $1 == -w && $2 == 60 ]]; then shift 2; exec /usr/bin/flock -w 0 "$@"; fi\nexec /usr/bin/flock "$@"\n')
+        wrapper.chmod(0o755)
+        with open('/run/app-backup-upgrade.lock','a') as lock:
+            fcntl.flock(lock,fcntl.LOCK_EX | fcntl.LOCK_NB)
+            failed=subprocess.run(['bash',str(REPO/'update.sh'),'--worker',str(state)],env=self.env,capture_output=True,text=True,timeout=10)
+        self.assertNotEqual(failed.returncode,0)
+        self.assertIn('Timed out waiting for update coordinator lock',(state/'update.log').read_text())
+        self.assertIn('Installation not started',(state/'result.txt').read_text())
+        self.assertTrue((state/'result.txt').read_text().startswith('FAILED'))
+        self.assertEqual(canonical.read_bytes(),original)
+        self.assertEqual(json.loads(self.state.read_text())['services']['app-backup.timer'],'active')
+        self.assertNotIn('systemctl stop',self.events.read_text())
+
     def test_cron_inventory_requires_explicit_policy_and_sorts_timer_first(self):
         canonical=self.configure_upgrade()
         cron=Path('/etc/cron.d/app-backup-test-nextcloud')