upgrades: Use bind mounts to edit sources file only upon completion

- Save changes to /etc/apt/sources.list as a different file.

- When launching the dist upgrade process via systemd-run, use bind mounting to
ensure that the newly created sources file is treated as the original
/etc/apt/sources.list.

- If the process completes successfully, rename the new file to the original
sources.list. If the process terminates abruptly or machine reboots,
sources.list will remain unchanged. This will also the dist upgrade process to
be restarted (and hopefully continued).

Tests:

- On a fresh stable container, running dist-upgrade succeeds.

- While dist-upgrade is running, /etc/apt/sources.list is unmodified. After the
operation is successfully completed, /etc/apt/sources.list has been updates
successfully. If the operation fails, /etc/apt/sources.list remains unmodified.

- During the run the following are run:
  - apt update
  - package holds
  - debconf selections
  - full-upgrade
  - autoremove
  - unattended-upgrades
  - restarting freedombox service
  - waiting 10 minutes
  - apt update

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/upgrades/distupgrade.py

@@ -17,8 +17,6 @@ from . import utils
 
 logger = logging.getLogger(__name__)
 
-SOURCES_LIST = '/etc/apt/sources.list'
-
 OBSOLETE_PACKAGES: list[str] = []
 
 PACKAGES_WITH_PROMPTS = ['firewalld', 'minidlna', 'radicale']
@@ -28,6 +26,9 @@ PRE_DEBCONF_SELECTIONS: list[str] = [
     'grub-pc grub-pc/install_devices_empty boolean true'
 ]
 
+sources_list = pathlib.Path('/etc/apt/sources.list')
+temp_sources_list = pathlib.Path('/etc/apt/sources.list.fbx-dist-upgrade')
+
 
 def _apt_run(arguments):
     """Run an apt command and ensure that output is written to stdout."""
@@ -39,8 +40,9 @@ def _sources_list_update(old_codename: str, new_codename: str):
     logger.info('Upgrading from %s to %s...', old_codename, new_codename)
     aug = augeas.Augeas(flags=augeas.Augeas.NO_LOAD +
                         augeas.Augeas.NO_MODL_AUTOLOAD)
-    aug.transform('aptsources', SOURCES_LIST)
-    aug.set('/augeas/context', '/files' + SOURCES_LIST)
+    aug.transform('aptsources', str(sources_list))
+    aug.set('/augeas/context', '/files' + str(sources_list))
+    aug.set('/augeas/save', 'newfile')  # Save to a new file
     aug.load()
 
     for match_ in aug.match('*'):
@@ -55,6 +57,9 @@ def _sources_list_update(old_codename: str, new_codename: str):
 
     aug.save()
 
+    aug_path = sources_list.with_suffix('.list.augnew')
+    aug_path.rename(temp_sources_list)
+
 
 def _get_new_codename(test_upgrade: bool) -> str | None:
     """Return the codename for the next release."""
@@ -258,6 +263,21 @@ def _wait():
     time.sleep(10 * 60)
 
 
+def _trigger_on_complete():
+    """Trigger the on complete step in a separate service."""
+    # The dist-upgrade process will be run /etc/apt/sources.list file bind
+    # mounted on with a modified file. So, moving modified file to the original
+    # file will not be possible. For that, we need to launch a new process with
+    # a different systemd service (which does not have the bind mounts).
+    logger.info('Triggering on-complete to commit sources.lists')
+    subprocess.run([
+        'systemd-run', '--unit=freedombox-dist-upgrade-on-complete',
+        '--description=Finish up upgrade to new stable Debian release',
+        '/usr/share/plinth/actions/actions', 'upgrades',
+        'dist_upgrade_on_complete', '--no-args'
+    ], check=True)
+
+
 def _logging_setup():
     """Log to journal via console logging.
 
@@ -287,27 +307,37 @@ def perform():
     _freedombox_restart()
     _wait()
     _apt_update()
+    _trigger_on_complete()
 
 
 def start_service(test_upgrade: bool):
     """Create dist upgrade service and start it."""
-    old_codename, new_codename = _check(test_upgrade)
-
-    _sources_list_update(old_codename, new_codename)
-
+    # Cleanup old service
     old_service_path = pathlib.Path(
         '/run/systemd/system/freedombox-dist-upgrade.service')
     if old_service_path.exists():
         old_service_path.unlink(missing_ok=True)
         action_utils.service_daemon_reload()
 
+    old_codename, new_codename = _check(test_upgrade)
+
+    _sources_list_update(old_codename, new_codename)
+
     args = [
         '--unit=freedombox-dist-upgrade',
         '--description=Upgrade to new stable Debian release',
-        '--property=KillMode=process',
-        '--property=TimeoutSec=12hr',
+        '--property=KillMode=process', '--property=TimeoutSec=12hr',
+        f'--property=BindPaths={temp_sources_list}:{sources_list}'
     ]
     subprocess.run(['systemd-run'] + args + [
         'systemd-inhibit', '/usr/share/plinth/actions/actions', 'upgrades',
         'dist_upgrade', '--no-args'
     ], check=True)
+
+
+def on_complete():
+    """Perform cleanup operations."""
+    _logging_setup()
+    logger.info('Dist upgrade complete.')
+    logger.info('Committing changes to /etc/apt/sources.list')
+    temp_sources_list.rename(sources_list)

plinth/modules/upgrades/privileged.py

@@ -251,3 +251,9 @@ def start_dist_upgrade(test: bool = False):
 def dist_upgrade():
     """Perform major distribution upgrade."""
     distupgrade.perform()
+
+
+@privileged
+def dist_upgrade_on_complete():
+    """Perform cleanup operations after distribution upgrade."""
+    distupgrade.on_complete()

plinth/modules/upgrades/tests/test_distupgrade.py

@@ -57,20 +57,21 @@ deb https://deb.debian.org/debian bookwormish main
 '''  # noqa: E501
 
     sources_list = tmp_path / 'sources.list'
-    sources_list.write_text(original)
-    with patch('plinth.modules.upgrades.distupgrade.SOURCES_LIST',
-               str(sources_list)):
+    temp_sources_list = tmp_path / 'sources.list.fbx-dist-upgrade'
+
+    module = 'plinth.modules.upgrades.distupgrade'
+    with patch(f'{module}.sources_list', sources_list), \
+         patch(f'{module}.temp_sources_list', temp_sources_list):
+        sources_list.write_text(original)
         distupgrade._sources_list_update('bookworm', 'trixie')
 
-    assert sources_list.read_text() == modified
+        assert temp_sources_list.read_text() == modified
 
-    original = re.sub(r'bookworm([ -])', r'stable\1', original)
-    sources_list.write_text(original)
-    with patch('plinth.modules.upgrades.distupgrade.SOURCES_LIST',
-               str(sources_list)):
+        original = re.sub(r'bookworm([ -])', r'stable\1', original)
+        sources_list.write_text(original)
         distupgrade._sources_list_update('bookworm', 'trixie')
 
-    assert sources_list.read_text() == modified
+        assert temp_sources_list.read_text() == modified
 
 
 @patch('plinth.modules.upgrades.utils.get_http_protocol')
@@ -320,3 +321,30 @@ def test_wait(sleep):
     """Test that sleeping works."""
     distupgrade._wait()
     sleep.assert_called_with(600)
+
+
+@patch('subprocess.run')
+def test_trigger_on_complete(run):
+    """Test triggering post completion process."""
+    distupgrade._trigger_on_complete()
+    run.assert_called_with([
+        'systemd-run', '--unit=freedombox-dist-upgrade-on-complete',
+        '--description=Finish up upgrade to new stable Debian release',
+        '/usr/share/plinth/actions/actions', 'upgrades',
+        'dist_upgrade_on_complete', '--no-args'
+    ], check=True)
+
+
+def test_on_complete(tmp_path):
+    """Test that /etc/apt/sources.list is committed."""
+    sources_list = tmp_path / 'sources.list'
+    sources_list.write_text('before')
+    temp_sources_list = tmp_path / 'sources.list.fbx-dist-upgrade'
+    temp_sources_list.write_text('after')
+
+    module = 'plinth.modules.upgrades.distupgrade'
+    with patch(f'{module}.sources_list', sources_list), \
+         patch(f'{module}.temp_sources_list', temp_sources_list):
+        distupgrade.on_complete()
+        assert sources_list.read_text() == 'after'
+        assert not temp_sources_list.exists()