Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions .github/workflows/code_style.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,13 @@ jobs:
# runner, so it falls back to a GitHub-hosted one. Branch
# pull requests, which are the ones a maintainer waits on,
# keep the paid shape.
# Dependabot pull requests run on the hosted pool too (free on a public
# repository). The author is tested, not the actor, which changes when
# someone else pushes to the branch.
runs-on: >-
${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-4' }}
${{ (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-4' }}
timeout-minutes: 30
steps:
- name: Checkout repository
Expand Down Expand Up @@ -132,8 +136,9 @@ jobs:
# pull requests, which are the ones a maintainer waits on,
# keep the paid shape.
runs-on: >-
${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-4' }}
${{ (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-4' }}
timeout-minutes: 75
env:
# sccache is the sole owner of compiler output: no cache step archives a
Expand Down
8 changes: 6 additions & 2 deletions .github/workflows/codescene-coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,13 @@ jobs:
# runner, so it falls back to a GitHub-hosted one. Branch
# pull requests, which are the ones a maintainer waits on,
# keep the paid shape.
# Dependabot pull requests run on the hosted pool too (free on a public
# repository). The author is tested, not the actor, which changes when
# someone else pushes to the branch.
runs-on: >-
${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-4' }}
${{ (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-4' }}
timeout-minutes: 90
needs: [compile-contracts]
env:
Expand Down
13 changes: 9 additions & 4 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ jobs:
# fork cannot obtain an Ubicloud runner, so it runs
# GitHub-hosted. A branch pull request, which is the one a
# maintainer waits on, keeps the paid shape.
# Dependabot pull requests run on the hosted pool too (free on a public
# repository). The author is tested, not the actor, which changes when
# someone else pushes to the branch.
runs-on: >-
${{ github.event_name == 'schedule' && 'ubuntu-latest'
|| github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-4' }}
|| (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-4' }}
timeout-minutes: 30
env:
# sccache is the sole owner of compiler output: no cache step archives a
Expand Down Expand Up @@ -147,8 +151,9 @@ jobs:
# maintainer waits on, keeps the paid shape.
runs-on: >-
${{ github.event_name == 'schedule' && 'ubuntu-latest'
|| github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-2' }}
|| (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-2' }}
timeout-minutes: 30
strategy:
fail-fast: false
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/pr-label-classify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ jobs:
# takes the smallest ARM shape: on the hosted pool this 13-20 s job
# waited up to 2,098 s to start (ADR 0013). `pull_request_target` runs in
# this repository's context for a fork too, so it needs no fork fallback.
# dependabot-runner: exempt. A few seconds of labelling on a `pull_request_target`
# event; a hosted arm would need a new hosted arm64 label in the registry and
# contracts for no measurable saving.
runs-on: ubicloud-standard-2-arm
timeout-minutes: 10
steps:
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/pr-label-scope.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ jobs:
# architecture, so the smallest ARM shape: on the hosted pool this
# 4-7 s job waited up to 1,495 s to start (ADR 0013). Like the classifier
# it runs on `pull_request_target`, so a fork needs no fallback.
# dependabot-runner: exempt. A few seconds of labelling on a `pull_request_target`
# event; a hosted arm would need a new hosted arm64 label in the registry and
# contracts for no measurable saving.
runs-on: ubicloud-standard-2-arm
timeout-minutes: 10
steps:
Expand Down
8 changes: 6 additions & 2 deletions .github/workflows/regression-test-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@ jobs:
# the smallest ARM shape: on the hosted pool this 6-8 s job waited up to
# 734 s to start (ADR 0013). A fork's pull request cannot obtain an
# Ubicloud runner, so it falls back to a hosted one.
# Dependabot pull requests run on the hosted pool too (free on a public
# repository). The author is tested, not the actor, which changes when
# someone else pushes to the branch.
runs-on: >-
${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-2-arm' }}
${{ (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-2-arm' }}
timeout-minutes: 10
steps:
- name: Checkout repository
Expand Down
33 changes: 21 additions & 12 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,14 @@ jobs:
# fork cannot obtain an Ubicloud runner, so it runs
# GitHub-hosted. A branch pull request, which is the one a
# maintainer waits on, keeps the paid shape.
# Dependabot pull requests run on the hosted pool too (free on a public
# repository). The author is tested, not the actor, which changes when
# someone else pushes to the branch.
runs-on: >-
${{ github.event_name == 'schedule' && 'ubuntu-latest'
|| github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-4' }}
|| (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-4' }}
timeout-minutes: 90
env:
# sccache is the sole owner of compiler output: no cache step archives a
Expand Down Expand Up @@ -274,8 +278,9 @@ jobs:
# maintainer waits on, keeps the paid shape.
runs-on: >-
${{ github.event_name == 'schedule' && 'ubuntu-latest'
|| github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-2' }}
|| (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-2' }}
timeout-minutes: 45
env:
# sccache is the sole owner of compiler output: no cache step archives a
Expand Down Expand Up @@ -377,8 +382,9 @@ jobs:
# maintainer waits on, keeps the paid shape.
runs-on: >-
${{ github.event_name == 'schedule' && 'ubuntu-latest'
|| github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-2' }}
|| (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-2' }}
timeout-minutes: 20
env:
# sccache is the sole owner of compiler output: no cache step archives a
Expand Down Expand Up @@ -475,8 +481,9 @@ jobs:
# This small member-specific matrix leg also exercises HTTP/2 policy.
runs-on: >-
${{ github.event_name == 'schedule' && 'ubuntu-latest'
|| github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-2' }}
|| (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-2' }}
timeout-minutes: 30
env:
# sccache is the sole owner of compiler output: no cache step archives a
Expand Down Expand Up @@ -640,8 +647,9 @@ jobs:
# maintainer waits on, keeps the paid shape.
runs-on: >-
${{ github.event_name == 'schedule' && 'ubuntu-latest'
|| github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-4' }}
|| (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-4' }}
timeout-minutes: 75
env:
# sccache is the sole owner of compiler output: no cache step archives a
Expand Down Expand Up @@ -758,8 +766,9 @@ jobs:
# maintainer waits on, keeps the paid shape.
runs-on: >-
${{ github.event_name == 'schedule' && 'ubuntu-latest'
|| github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-4' }}
|| (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-4' }}
timeout-minutes: 45
steps:
- name: Checkout repository
Expand Down
61 changes: 36 additions & 25 deletions docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,18 @@ to this repository and the fork's run does not reach it, so a lane that asks
for one does not fail: it queues, waits, and is eventually reported as a stuck
or cancelled check on somebody else's contribution, which reads as this
repository being broken. Every lane a `pull_request` can dispatch therefore
names a GitHub-hosted runner for that case:
names a GitHub-hosted runner for that case. A pull request Dependabot opened
takes the same hosted arm: hosted minutes cost nothing on a public repository,
and Dependabot opens many pull requests, which therefore run on the hosted pool
without Ubicloud's cache store. The condition tests the pull request's author,
not `github.actor`, because the actor is whoever triggered the run, so a person
pushing to a Dependabot branch would flip the runner:

```yaml
runs-on: >-
${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-4' }}
${{ (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-4' }}
```

A lane that also serves a cron composes both conditions in one expression, in
Expand All @@ -371,33 +377,38 @@ that order, because `runs-on` is the only place either distinction can live:
```yaml
runs-on: >-
${{ github.event_name == 'schedule' && 'ubuntu-latest'
|| github.event.pull_request.head.repo.fork && 'ubuntu-latest'
|| 'ubicloud-standard-2' }}
|| (github.event.pull_request.head.repo.fork
|| github.event.pull_request.user.login == 'dependabot[bot]')
&& 'ubuntu-latest' || 'ubicloud-standard-2' }}
```

The reader splits the chain on the `||` outside parentheses, so the hosted
condition's own `||` is not a chain separator, and it recognizes that condition
only as written (the fork field, then the Dependabot author).

`Job.runner_labels` reads a chain of any length and reports every distinct
label, so a lane still answers `uses_ubicloud` and stays inside the timeout,
sizing and sccache contracts. `Job.labels_for_event` answers which arm an event
selects, and it answers a fork condition false on purpose: these contracts ask
what a lane costs this repository, a fork's run costs nothing here, and reading
the fork arm as the pull-request answer would report every one of these lanes
as free while hiding the shape they buy for a branch pull request. A chain the
helpers cannot read, one with no fallback arm or naming a condition they cannot
evaluate, stays one opaque label rather than being split into arms nobody
checked.

`tests/workflow_contracts/fork_fallback_test.py` asserts the field rather than
the shape, because the failure worth stopping looks right. Swapping
`head.repo.fork` for `head.repo.private` leaves an expression of the same shape
with the same two labels; it sends every branch pull request to a free runner
while a fork's still queues for a paid one, and nothing in the workflow shows
it. The contract also asserts the other direction, that a branch pull request
still selects the Ubicloud shape, so a lane rewritten to send every pull
request to `ubuntu-latest` fails rather than passing as a fallback. It reads
the arms in order, too: an arm ahead of the fork arm that selects Ubicloud on a
pull request takes a fork's run first, so it fails even though the fork field
and its hosted label are both present further along. An earlier cron arm is
fine, since a pull request cannot take it.
selects, and it answers a fork or Dependabot condition false on purpose: these
contracts ask what a lane costs this repository, a fork's run costs nothing
here, and reading the fork arm as the pull-request answer would report every
one of these lanes as free while hiding the shape they buy for a branch pull
request. A chain the helpers cannot read, one with no fallback arm or naming a
condition they cannot evaluate, stays one opaque label rather than being split
into arms nobody checked.

`tests/workflow_contracts/fork_fallback_test.py` asserts the exact hosted
condition rather than the shape, because the failure worth stopping looks
right. Swapping `head.repo.fork` for `head.repo.private` leaves an expression
of the same shape with the same two labels; it sends every branch pull request
to a free runner while a fork's still queues for a paid one, and nothing in the
workflow shows it. The contract also asserts the other direction, that a branch
pull request still selects the Ubicloud shape, so a lane rewritten to send
every pull request to `ubuntu-latest` fails rather than passing as a fallback.
It reads the arms in order, too: an arm ahead of the fork arm that selects
Ubicloud on a pull request takes a fork's run first, so it fails even though
the fork field and its hosted label are both present further along. An earlier
cron arm is fine, since a pull request cannot take it.

Reading either expression as one opaque label would drop the job out of the
placement, timeout, sizing and sccache contracts at once, which is the failure
Expand Down
4 changes: 2 additions & 2 deletions tests/workflow_contracts/_fork_lanes.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import typing as typ

from _runs_on import EVENT_CONDITION_RE, conditional_runs_on_arms
from _workflow_policy import FORK_CONDITION, UBICLOUD_LABEL_PREFIX
from _workflow_policy import FORK_CONDITION, HOSTED_CONDITION, UBICLOUD_LABEL_PREFIX

if typ.TYPE_CHECKING: # pragma: no cover - typing only
from _workflow_policy import Job
Expand Down Expand Up @@ -126,7 +126,7 @@ def paid_arms_before_the_fork(declared: str) -> tuple[str, ...]:
"""
shadowing: list[str] = []
for condition, label in conditional_runs_on_arms(declared) or ():
if condition == FORK_CONDITION:
if condition in (FORK_CONDITION, HOSTED_CONDITION):
break
if label.startswith(UBICLOUD_LABEL_PREFIX) and _holds_on_a_pull_request(
condition
Expand Down
32 changes: 31 additions & 1 deletion tests/workflow_contracts/_runs_on.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,35 @@
r"^github\.event\.pull_request\.head\.repo\.(?P<field>[a-z_]+)$"
)

#: The hosted arm's exact condition, parenthesized: a fork, or a pull request
#: Dependabot opened. Compared as written, because its inner `||` must not be
#: read as the chain's own separator and a looser pattern would accept a sibling
#: field or `github.actor` as the author.
HOSTED_CONDITION_RE: re.Pattern[str] = re.compile(
r"^\(github\.event\.pull_request\.head\.repo\.fork\s*\|\|\s*"
r"github\.event\.pull_request\.user\.login\s*==\s*'dependabot\[bot\]'\)$"
)


def _depth_change(character: str) -> int:
"""Return how one character changes the parenthesis depth."""
return {"(": 1, ")": -1}.get(character, 0)


def _split_chain(body: str) -> list[str]:
"""Split an expression body on the `||` that sit outside parentheses."""
parts: list[str] = []
depth = 0
start = 0
for index, character in enumerate(body):
depth += _depth_change(character)
if depth == 0 and body.startswith("||", index):
parts.append(body[start:index].strip())
start = index + 2
parts.append(body[start:].strip())
return parts


def _expression_body(declared: str) -> str | None:
"""Return the inside of a `${{ ... }}` scalar, or None if it is not one.

Expand Down Expand Up @@ -123,7 +152,7 @@ def _runs_on_chain(declared: str) -> tuple[tuple[str | None, str], ...] | None:
body = _expression_body(declared)
if body is None:
return None
parts = [part.strip() for part in body.split("||")]
parts = _split_chain(body)
if len(parts) < 2:
return None
arms = [
Expand Down Expand Up @@ -204,6 +233,7 @@ def _recognized_condition(condition: str) -> bool:
return (
EVENT_CONDITION_RE.match(condition) is not None
or HEAD_REPO_CONDITION_RE.match(condition) is not None
or HOSTED_CONDITION_RE.match(condition) is not None
)


Expand Down
9 changes: 9 additions & 0 deletions tests/workflow_contracts/_workflow_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@
#: The field that decides whether a pull request came from a fork.
FORK_CONDITION = "github.event.pull_request.head.repo.fork"

#: The pull request's author, not `github.actor`: the actor is whoever
#: triggered the run, so a person pushing to a Dependabot branch would flip the
#: runner. Hosted minutes cost nothing on a public repository, so Dependabot's
#: pull requests take the hosted arm as a fork's do.
DEPENDABOT_CONDITION = "github.event.pull_request.user.login == 'dependabot[bot]'"

#: The exact condition of the hosted arm: a fork, or Dependabot.
HOSTED_CONDITION = f"({FORK_CONDITION} || {DEPENDABOT_CONDITION})"

#: Prefix shared by every Ubicloud runner label. Match on the prefix, not on
#: one exact label: the migration wave introduces `ubicloud-standard-2`, and a
#: contract keyed to the current label would wave the new one through.
Expand Down
10 changes: 5 additions & 5 deletions tests/workflow_contracts/fork_fallback_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
from _trigger_reading import runs_on_event, triggers
from _workflow_files import jobs_of
from _workflow_policy import (
FORK_CONDITION,
HOSTED_CONDITION,
UBICLOUD_LABEL_PREFIX,
Job,
conditional_runs_on_arms,
Expand Down Expand Up @@ -156,18 +156,18 @@ def test_every_pull_request_lane_falls_back_for_a_fork(job: Job) -> None:
Nothing about the workflow's appearance would show it.
"""
declared = runs_on_scalar(job)
assert FORK_CONDITION in declared, (
assert HOSTED_CONDITION in declared, (
f"{job} can run on a fork's pull request and asks for "
f"{job.runner_summary} without a fork fallback. Add "
f"`{FORK_CONDITION} && '{HOSTED_LABEL}'` ahead of the paid label: a "
f"`{HOSTED_CONDITION} && '{HOSTED_LABEL}'` ahead of the paid label: a "
"fork cannot obtain an Ubicloud runner and its check will hang."
)


def test_the_fork_arm_selects_a_hosted_runner(job: Job) -> None:
"""The fallback must be free, and it must be the fork arm's own label."""
declared = runs_on_scalar(job)
_, found, arm = declared.partition(FORK_CONDITION)
_, found, arm = declared.partition(HOSTED_CONDITION)
assert found, f"{job} names no fork condition, so there is no fallback arm to read"
assert arm.lstrip().startswith(f"&& '{HOSTED_LABEL}'"), (
f"{job} tests the fork field but does not hand a fork "
Expand All @@ -186,7 +186,7 @@ def test_no_earlier_arm_hands_a_fork_a_paid_runner(job: Job) -> None:
shadowing = paid_arms_before_the_fork(runs_on_scalar(job))
assert not shadowing, (
f"{job} selects a paid runner for a pull request before its fork arm "
f"is reached: {list(shadowing)}. Put `{FORK_CONDITION} && "
f"is reached: {list(shadowing)}. Put `{HOSTED_CONDITION} && "
f"'{HOSTED_LABEL}'` ahead of every arm a pull request can take."
)

Expand Down
Loading
Loading