From 9772faa47b26d34fd3983b8f3ed8d9b8467ac388 Mon Sep 17 00:00:00 2001 From: Sharpz7 Date: Wed, 20 Aug 2025 16:02:20 +0000 Subject: [PATCH 1/4] Added control of docker networks of containers --- README.md | 11 +++- pman/config.py | 9 ++-- pman/dockermgr.py | 19 ++++--- tests/test_dockermgr.py | 117 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 10 deletions(-) create mode 100644 tests/test_dockermgr.py diff --git a/README.md b/README.md index aba3f16..0d5f2ae 100755 --- a/README.md +++ b/README.md @@ -130,12 +130,13 @@ https://github.com/containers/podman/blob/main/troubleshooting.md#symptom-23 | `STOREBASE` | where job data is stored, valid when `STORAGE_TYPE=host`, conflicts with `VOLUME_NAME` | | `VOLUME_NAME` | name of data volume, valid when `STORAGE_TYPE=docker_local_volume` or `STORAGE_TYPE=kubernetes_pvc` | | `PFCON_SELECTOR` | label on the pfcon container, may be specified for pman to self-discover `VOLUME_NAME` (default: `org.chrisproject.role=pfcon`) | -| `CONTAINER_USER` | Set job container user in the form `UID:GID`, may be a range for random values | +| `CONTAINER_USER` | Set job container user in the form `UID:GID`, may be a range for random values | | `ENABLE_HOME_WORKAROUND` | If set to "yes" then set job environment variable `HOME=/tmp` | | `SHM_SIZE` | Size of `/dev/shm` in mebibytes. (Supported only in Docker, Podman, and Kubernetes.) | | `JOB_LABELS` | CSV list of key=value pairs, labels to apply to container jobs | | `JOB_LOGS_TAIL` | (int) maximum size of job logs | | `IGNORE_LIMITS` | If set to "yes" then do not set resource limits on container jobs (for making things work without effort) | +| `DOCKER_NETWORKS` | Comma-separated list of Docker networks to connect containers to (e.g., "network1,network2") | | `REMOVE_JOBS` | If set to "no" then pman will not delete jobs (for debugging) | [flask docs]: https://flask.palletsprojects.com/en/2.1.x/config/#SECRET_KEY @@ -162,6 +163,14 @@ PersistentVolumeClaim configured as ReadWriteMany. In cases where the volume is only writable to a specific UNIX user, such as a NFS-backed volume, `CONTAINER_USER` can be used as a workaround. +### Docker-Specific Options + +Applicable when `CONTAINER_ENV=docker` + +| Environment Variable | Description | +|---------------------------|-------------------------------------------------| +| `DOCKER_NETWORKS` | Comma-separated list of Docker networks to connect containers to (e.g., "network1,network2") | + ### Kubernetes-Specific Options Applicable when `CONTAINER_ENV=kubernetes` diff --git a/pman/config.py b/pman/config.py index 20d1f9b..6f5ed74 100755 --- a/pman/config.py +++ b/pman/config.py @@ -1,12 +1,11 @@ +from importlib.metadata import Distribution from logging.config import dictConfig from environs import Env -from importlib.metadata import Distribution - -from pman.memsize import Memsize from pman._helpers import get_storebase_from_docker +from pman.memsize import Memsize pkg = Distribution.from_name(__package__) @@ -32,6 +31,10 @@ def __init__(self): shm_size = env.int('SHM_SIZE', None) self.SHM_SIZE = None if shm_size is None else Memsize(shm_size) + # Docker networks configuration + docker_networks = env('DOCKER_NETWORKS', '') + self.DOCKER_NETWORKS = None if not docker_networks else docker_networks.split(',') + self.CONTAINER_ENV = env('CONTAINER_ENV', 'docker') if self.CONTAINER_ENV == 'podman': # podman is just an alias for docker self.CONTAINER_ENV = 'docker' diff --git a/pman/dockermgr.py b/pman/dockermgr.py index a6ff4db..01f7339 100755 --- a/pman/dockermgr.py +++ b/pman/dockermgr.py @@ -1,13 +1,14 @@ import shlex -from typing import List, Optional, AnyStr +from typing import AnyStr, List, Optional +import docker from docker import DockerClient -from docker.types import DeviceRequest from docker.models.containers import Container +from docker.types import DeviceRequest -from pman.abstractmgr import (AbstractManager, Image, JobName, ResourcesDict, - MountsDict, JobInfo, TimeStamp, ManagerException, JobStatus) -import docker +from pman.abstractmgr import (AbstractManager, Image, JobInfo, JobName, + JobStatus, ManagerException, MountsDict, + ResourcesDict, TimeStamp) class DockerManager(AbstractManager[Container]): @@ -67,6 +68,11 @@ def schedule_job(self, image: Image, command: List[str], name: JobName, if (s := self.config.get('SHM_SIZE')) is not None: shm_size['shm_size'] = s.as_mb() + # Docker networks configuration + networks = {} + if (docker_networks := self.config.get('DOCKER_NETWORKS')) is not None: + networks['network'] = docker_networks[0] if len(docker_networks) == 1 else docker_networks + return self.__docker.containers.run( image=image, command=command, @@ -78,7 +84,8 @@ def schedule_job(self, image: Image, command: List[str], name: JobName, **limits, **user_spec, **shm_size, - **volumes + **volumes, + **networks ) def get_job(self, name: JobName) -> Container: diff --git a/tests/test_dockermgr.py b/tests/test_dockermgr.py new file mode 100644 index 0000000..9f5550b --- /dev/null +++ b/tests/test_dockermgr.py @@ -0,0 +1,117 @@ +from unittest.mock import Mock, patch + +import pytest + +from pman.dockermgr import DockerManager + + +class TestDockerManager: + """Test cases for DockerManager class.""" + + def test_docker_networks_single_network(self): + """Test that a single network is correctly configured.""" + config = {'DOCKER_NETWORKS': ['test-network']} + mock_docker_client = Mock() + + with patch('docker.from_env', return_value=mock_docker_client): + manager = DockerManager(config, mock_docker_client) + + # Mock the containers.run method + mock_container = Mock() + mock_docker_client.containers.run.return_value = mock_container + + # Test parameters + image = 'test-image' + command = ['test', 'command'] + name = 'test-job' + resources_dict = {'number_of_workers': 1, 'cpu_limit': 1, 'memory_limit': 100, 'gpu_limit': 0} + env = [] + uid = None + gid = None + mounts_dict = { + 'inputdir_source': '/input', + 'inputdir_target': '/share/incoming', + 'outputdir_source': '/output', + 'outputdir_target': '/share/outgoing' + } + + # Call schedule_job + result = manager.schedule_job(image, command, name, resources_dict, env, uid, gid, mounts_dict) + + # Verify that containers.run was called with the correct network parameter + mock_docker_client.containers.run.assert_called_once() + call_args = mock_docker_client.containers.run.call_args + assert call_args[1]['network'] == 'test-network' + assert result == mock_container + + def test_docker_networks_multiple_networks(self): + """Test that multiple networks are correctly configured.""" + config = {'DOCKER_NETWORKS': ['network1', 'network2']} + mock_docker_client = Mock() + + with patch('docker.from_env', return_value=mock_docker_client): + manager = DockerManager(config, mock_docker_client) + + # Mock the containers.run method + mock_container = Mock() + mock_docker_client.containers.run.return_value = mock_container + + # Test parameters + image = 'test-image' + command = ['test', 'command'] + name = 'test-job' + resources_dict = {'number_of_workers': 1, 'cpu_limit': 1, 'memory_limit': 100, 'gpu_limit': 0} + env = [] + uid = None + gid = None + mounts_dict = { + 'inputdir_source': '/input', + 'inputdir_target': '/share/incoming', + 'outputdir_source': '/output', + 'outputdir_target': '/share/outgoing' + } + + # Call schedule_job + result = manager.schedule_job(image, command, name, resources_dict, env, uid, gid, mounts_dict) + + # Verify that containers.run was called with the correct network parameter + mock_docker_client.containers.run.assert_called_once() + call_args = mock_docker_client.containers.run.call_args + assert call_args[1]['network'] == ['network1', 'network2'] + assert result == mock_container + + def test_docker_networks_none(self): + """Test that no network parameter is passed when DOCKER_NETWORKS is not configured.""" + config = {} + mock_docker_client = Mock() + + with patch('docker.from_env', return_value=mock_docker_client): + manager = DockerManager(config, mock_docker_client) + + # Mock the containers.run method + mock_container = Mock() + mock_docker_client.containers.run.return_value = mock_container + + # Test parameters + image = 'test-image' + command = ['test', 'command'] + name = 'test-job' + resources_dict = {'number_of_workers': 1, 'cpu_limit': 1, 'memory_limit': 100, 'gpu_limit': 0} + env = [] + uid = None + gid = None + mounts_dict = { + 'inputdir_source': '/input', + 'inputdir_target': '/share/incoming', + 'outputdir_source': '/output', + 'outputdir_target': '/share/outgoing' + } + + # Call schedule_job + result = manager.schedule_job(image, command, name, resources_dict, env, uid, gid, mounts_dict) + + # Verify that containers.run was called without network parameter + mock_docker_client.containers.run.assert_called_once() + call_args = mock_docker_client.containers.run.call_args + assert 'network' not in call_args[1] + assert result == mock_container \ No newline at end of file From b62e4d4252081181176cdaa57cd9d3be7d8e7dc1 Mon Sep 17 00:00:00 2001 From: NIDUS Date: Sun, 24 Aug 2025 02:42:05 +0000 Subject: [PATCH 2/4] Fixes - env.list and handle multi-network --- pman/config.py | 3 +-- pman/dockermgr.py | 27 ++++++++++++++++++++++++--- tests/test_dockermgr.py | 15 +++++++++++---- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/pman/config.py b/pman/config.py index 6f5ed74..a254f3e 100755 --- a/pman/config.py +++ b/pman/config.py @@ -32,8 +32,7 @@ def __init__(self): self.SHM_SIZE = None if shm_size is None else Memsize(shm_size) # Docker networks configuration - docker_networks = env('DOCKER_NETWORKS', '') - self.DOCKER_NETWORKS = None if not docker_networks else docker_networks.split(',') + self.DOCKER_NETWORKS = env.list('DOCKER_NETWORKS', []) self.CONTAINER_ENV = env('CONTAINER_ENV', 'docker') if self.CONTAINER_ENV == 'podman': # podman is just an alias for docker diff --git a/pman/dockermgr.py b/pman/dockermgr.py index 01f7339..3326323 100755 --- a/pman/dockermgr.py +++ b/pman/dockermgr.py @@ -1,4 +1,5 @@ import shlex +import logging from typing import AnyStr, List, Optional import docker @@ -11,6 +12,9 @@ ResourcesDict, TimeStamp) +logger = logging.getLogger(__name__) + + class DockerManager(AbstractManager[Container]): """ Interface between pman and Docker Engine or Podman API. @@ -70,10 +74,13 @@ def schedule_job(self, image: Image, command: List[str], name: JobName, # Docker networks configuration networks = {} - if (docker_networks := self.config.get('DOCKER_NETWORKS')) is not None: - networks['network'] = docker_networks[0] if len(docker_networks) == 1 else docker_networks + if (docker_networks := self.config.get('DOCKER_NETWORKS')) and len(docker_networks) > 0: + # Only use the first network for container creation + networks['network'] = docker_networks[0] - return self.__docker.containers.run( + logger.info(f"networks: {networks}") + + container = self.__docker.containers.run( image=image, command=command, name=name, @@ -88,6 +95,20 @@ def schedule_job(self, image: Image, command: List[str], name: JobName, **networks ) + # Connect to additional networks if multiple networks are specified + if (docker_networks := self.config.get('DOCKER_NETWORKS')) and len(docker_networks) > 1: + for network_name in docker_networks[1:]: + try: + network = self.__docker.networks.get(network_name) + network.connect(container) + logger.info(f"Connected container {name} to additional network: {network_name}") + except docker.errors.NotFound: + logger.warning(f"Network {network_name} not found, skipping connection") + except docker.errors.APIError as e: + logger.error(f"Failed to connect container {name} to network {network_name}: {e}") + + return container + def get_job(self, name: JobName) -> Container: try: return self.__docker.containers.get(name) diff --git a/tests/test_dockermgr.py b/tests/test_dockermgr.py index 9f5550b..94dc195 100644 --- a/tests/test_dockermgr.py +++ b/tests/test_dockermgr.py @@ -1,7 +1,5 @@ from unittest.mock import Mock, patch -import pytest - from pman.dockermgr import DockerManager @@ -56,6 +54,10 @@ def test_docker_networks_multiple_networks(self): mock_container = Mock() mock_docker_client.containers.run.return_value = mock_container + # Mock the networks.get method and network.connect method for additional networks + mock_network = Mock() + mock_docker_client.networks.get.return_value = mock_network + # Test parameters image = 'test-image' command = ['test', 'command'] @@ -74,10 +76,15 @@ def test_docker_networks_multiple_networks(self): # Call schedule_job result = manager.schedule_job(image, command, name, resources_dict, env, uid, gid, mounts_dict) - # Verify that containers.run was called with the correct network parameter + # Verify that containers.run was called with only the first network mock_docker_client.containers.run.assert_called_once() call_args = mock_docker_client.containers.run.call_args - assert call_args[1]['network'] == ['network1', 'network2'] + assert call_args[1]['network'] == 'network1' + + # Verify that the second network was connected separately + mock_docker_client.networks.get.assert_called_once_with('network2') + mock_network.connect.assert_called_once_with(mock_container) + assert result == mock_container def test_docker_networks_none(self): From f968b1f3ecc40a684a44f1ef29b0598a3ad1c0d4 Mon Sep 17 00:00:00 2001 From: Sharpz7 Date: Thu, 11 Sep 2025 19:16:42 -0600 Subject: [PATCH 3/4] Implemented suggested changes --- pman/config.py | 2 +- pman/dockermgr.py | 19 +----- tests/test_dockermgr.py | 124 ---------------------------------------- 3 files changed, 4 insertions(+), 141 deletions(-) delete mode 100644 tests/test_dockermgr.py diff --git a/pman/config.py b/pman/config.py index a254f3e..408ae18 100755 --- a/pman/config.py +++ b/pman/config.py @@ -32,7 +32,7 @@ def __init__(self): self.SHM_SIZE = None if shm_size is None else Memsize(shm_size) # Docker networks configuration - self.DOCKER_NETWORKS = env.list('DOCKER_NETWORKS', []) + self.DOCKER_NETWORKS = env('DOCKER_NETWORKS', None) self.CONTAINER_ENV = env('CONTAINER_ENV', 'docker') if self.CONTAINER_ENV == 'podman': # podman is just an alias for docker diff --git a/pman/dockermgr.py b/pman/dockermgr.py index 3326323..c1b5edd 100755 --- a/pman/dockermgr.py +++ b/pman/dockermgr.py @@ -74,11 +74,10 @@ def schedule_job(self, image: Image, command: List[str], name: JobName, # Docker networks configuration networks = {} - if (docker_networks := self.config.get('DOCKER_NETWORKS')) and len(docker_networks) > 0: - # Only use the first network for container creation - networks['network'] = docker_networks[0] + if docker_network := self.config.get('DOCKER_NETWORKS'): + networks['network'] = docker_network - logger.info(f"networks: {networks}") + logger.debug(f"networks: {networks}") container = self.__docker.containers.run( image=image, @@ -95,18 +94,6 @@ def schedule_job(self, image: Image, command: List[str], name: JobName, **networks ) - # Connect to additional networks if multiple networks are specified - if (docker_networks := self.config.get('DOCKER_NETWORKS')) and len(docker_networks) > 1: - for network_name in docker_networks[1:]: - try: - network = self.__docker.networks.get(network_name) - network.connect(container) - logger.info(f"Connected container {name} to additional network: {network_name}") - except docker.errors.NotFound: - logger.warning(f"Network {network_name} not found, skipping connection") - except docker.errors.APIError as e: - logger.error(f"Failed to connect container {name} to network {network_name}: {e}") - return container def get_job(self, name: JobName) -> Container: diff --git a/tests/test_dockermgr.py b/tests/test_dockermgr.py deleted file mode 100644 index 94dc195..0000000 --- a/tests/test_dockermgr.py +++ /dev/null @@ -1,124 +0,0 @@ -from unittest.mock import Mock, patch - -from pman.dockermgr import DockerManager - - -class TestDockerManager: - """Test cases for DockerManager class.""" - - def test_docker_networks_single_network(self): - """Test that a single network is correctly configured.""" - config = {'DOCKER_NETWORKS': ['test-network']} - mock_docker_client = Mock() - - with patch('docker.from_env', return_value=mock_docker_client): - manager = DockerManager(config, mock_docker_client) - - # Mock the containers.run method - mock_container = Mock() - mock_docker_client.containers.run.return_value = mock_container - - # Test parameters - image = 'test-image' - command = ['test', 'command'] - name = 'test-job' - resources_dict = {'number_of_workers': 1, 'cpu_limit': 1, 'memory_limit': 100, 'gpu_limit': 0} - env = [] - uid = None - gid = None - mounts_dict = { - 'inputdir_source': '/input', - 'inputdir_target': '/share/incoming', - 'outputdir_source': '/output', - 'outputdir_target': '/share/outgoing' - } - - # Call schedule_job - result = manager.schedule_job(image, command, name, resources_dict, env, uid, gid, mounts_dict) - - # Verify that containers.run was called with the correct network parameter - mock_docker_client.containers.run.assert_called_once() - call_args = mock_docker_client.containers.run.call_args - assert call_args[1]['network'] == 'test-network' - assert result == mock_container - - def test_docker_networks_multiple_networks(self): - """Test that multiple networks are correctly configured.""" - config = {'DOCKER_NETWORKS': ['network1', 'network2']} - mock_docker_client = Mock() - - with patch('docker.from_env', return_value=mock_docker_client): - manager = DockerManager(config, mock_docker_client) - - # Mock the containers.run method - mock_container = Mock() - mock_docker_client.containers.run.return_value = mock_container - - # Mock the networks.get method and network.connect method for additional networks - mock_network = Mock() - mock_docker_client.networks.get.return_value = mock_network - - # Test parameters - image = 'test-image' - command = ['test', 'command'] - name = 'test-job' - resources_dict = {'number_of_workers': 1, 'cpu_limit': 1, 'memory_limit': 100, 'gpu_limit': 0} - env = [] - uid = None - gid = None - mounts_dict = { - 'inputdir_source': '/input', - 'inputdir_target': '/share/incoming', - 'outputdir_source': '/output', - 'outputdir_target': '/share/outgoing' - } - - # Call schedule_job - result = manager.schedule_job(image, command, name, resources_dict, env, uid, gid, mounts_dict) - - # Verify that containers.run was called with only the first network - mock_docker_client.containers.run.assert_called_once() - call_args = mock_docker_client.containers.run.call_args - assert call_args[1]['network'] == 'network1' - - # Verify that the second network was connected separately - mock_docker_client.networks.get.assert_called_once_with('network2') - mock_network.connect.assert_called_once_with(mock_container) - - assert result == mock_container - - def test_docker_networks_none(self): - """Test that no network parameter is passed when DOCKER_NETWORKS is not configured.""" - config = {} - mock_docker_client = Mock() - - with patch('docker.from_env', return_value=mock_docker_client): - manager = DockerManager(config, mock_docker_client) - - # Mock the containers.run method - mock_container = Mock() - mock_docker_client.containers.run.return_value = mock_container - - # Test parameters - image = 'test-image' - command = ['test', 'command'] - name = 'test-job' - resources_dict = {'number_of_workers': 1, 'cpu_limit': 1, 'memory_limit': 100, 'gpu_limit': 0} - env = [] - uid = None - gid = None - mounts_dict = { - 'inputdir_source': '/input', - 'inputdir_target': '/share/incoming', - 'outputdir_source': '/output', - 'outputdir_target': '/share/outgoing' - } - - # Call schedule_job - result = manager.schedule_job(image, command, name, resources_dict, env, uid, gid, mounts_dict) - - # Verify that containers.run was called without network parameter - mock_docker_client.containers.run.assert_called_once() - call_args = mock_docker_client.containers.run.call_args - assert 'network' not in call_args[1] - assert result == mock_container \ No newline at end of file From 10d1f2a74882e1d991b13dc2de8b47ad0b667676 Mon Sep 17 00:00:00 2001 From: Sharpz7 Date: Thu, 11 Sep 2025 19:19:04 -0600 Subject: [PATCH 4/4] Removed logging --- pman/dockermgr.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pman/dockermgr.py b/pman/dockermgr.py index c1b5edd..b8b2245 100755 --- a/pman/dockermgr.py +++ b/pman/dockermgr.py @@ -1,5 +1,4 @@ import shlex -import logging from typing import AnyStr, List, Optional import docker @@ -12,9 +11,6 @@ ResourcesDict, TimeStamp) -logger = logging.getLogger(__name__) - - class DockerManager(AbstractManager[Container]): """ Interface between pman and Docker Engine or Podman API. @@ -77,8 +73,6 @@ def schedule_job(self, image: Image, command: List[str], name: JobName, if docker_network := self.config.get('DOCKER_NETWORKS'): networks['network'] = docker_network - logger.debug(f"networks: {networks}") - container = self.__docker.containers.run( image=image, command=command,