nextcloud: Refactor container creation code

- Reduce nesting necessary nesting.

- Add some type annotations.

- Simplify writing command output to a file by passing file handle to
subprocess.run().

- Create a path for volume to eliminate some duplication.

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

plinth/action_utils.py

@@ -489,69 +489,60 @@ def is_package_manager_busy():
         return False
 
 
-def podman_run(network_name, subnet, bridge_ip, host_port, container_port,
+def podman_run(network_name: str, subnet: str, bridge_ip: str, host_port: str,
-               container_ip, volume_name, container_name, image_name,
+               container_port: str, container_ip: str, volume_name: str,
-               extra_run_options=None, extra_network_options=None):
+               container_name: str, image_name: str,
+               extra_run_options: list[str] | None = None,
+               extra_network_options: list[str] | None = None):
     """Remove, recreate and run a podman container."""
     try:
         service_stop(container_name)
         subprocess.run(['podman', 'network', 'rm', '--force', network_name],
                        check=False)
-    finally:
+    except subprocess.CalledProcessError:
-        network_create_command = [
+        pass
-            'podman', 'network', 'create', '--driver', 'bridge', '--subnet',
-            subnet, '--gateway', bridge_ip, '--dns', bridge_ip,
-            '--interface-name', network_name, network_name
-        ]
-        if extra_network_options:
-            network_create_command.extend(extra_network_options)
-        # create bridge network
-        subprocess.run(network_create_command, check=True)
 
-        run_command = [
+    network_create_command = [
-            'podman',
+        'podman', 'network', 'create', '--driver', 'bridge', '--subnet',
-            'run',
+        subnet, '--gateway', bridge_ip, '--dns', bridge_ip, '--interface-name',
-            '--detach',
+        network_name, network_name
-            # Only listen on localhost. This is to prevent
+    ] + (extra_network_options or [])
-            # exposing the host port to the internet
-            '--publish',
-            f'127.0.0.1:{host_port}:{container_port}',
-            '--network',
-            network_name,
-            '--ip',
-            container_ip,
-            '--volume',
-            f'{volume_name}:/var/www/html',
-            '--name',
-            container_name,
-            '--restart',
-            'unless-stopped',
-            '--quiet',
-            # enable automatic updates
-            '--label',
-            'io.containers.autoupdate=registry',
-            # If another container with the same name already
-            # exists, replace and remove it.
-            '--replace',
-        ] + (extra_run_options or []) + [image_name]
-        subprocess.run(run_command, check=True)
 
-        systemd_content = subprocess.run(
+    # Create bridge network
+    subprocess.run(network_create_command, check=True)
+
+    args = [
+        'podman', 'run', '--detach', '--network', network_name, '--ip',
+        container_ip, '--volume', f'{volume_name}:/var/www/html', '--name',
+        container_name, '--restart', 'unless-stopped', '--quiet'
+    ]
+    # Only listen on localhost. This is to prevent exposing the host port to
+    # the internet.
+    args += ['--publish', f'127.0.0.1:{host_port}:{container_port}']
+    # Enable automatic updates.
+    args += ['--label', 'io.containers.autoupdate=registry']
+    # If another container with the same name already exists, replace and
+    # remove it.
+    args += ['--replace']
+    args += (extra_run_options or []) + [image_name]
+    subprocess.run(args, check=True)
+
+    # Create service file for starting/stopping container using systemd
+    service_file = pathlib.Path(
+        '/etc/systemd/system') / f'{container_name}.service'
+    with service_file.open('wb') as file_handle:
+        subprocess.run(
             ['podman', 'generate', 'systemd', '--new', container_name],
-            capture_output=True, check=True).stdout.decode()
+            stdout=file_handle, check=True)
-        pathlib.Path('/etc/systemd/system/'
-                     '{container_name}.service').write_text(
-                         systemd_content, encoding='utf-8')
-        service_daemon_reload()
 
 
-def podman_uninstall(container_name, network_name, volume_name, image_name):
+def podman_uninstall(container_name: str, network_name: str, volume_name: str,
+                     image_name: str):
     """Remove a podman container's components and systemd unit."""
-    components = [('network', network_name), ('volume', volume_name),
+    subprocess.run(['podman', 'network', 'rm', network_name], check=True)
-                  ('image', image_name)]
+    subprocess.run(['podman', 'volume', 'rm', volume_name], check=True)
-    for component in components:
+    subprocess.run(['podman', 'image', 'rm', image_name], check=True)
-        subprocess.run(['podman', component[0], 'rm', component[1]],
+    service_file = pathlib.Path(
-                       check=True)
+        '/etc/systemd/system/') / f'{container_name}.service'
-    pathlib.Path('/etc/systemd/system/{container_name}.service').unlink(
+    service_file.unlink(missing_ok=True)
-        missing_ok=True)
     service_daemon_reload()

plinth/modules/nextcloud/manifest.py

@@ -49,7 +49,7 @@ clients = [{
 backup = {
     'data': {
         'directories': [
-            '/var/lib/containers/storage/volumes/nextcloud-volume-fbx/'
+            '/var/lib/containers/storage/volumes/nextcloud-volume-freedombox/'
         ],
         'files': [
             '/var/lib/plinth/backups-data/nextcloud-database.sql',

plinth/modules/nextcloud/privileged.py

@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
 """Configure Nextcloud."""
 
-import os
 import pathlib
 import random
 import re
@@ -19,7 +18,7 @@ NETWORK_NAME = 'nextcloud-fbx'
 BRIDGE_IP = '172.16.16.1'
 CONTAINER_IP = '172.16.16.2'
 CONTAINER_NAME = 'nextcloud-freedombox'
-VOLUME_NAME = 'nextcloud-volume-fbx'
+VOLUME_NAME = 'nextcloud-volume-freedombox'
 IMAGE_NAME = 'docker.io/library/nextcloud:stable-apache'
 
 DB_HOST = 'localhost'
@@ -27,6 +26,8 @@ DB_NAME = 'nextcloud_fbx'
 DB_USER = 'nextcloud_fbx'
 GUI_ADMIN = 'nextcloud-admin'
 
+_volume_path = pathlib.Path(
+    '/var/lib/containers/storage/volumes/') / VOLUME_NAME
 _socket_config_file = pathlib.Path('/etc/mysql/mariadb.conf.d/'
                                    '99-freedombox.cnf')
 _systemd_location = pathlib.Path('/etc/systemd/system/')
@@ -63,9 +64,9 @@ def setup():
     # Wait until CAN_INSTALL file is available.
     timeout = 300
     while timeout > 0:
-        if os.path.exists('/var/lib/containers/storage/volumes/'
+        if (_volume_path / '_data/config/CAN_INSTALL').exists():
-                          'nextcloud-volume-fbx/_data/config/CAN_INSTALL'):
             break
+
         timeout = timeout - 1
         time.sleep(1)
 
@@ -198,9 +199,7 @@ FLUSH PRIVILEGES;
 
 
 def _nextcloud_setup_wizard(db_password, admin_password):
-    admin_data_dir = pathlib.Path(
+    admin_data_dir = _volume_path / '_data/data' / GUI_ADMIN
-        '/var/lib/containers/storage/volumes/nextcloud-volume-fbx/'
-        f'_data/data/{GUI_ADMIN}')
     if not admin_data_dir.exists():
         _run_occ('maintenance:install', '--database=mysql',
                  f'--database-name={DB_NAME}', f'--database-host={BRIDGE_IP}',
@@ -368,8 +367,7 @@ def _get_dbpassword():
 
     OCC cannot run unless Nextcloud can already connect to the database.
     """
-    config_file = ('/var/lib/containers/storage/volumes/nextcloud-volume-fbx'
+    config_file = _volume_path / '_data/config/config.php'
-                   '/_data/config/config.php')
     with open(config_file, 'r', encoding='utf-8') as config:
         config_contents = config.read()
 
@@ -381,9 +379,7 @@ def _get_dbpassword():
 
 def _create_redis_config(password):
     """Create a php file for Redis configuration."""
-    config_file = pathlib.Path(
+    config_file = _volume_path / '_data/config/freedombox.config.php'
-        '/var/lib/containers/storage/volumes/nextcloud-volume-fbx/_data/'
-        'config/freedombox.config.php')
     file_content = f'''<?php
 $CONFIG = [
 'filelocking.enabled' => true,