From 70c73af7e8a16cc23a94f745cac138032039b6df Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Mon, 7 Oct 2024 10:06:36 -0400 Subject: [PATCH] refactor!: strip 'build-' prefix from docker image task labels BREAKING CHANGE: `build-` prefix stripped from docker image tasks This is being done to facilitate merging some docker related logic with gecko_taskgraph. The issue with adding a `build-` prefix to the labels, is that it is common for there to exist a `build` kind. While it isn't best practice to parse labels to obtain information like the kind, in reality this happens pretty frequently. So making this change: 1. Makes the label consistent with Gecko (needed to merge some docker related logic) 2. Makes the label consistent with other tasks (prefix should just be the kind name) 3. Reduces footguns for folks that are parsing the label. --- docs/index.rst | 2 +- docs/reference/migrations.rst | 1 + src/taskgraph/docker.py | 4 ++-- src/taskgraph/task.py | 2 +- src/taskgraph/transforms/cached_tasks.py | 5 +---- src/taskgraph/transforms/docker_image.py | 4 ++-- src/taskgraph/transforms/task.py | 2 +- taskcluster/kinds/push-image/kind.yml | 4 ++-- test/test_morph.py | 2 +- test/test_transform_docker_image.py | 4 ++-- 10 files changed, 14 insertions(+), 16 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 5d6911962..762c085f7 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -70,7 +70,7 @@ of tasks, and submit them all to Taskcluster. But you can also test out the taskgraph full You'll notice that ``taskgraph init`` has created a couple of tasks for us -already, namely ``build-docker-image-linux`` and ``hello-world``. +already, namely ``docker-image-linux`` and ``hello-world``. .. note:: diff --git a/docs/reference/migrations.rst b/docs/reference/migrations.rst index 4601e24aa..f19caee75 100644 --- a/docs/reference/migrations.rst +++ b/docs/reference/migrations.rst @@ -22,6 +22,7 @@ This page can help when migrating Taskgraph across major versions. filters: ["target_tasks_method", "my_custom_filter"] No action is necessary if the ``filters`` parameter was empty. +* Change all references from ``build-docker-image`` to ``docker-image``. 10.x -> 11.x ------------ diff --git a/src/taskgraph/docker.py b/src/taskgraph/docker.py index e379a1847..fdb2020cb 100644 --- a/src/taskgraph/docker.py +++ b/src/taskgraph/docker.py @@ -47,7 +47,7 @@ def get_image_digest(image_name): strict=False, ) tasks = load_tasks_for_kind(params, "docker-image") - task = tasks[f"build-docker-image-{image_name}"] + task = tasks[f"docker-image-{image_name}"] return task.attributes["cached_task"]["digest"] @@ -61,7 +61,7 @@ def load_image_by_name(image_name, tag=None): strict=False, ) tasks = load_tasks_for_kind(params, "docker-image") - task = tasks[f"build-docker-image-{image_name}"] + task = tasks[f"docker-image-{image_name}"] indexes = task.optimization.get("index-search", []) task_id = IndexSearch().should_replace_task(task, {}, None, indexes) diff --git a/src/taskgraph/task.py b/src/taskgraph/task.py index 45427ac4f..8852d8a0a 100644 --- a/src/taskgraph/task.py +++ b/src/taskgraph/task.py @@ -17,7 +17,7 @@ class Task: - task: the task definition (JSON-able dictionary) - optimization: optimization to apply to the task (see taskgraph.optimize) - dependencies: tasks this one depends on, in the form {name: label}, for example - {'build': 'build-linux64/opt', 'docker-image': 'build-docker-image-desktop-test'} + {'build': 'build-linux64/opt', 'docker-image': 'docker-image-desktop-test'} - soft_dependencies: tasks this one may depend on if they are available post optimisation. They are set as a list of tasks label. - if_dependencies: only run this task if at least one of these dependencies diff --git a/src/taskgraph/transforms/cached_tasks.py b/src/taskgraph/transforms/cached_tasks.py index a507e965e..798187c4a 100644 --- a/src/taskgraph/transforms/cached_tasks.py +++ b/src/taskgraph/transforms/cached_tasks.py @@ -14,10 +14,7 @@ def order_tasks(config, tasks): """Iterate image tasks in an order where parent tasks come first.""" - if config.kind == "docker-image": - kind_prefix = "build-docker-image-" - else: - kind_prefix = config.kind + "-" + kind_prefix = config.kind + "-" pending = deque(tasks) task_labels = {task["label"] for task in pending} diff --git a/src/taskgraph/transforms/docker_image.py b/src/taskgraph/transforms/docker_image.py index 191e9641e..50bdd5b1a 100644 --- a/src/taskgraph/transforms/docker_image.py +++ b/src/taskgraph/transforms/docker_image.py @@ -131,7 +131,7 @@ def fill_template(config, tasks): # include some information that is useful in reconstructing this task # from JSON taskdesc = { - "label": "build-docker-image-" + image_name, + "label": "docker-image-" + image_name, "description": description, "attributes": { "image_name": image_name, @@ -193,7 +193,7 @@ def fill_template(config, tasks): if parent: deps = taskdesc.setdefault("dependencies", {}) - deps["parent"] = f"build-docker-image-{parent}" + deps["parent"] = f"docker-image-{parent}" worker["env"]["PARENT_TASK_ID"] = { "task-reference": "", } diff --git a/src/taskgraph/transforms/task.py b/src/taskgraph/transforms/task.py index 9733e41d2..7e4e7137e 100644 --- a/src/taskgraph/transforms/task.py +++ b/src/taskgraph/transforms/task.py @@ -371,7 +371,7 @@ def build_docker_worker_payload(config, task, task_def): if isinstance(image, dict): if "in-tree" in image: name = image["in-tree"] - docker_image_task = "build-docker-image-" + image["in-tree"] + docker_image_task = "docker-image-" + image["in-tree"] assert "docker-image" not in task.get( "dependencies", () ), "docker-image key in dependencies object is reserved" diff --git a/taskcluster/kinds/push-image/kind.yml b/taskcluster/kinds/push-image/kind.yml index f59af96d6..36d91262e 100644 --- a/taskcluster/kinds/push-image/kind.yml +++ b/taskcluster/kinds/push-image/kind.yml @@ -42,7 +42,7 @@ task-defaults: tasks: decision: dependencies: - image: build-docker-image-decision + image: docker-image-decision run-task: dependencies: - image: build-docker-image-run-task + image: docker-image-run-task diff --git a/test/test_morph.py b/test/test_morph.py index 4f427361f..a894d14ea 100644 --- a/test/test_morph.py +++ b/test/test_morph.py @@ -49,7 +49,7 @@ def test_make_index_tasks(make_taskgraph, graph_config): task = Task(kind="test", label="a", attributes={}, task=task_def) docker_task = Task( kind="docker-image", - label="build-docker-image-index-task", + label="docker-image-index-task", attributes={}, task={}, ) diff --git a/test/test_transform_docker_image.py b/test/test_transform_docker_image.py index a156ce918..85770a188 100644 --- a/test/test_transform_docker_image.py +++ b/test/test_transform_docker_image.py @@ -27,7 +27,7 @@ pytest.param( {"parent": "fake-parent"}, {}, - {"dependencies": {"parent": "build-docker-image-fake-parent"}}, + {"dependencies": {"parent": "docker-image-fake-parent"}}, id="parent", ), pytest.param( @@ -58,7 +58,7 @@ def test_transforms( config = make_transform_config() config.params.update(extra_params) - expected_task_output["label"] = "build-docker-image-" + task["name"] + expected_task_output["label"] = "docker-image-" + task["name"] expected_task_output["index"] = task["index"] expected_task_output["description"] = ( "Build the docker image {} for use by dependent tasks".format(task["name"])