From 07c29dca7ec7bc082db969babe5e6d9cdd0c60d1 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Sun, 31 Mar 2024 21:03:16 -0700 Subject: [PATCH] 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 Reviewed-by: James Valleroy --- plinth/action_utils.py | 99 ++++++++++++-------------- plinth/modules/nextcloud/manifest.py | 2 +- plinth/modules/nextcloud/privileged.py | 20 +++--- 3 files changed, 54 insertions(+), 67 deletions(-) diff --git a/plinth/action_utils.py b/plinth/action_utils.py index c6286632b..c4d39e582 100644 --- a/plinth/action_utils.py +++ b/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, - container_ip, volume_name, container_name, image_name, - extra_run_options=None, extra_network_options=None): +def podman_run(network_name: str, subnet: str, bridge_ip: str, host_port: str, + container_port: str, container_ip: str, volume_name: str, + 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: - network_create_command = [ - '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) + except subprocess.CalledProcessError: + pass - run_command = [ - 'podman', - 'run', - '--detach', - # Only listen on localhost. This is to prevent - # 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) + network_create_command = [ + 'podman', 'network', 'create', '--driver', 'bridge', '--subnet', + subnet, '--gateway', bridge_ip, '--dns', bridge_ip, '--interface-name', + network_name, network_name + ] + (extra_network_options or []) - 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() - pathlib.Path('/etc/systemd/system/' - '{container_name}.service').write_text( - systemd_content, encoding='utf-8') - service_daemon_reload() + stdout=file_handle, check=True) -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), - ('image', image_name)] - for component in components: - subprocess.run(['podman', component[0], 'rm', component[1]], - check=True) - pathlib.Path('/etc/systemd/system/{container_name}.service').unlink( - missing_ok=True) + subprocess.run(['podman', 'network', 'rm', network_name], check=True) + subprocess.run(['podman', 'volume', 'rm', volume_name], check=True) + subprocess.run(['podman', 'image', 'rm', image_name], check=True) + service_file = pathlib.Path( + '/etc/systemd/system/') / f'{container_name}.service' + service_file.unlink(missing_ok=True) service_daemon_reload() diff --git a/plinth/modules/nextcloud/manifest.py b/plinth/modules/nextcloud/manifest.py index 3967ed444..7d3543ea3 100644 --- a/plinth/modules/nextcloud/manifest.py +++ b/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', diff --git a/plinth/modules/nextcloud/privileged.py b/plinth/modules/nextcloud/privileged.py index 21d9a13c9..a74a3dd2e 100644 --- a/plinth/modules/nextcloud/privileged.py +++ b/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/' - 'nextcloud-volume-fbx/_data/config/CAN_INSTALL'): + if (_volume_path / '_data/config/CAN_INSTALL').exists(): 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( - '/var/lib/containers/storage/volumes/nextcloud-volume-fbx/' - f'_data/data/{GUI_ADMIN}') + admin_data_dir = _volume_path / '_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' - '/_data/config/config.php') + config_file = _volume_path / '_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( - '/var/lib/containers/storage/volumes/nextcloud-volume-fbx/_data/' - 'config/freedombox.config.php') + config_file = _volume_path / '_data/config/freedombox.config.php' file_content = f''' true,