From 7e2c1bdbc33bfd096c6d000b819f533d0c14115d Mon Sep 17 00:00:00 2001 From: abhishekmadan30 Date: Mon, 28 Jul 2025 17:00:04 -0400 Subject: [PATCH 1/4] fix: removed hardcoding from docker-image dir and kinds --- src/taskgraph/config.py | 8 ++++++++ src/taskgraph/docker.py | 8 ++++---- src/taskgraph/main.py | 17 +++++++++++++++-- src/taskgraph/transforms/task.py | 2 +- src/taskgraph/util/docker.py | 21 +++++++++++++++------ 5 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/taskgraph/config.py b/src/taskgraph/config.py index 1f698cbc5..a1efe9177 100644 --- a/src/taskgraph/config.py +++ b/src/taskgraph/config.py @@ -26,6 +26,10 @@ # The trust-domain for this graph. # (See https://firefox-source-docs.mozilla.org/taskcluster/taskcluster/taskgraph.html#taskgraph-trust-domain) # noqa Required("trust-domain"): str, + Optional( + "docker-image-kind", + description="What kind of docker image to use for the task.", + ): [str], Required("task-priority"): optionally_keyed_by( "project", "level", @@ -157,6 +161,10 @@ def vcs_root(self): def taskcluster_yml(self): return os.path.join(self.vcs_root, ".taskcluster.yml") + @property + def docker_dir(self): + return os.path.join(self.root_dir, "docker") + def validate_graph_config(config): validate_schema(graph_config_schema, config, "Invalid graph configuration:") diff --git a/src/taskgraph/docker.py b/src/taskgraph/docker.py index 17db732e9..77ae3d80b 100644 --- a/src/taskgraph/docker.py +++ b/src/taskgraph/docker.py @@ -97,21 +97,21 @@ def load_image_by_task_id(task_id, tag=None): return tag -def build_context(name, outputFile, args=None): +def build_context(name, outputFile, args=None, graph_config=None): """Build a context.tar for image with specified name.""" if not name: raise ValueError("must provide a Docker image name") if not outputFile: raise ValueError("must provide a outputFile") - image_dir = docker.image_path(name) + image_dir = docker.image_path(name, graph_config) if not os.path.isdir(image_dir): raise Exception(f"image directory does not exist: {image_dir}") docker.create_context_tar(".", image_dir, outputFile, args) -def build_image(name, tag, args=None): +def build_image(name, tag, args=None, graph_config=None): """Build a Docker image of specified name. Output from image building process will be printed to stdout. @@ -119,7 +119,7 @@ def build_image(name, tag, args=None): if not name: raise ValueError("must provide a Docker image name") - image_dir = docker.image_path(name) + image_dir = docker.image_path(name, graph_config) if not os.path.isdir(image_dir): raise Exception(f"image directory does not exist: {image_dir}") diff --git a/src/taskgraph/main.py b/src/taskgraph/main.py index b683ef3f7..e017c8148 100644 --- a/src/taskgraph/main.py +++ b/src/taskgraph/main.py @@ -565,6 +565,12 @@ def show_taskgraph(options): @command("build-image", help="Build a Docker image") @argument("image_name", help="Name of the image to build") +@argument( + "--root", + "-r", + default="taskcluster", + help="relative path for the root of the taskgraph definition", +) @argument( "-t", "--tag", help="tag that the image should be built as.", metavar="name:tag" ) @@ -575,13 +581,20 @@ def show_taskgraph(options): metavar="context.tar", ) def build_image(args): + from taskgraph.config import load_graph_config # noqa: PLC0415 from taskgraph.docker import build_context, build_image # noqa: PLC0415 validate_docker() + + root = args["root"] + graph_config = load_graph_config(root) + if args["context_only"] is None: - build_image(args["image_name"], args["tag"], os.environ) + build_image(args["image_name"], args["tag"], os.environ, graph_config) else: - build_context(args["image_name"], args["context_only"], os.environ) + build_context( + args["image_name"], args["context_only"], os.environ, graph_config + ) @command( diff --git a/src/taskgraph/transforms/task.py b/src/taskgraph/transforms/task.py index aa62b1b0d..09b8836d9 100644 --- a/src/taskgraph/transforms/task.py +++ b/src/taskgraph/transforms/task.py @@ -575,7 +575,7 @@ def build_docker_worker_payload(config, task, task_def): } # Find VOLUME in Dockerfile. - volumes = dockerutil.parse_volumes(name) + volumes = dockerutil.parse_volumes(name, config.graph_config) for v in sorted(volumes): if v in worker["volumes"]: raise Exception( diff --git a/src/taskgraph/util/docker.py b/src/taskgraph/util/docker.py index 2a1990eac..01df62d1e 100644 --- a/src/taskgraph/util/docker.py +++ b/src/taskgraph/util/docker.py @@ -206,28 +206,37 @@ def stream_context_tar(topsrcdir, context_dir, out_file, args=None): @functools.lru_cache(maxsize=None) -def image_paths(): +def image_paths(graph_config=None): """Return a map of image name to paths containing their Dockerfile.""" - config = load_yaml("taskcluster", "kinds", "docker-image", "kind.yml") + if graph_config: + config = load_yaml( + graph_config.docker_dir, + "kinds", + graph_config["docker_image_kind"], + "kind.yml", + ) + else: + config = load_yaml("taskcluster", "kinds", "docker-image", "kind.yml") + return { k: os.path.join(IMAGE_DIR, v.get("definition", k)) for k, v in config["tasks"].items() } -def image_path(name): - paths = image_paths() +def image_path(name, graph_config=None): + paths = image_paths(graph_config) if name in paths: return paths[name] return os.path.join(IMAGE_DIR, name) @functools.lru_cache(maxsize=None) -def parse_volumes(image): +def parse_volumes(image, graph_config=None): """Parse VOLUME entries from a Dockerfile for an image.""" volumes = set() - path = image_path(image) + path = image_path(image, graph_config) with open(os.path.join(path, "Dockerfile"), "rb") as fh: for line in fh: From db8b16187776ab8d88e0fc63c0be4ed69ffd7a3c Mon Sep 17 00:00:00 2001 From: abhishekmadan30 Date: Mon, 28 Jul 2025 19:13:55 -0400 Subject: [PATCH 2/4] fix: updated to review and added tests --- src/taskgraph/config.py | 5 ++- src/taskgraph/util/docker.py | 26 ++++++------ taskcluster/config.yml | 2 + test/test_util_docker.py | 79 ++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 16 deletions(-) diff --git a/src/taskgraph/config.py b/src/taskgraph/config.py index a1efe9177..56753dd52 100644 --- a/src/taskgraph/config.py +++ b/src/taskgraph/config.py @@ -28,8 +28,9 @@ Required("trust-domain"): str, Optional( "docker-image-kind", - description="What kind of docker image to use for the task.", - ): [str], + default="docker-image", + description="Name of the docker image kind (default: docker-image)", + ): str, Required("task-priority"): optionally_keyed_by( "project", "level", diff --git a/src/taskgraph/util/docker.py b/src/taskgraph/util/docker.py index 01df62d1e..6749867b8 100644 --- a/src/taskgraph/util/docker.py +++ b/src/taskgraph/util/docker.py @@ -206,33 +206,31 @@ def stream_context_tar(topsrcdir, context_dir, out_file, args=None): @functools.lru_cache(maxsize=None) -def image_paths(graph_config=None): +def image_paths(graph_config): """Return a map of image name to paths containing their Dockerfile.""" - if graph_config: - config = load_yaml( - graph_config.docker_dir, - "kinds", - graph_config["docker_image_kind"], - "kind.yml", - ) - else: - config = load_yaml("taskcluster", "kinds", "docker-image", "kind.yml") + + config = load_yaml( + graph_config.root_dir, + "kinds", + graph_config["docker-image-kind"], + "kind.yml", + ) return { - k: os.path.join(IMAGE_DIR, v.get("definition", k)) + k: os.path.join(graph_config.docker_dir, v.get("definition", k)) for k, v in config["tasks"].items() } -def image_path(name, graph_config=None): +def image_path(name, graph_config): paths = image_paths(graph_config) if name in paths: return paths[name] - return os.path.join(IMAGE_DIR, name) + return os.path.join(graph_config.docker_dir, name) @functools.lru_cache(maxsize=None) -def parse_volumes(image, graph_config=None): +def parse_volumes(image, graph_config): """Parse VOLUME entries from a Dockerfile for an image.""" volumes = set() diff --git a/taskcluster/config.yml b/taskcluster/config.yml index 280296fb6..1d0aef94e 100644 --- a/taskcluster/config.yml +++ b/taskcluster/config.yml @@ -14,6 +14,8 @@ index: task-priority: low +docker-image-kind: docker-image + taskgraph: register: self_taskgraph:register decision-parameters: 'self_taskgraph.custom_parameters:decision_parameters' diff --git a/test/test_util_docker.py b/test/test_util_docker.py index 977f7d624..f140b1668 100644 --- a/test/test_util_docker.py +++ b/test/test_util_docker.py @@ -14,6 +14,7 @@ import taskcluster_urls as liburls +from taskgraph.config import GraphConfig from taskgraph.util import docker from .mockedopen import MockedOpen @@ -269,3 +270,81 @@ def test_stream_context_tar(self): ) finally: shutil.rmtree(tmp) + + def test_image_paths_with_custom_kind(self): + """Test image_paths function with graph_config parameter.""" + temp_dir = tempfile.mkdtemp() + try: + # Create the kinds directory structure + kinds_dir = os.path.join(temp_dir, "kinds", "docker-test-image") + os.makedirs(kinds_dir) + + # Create the kind.yml file with task definitions + kind_yml_path = os.path.join(kinds_dir, "kind.yml") + with open(kind_yml_path, "w") as f: + f.write("tasks:\n") + f.write(" test-image:\n") + f.write(" definition: test-image\n") + f.write(" another-image:\n") + f.write(" definition: custom-path\n") + + # Create graph config pointing to our test directory + temp_graph_config = GraphConfig( + { + "trust-domain": "test-domain", + "docker-image-kind": "docker-test-image", + }, + temp_dir, + ) + + paths = docker.image_paths(temp_graph_config) + + expected_docker_dir = os.path.join(temp_graph_config.root_dir, "docker") + self.assertEqual( + paths["test-image"], os.path.join(expected_docker_dir, "test-image") + ) + self.assertEqual( + paths["another-image"], os.path.join(expected_docker_dir, "custom-path") + ) + finally: + shutil.rmtree(temp_dir) + + def test_parse_volumes_with_graph_config(self): + """Test parse_volumes function with graph_config parameter.""" + temp_dir = tempfile.mkdtemp() + try: + kinds_dir = os.path.join(temp_dir, "kinds", "docker-test-image") + os.makedirs(kinds_dir) + + kind_yml_path = os.path.join(kinds_dir, "kind.yml") + with open(kind_yml_path, "w") as f: + f.write("tasks:\n") + f.write(" test-image:\n") + f.write(" definition: test-image\n") + + docker_dir = os.path.join(temp_dir, "docker") + os.makedirs(docker_dir) + + image_dir = os.path.join(docker_dir, "test-image") + os.makedirs(image_dir) + + dockerfile_path = os.path.join(image_dir, "Dockerfile") + with open(dockerfile_path, "wb") as fh: + fh.write(b"VOLUME /foo/bar \n") + fh.write(b"VOLUME /hello /world \n") + + test_graph_config = GraphConfig( + { + "trust-domain": "test-domain", + "docker-image-kind": "docker-test-image", + }, + temp_dir, + ) + + volumes = docker.parse_volumes("test-image", test_graph_config) + + expected_volumes = {"/foo/bar", "/hello", "/world"} + self.assertEqual(volumes, expected_volumes) + + finally: + shutil.rmtree(temp_dir) From 53e3c20921042a8754a5cedc098a645b36e6385c Mon Sep 17 00:00:00 2001 From: abhishekmadan30 Date: Tue, 29 Jul 2025 09:21:54 -0400 Subject: [PATCH 3/4] fix: updated docker tests --- src/taskgraph/docker.py | 4 +- .../taskcluster/kinds/docker-image/kind.yml | 38 +++++++++++++++++++ test/test_docker.py | 31 +++++++++++++-- 3 files changed, 68 insertions(+), 5 deletions(-) create mode 100644 test/data/taskcluster/kinds/docker-image/kind.yml diff --git a/src/taskgraph/docker.py b/src/taskgraph/docker.py index 71f2d18df..e1f67fba2 100644 --- a/src/taskgraph/docker.py +++ b/src/taskgraph/docker.py @@ -97,7 +97,7 @@ def load_image_by_task_id(task_id, tag=None): return tag -def build_context(name, outputFile, args=None, graph_config=None): +def build_context(name, outputFile, graph_config, args=None): """Build a context.tar for image with specified name.""" if not name: raise ValueError("must provide a Docker image name") @@ -111,7 +111,7 @@ def build_context(name, outputFile, args=None, graph_config=None): docker.create_context_tar(".", image_dir, outputFile, args) -def build_image(name, tag, args=None, graph_config=None): +def build_image(name, tag, graph_config, args=None): """Build a Docker image of specified name. Output from image building process will be printed to stdout. diff --git a/test/data/taskcluster/kinds/docker-image/kind.yml b/test/data/taskcluster/kinds/docker-image/kind.yml new file mode 100644 index 000000000..06914ebf2 --- /dev/null +++ b/test/data/taskcluster/kinds/docker-image/kind.yml @@ -0,0 +1,38 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +--- +meta: + - &uv_version 0.6.1 + +loader: taskgraph.loader.transform:loader + +transforms: + - taskgraph.transforms.docker_image:transforms + - taskgraph.transforms.cached_tasks:transforms + - taskgraph.transforms.task:transforms + +# make a task for each docker-image we might want. For the moment, since we +# write artifacts for each, these are whitelisted, but ideally that will change +# (to use subdirectory clones of the proper directory), at which point we can +# generate tasks for every docker image in the directory, secure in the +# knowledge that unnecessary images will be omitted from the target task graph +tasks: + decision: + symbol: I(d) + parent: run-task + fetch: + symbol: I(fetch) + index-task: + symbol: I(idx) + python: + symbol: I(py) + args: + PYTHON_VERSIONS: "3.13 3.12 3.11 3.10 3.9 3.8" + UV_VERSION: *uv_version + run-task: + symbol: I(rt) + args: + UV_VERSION: *uv_version + skopeo: + symbol: I(skopeo) diff --git a/test/test_docker.py b/test/test_docker.py index 3bc3d3638..4f8d79d4d 100644 --- a/test/test_docker.py +++ b/test/test_docker.py @@ -4,6 +4,7 @@ import pytest from taskgraph import docker +from taskgraph.config import GraphConfig @pytest.fixture(autouse=True, scope="module") @@ -30,7 +31,15 @@ def test_build_image(capsys, mock_docker_build): image = "hello-world-tag" tag = f"test/{image}:1.0" - assert docker.build_image(image, None) is None + graph_config = GraphConfig( + { + "trust-domain": "test-domain", + "docker-image-kind": "docker-image", + }, + "test/data/taskcluster", + ) + + assert docker.build_image(image, None, graph_config=graph_config) is None m_stream.assert_called_once() m_run.assert_called_once_with( ["docker", "image", "build", "--no-cache", f"-t={tag}", "-"], @@ -47,7 +56,15 @@ def test_build_image_no_tag(capsys, mock_docker_build): m_stream, m_run = mock_docker_build image = "hello-world" - assert docker.build_image(image, None) is None + graph_config = GraphConfig( + { + "trust-domain": "test-domain", + "docker-image-kind": "docker-image", + }, + "test/data/taskcluster", + ) + + assert docker.build_image(image, None, graph_config=graph_config) is None m_stream.assert_called_once() m_run.assert_called_once_with( ["docker", "image", "build", "--no-cache", "-"], @@ -71,8 +88,16 @@ def mock_run(*popenargs, check=False, **kwargs): m_run.side_effect = mock_run image = "hello-world" + graph_config = GraphConfig( + { + "trust-domain": "test-domain", + "docker-image-kind": "docker-image", + }, + "test/data/taskcluster", + ) + with pytest.raises(Exception): - docker.build_image(image, None) + docker.build_image(image, None, graph_config=graph_config) m_stream.assert_called_once() m_run.assert_called_once_with( ["docker", "image", "build", "--no-cache", "-"], From ef11bfcfb32342af4ad8e4de7b0370eafe274774 Mon Sep 17 00:00:00 2001 From: abhishekmadan30 Date: Tue, 29 Jul 2025 11:33:46 -0400 Subject: [PATCH 4/4] fix: updated default case and add kinds dir to graph config --- src/taskgraph/config.py | 5 ++++- src/taskgraph/util/docker.py | 5 ++--- taskcluster/config.yml | 2 -- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/taskgraph/config.py b/src/taskgraph/config.py index 56753dd52..41954b9d1 100644 --- a/src/taskgraph/config.py +++ b/src/taskgraph/config.py @@ -28,7 +28,6 @@ Required("trust-domain"): str, Optional( "docker-image-kind", - default="docker-image", description="Name of the docker image kind (default: docker-image)", ): str, Required("task-priority"): optionally_keyed_by( @@ -166,6 +165,10 @@ def taskcluster_yml(self): def docker_dir(self): return os.path.join(self.root_dir, "docker") + @property + def kinds_dir(self): + return os.path.join(self.root_dir, "kinds") + def validate_graph_config(config): validate_schema(graph_config_schema, config, "Invalid graph configuration:") diff --git a/src/taskgraph/util/docker.py b/src/taskgraph/util/docker.py index 6749867b8..6027bd0da 100644 --- a/src/taskgraph/util/docker.py +++ b/src/taskgraph/util/docker.py @@ -210,9 +210,8 @@ def image_paths(graph_config): """Return a map of image name to paths containing their Dockerfile.""" config = load_yaml( - graph_config.root_dir, - "kinds", - graph_config["docker-image-kind"], + graph_config.kinds_dir, + graph_config.get("docker-image-kind", "docker-image"), "kind.yml", ) diff --git a/taskcluster/config.yml b/taskcluster/config.yml index 1d0aef94e..280296fb6 100644 --- a/taskcluster/config.yml +++ b/taskcluster/config.yml @@ -14,8 +14,6 @@ index: task-priority: low -docker-image-kind: docker-image - taskgraph: register: self_taskgraph:register decision-parameters: 'self_taskgraph.custom_parameters:decision_parameters'