Skip to content

Commit

Permalink
refactor!: strip 'build-' prefix from docker image task labels
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ahal committed Oct 25, 2024
1 parent 0f46df6 commit 70c73af
Show file tree
Hide file tree
Showing 10 changed files with 14 additions and 16 deletions.
2 changes: 1 addition & 1 deletion docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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::

Expand Down
1 change: 1 addition & 0 deletions docs/reference/migrations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
------------
Expand Down
4 changes: 2 additions & 2 deletions src/taskgraph/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]


Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/taskgraph/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions src/taskgraph/transforms/cached_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
4 changes: 2 additions & 2 deletions src/taskgraph/transforms/docker_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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": "<parent>",
}
Expand Down
2 changes: 1 addition & 1 deletion src/taskgraph/transforms/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions taskcluster/kinds/push-image/kind.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/test_morph.py
Original file line number Diff line number Diff line change
Expand Up @@ -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={},
)
Expand Down
4 changes: 2 additions & 2 deletions test/test_transform_docker_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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"])
Expand Down

0 comments on commit 70c73af

Please sign in to comment.