Skip to content

Commit 0a19b4f

Browse files
committed
fix(load-task): remove TASKCLUSTER_CACHES env
run-task expects the worker to mount a volume at each path passed in here. Let's avoid needing the user of `taskgraph load-task` needing to do the same.
1 parent 073f0b3 commit 0a19b4f

File tree

2 files changed

+61
-6
lines changed

2 files changed

+61
-6
lines changed

src/taskgraph/docker.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,11 @@ def load_task(task_id, remove=True, user=None):
327327
# Add the task's environment variables.
328328
env.update(task_def["payload"].get("env", {}))
329329

330+
# run-task expects the worker to mount a volume for each path defined in
331+
# TASKCLUSTER_CACHES, delete them to avoid needing to do the same.
332+
if "TASKCLUSTER_CACHES" in env:
333+
del env["TASKCLUSTER_CACHES"]
334+
330335
envfile = None
331336
initfile = None
332337
try:

test/test_docker.py

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@ def test_load_task(run_load_task):
166166
],
167167
"image": {"taskId": image_task_id, "type": "task-image"},
168168
},
169-
"tags": {"worker-implementation": "docker-worker"},
170169
}
171170
ret, mocks = run_load_task(task)
172171
assert ret == 0
@@ -202,7 +201,33 @@ def test_load_task(run_load_task):
202201
assert exp == actual[i]
203202

204203

205-
def test_load_task_env_and_remove(run_load_task):
204+
def test_load_task_env_init_and_remove(mocker, run_load_task):
205+
# Mock NamedTemporaryFile to capture what's written to it
206+
mock_envfile = mocker.MagicMock()
207+
mock_envfile.name = "/tmp/test_envfile"
208+
mock_envfile.fileno.return_value = 123 # Mock file descriptor
209+
210+
written_env_content = []
211+
mock_envfile.write = lambda content: written_env_content.append(content)
212+
mock_envfile.close = mocker.MagicMock()
213+
214+
mock_initfile = mocker.MagicMock()
215+
mock_initfile.name = "/tmp/test_initfile"
216+
mock_initfile.fileno.return_value = 456 # Mock file descriptor
217+
written_init_content = []
218+
mock_initfile.write = lambda content: written_init_content.append(content)
219+
mock_initfile.close = mocker.MagicMock()
220+
221+
# Return different mocks for each call to NamedTemporaryFile
222+
mock_tempfile = mocker.patch.object(docker.tempfile, "NamedTemporaryFile")
223+
mock_tempfile.side_effect = [mock_envfile, mock_initfile]
224+
225+
# Mock os.remove to prevent file deletion errors
226+
mock_os_remove = mocker.patch.object(docker.os, "remove")
227+
228+
# Mock os.fchmod
229+
mocker.patch.object(docker.os, "fchmod")
230+
206231
image_task_id = "def"
207232
task = {
208233
"payload": {
@@ -213,16 +238,43 @@ def test_load_task_env_and_remove(run_load_task):
213238
"--",
214239
"echo foo",
215240
],
216-
"env": {"FOO": "BAR", "BAZ": 1},
241+
"env": {"FOO": "BAR", "BAZ": "1", "TASKCLUSTER_CACHES": "path"},
217242
"image": {"taskId": image_task_id, "type": "task-image"},
218243
},
219244
}
220245
ret, mocks = run_load_task(task, remove=True)
221246
assert ret == 0
222247

248+
# NamedTemporaryFile was called twice (once for env, once for init)
249+
assert mock_tempfile.call_count == 2
250+
251+
# Verify the environment content written to the file
252+
assert len(written_env_content) == 1
253+
env_lines = written_env_content[0].split("\n")
254+
255+
# Verify written env is expected
256+
assert "TASKCLUSTER_CACHES=path" not in env_lines
257+
assert "FOO=BAR" in env_lines
258+
assert "BAZ=1" in env_lines
259+
260+
# Check that the default env vars were included
261+
assert any("RUN_ID=0" in line for line in env_lines)
262+
assert any("TASK_ID=abc" in line for line in env_lines)
263+
assert any("TASK_GROUP_ID=" in line for line in env_lines)
264+
assert any("TASKCLUSTER_ROOT_URL=" in line for line in env_lines)
265+
266+
# Both files were closed and removed
267+
mock_envfile.close.assert_called_once()
268+
mock_initfile.close.assert_called_once()
269+
assert mock_os_remove.call_count == 2
270+
assert mock_os_remove.call_args_list[0] == mocker.call("/tmp/test_envfile")
271+
assert mock_os_remove.call_args_list[1] == mocker.call("/tmp/test_initfile")
272+
273+
# Verify subprocess was called with the correct env file and init file
223274
mocks["subprocess_run"].assert_called_once()
224275
actual = mocks["subprocess_run"].call_args[0][0]
225-
assert re.match(r"--env-file=/tmp/tmp.*", actual[4])
276+
assert actual[3] == "/tmp/test_initfile:/builds/worker/.bashrc"
277+
assert actual[4] == "--env-file=/tmp/test_envfile"
226278
assert actual[5] == "--rm"
227279

228280

@@ -254,7 +306,6 @@ def test_load_task_with_different_image_types(
254306
],
255307
"image": image,
256308
},
257-
"tags": {"worker-implementation": "docker-worker"},
258309
}
259310

260311
mocker.patch.object(docker, "find_task_id", return_value=image_task_id)
@@ -277,7 +328,6 @@ def test_load_task_with_unsupported_image_type(capsys, run_load_task):
277328
],
278329
"image": {"type": "unsupported-type", "path": "/some/path"},
279330
},
280-
"tags": {"worker-implementation": "docker-worker"},
281331
}
282332

283333
ret, mocks = run_load_task(task)

0 commit comments

Comments
 (0)