mirror of
https://github.com/MacRimi/ProxMenux.git
synced 2026-11-01 10:22:08 +00:00
fix: guard host reboot with native package locks
This commit is contained in:
@@ -4,18 +4,64 @@ import os
|
||||
from pathlib import Path
|
||||
import socket
|
||||
import stat
|
||||
import subprocess
|
||||
import sys
|
||||
import time
|
||||
|
||||
from kernel_reboot import reboot_status
|
||||
from host_update_jobs import list_jobs
|
||||
|
||||
LOCK = '/run/proxmenux-host-update.lock'
|
||||
BOOT_ID = Path('/proc/sys/kernel/random/boot_id')
|
||||
PROC = Path('/proc')
|
||||
PACKAGE_LOCKS = ('/var/lib/dpkg/lock-frontend', '/var/lib/dpkg/lock')
|
||||
UNIT = 'proxmenux-action-reboot-after-update'
|
||||
# The unit obtains the SAME lock as CLI/legacy/web updates. Holding it for
|
||||
# the shutdown transition closes the precheck-to-launch race. If an updater
|
||||
# wins that race, flock exits75 and systemctl reboot is never invoked.
|
||||
COMMAND = ['/usr/bin/flock', '-n', '-E', '75', LOCK, '/bin/sh', '-c',
|
||||
'/bin/systemctl reboot && /bin/sleep 60']
|
||||
COMMAND = ['/usr/bin/flock', '-n', '-E', '75', LOCK, '/usr/bin/python3',
|
||||
'/usr/local/share/proxmenux/monitor-app/usr/bin/host_update_controls.py', '--execute-reboot']
|
||||
|
||||
|
||||
def package_process_active():
|
||||
try:
|
||||
for path in PROC.iterdir():
|
||||
if not path.name.isdigit():
|
||||
continue
|
||||
try:
|
||||
if (path/'comm').read_text().strip() in ('apt', 'apt-get', 'dpkg', 'unattended-upgr'):
|
||||
return True
|
||||
except (FileNotFoundError, ProcessLookupError):
|
||||
continue
|
||||
except OSError:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def execute_reboot():
|
||||
"""Unit-only final guard: POSIX dpkg locks also protect direct APT callers.
|
||||
|
||||
Do not use flock for these files: APT/dpkg use POSIX record locks. The
|
||||
parent retains both locks while systemctl runs and during shutdown.
|
||||
"""
|
||||
handles = []
|
||||
try:
|
||||
for path in PACKAGE_LOCKS:
|
||||
fd = os.open(path, os.O_RDWR | os.O_NOFOLLOW | os.O_NONBLOCK)
|
||||
handles.append(fd)
|
||||
if not stat.S_ISREG(os.fstat(fd).st_mode):
|
||||
return 76
|
||||
fcntl.lockf(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||
if reboot_status()['state'] != 'required':
|
||||
return 77
|
||||
subprocess.run(['/bin/systemctl', 'reboot'], check=True, timeout=10)
|
||||
time.sleep(60)
|
||||
return 0
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
return 75
|
||||
finally:
|
||||
for fd in handles:
|
||||
os.close(fd)
|
||||
|
||||
|
||||
def reboot_context():
|
||||
@@ -35,6 +81,8 @@ def reboot_context():
|
||||
return result
|
||||
if any(job.get('state') != 'complete' for job in jobs):
|
||||
return dict(result, reason='busy')
|
||||
if package_process_active():
|
||||
return dict(result, reason='busy')
|
||||
try:
|
||||
fd = os.open(LOCK, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)
|
||||
except FileNotFoundError:
|
||||
@@ -55,3 +103,7 @@ def reboot_context():
|
||||
reboot = reboot_status()
|
||||
return dict(result, allowed=reboot['state']=='required',
|
||||
reason='required' if reboot['state']=='required' else 'current' if reboot['state']=='current' else 'unknown')
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
sys.exit(execute_reboot() if sys.argv[1:] == ['--execute-reboot'] else 2)
|
||||
|
||||
@@ -94,6 +94,8 @@ class RebootGuards(unittest.TestCase):
|
||||
def setUp(self):
|
||||
temporary=tempfile.TemporaryDirectory();self.addCleanup(temporary.cleanup)
|
||||
self.root=Path(temporary.name); self.lock=self.root/'update.lock'
|
||||
(self.root/'proc').mkdir()
|
||||
self.proc=patch.object(controls,'PROC',self.root/'proc');self.proc.start();self.addCleanup(self.proc.stop)
|
||||
boot=self.root/'boot_id';boot.write_text('fixture-boot')
|
||||
self.boot=patch.object(controls,'BOOT_ID',boot);self.boot.start();self.addCleanup(self.boot.stop)
|
||||
self.a=patch.object(controls,'LOCK',str(self.lock)); self.a.start(); self.addCleanup(self.a.stop)
|
||||
@@ -121,8 +123,8 @@ class RebootGuards(unittest.TestCase):
|
||||
self.lock.touch()
|
||||
marker=self.root/'inert-action'
|
||||
command=list(controls.COMMAND)
|
||||
self.assertEqual(command[-1],'/bin/systemctl reboot && /bin/sleep 60')
|
||||
command[4]=str(self.lock);command[-1]='printf reached > '+shlex.quote(str(marker))
|
||||
self.assertEqual(command[5:],['/usr/bin/python3','/usr/local/share/proxmenux/monitor-app/usr/bin/host_update_controls.py','--execute-reboot'])
|
||||
command[4]=str(self.lock);command[5:]=['/bin/sh','-c','printf reached > '+shlex.quote(str(marker))]
|
||||
with self.lock.open('r') as owner:
|
||||
fcntl.flock(owner,fcntl.LOCK_EX|fcntl.LOCK_NB)
|
||||
self.assertFalse(controls.reboot_context()['allowed'])
|
||||
@@ -137,6 +139,39 @@ class RebootGuards(unittest.TestCase):
|
||||
self.probe.return_value={'state':state}
|
||||
self.assertFalse(controls.reboot_context()['allowed'])
|
||||
|
||||
def test_direct_package_process_blocks_before_kernel_probe(self):
|
||||
path=self.root/'proc/123';path.mkdir();(path/'comm').write_text('apt-get\n')
|
||||
self.assertFalse(controls.reboot_context()['allowed'])
|
||||
self.probe.assert_not_called()
|
||||
|
||||
def test_worker_package_locks_missing_or_unknown_kernel_never_reboot(self):
|
||||
paths=[str(self.root/name) for name in ('frontend','backend')]
|
||||
with patch.object(controls,'PACKAGE_LOCKS',paths),patch.object(controls.subprocess,'run') as reboot,patch.object(controls.time,'sleep') as delay:
|
||||
self.assertEqual(controls.execute_reboot(),75)
|
||||
reboot.assert_not_called();delay.assert_not_called()
|
||||
for path in paths:Path(path).touch()
|
||||
self.probe.return_value={'state':'unknown'}
|
||||
self.assertEqual(controls.execute_reboot(),77)
|
||||
reboot.assert_not_called()
|
||||
self.probe.return_value={'state':'required'}
|
||||
self.assertEqual(controls.execute_reboot(),0)
|
||||
reboot.assert_called_once_with(['/bin/systemctl','reboot'],check=True,timeout=10)
|
||||
delay.assert_called_once_with(60)
|
||||
|
||||
def test_actual_posix_package_lock_blocks_worker_before_reboot(self):
|
||||
paths=[str(self.root/name) for name in ('frontend','backend')]
|
||||
for path in paths:Path(path).touch()
|
||||
# A separate process holds the same POSIX lock, just as APT does.
|
||||
code='import fcntl,sys,time; f=open(sys.argv[1],"r+"); fcntl.lockf(f,fcntl.LOCK_EX); print("locked",flush=True); time.sleep(5)'
|
||||
child=subprocess.Popen([sys.executable,'-c',code,paths[0]],stdout=subprocess.PIPE,text=True)
|
||||
try:
|
||||
self.assertEqual(child.stdout.readline().strip(),'locked')
|
||||
with patch.object(controls,'PACKAGE_LOCKS',paths),patch.object(controls.subprocess,'run') as reboot:
|
||||
self.assertEqual(controls.execute_reboot(),75)
|
||||
reboot.assert_not_called()
|
||||
finally:
|
||||
child.terminate();child.wait(timeout=2);child.stdout.close()
|
||||
|
||||
|
||||
class Routes(unittest.TestCase):
|
||||
def setUp(self):
|
||||
|
||||
Reference in New Issue
Block a user