From 564f769ff7c027eb543724e599e29c772e05269f Mon Sep 17 00:00:00 2001 From: allegroai <> Date: Wed, 20 Dec 2023 17:42:36 +0200 Subject: [PATCH] Add `agent.docker_args_extra_precedes_task`, `agent.protected_docker_extra_args` to prevent the same switch to be used by both `extra_docker_args` and the a Task's docker args --- .../backend_api/config/default/agent.conf | 7 ++ clearml_agent/commands/worker.py | 11 ++- clearml_agent/helper/docker_args.py | 70 +++++++++++++++++++ docs/clearml.conf | 7 ++ 4 files changed, 92 insertions(+), 3 deletions(-) diff --git a/clearml_agent/backend_api/config/default/agent.conf b/clearml_agent/backend_api/config/default/agent.conf index ec85567..58bde4b 100644 --- a/clearml_agent/backend_api/config/default/agent.conf +++ b/clearml_agent/backend_api/config/default/agent.conf @@ -177,6 +177,13 @@ # these are local for this agent and will not be updated in the experiment's docker_cmd section # extra_docker_arguments: ["--ipc=host", ] + # Allow the extra docker arg to override task level docker arg (if the same argument is passed on both), + # if set to False, a task docker arg will override the docker extra arg + # docker_args_extra_precedes_task: true + + # allows the following task docker args to be overridden by the extra_docker_arguments + # protected_docker_extra_args: ["privileged", "security-opt", "network", "ipc"] + # optional shell script to run in docker when started before the experiment is started # extra_docker_shell_script: ["apt-get install -y bindfs", ] diff --git a/clearml_agent/commands/worker.py b/clearml_agent/commands/worker.py index a0b9e1c..6656b98 100644 --- a/clearml_agent/commands/worker.py +++ b/clearml_agent/commands/worker.py @@ -4026,15 +4026,20 @@ class Worker(ServiceCommandSection): docker_arguments = self._filter_docker_args(docker_arguments) if self._session.config.get("agent.docker_allow_host_environ", None): docker_arguments = self._resolve_docker_env_args(docker_arguments) - base_cmd += [a for a in docker_arguments if a] if extra_docker_arguments: # we always resolve environments in the `extra_docker_arguments` becuase the admin set them (not users) extra_docker_arguments = self._resolve_docker_env_args(extra_docker_arguments) - extra_docker_arguments = [extra_docker_arguments] \ if isinstance(extra_docker_arguments, six.string_types) else extra_docker_arguments - base_cmd += [str(a) for a in extra_docker_arguments if a] + + # decide on order of docker args when merging overlapping arguments + # from extra_docker_args and the Task's docker_args + base_cmd += DockerArgsSanitizer.merge_docker_args( + config=self._session.config, + task_docker_arguments=docker_arguments, + extra_docker_arguments=extra_docker_arguments + ) # set docker labels base_cmd += ['-l', self._worker_label.format(worker_id)] diff --git a/clearml_agent/helper/docker_args.py b/clearml_agent/helper/docker_args.py index 83cf8d4..3a1d252 100644 --- a/clearml_agent/helper/docker_args.py +++ b/clearml_agent/helper/docker_args.py @@ -94,3 +94,73 @@ class DockerArgsSanitizer: res.fragment )) return " ".join(tokens) if changed else s, changed + + @staticmethod + def get_list_of_switches(docker_args: List[str]) -> List[str]: + args = [] + for token in docker_args: + if token.strip().startswith("-"): + args += [token.strip().split("=")[0].lstrip("-")] + + return args + + @staticmethod + def filter_switches(docker_args: List[str], exclude_switches: List[str]) -> List[str]: + # shortcut if we are sure we have no matches + if (not exclude_switches or + not any("-{}".format(s) in " ".join(docker_args) for s in exclude_switches)): + return docker_args + + args = [] + in_switch_args = True + for token in docker_args: + if token.strip().startswith("-"): + if "=" in token: + switch = token.strip().split("=")[0] + in_switch_args = False + else: + switch = token + in_switch_args = True + + if switch.lstrip("-") in exclude_switches: + # if in excluded, skip the switch and following arguments + in_switch_args = False + else: + args += [token] + + elif in_switch_args: + args += [token] + else: + # this is the switch arguments we need to skip + pass + + return args + + @staticmethod + def merge_docker_args(config, task_docker_arguments: List[str], extra_docker_arguments: List[str]) -> List[str]: + base_cmd = [] + # currently only resolving --network, --ipc, --privileged + override_switches = config.get( + "agent.protected_docker_extra_args", + ["privileged", "security-opt", "network", "ipc"] + ) + + if config.get("agent.docker_args_extra_precedes_task", True): + switches = [] + if extra_docker_arguments: + switches = DockerArgsSanitizer.get_list_of_switches(extra_docker_arguments) + switches = list(set(switches) & set(override_switches)) + base_cmd += [str(a) for a in extra_docker_arguments if a] + if task_docker_arguments: + docker_arguments = DockerArgsSanitizer.filter_switches(task_docker_arguments, switches) + base_cmd += [a for a in docker_arguments if a] + else: + switches = [] + if task_docker_arguments: + switches = DockerArgsSanitizer.get_list_of_switches(task_docker_arguments) + switches = list(set(switches) & set(override_switches)) + base_cmd += [a for a in task_docker_arguments if a] + if extra_docker_arguments: + extra_docker_arguments = DockerArgsSanitizer.filter_switches(extra_docker_arguments, switches) + base_cmd += [a for a in extra_docker_arguments if a] + return base_cmd diff --git a/docs/clearml.conf b/docs/clearml.conf index 7de6934..d511b30 100644 --- a/docs/clearml.conf +++ b/docs/clearml.conf @@ -189,6 +189,13 @@ agent { # You can also pass host environments into the container with ["-e", "HOST_NAME=$HOST_NAME"] # extra_docker_arguments: ["--ipc=host", "-v", "/mnt/host/data:/mnt/data"] + # Allow the extra docker arg to override task level docker arg (if the same argument is passed on both), + # if set to False, a task docker arg will override the docker extra arg + # docker_args_extra_precedes_task: true + + # prevent a task docker args to be used if already specified in the extra_docker_arguments + # protected_docker_extra_args: ["privileged", "security-opt", "network", "ipc"] + # optional shell script to run in docker when started before the experiment is started # extra_docker_shell_script: ["apt-get install -y bindfs", ]