potiuk commented on code in PR #74889:
URL: https://github.com/apache/airflow/pull/74889#discussion_r4307830831
##########
dev/breeze/src/airflow_breeze/utils/selective_checks.py:
##########
@@ -2126,6 +2129,96 @@ def amd_runners(self) -> str:
def arm_runners(self) -> str:
return PUBLIC_ARM_RUNNERS
+ @cached_property
+ def runner_type(self) -> str:
+ """Runner labels for the jobs of a CI run on this run's platform.
+
+ The RunsOn label is a template the jobs fill in with their size class.
+ """
+ # RunsOn needs the run id in the label; without one (e.g. a local run)
the jobs would never start.
+ run_id = self._github_context_dict.get("run_id")
+ if not run_id or not self._use_runs_on_runners():
+ return PUBLIC_ARM_RUNNERS if self._platform == CI_ARM_PLATFORM
else PUBLIC_AMD_RUNNERS
+ template = (
+ RUNS_ON_ARM_RUNNER_TEMPLATE if self._platform == CI_ARM_PLATFORM
else RUNS_ON_AMD_RUNNER_TEMPLATE
+ )
+ return json.dumps([template.format(run_id=run_id)])
+
+ def _use_runs_on_runners(self) -> bool:
+ """Whether this run may use the self-hosted RunsOn runners.
+
+ Code that runs on them can reach the RunsOn stack's AWS resources, so
only code from
+ apache/airflow itself goes there: pushes, schedules and dispatches,
and PRs from its own
+ branches except bots' (e.g. Dependabot's), which carry code nobody has
reviewed. A PR from a fork
+ goes there only after a committer adds the ``use self-hosted runners``
label.
Review Comment:
So the docstring doesn't present this as the security boundary (see the
review summary):
```suggestion
"""Whether this run should use the self-hosted RunsOn runners.
This only routes runs; it does not keep fork code off RunsOn. A
``pull_request`` run uses
the PR's own workflow files and breeze, so a fork can request a
RunsOn label directly -
that restriction has to be enforced by the RunsOn stack / runner
group, not here. Routed
there: pushes, schedules and dispatches, and PRs from apache/airflow
branches except
bots' (e.g. Dependabot's). A fork PR goes there after a committer
adds the
``use self-hosted runners`` label.
```
##########
scripts/ci/move_docker_to_mnt.sh:
##########
@@ -29,7 +29,11 @@ function cleanup_runner {
# Hence we only move docker to /mnt on x86_64 where we have a separate
/mnt mount
# If we get short on disk space on arm64 as well we need to revisit this
logic
# and make the idle nvme being used as well.
- if uname -i|grep -q x86_64; then
+ if [[ "${RUNNER_ENVIRONMENT:-}" == "self-hosted" ]] && mountpoint -q
/var/lib/docker; then
+ # RunsOn already keeps docker on the instance's local NVMe disks,
which may also back
+ # /home/runner and /tmp, so emptying /var/lib/docker could delete the
running job's files.
+ echo "/var/lib/docker is already on a dedicated disk - keeping it"
Review Comment:
On RunsOn instance types without local NVMe, `/var/lib/docker` is not a
mount point, so this falls through to the GitHub-hosted branch and moves docker
onto `/mnt`, which there is the same EBS root volume:
```suggestion
if [[ "${RUNNER_ENVIRONMENT:-}" == "self-hosted" ]]; then
# RunsOn keeps docker either on the instance's local NVMe disks,
which may also back
# /home/runner and /tmp, or - on instance types without them - on
the same EBS root
# volume /mnt is on. Moving it gains nothing and could delete the
running job's files.
echo "Self-hosted runner - keeping /var/lib/docker where it is"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
