upgrades: Refactor dist upgrade process

No change in functionality.

Signed-off-by: James Valleroy <jvalleroy@mailbox.org>
[sunil: Fix copy/paste error with indentation in start_dist_upgrade_service]
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>

actions/upgrades

@@ -89,6 +89,26 @@ Pin: release a=buster-backports
 Pin-Priority: 500
 '''
 
+DIST_UPGRADE_OBSOLETE_PACKAGES = ['libgcc1']
+
+DIST_UPGRADE_PACKAGES_WITH_PROMPTS = [
+    'firewalld', 'mumble-server', 'radicale', 'roundcube-core'
+]
+
+# These are packages that may not be available in the next stable release.
+DIST_UPGRADE_OPTIONAL_PACKAGES_WITH_PROMPTS = ['tt-rss']
+
+DIST_UPGRADE_PRE_INSTALL_PACKAGES = [
+    'base-files', 'python3-systemd', 'unattended-upgrades'
+]
+
+DIST_UPGRADE_PRE_DEBCONF_SELECTIONS = [
+    # Tell grub-pc to continue without installing grub again.
+    'grub-pc grub-pc/install_devices_empty boolean true'
+]
+
+DIST_UPGRADE_REQUIRED_FREE_SPACE = 5000000
+
 DIST_UPGRADE_SERVICE = '''
 [Unit]
 Description=Upgrade to new stable Debian release
@@ -138,15 +158,18 @@ def parse_arguments():
     return parser.parse_args()
 
 
+def _release_held_freedombox():
+    """In case freedombox package was left in held state by an interrupted
+    process, release it."""
+    if apt_hold_flag.exists() and not is_package_manager_busy():
+        apt_unhold_freedombox()
+
+
 def _run():
     """Run unattended-upgrades"""
     subprocess.run(['dpkg', '--configure', '-a'], check=False)
     run_apt_command(['--fix-broken', 'install'])
-
+    _release_held_freedombox()
-    # In case freedombox package was left in held state by an
-    # interrupted process, release it.
-    if apt_hold_flag.exists() and not is_package_manager_busy():
-        apt_unhold_freedombox()
 
     subprocess.Popen(['systemctl', 'start', 'freedombox-manual-upgrade'],
                      stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL,
@@ -326,6 +349,13 @@ def _add_apt_preferences():
             file_handle.write(APT_PREFERENCES_APPS)
 
 
+def _is_sufficient_free_space():
+    """Return whether there is sufficient free space for dist upgrade."""
+    output = subprocess.check_output(['df', '--output=avail', '/'])
+    free_space = int(output.decode().split('\n')[1])
+    return free_space >= DIST_UPGRADE_REQUIRED_FREE_SPACE
+
+
 def _check_dist_upgrade(test_upgrade=False):
     """Check for new stable release, if updates are enabled, and if there is
     enough free space for the dist upgrade.
@@ -377,9 +407,7 @@ def _check_dist_upgrade(test_upgrade=False):
     if check_dist == 'testing' and not test_upgrade:
         return (False, 'test-not-set')
 
-    output = subprocess.check_output(['df', '--output=avail', '/'])
+    if not _is_sufficient_free_space():
-    free_space = int(output.decode().split('\n')[1])
-    if free_space < 5000000:
         return (False, 'not-enough-free-space')
 
     logging.info('Upgrading from %s to %s...', dist, codename)
@@ -405,29 +433,44 @@ def _check_dist_upgrade(test_upgrade=False):
     return (True, 'started-dist-upgrade')
 
 
-def _perform_dist_upgrade():
+def _take_snapshot_and_disable():
-    """Perform upgrade to next release of Debian."""
+    """Take a snapshot if supported and enabled, then disable snapshots.
-    # Take a snapshot if supported and enabled, then disable snapshots.
+
-    snapshots_supported = snapshot_is_supported()
+    Return whether snapshots shall be re-enabled at the end."""
-    if snapshots_supported:
+    if snapshot_is_supported():
         print('Taking a snapshot before dist upgrade...', flush=True)
         subprocess.run(['/usr/share/plinth/actions/snapshot', 'create'],
                        check=True)
         aug = snapshot_load_augeas()
-        apt_snapshots_enabled = is_apt_snapshots_enabled(aug)
+        if is_apt_snapshots_enabled(aug):
-        if apt_snapshots_enabled:
             print('Disable apt snapshots during dist upgrade...', flush=True)
             subprocess.run([
                 '/usr/share/plinth/actions/snapshot', 'disable-apt-snapshot',
                 'yes'
             ], check=True)
+            return True
         else:
             print('Apt snapshots already disabled.', flush=True)
     else:
         print('Snapshots are not supported, skip taking a snapshot.',
               flush=True)
 
-    # If searx is enabled, disable it until we can upgrade it properly.
+    return False
+
+
+def _restore_snapshots_config(reenable):
+    """Restore original snapshots configuration."""
+    if reenable:
+        print('Re-enable apt snapshots...', flush=True)
+        subprocess.run([
+            '/usr/share/plinth/actions/snapshot', 'disable-apt-snapshot', 'no'
+        ], check=True)
+
+
+def _disable_searx():
+    """If searx is enabled, disable it until we can upgrade it properly.
+
+    Return whether searx was originally enabled."""
     searx_is_enabled = pathlib.Path(
         '/etc/uwsgi/apps-enabled/searx.ini').exists()
     if searx_is_enabled:
@@ -437,54 +480,75 @@ def _perform_dist_upgrade():
             'searx'
         ], check=True)
 
+    return searx_is_enabled
+
+
+def _update_searx(reenable):
+    """If searx is installed, update search engines list.
+    Re-enable if previously enabled."""
+    if pathlib.Path('/etc/searx/settings.yml').exists():
+        print('Updating searx search engines list...', flush=True)
+        subprocess.run(['/usr/share/plinth/actions/searx', 'setup'],
+                       check=True)
+        if reenable:
+            print('Re-enabling searx after upgrade...', flush=True)
+            subprocess.run([
+                '/usr/share/plinth/actions/apache', 'uwsgi-enable', '--name',
+                'searx'
+            ], check=True)
+
+
+def _perform_dist_upgrade():
+    """Perform upgrade to next release of Debian."""
+    reenable_snapshots = _take_snapshot_and_disable()
+    reenable_searx = _disable_searx()
+
     # Hold freedombox package during entire dist upgrade.
     print('Holding freedombox package...', flush=True)
     with apt_hold_freedombox():
         print('Updating Apt cache...', flush=True)
         run_apt_command(['update'])
 
-        print('Upgrading base-files and unattended-upgrades...', flush=True)
+        # Install packages that are necessary for unattended-upgrades
-        run_apt_command(['install', 'base-files'])
+        # to start the dist upgrade.
-        run_apt_command(['install', 'python3-systemd', 'unattended-upgrades'])
+        print(f'Upgrading packages: {DIST_UPGRADE_PRE_INSTALL_PACKAGES}...',
+              flush=True)
+        run_apt_command(['install'] + DIST_UPGRADE_PRE_INSTALL_PACKAGES)
 
-        # Tell grub-pc to continue without installing grub again.
+        # Pre-set debconf selections if they are required during the
-        print('Set grub-pc to not require re-installing grub...', flush=True)
+        # dist upgrade.
-        debconf_set_selections(
+        print(
-            ['grub-pc grub-pc/install_devices_empty boolean true'])
+            f'Setting debconf selections: '
+            f'{DIST_UPGRADE_PRE_DEBCONF_SELECTIONS}', flush=True)
+        debconf_set_selections(DIST_UPGRADE_PRE_DEBCONF_SELECTIONS)
 
+        # This will upgrade most of the packages.
         print('Running unattended-upgrade...', flush=True)
         subprocess.run(['unattended-upgrade', '--verbose'], check=False)
 
         # Remove obsolete packages that may prevent other packages from
         # upgrading.
-        print('Removing libgcc1...', flush=True)
+        print(f'Removing packages: {DIST_UPGRADE_OBSOLETE_PACKAGES}...',
-        run_apt_command(['remove', 'libgcc1'])
+              flush=True)
+        run_apt_command(['remove'] + DIST_UPGRADE_OBSOLETE_PACKAGES)
 
         # Hold packages known to have conffile prompts. FreedomBox service
         # will handle their upgrade later.
-        packages_with_prompts = [
-            'firewalld', 'mumble-server', 'radicale', 'roundcube-core'
-        ]
         print(
             'Holding packages with conffile prompts: ' +
-            ', '.join(packages_with_prompts) + '...', flush=True)
+            ', '.join(DIST_UPGRADE_PACKAGES_WITH_PROMPTS) + '...', flush=True)
-        with apt_hold(packages_with_prompts):
+        with apt_hold(DIST_UPGRADE_PACKAGES_WITH_PROMPTS):
-            print('Holding tt-rss package if available...', flush=True)
+            # TODO: Support holding more than 1 package here.
-            with apt_hold(['tt-rss'], ignore_errors=True):
+            print(
+                f'Holding packages if available: '
+                f'{DIST_UPGRADE_OPTIONAL_PACKAGES_WITH_PROMPTS[0]}...',
+                flush=True)
+            with apt_hold([DIST_UPGRADE_OPTIONAL_PACKAGES_WITH_PROMPTS[0]],
+                          ignore_errors=True):
                 print('Running apt full-upgrade...', flush=True)
                 run_apt_command(['full-upgrade'])
 
-        # If searx is installed, update search engines list.
+        _update_searx(reenable_searx)
-        if pathlib.Path('/etc/searx/settings.yml').exists():
-            print('Updating searx search engines list...', flush=True)
-            subprocess.run(['/usr/share/plinth/actions/searx', 'setup'],
-                           check=True)
-            if searx_is_enabled:
-                print('Re-enabling searx after upgrade...', flush=True)
-                subprocess.run([
-                    '/usr/share/plinth/actions/apache', 'uwsgi-enable',
-                    '--name', 'searx'
-                ], check=True)
 
         print('Running apt autoremove...', flush=True)
         run_apt_command(['autoremove'])
@@ -494,12 +558,7 @@ def _perform_dist_upgrade():
     print('Running unattended-upgrade...', flush=True)
     subprocess.run(['unattended-upgrade', '--verbose'], check=False)
 
-    # Restore original snapshots configuration.
+    _restore_snapshots_config(reenable_snapshots)
-    if snapshots_supported and apt_snapshots_enabled:
-        print('Re-enable apt snapshots...', flush=True)
-        subprocess.run([
-            '/usr/share/plinth/actions/snapshot', 'disable-apt-snapshot', 'no'
-        ], check=True)
 
     # Restart FreedomBox service to ensure it is using the latest
     # dependencies.
@@ -532,27 +591,29 @@ def subcommand_activate_backports(arguments):
     _check_and_backports_sources(arguments.develop)
 
 
+def _start_dist_upgrade_service():
+    """Create dist upgrade service and start it."""
+    with open(DIST_UPGRADE_SERVICE_PATH, 'w') as service_file:
+        service_file.write(DIST_UPGRADE_SERVICE)
+
+    service_daemon_reload()
+    subprocess.Popen(['systemctl', 'start', 'freedombox-dist-upgrade'],
+                     stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL,
+                     stderr=subprocess.DEVNULL, close_fds=True,
+                     start_new_session=True)
+
+
 def subcommand_start_dist_upgrade(arguments):
     """Start dist upgrade process.
 
     Check if a new stable release is available, and start dist-upgrade process
     if updates are enabled.
     """
-    # In case freedombox package was left in held state by an
+    _release_held_freedombox()
-    # interrupted process, release it.
-    if apt_hold_flag.exists() and not is_package_manager_busy():
-        apt_unhold_freedombox()
 
     upgrade_ready, reason = _check_dist_upgrade(arguments.test)
     if upgrade_ready:
-        with open(DIST_UPGRADE_SERVICE_PATH, 'w') as service_file:
+        _start_dist_upgrade_service()
-            service_file.write(DIST_UPGRADE_SERVICE)
-
-        service_daemon_reload()
-        subprocess.Popen(['systemctl', 'start', 'freedombox-dist-upgrade'],
-                         stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL,
-                         stderr=subprocess.DEVNULL, close_fds=True,
-                         start_new_session=True)
 
     print(
         json.dumps({