Skip to content

Commit 163d976

Browse files
committed
Stop chowning all of ${RUNNER_TEMP} in linux_job/linux_job_v2
${RUNNER_TEMP} is not ours to chown. It also holds the runner's own control files -- _github_workflow/event.json, _runner_file_commands/ -- and the recursive chown we issue from inside an alpine container is not guaranteed to land on the runner's UID. On a runner whose docker daemon remaps container UIDs it does not. The gfx1100 ROCm runners (ctr-navi3x-b13-ws*) run rootless docker (`docker info` reports DockerRootDir=/home/pytorchci/.local/share/docker), where container UID 0 maps to the host runner user and container UID N>0 maps into that user's subuid range. `chown -R $(id -u)` -- the runner's *host* UID, expanded outside the container -- therefore lands on a subordinate UID that nothing on the host owns. The runner then cannot read event.json, and every remaining step of the job fails with ##[error]Access to the path '/home/pytorchci/actions-runner/_work/_temp/_github_workflow/event.json' is denied. Chown only the three directories we actually hand to the container: $RUNNER_ARTIFACT_DIR, $RUNNER_DOCS_DIR and $RUNNER_TEST_RESULTS_DIR. Those are the bind mounts the exec container writes to as root, so they are the only ones that need restoring, and none of them belong to the runner. setup-linux, setup-rocm and setup-xpu all set the three env vars to ${RUNNER_TEMP}/{artifacts,docs,test-results}. chown-directory now no-ops on an empty or missing directory, since v1 runs these steps with `if: always()` and a skipped setup-* action leaves the env vars unset. That also stops docker from creating the bind mount source itself, which it would do as root. Hoist the repeated ALPINE_IMAGE expression to a job-level env var rather than repeat it four times.
1 parent 936e72e commit 163d976

3 files changed

Lines changed: 45 additions & 8 deletions

File tree

.github/actions/chown-directory/action.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,9 @@ runs:
2222
ALPINE_IMAGE: ${{ inputs.ALPINE_IMAGE }}
2323
DIRECTORY: ${{ inputs.directory }}
2424
run: |
25+
# Bail rather than let docker create the bind mount source as root
26+
if [[ -z "${DIRECTORY}" || ! -d "${DIRECTORY}" ]]; then
27+
echo "Nothing to chown, ${DIRECTORY:-<empty>} is not a directory"
28+
exit 0
29+
fi
2530
docker run --rm -v "${DIRECTORY}":/v -w /v "${ALPINE_IMAGE}" chown -R "$(id -u):$(id -g)" .

.github/workflows/linux_job.yml

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ jobs:
119119
# Will be blank outside of this
120120
PR_NUMBER: ${{ github.event.pull_request.number }}
121121
SCRIPT: ${{ inputs.script }}
122+
ALPINE_IMAGE: ${{ startsWith(inputs.runner, 'linux.arm64') && 'arm64v8/alpine' || '308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine' }}
122123
runs-on: ${{ inputs.runner }}
123124
# TODO: Eventually this should run in a container, we need to make a container that matches up
124125
# with the users for our self hosted runner infra since using actions/checkout with a root
@@ -280,14 +281,30 @@ jobs:
280281
uses: ./test-infra/.github/actions/chown-directory
281282
with:
282283
directory: ${{ github.workspace }}/${{ env.repository }}
283-
ALPINE_IMAGE: ${{ startsWith(inputs.runner, 'linux.arm64') && 'arm64v8/alpine' || '308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine' }}
284+
ALPINE_IMAGE: ${{ env.ALPINE_IMAGE }}
284285

285-
- name: Chown runner temp
286+
# Not all of ${RUNNER_TEMP} — a container-issued chown lands on a subordinate
287+
# UID under rootless docker, locking the runner out of _github_workflow/.
288+
- name: Chown runner artifacts dir
286289
if: always()
287290
uses: ./test-infra/.github/actions/chown-directory
288291
with:
289-
directory: ${{ runner.temp }}
290-
ALPINE_IMAGE: ${{ startsWith(inputs.runner, 'linux.arm64') && 'arm64v8/alpine' || '308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine' }}
292+
directory: ${{ env.RUNNER_ARTIFACT_DIR }}
293+
ALPINE_IMAGE: ${{ env.ALPINE_IMAGE }}
294+
295+
- name: Chown runner docs dir
296+
if: always()
297+
uses: ./test-infra/.github/actions/chown-directory
298+
with:
299+
directory: ${{ env.RUNNER_DOCS_DIR }}
300+
ALPINE_IMAGE: ${{ env.ALPINE_IMAGE }}
301+
302+
- name: Chown runner test results dir
303+
if: always()
304+
uses: ./test-infra/.github/actions/chown-directory
305+
with:
306+
directory: ${{ env.RUNNER_TEST_RESULTS_DIR }}
307+
ALPINE_IMAGE: ${{ env.ALPINE_IMAGE }}
291308

292309
- name: Prepare artifacts for upload
293310
if: always()

.github/workflows/linux_job_v2.yml

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ jobs:
137137
# Will be blank outside of this
138138
PR_NUMBER: ${{ github.event.pull_request.number }}
139139
SCRIPT: ${{ inputs.script }}
140+
ALPINE_IMAGE: ${{ startsWith(inputs.runner, 'linux.arm64') && 'arm64v8/alpine' || startsWith(inputs.runner, 'linux.idc') && 'alpine' || '308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine' }}
140141
filter: ${{ inputs.checkout-mode == 'blobless' && 'blob:none' || inputs.checkout-mode == 'treeless' && 'tree:0' || null }}
141142
runs-on: ${{ inputs.runner }}
142143
# TODO: Eventually this should run in a container, we need to make a container that matches up
@@ -311,13 +312,27 @@ jobs:
311312
uses: ./test-infra/.github/actions/chown-directory
312313
with:
313314
directory: ${{ github.workspace }}/${{ env.repository }}
314-
ALPINE_IMAGE: ${{ startsWith(inputs.runner, 'linux.arm64') && 'arm64v8/alpine' || startsWith(inputs.runner, 'linux.idc') && 'alpine' || '308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine' }}
315+
ALPINE_IMAGE: ${{ env.ALPINE_IMAGE }}
315316

316-
- name: Chown runner temp
317+
# Not all of ${RUNNER_TEMP} — a container-issued chown lands on a subordinate
318+
# UID under rootless docker, locking the runner out of _github_workflow/.
319+
- name: Chown runner artifacts dir
317320
uses: ./test-infra/.github/actions/chown-directory
318321
with:
319-
directory: ${{ runner.temp }}
320-
ALPINE_IMAGE: ${{ startsWith(inputs.runner, 'linux.arm64') && 'arm64v8/alpine' || startsWith(inputs.runner, 'linux.idc') && 'alpine' || '308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine' }}
322+
directory: ${{ env.RUNNER_ARTIFACT_DIR }}
323+
ALPINE_IMAGE: ${{ env.ALPINE_IMAGE }}
324+
325+
- name: Chown runner docs dir
326+
uses: ./test-infra/.github/actions/chown-directory
327+
with:
328+
directory: ${{ env.RUNNER_DOCS_DIR }}
329+
ALPINE_IMAGE: ${{ env.ALPINE_IMAGE }}
330+
331+
- name: Chown runner test results dir
332+
uses: ./test-infra/.github/actions/chown-directory
333+
with:
334+
directory: ${{ env.RUNNER_TEST_RESULTS_DIR }}
335+
ALPINE_IMAGE: ${{ env.ALPINE_IMAGE }}
321336

322337
- name: Prepare artifacts for upload
323338
if: always()

0 commit comments

Comments
 (0)