Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
61ea71cc35 | ||
|
|
5ffd0c98bc | ||
|
|
f0994c11a0 | ||
|
|
b52c8d1af6 | ||
|
|
1677dff568 |
No files matched your search
@@ -40,13 +40,21 @@ missing credential is a lock-out, here a skip would be a bypass.
|
||||
**The administrator override (2026-10-01).** The rule above was unsatisfiable for the one human
|
||||
who runs this repo: Gitea refuses to let an author approve their own PR, and `jakman85` is the
|
||||
only collaborator — so a sensitive-path PR the operator opened could never be merged, *including
|
||||
the PR that would fix that.* An administrator may therefore waive the review by commenting on
|
||||
the PR that would fix that.* An administrator may therefore waive the review by posting this on
|
||||
the PR:
|
||||
|
||||
/approve-override <reason>
|
||||
|
||||
The directive must **start a line**, the **reason is mandatory**, the commenter's `admin`/`owner`
|
||||
permission is re-checked **live** when the gate runs, and the comment must be **newer than the
|
||||
**Post it as a PR *review* of type "Comment", not in the plain comment box.** CI's
|
||||
`SETTINGS_API_TOKEN` is minted `[read:repository, read:admin]`, and `/issues/{n}/comments` sits
|
||||
behind Gitea's *issue* scope — so an override typed into the comment box is **invisible to CI**,
|
||||
and the stage reports it as a missing approval rather than as a permission problem (measured on
|
||||
PR #38, 2026-10-02). `/pulls/{n}/reviews` is readable, so a review body works today. Both sources
|
||||
are accepted and the gate names which it used; the comment box starts working once
|
||||
`infra/gitea/set-ci-token.sh` is re-run to pick up `read:issue`.
|
||||
|
||||
The directive must **start a line**, the **reason is mandatory**, the poster's `admin`/`owner`
|
||||
permission is re-checked **live** when the gate runs, and it must be **newer than the
|
||||
head commit** — push again and you must override again. The overrider, the timestamp and the
|
||||
reason are printed in the gate log and the job summary. For an administrator, "a change to the
|
||||
gate cannot be gated by the gate" is therefore **advisory, not structural**; what is kept is an
|
||||
|
||||
+105
-50
@@ -1,39 +1,54 @@
|
||||
#!/usr/bin/env python3
|
||||
"""ci/approval-override.py — select the admin-override comment on a PR, if there is a valid one.
|
||||
"""ci/approval-override.py — select the admin-override on a PR, if there is a valid one.
|
||||
|
||||
This script decides NOTHING about authority. It parses a PR's comment list and returns the
|
||||
single override *candidate* that ci/approval-policy.sh should then try to authorise against
|
||||
This script decides NOTHING about authority. It parses a PR's comments and reviews and returns
|
||||
the single override *candidate* that ci/approval-policy.sh should then try to authorise against
|
||||
the live Gitea permission API. Keeping the two apart is the point:
|
||||
|
||||
* parsing is pure, offline, and therefore TESTABLE (ci/tests/approval-override.test.sh);
|
||||
* authorisation needs the API and must stay in the stage that fails closed.
|
||||
|
||||
A candidate is a comment whose body contains, AT THE START OF A LINE, the directive followed
|
||||
by a non-empty reason:
|
||||
A candidate is a comment OR a review whose body contains, AT THE START OF A LINE, the directive
|
||||
followed by a non-empty reason:
|
||||
|
||||
/approve-override the gate cannot gate itself and I am the only human reviewer
|
||||
|
||||
TWO SOURCES, AND WHY THAT IS NOT BELT-AND-BRACES. Measured 2026-10-02 on PR #38: the override was
|
||||
posted as an ordinary PR comment, was well-formed, was newer than the head commit — and the stage
|
||||
still blocked, because CI could not READ it. `SETTINGS_API_TOKEN` is minted with scopes
|
||||
`[read:repository, read:admin]`, and Gitea puts /issues/{n}/comments behind the *issue* scope, so
|
||||
the request came back as an error OBJECT rather than an array. `/pulls/{n}/reviews` is under
|
||||
read:repository and was readable in the same run. So:
|
||||
|
||||
* a REVIEW body works with the token CI has today (leave a review of type "Comment");
|
||||
* a COMMENT body works once the token carries read:issue (see infra/gitea/set-ci-token.sh).
|
||||
|
||||
Accepting both means the hatch is not hostage to one API scope. A source that cannot be read
|
||||
contributes no candidates and is reported on stderr; it can only cause a false BLOCK, never a
|
||||
false pass, which is why degrading it is safe and blocking on it was not necessary.
|
||||
|
||||
WHY EACH RULE IS HERE
|
||||
|
||||
* START OF A LINE, not "anywhere in the body". Otherwise quoting someone else's override
|
||||
("you could just /approve-override this") re-arms it. Markdown quotes are stripped of a
|
||||
leading '>' before matching for the same reason -- a quoted directive must NOT count, so
|
||||
lines beginning with '>' are DROPPED, not unquoted.
|
||||
* A REASON IS MANDATORY. A bare directive records that a review was waived but not why,
|
||||
which is the difference between an audit trail and a rubber stamp.
|
||||
* STALE OVERRIDES ARE DISCARDED. An override earned at commit A must not silently cover
|
||||
commit B. Any candidate created at or before the head commit's timestamp is dropped --
|
||||
the same semantics Gitea's own `dismiss_stale_approvals` applies to reviews. Push new
|
||||
commits and you must re-override.
|
||||
* THE LATEST CANDIDATE WINS, so a second override comment supersedes the first rather than
|
||||
both being live.
|
||||
("you could just /approve-override this") re-arms it. Lines beginning with '>' are DROPPED
|
||||
rather than unquoted, for exactly that reason.
|
||||
* A REASON IS MANDATORY. A bare directive records that a review was waived but not why, which
|
||||
is the difference between an audit trail and a rubber stamp.
|
||||
* STALE OVERRIDES ARE DISCARDED. An override earned at commit A must not silently cover commit
|
||||
B. Any candidate created at or before the head commit's timestamp is dropped — the same
|
||||
semantics Gitea's own `dismiss_stale_approvals` applies to approvals. Push new commits and
|
||||
you must re-override.
|
||||
* A PENDING REVIEW IS NOT SUBMITTED. It is a private draft visible only to its author, so it is
|
||||
not an act of anything and is dropped.
|
||||
* THE LATEST CANDIDATE WINS, across both sources, so a later override supersedes an earlier one.
|
||||
|
||||
Output: one TSV line (login, created_at, reason) on stdout, or nothing at all when there is no
|
||||
valid candidate. Never exits non-zero for "no override" -- absence is a normal answer, and the
|
||||
Output: one TSV line (login, timestamp, source, reason) on stdout, or nothing at all when there is
|
||||
no valid candidate. Never exits non-zero for "no override" — absence is a normal answer and the
|
||||
caller must not be able to confuse it with a parse crash.
|
||||
|
||||
Input, all via env so nothing lands in `ps`:
|
||||
COMMENTS_JSON the Gitea /issues/{n}/comments array
|
||||
COMMENTS_JSON the Gitea /issues/{n}/comments array (or an error object; or absent)
|
||||
REVIEWS_JSON the Gitea /pulls/{n}/reviews array (or an error object; or absent)
|
||||
HEAD_TIME ISO-8601 timestamp of the PR head commit
|
||||
DIRECTIVE the override directive, e.g. /approve-override
|
||||
"""
|
||||
@@ -81,57 +96,97 @@ def extract_reason(body, directive):
|
||||
return None
|
||||
|
||||
|
||||
def main():
|
||||
directive = os.environ.get("DIRECTIVE", "/approve-override").strip()
|
||||
head_time = parse_ts(os.environ.get("HEAD_TIME", ""))
|
||||
|
||||
def load_source(env_name, label):
|
||||
"""Return a list of items for one source, or [] with a stderr note if it is unusable."""
|
||||
raw = os.environ.get(env_name)
|
||||
if not raw or not raw.strip():
|
||||
return []
|
||||
try:
|
||||
comments = json.loads(os.environ.get("COMMENTS_JSON") or "[]")
|
||||
parsed = json.loads(raw)
|
||||
except (ValueError, TypeError):
|
||||
# The caller validated this JSON already; if it is broken here, say so loudly on
|
||||
# stderr and emit no candidate. Silence on stdout means "no override".
|
||||
print("approval-override: COMMENTS_JSON is not valid JSON", file=sys.stderr)
|
||||
return 0
|
||||
if not isinstance(comments, list):
|
||||
print("approval-override: COMMENTS_JSON is not a list", file=sys.stderr)
|
||||
return 0
|
||||
print(
|
||||
"approval-override: %s is not valid JSON — ignoring that source" % label,
|
||||
file=sys.stderr,
|
||||
)
|
||||
return []
|
||||
if isinstance(parsed, list):
|
||||
return parsed
|
||||
# An error object, typically {"message": "token does not have ... scope"}. Say which,
|
||||
# because "no override found" and "could not read the overrides" look identical from
|
||||
# the outside and the first wasted a real debugging session.
|
||||
detail = ""
|
||||
if isinstance(parsed, dict):
|
||||
detail = str(parsed.get("message") or parsed.get("errors") or "")[:200]
|
||||
print(
|
||||
"approval-override: %s is not a list (Gitea returned %s%s) — ignoring that source"
|
||||
% (label, type(parsed).__name__, ": " + detail if detail else ""),
|
||||
file=sys.stderr,
|
||||
)
|
||||
return []
|
||||
|
||||
candidates = []
|
||||
for comment in comments:
|
||||
if not isinstance(comment, dict):
|
||||
|
||||
def candidates_from(items, source, directive, head_time):
|
||||
"""Yield (when, login, reason, raw_when, source) for every valid override in one source."""
|
||||
found = []
|
||||
for item in items:
|
||||
if not isinstance(item, dict):
|
||||
continue
|
||||
login = ((comment.get("user") or {}).get("login") or "").strip()
|
||||
reason = extract_reason(comment.get("body"), directive)
|
||||
login = ((item.get("user") or {}).get("login") or "").strip()
|
||||
reason = extract_reason(item.get("body"), directive)
|
||||
if not login or not reason:
|
||||
continue
|
||||
created_raw = comment.get("created_at") or comment.get("updated_at") or ""
|
||||
created = parse_ts(created_raw)
|
||||
if created is None:
|
||||
# A PENDING review is an unsubmitted private draft — not an act of anything.
|
||||
state = str(item.get("state") or "").upper()
|
||||
if source == "review" and state == "PENDING":
|
||||
print(
|
||||
"approval-override: dropping override by '%s' — unreadable timestamp %r"
|
||||
% (login, created_raw),
|
||||
"approval-override: dropping PENDING (unsubmitted) review by '%s'" % login,
|
||||
file=sys.stderr,
|
||||
)
|
||||
continue
|
||||
raw_when = (
|
||||
item.get("submitted_at") or item.get("created_at") or item.get("updated_at") or ""
|
||||
)
|
||||
when = parse_ts(raw_when)
|
||||
if when is None:
|
||||
print(
|
||||
"approval-override: dropping %s override by '%s' — unreadable timestamp %r"
|
||||
% (source, login, raw_when),
|
||||
file=sys.stderr,
|
||||
)
|
||||
continue
|
||||
# An override must post-date the commit it covers.
|
||||
if head_time is not None and created <= head_time:
|
||||
if head_time is not None and when <= head_time:
|
||||
print(
|
||||
"approval-override: dropping STALE override by '%s' (%s) — not newer than "
|
||||
"head commit (%s). Re-comment to override the current commit."
|
||||
% (login, created.isoformat(), head_time.isoformat()),
|
||||
"approval-override: dropping STALE %s override by '%s' (%s) — not newer than "
|
||||
"head commit (%s). Re-post it to override the current commit."
|
||||
% (source, login, when.isoformat(), head_time.isoformat()),
|
||||
file=sys.stderr,
|
||||
)
|
||||
continue
|
||||
candidates.append((created, login, reason, created_raw))
|
||||
found.append((when, login, reason, raw_when, source))
|
||||
return found
|
||||
|
||||
if not candidates:
|
||||
|
||||
def main():
|
||||
directive = os.environ.get("DIRECTIVE", "/approve-override").strip()
|
||||
head_time = parse_ts(os.environ.get("HEAD_TIME", ""))
|
||||
|
||||
found = []
|
||||
found += candidates_from(
|
||||
load_source("COMMENTS_JSON", "COMMENTS_JSON"), "comment", directive, head_time
|
||||
)
|
||||
found += candidates_from(
|
||||
load_source("REVIEWS_JSON", "REVIEWS_JSON"), "review", directive, head_time
|
||||
)
|
||||
|
||||
if not found:
|
||||
return 0
|
||||
|
||||
# Latest wins: a later override supersedes an earlier one.
|
||||
created, login, reason, created_raw = max(candidates, key=lambda c: c[0])
|
||||
# Latest wins, across both sources: a later override supersedes an earlier one.
|
||||
when, login, reason, raw_when, source = max(found, key=lambda c: c[0])
|
||||
# Keep the reason on one line — this becomes a TSV field.
|
||||
reason = " ".join(reason.split())
|
||||
print("%s\t%s\t%s" % (login, created_raw, reason))
|
||||
print("%s\t%s\t%s\t%s" % (login, raw_when, source, reason))
|
||||
return 0
|
||||
|
||||
|
||||
|
||||
+34
-8
@@ -103,8 +103,15 @@ REVIEWS_JSON="$(api "${GITEA_URL}/api/v1/repos/${GITEA_REPO}/pulls/${PR}/reviews
|
||||
# PR conversation comments live under the ISSUES endpoint. `/pulls/{n}/comments` returns review
|
||||
# (diff) comments ONLY, so an override posted in the normal comment box would never appear in it.
|
||||
# Verified against this Gitea (1.24.3) rather than assumed.
|
||||
#
|
||||
# This one is NOT in the must-parse loop below, and that is deliberate. /issues/{n}/comments sits
|
||||
# behind Gitea's *issue* token scope, which `SETTINGS_API_TOKEN` does not carry (it is minted
|
||||
# `[read:repository, read:admin]`) — so it returns an error OBJECT, not an array. Blocking on that
|
||||
# turned "CI cannot read your override" into a message that said "you did not post one", and cost a
|
||||
# real debugging session on PR #38. It is handled explicitly, and loudly, further down: an
|
||||
# unreadable source can only cause a false BLOCK, never a false pass.
|
||||
COMMENTS_JSON="$(api "${GITEA_URL}/api/v1/repos/${GITEA_REPO}/issues/${PR}/comments?limit=1000" || true)"
|
||||
for v in FILES_JSON PR_JSON REVIEWS_JSON COMMENTS_JSON; do
|
||||
for v in FILES_JSON PR_JSON REVIEWS_JSON; do
|
||||
jq -e . >/dev/null 2>&1 <<<"${!v}" || {
|
||||
err "could not read ${v} from Gitea for PR #${PR} — BLOCKING rather than assuming."
|
||||
summary "- approval-policy: **BLOCKED — Gitea unreadable**"
|
||||
@@ -170,8 +177,23 @@ if [ "$OVERRIDE_ENABLED" = "true" ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Pure, offline, unit-tested selection: which comment, if any, is a live override?
|
||||
OVERRIDE_TSV="$(COMMENTS_JSON="$COMMENTS_JSON" HEAD_TIME="$HEAD_TIME" DIRECTIVE="$DIRECTIVE" \
|
||||
# Is the comments source actually readable? Say so plainly if not — a scope problem and "you
|
||||
# never posted an override" are indistinguishable from the outside, and conflating them is what
|
||||
# made PR #38 look like a code bug.
|
||||
if ! jq -e 'type == "array"' >/dev/null 2>&1 <<<"${COMMENTS_JSON:-}"; then
|
||||
SCOPE_MSG="$(jq -r '.message // empty' <<<"${COMMENTS_JSON:-null}" 2>/dev/null || true)"
|
||||
warn "cannot read PR COMMENTS from Gitea${SCOPE_MSG:+ — ${SCOPE_MSG}}."
|
||||
warn " /issues/${PR}/comments needs the *issue* token scope; SETTINGS_API_TOKEN is minted"
|
||||
warn " [read:repository, read:admin] (infra/gitea/set-ci-token.sh). An override posted as an"
|
||||
warn " ordinary PR COMMENT is therefore invisible to CI — post it as a REVIEW of type"
|
||||
warn " 'Comment' instead, which /pulls/${PR}/reviews exposes under read:repository."
|
||||
summary "- approval-policy: note — PR comments unreadable (token lacks the issue scope); use a **review** to override"
|
||||
COMMENTS_JSON='[]'
|
||||
fi
|
||||
|
||||
# Pure, offline, unit-tested selection: which comment or review, if any, is a live override?
|
||||
OVERRIDE_TSV="$(COMMENTS_JSON="$COMMENTS_JSON" REVIEWS_JSON="$REVIEWS_JSON" \
|
||||
HEAD_TIME="$HEAD_TIME" DIRECTIVE="$DIRECTIVE" \
|
||||
python3 "${CI_DIR}/approval-override.py")" || {
|
||||
err "ci/approval-override.py failed — BLOCKING rather than assuming there is no override."
|
||||
summary "- approval-policy: **BLOCKED — override parser failed**"
|
||||
@@ -180,7 +202,8 @@ if [ "$OVERRIDE_ENABLED" = "true" ]; then
|
||||
if [ -n "$OVERRIDE_TSV" ]; then
|
||||
OV_USER="$(cut -f1 <<<"$OVERRIDE_TSV")"
|
||||
OV_WHEN="$(cut -f2 <<<"$OVERRIDE_TSV")"
|
||||
OV_WHY="$(cut -f3- <<<"$OVERRIDE_TSV")"
|
||||
OV_SRC="$(cut -f3 <<<"$OVERRIDE_TSV")"
|
||||
OV_WHY="$(cut -f4- <<<"$OVERRIDE_TSV")"
|
||||
|
||||
# AUTHORITY IS MEASURED LIVE, never inferred from the comment or from a name in git.
|
||||
PERM_JSON="$(api "${GITEA_URL}/api/v1/repos/${GITEA_REPO}/collaborators/${OV_USER}/permission" || true)"
|
||||
@@ -214,12 +237,13 @@ print("yes" if (ok_perm and ok_list) else "no")
|
||||
else
|
||||
warn "ADMIN OVERRIDE by '${OV_USER}' (permission '${OV_PERM}'); author is '${AUTHOR}'."
|
||||
fi
|
||||
warn " via: PR ${OV_SRC}"
|
||||
warn " when: ${OV_WHEN} (head commit ${HEAD_SHA:0:8}, ${HEAD_TIME})"
|
||||
warn " reason: ${OV_WHY}"
|
||||
warn " review waived on: $(printf '%s ' $SENSITIVE)"
|
||||
ok "sensitive paths cleared by administrator override — this stage does not block."
|
||||
SELFNOTE=""; [ "$OV_USER" = "$AUTHOR" ] && SELFNOTE=", **self-waived as author**"
|
||||
summary "- approval-policy: **OVERRIDDEN by \`${OV_USER}\`** (permission \`${OV_PERM}\`${SELFNOTE})"
|
||||
summary "- approval-policy: **OVERRIDDEN by \`${OV_USER}\`** (permission \`${OV_PERM}\`, via PR ${OV_SRC}${SELFNOTE})"
|
||||
summary " - reason: ${OV_WHY}"
|
||||
summary " - review waived on: $(printf '\`%s\` ' $SENSITIVE)"
|
||||
exit 0
|
||||
@@ -239,10 +263,12 @@ err " honestly approve for itself. Get a human review, or move the change out o
|
||||
printf ' %s\n' $SENSITIVE >&2
|
||||
err " The list lives in infra/gitea/approval-policy.json, with a reason per entry."
|
||||
if [ "$OVERRIDE_ENABLED" = "true" ]; then
|
||||
err " Or, if you hold admin/owner on this repository, comment on the PR:"
|
||||
err " Or, if you hold admin/owner on this repository, post this on the PR:"
|
||||
err " ${DIRECTIVE} <reason>"
|
||||
err " The reason is mandatory, the directive must START a line, and the comment must be"
|
||||
err " NEWER than head commit ${HEAD_SHA:0:8} — push again and you must override again."
|
||||
err " as a REVIEW of type 'Comment' (not the plain comment box — CI's token cannot read"
|
||||
err " /issues/${PR}/comments without the issue scope; see infra/gitea/set-ci-token.sh)."
|
||||
err " The reason is mandatory, the directive must START a line, and it must be NEWER than"
|
||||
err " head commit ${HEAD_SHA:0:8} — push again and you must override again."
|
||||
fi
|
||||
summary "- approval-policy: **BLOCKED — needs a non-author approval, or an admin \`${DIRECTIVE}\` comment**"
|
||||
exit 1
|
||||
@@ -26,7 +26,13 @@ BEFORE="2026-10-01T11:30:00Z"
|
||||
|
||||
# run <comments-json> -> stdout of the parser (stderr is diagnostic only)
|
||||
run() {
|
||||
COMMENTS_JSON="$1" HEAD_TIME="${2:-$HEAD_TIME}" DIRECTIVE="${3:-/approve-override}" \
|
||||
COMMENTS_JSON="$1" REVIEWS_JSON="" HEAD_TIME="${2:-$HEAD_TIME}" DIRECTIVE="${3:-/approve-override}" \
|
||||
python3 "$SUT" 2>/dev/null
|
||||
}
|
||||
|
||||
# run_rev <reviews-json> — the same, but through the REVIEWS source.
|
||||
run_rev() {
|
||||
COMMENTS_JSON="" REVIEWS_JSON="$1" HEAD_TIME="${2:-$HEAD_TIME}" DIRECTIVE="${3:-/approve-override}" \
|
||||
python3 "$SUT" 2>/dev/null
|
||||
}
|
||||
|
||||
@@ -54,10 +60,24 @@ expect_user() {
|
||||
expect_reason() {
|
||||
local label="$1" json="$2" want="$3" out got
|
||||
out="$(run "$json")"
|
||||
got="$(cut -f3- <<<"$out")"
|
||||
got="$(cut -f4- <<<"$out")"
|
||||
if [ "$got" = "$want" ]; then ok "$label"; else bad "$label (wanted '${want}', got '${got}')"; fi
|
||||
}
|
||||
|
||||
# check <label> <actual> <wanted>
|
||||
check() {
|
||||
if [ "$2" = "$3" ]; then ok "$1"; else bad "$1 (wanted '${3}', got '${2}')"; fi
|
||||
}
|
||||
|
||||
# review <login> <submitted_at> <state> <body>
|
||||
review() {
|
||||
python3 -c '
|
||||
import json, sys
|
||||
print(json.dumps([{"user": {"login": sys.argv[1]}, "submitted_at": sys.argv[2],
|
||||
"state": sys.argv[3], "body": sys.argv[4]}]))
|
||||
' "$1" "$2" "$3" "$4"
|
||||
}
|
||||
|
||||
echo "== ci/approval-override.py"
|
||||
|
||||
# --- the happy path -------------------------------------------------------------------
|
||||
@@ -141,5 +161,72 @@ else
|
||||
bad "with no HEAD_TIME the candidate should still be returned (got '${out}')"
|
||||
fi
|
||||
|
||||
# --- the REVIEWS source ---------------------------------------------------------------
|
||||
# Load-bearing, not redundant. Measured on PR #38: SETTINGS_API_TOKEN is minted
|
||||
# [read:repository, read:admin], /issues/{n}/comments needs the ISSUE scope, so a well-formed
|
||||
# override in the comment box came back as an error object and was invisible. /pulls/{n}/reviews
|
||||
# IS readable with that token, so a review body is the route that works today.
|
||||
check "a review body carries an override" \
|
||||
"$(cut -f1 <<<"$(run_rev "$(review jakman85 "$AFTER" COMMENT '/approve-override via a review')")")" \
|
||||
jakman85
|
||||
check "the source is reported as 'review'" \
|
||||
"$(cut -f3 <<<"$(run_rev "$(review jakman85 "$AFTER" COMMENT '/approve-override via a review')")")" \
|
||||
review
|
||||
check "the source is reported as 'comment' for a comment" \
|
||||
"$(cut -f3 <<<"$(run "$(comment jakman85 "$AFTER" '/approve-override via a comment')")")" \
|
||||
comment
|
||||
|
||||
out="$(run_rev "$(review jakman85 "$BEFORE" COMMENT '/approve-override stale review')")"
|
||||
check "a stale review override is dropped too" "${out:-<none>}" "<none>"
|
||||
|
||||
out="$(run_rev "$(review jakman85 "$AFTER" PENDING '/approve-override unsubmitted draft')")"
|
||||
check "a PENDING (unsubmitted) review is NOT an act of anything" "${out:-<none>}" "<none>"
|
||||
|
||||
check "an APPROVED review carrying the directive still counts as an override" \
|
||||
"$(cut -f1 <<<"$(run_rev "$(review jakman85 "$AFTER" APPROVED '/approve-override belt and braces')")")" \
|
||||
jakman85
|
||||
|
||||
out="$(run_rev "$(review jakman85 "$AFTER" COMMENT 'ordinary review prose, no directive')")"
|
||||
check "an ordinary review body is not an override" "${out:-<none>}" "<none>"
|
||||
|
||||
# --- an unreadable source degrades to 'no candidates', never to a pass ----------------
|
||||
# The real-world shape: Gitea answers with {"message": "token does not have ... scope"}.
|
||||
ERR_OBJ='{"message":"token does not have at least one of required scope(s): [read:issue]"}'
|
||||
out="$(COMMENTS_JSON="$ERR_OBJ" REVIEWS_JSON="" HEAD_TIME="$HEAD_TIME" \
|
||||
DIRECTIVE=/approve-override python3 "$SUT" 2>/dev/null)"
|
||||
check "an error OBJECT from the comments API yields no override" "${out:-<none>}" "<none>"
|
||||
|
||||
out="$(COMMENTS_JSON="$ERR_OBJ" \
|
||||
REVIEWS_JSON="$(review jakman85 "$AFTER" COMMENT '/approve-override reviews still work')" \
|
||||
HEAD_TIME="$HEAD_TIME" DIRECTIVE=/approve-override python3 "$SUT" 2>/dev/null)"
|
||||
check "an unreadable COMMENTS source does not suppress a valid REVIEW override" \
|
||||
"$(cut -f1 <<<"$out")" jakman85
|
||||
|
||||
# and it must SAY so, or a scope problem reads as 'you never posted one' (it did, on PR #38)
|
||||
errout="$(COMMENTS_JSON="$ERR_OBJ" REVIEWS_JSON="" HEAD_TIME="$HEAD_TIME" \
|
||||
DIRECTIVE=/approve-override python3 "$SUT" 2>&1 >/dev/null)"
|
||||
case "$errout" in
|
||||
*"not a list"*read:issue*) ok "the diagnostic names the scope Gitea complained about" ;;
|
||||
*) bad "the diagnostic should quote Gitea's message (got: ${errout})" ;;
|
||||
esac
|
||||
|
||||
# --- latest wins ACROSS the two sources ----------------------------------------------
|
||||
out="$(COMMENTS_JSON="$(comment early "2026-10-01T12:10:00Z" '/approve-override from a comment')" \
|
||||
REVIEWS_JSON="$(review later "2026-10-01T12:50:00Z" COMMENT '/approve-override from a review')" \
|
||||
HEAD_TIME="$HEAD_TIME" DIRECTIVE=/approve-override python3 "$SUT" 2>/dev/null)"
|
||||
check "the latest override wins across both sources (review newer)" "$(cut -f1 <<<"$out")" later
|
||||
check "and its source is labelled" "$(cut -f3 <<<"$out")" review
|
||||
|
||||
out="$(COMMENTS_JSON="$(comment later "2026-10-01T12:50:00Z" '/approve-override from a comment')" \
|
||||
REVIEWS_JSON="$(review early "2026-10-01T12:10:00Z" COMMENT '/approve-override from a review')" \
|
||||
HEAD_TIME="$HEAD_TIME" DIRECTIVE=/approve-override python3 "$SUT" 2>/dev/null)"
|
||||
check "the latest override wins across both sources (comment newer)" "$(cut -f1 <<<"$out")" later
|
||||
check "and that source is labelled too" "$(cut -f3 <<<"$out")" comment
|
||||
|
||||
# --- both sources absent entirely -----------------------------------------------------
|
||||
out="$(COMMENTS_JSON="" REVIEWS_JSON="" HEAD_TIME="$HEAD_TIME" \
|
||||
DIRECTIVE=/approve-override python3 "$SUT" 2>/dev/null)"
|
||||
check "no sources at all yields no override, and no crash" "${out:-<none>}" "<none>"
|
||||
|
||||
printf '\n %d passed, %d failed\n' "$PASS" "$FAIL"
|
||||
[ "$FAIL" -eq 0 ] || exit 1
|
||||
@@ -87,6 +87,43 @@ Also updated: `CLAUDE.md` §1 and the §2 enforcement table (the "approval requi
|
||||
explicitly ⬜ with the reason, rather than a stale ✅), and `infra/gitea/README.md` gains §6 and
|
||||
marks its historical step 4 as reversed — do not re-apply.
|
||||
|
||||
**Second pass, 2026-10-02 — the override did not work the first time, and why.** The operator ran
|
||||
the live PATCH (`required_approvals: 0` confirmed live, with `block_admin_merge_override` and
|
||||
`enable_status_check` still `true`), merged PR #35, and posted `/approve-override Need to ensure
|
||||
approval override work as expected` on PR #38. The gate re-ran (run 203) and **still blocked.**
|
||||
|
||||
The comment was not at fault: posted by `jakman85`, directive at the start of the line, reason
|
||||
present, and 29 minutes **newer** than head commit `616a497`, so it cleared the staleness rule.
|
||||
What failed was that **CI could not read it.** Run 203's log:
|
||||
|
||||
approval-override: COMMENTS_JSON is not a list
|
||||
|
||||
`SETTINGS_API_TOKEN` is minted `["read:repository", "read:admin"]` by
|
||||
`infra/gitea/set-ci-token.sh`, and Gitea puts `/issues/{n}/comments` behind the **issue** scope. So
|
||||
Gitea returned an error *object*, `jq -e .` accepted it as valid JSON, and the stage fell through
|
||||
to "no approving review" — **a permission failure wearing the costume of a missing override.**
|
||||
`/pulls/{n}/files` and `/pulls/{n}/reviews` are under `read:repository`, which is exactly why every
|
||||
other part of the same stage worked and this one did not.
|
||||
|
||||
Three fixes, and the first is the one that mattered most:
|
||||
|
||||
1. **The diagnosis is no longer misleading.** `COMMENTS_JSON` is out of the must-parse-or-block
|
||||
loop and is explicitly checked for being an *array*; when it is not, the stage prints Gitea's
|
||||
own `.message`, names the missing scope, and says where to post the override instead. An
|
||||
unreadable source can only cause a false BLOCK, never a false pass, so degrading it to a loud
|
||||
warning is safe — and blocking on it was never the honest behaviour.
|
||||
2. **The override is accepted from a PR REVIEW body as well as a comment**, so the hatch is not
|
||||
hostage to one API scope. A review of type "Comment" works on the token CI has *today*. The
|
||||
gate names which source it used. `PENDING` (unsubmitted, author-only draft) reviews are dropped.
|
||||
3. **`set-ci-token.sh` now requests `read:issue`**, which makes the ordinary comment box work —
|
||||
the thing anybody reaches for first. It takes effect only on a **re-run**: an existing token's
|
||||
scopes cannot be widened in place.
|
||||
|
||||
`ci/tests/approval-override.test.sh` grows from 23 to **38 assertions**, covering the reviews
|
||||
source, `PENDING` reviews, an error *object* from the comments API, "an unreadable comments source
|
||||
must not suppress a valid review override", latest-wins across both sources, and that the
|
||||
diagnostic actually quotes Gitea's message.
|
||||
|
||||
**One adjacent fix, deliberately in scope: `ci/tests/gate-decision.test.sh` was already broken.**
|
||||
It hand-maintains a list of stub stage scripts that "must name EVERY stage `gate.sh` runs", and it
|
||||
did not name `approval-policy` — added to `gate.sh` by PR #17 and never added here. `run_stage`
|
||||
@@ -130,8 +167,30 @@ missing from its test. `make -C ci test` goes from **FAILED to all suites passed
|
||||
**FAIL, 15 failed / 35 passed**. After deriving the stub list from `gate.sh`: **PASS, 50
|
||||
assertions**.
|
||||
- **`make -C ci test` in the pinned toolbox: `ci/tests: all suites passed`, rc 0** — approval-override
|
||||
23, gate-decision 50, touched-requirements-base 31, test-attribution 15, touched-requirements 26,
|
||||
**38**, gate-decision 50, touched-requirements-base 31, test-attribution 15, touched-requirements 26,
|
||||
decisions-log pass. It was rc 2 (`ci/tests: FAILED`) before the stub-list fix.
|
||||
- **The live flip worked, and the drift detector proved it.** Run 202 (before): `gitea-settings:
|
||||
FAIL`, `required_approvals: DECLARED 0 but LIVE is 1`. Run 203 (after the operator ran the
|
||||
PATCH): `gitea-settings: pass`, with `block_admin_merge_override = true` among the keys it
|
||||
checked. So the narrow change held — the approval count moved and the gate's teeth did not.
|
||||
- **The first override attempt failed for a measured reason, not a guessed one.** Run 203 log:
|
||||
`approval-override: COMMENTS_JSON is not a list`. Token scopes read from `set-ci-token.sh`:
|
||||
`[read:repository, read:admin]`. The same run successfully read `/pulls/38` and
|
||||
`/pulls/38/files` (author and sensitive paths both resolved) and got past the reviews
|
||||
validation, which is how I know `/pulls/{n}/reviews` IS readable with that token — from
|
||||
behaviour, not from documentation.
|
||||
- **The comment itself was valid.** `created_at` 2026-10-02T05:31:24Z vs head commit `616a497`
|
||||
committer date 2026-10-02T00:02:20-05:00 → the comment is 29m04s newer, so it passed the
|
||||
staleness rule. Ruling the comment out is what left the token scope as the only candidate.
|
||||
- **`?limit=1000` is not the culprit.** Queried `/issues/38/comments` with `limit=1000`, `limit=50`
|
||||
and no limit using my own credentials: `type=array` in all three. So the difference is the
|
||||
credential, not the query.
|
||||
- **The root cause is confirmed by Gitea, in its own words.** Run 206 (the fix in place):
|
||||
`[warn] cannot read PR COMMENTS from Gitea — token does not have at least one of required
|
||||
scope(s), required=[read:issue], token scope=read:admin,read:repository.` followed by the
|
||||
instruction to post the override as a review. That is the diagnosis the first attempt should
|
||||
have produced, and `gitea-settings: pass` in the same run. The stage still ends `FAIL`, correctly
|
||||
— there is no valid override on the PR yet.
|
||||
- **Toolbox merge gate** (`./infra/devcontainer/dev.sh run bash -c './ci/gate.sh'`), run on the
|
||||
rebased branch: **rc 3, `MERGE GATE: UNDETERMINED`**. Green stages: spec-immutability,
|
||||
decisions-log, lint, l0 (23 offline cases, 0 failures), render-golden including the new
|
||||
@@ -142,20 +201,32 @@ missing from its test. `make -C ci test` goes from **FAILED to all suites passed
|
||||
|
||||
## What I did NOT verify
|
||||
|
||||
- **The live Gitea rule still says `required_approvals: 1`. I did not change it.** Mutating live
|
||||
platform settings is an operator action; `infra/gitea/allow-admin-self-merge.sh` is written,
|
||||
syntax-checked, and **unrun**. The declaration and the live rule must move together.
|
||||
*Now measured, not predicted:* PR #38's CI run (run 202, job 0) fails at **1m44s** with
|
||||
`required_approvals: DECLARED 0 but LIVE is 1` → `gitea-settings: FAIL`. Correct behaviour, and
|
||||
the PR stays red until the script is run.
|
||||
- **The override has never been ACCEPTED by a real run** — only offered. Measured in the same CI
|
||||
log: the stage reached `approval-policy: FAIL` on the sensitive paths and printed the *new*
|
||||
code's guidance, `Or, if you hold admin/owner on this repository, comment on the PR:
|
||||
/approve-override <reason> ... NEWER than head commit 7b9f733c`. So the new stage **is** what
|
||||
runs on this PR (self-bootstrapping confirmed by measurement, not by reasoning), and the
|
||||
authorising branch — comment parse → live permission lookup → exit 0 — is still **unexercised**.
|
||||
Posting the comment on PR #38 is its first real test. If it does not work, the recovery is to
|
||||
PATCH `required_approvals` and read run 202's successor log.
|
||||
- **The override has still never been ACCEPTED by a real run.** This is the second attempt at
|
||||
this bullet, which is itself the point: the first pass called the hatch unverified, and the
|
||||
first real test found a defect that no amount of local testing would have (an API scope CI has
|
||||
and I did not). What is now measured is that the parse → *reject* path works end to end and
|
||||
diagnoses itself. The **accepting** branch — directive found → live permission lookup → exit 0 —
|
||||
has never executed. Posting the override as a **review** on PR #38 is its first real test, and
|
||||
I would not be surprised by a third round.
|
||||
- **`read:issue` is the right scope name — Gitea said so itself, so this is no longer a guess.**
|
||||
Run 206 printed Gitea's own message: `token does not have at least one of required scope(s),
|
||||
required=[read:issue], token scope=read:admin,read:repository`. That both confirms the root
|
||||
cause and names the string, which matters because this repo has been burned by a token *name*
|
||||
that could not exist (`GITEA_SETTINGS_TOKEN`). What is **still** unverified is that the token
|
||||
*mint* accepts it — `POST /users/{u}/tokens` with that scope has not been run. The review route
|
||||
does not depend on it.
|
||||
- **I did not verify that Gitea lets a PR's own author submit a `COMMENT`-type review.** Gitea
|
||||
blocks author self-*approval*; "Comment" is expected to be allowed, and that expectation is now
|
||||
load-bearing for the recommended route. It is one click to find out, and I deliberately did not
|
||||
test it by posting an override myself: doing so as `gitea-admin` is precisely the agent
|
||||
self-override that `CLAUDE.md` now forbids.
|
||||
- **The whole re-run mechanism is manual.** `gate.yml` triggers on `pull_request` and `push: main`
|
||||
only — there is no `issue_comment` trigger, so posting an override does **not** re-run the gate.
|
||||
Re-run the job from the Actions UI, or close and reopen the PR (which fires `pull_request` and
|
||||
leaves the head SHA alone). **Do not push an empty commit to re-trigger it:** that moves the head
|
||||
commit and makes the override stale by the rule above, so it would have to be posted again. A
|
||||
comment trigger was considered and not built — it would need to check out PR head code while the
|
||||
job holds `SETTINGS_API_TOKEN`, which is the classic pwn-request shape and not worth it here.
|
||||
- **I have not seen the gate still REFUSE a merge** now that approvals are 0. The claim that
|
||||
`block_admin_merge_override: true` keeps a red gate unmergeable rests on Gitea's documented
|
||||
behaviour and on the 2026-09-29 session, **not** on a provoked refusal in this one. That probe
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
---
|
||||
date: 2026-10-02
|
||||
branch: fix/plex-quicksync-verification
|
||||
title: Hardware transcoding works — and `runbooks/plex-quicksync.md`'s verification section was wrong
|
||||
---
|
||||
|
||||
## What changed
|
||||
|
||||
**Hardware transcoding is confirmed working on prod.** Measured 2026-10-02 from
|
||||
`Plex Media Server.log` in the running pod: `vaapi` on `/dev/dri/renderD128`
|
||||
(`Intel CoffeeLake-S GT2 [UHD Graphics 630]`), `using hardware decode accelerator vaapi`,
|
||||
`zero-copy support present`, `using zero-copy transcoding`, and
|
||||
`final decoder: vaapi, final encoder: vaapi`. VAAPI for **both** directions, zero-copy, so frames
|
||||
never round-trip through the CPU. The device-plugin mechanism from PR #37 works end to end.
|
||||
|
||||
**And `runbooks/plex-quicksync.md`'s verification section was wrong, which is the correction this
|
||||
branch exists to publish.** The page said the Plex dashboard showing `Transcode (hw)` was "the *only*
|
||||
proof available, and nothing else will ever tell you". The operator had hardware transcoding fully
|
||||
working while **Dashboard → Activity showed nothing of the sort** — so the page's authoritative check
|
||||
was silent on a working system, and sent someone looking for a fault that did not exist. Three
|
||||
inference errors are now corrected on the page, two of them recorded as named false leads so the next
|
||||
reader does not re-walk them:
|
||||
|
||||
1. **The dashboard is not proof.** Replaced as the primary check by Plex's own log — the grep is in
|
||||
the page, in its **redacting** form, because `Plex Media Server.log` carries API URLs containing
|
||||
`X-Plex-Token` (CLAUDE.md §6; a credential has been leaked into a transcript here before). The
|
||||
page also now says explicitly that **`Plex Transcoder Statistics.log` is the wrong file** — it
|
||||
holds XML `<Variant .../>` records, not ffmpeg output, and the name invites the mistake — and why
|
||||
`grep -a` is required. The dashboard is **kept but demoted** to a secondary, best-effort signal,
|
||||
with its absence stated to prove nothing.
|
||||
2. **An fd snapshot is not evidence.** A `/proc/*/fd` sweep inside the pod found no holder of
|
||||
`renderD128` while hardware transcoding was active; transcode sessions are short and the scan
|
||||
landed in a gap. Recorded as a documented false lead, with the reliable process-level check in its
|
||||
place: the transcoder's **argv** (`-hwaccel`, `-hwaccel_device`, `vaapi`, `renderD128`), found via
|
||||
`ps -eo pid,user,comm | grep -i transcoder`.
|
||||
3. **A missing preference key does not mean the setting is off.** The page told readers to treat an
|
||||
absent `HardwareAcceleratedCodecs` in `Preferences.xml` as "never enabled". With hardware
|
||||
acceleration demonstrably on and working, that key was **absent from all 27 attributes** while
|
||||
`HardwareDevicePath` **was** written and the log said `hardware transcoding: enabled`. First-command
|
||||
#4 now reads both keys and states plainly that `HardwareDevicePath` present is a positive signal,
|
||||
absence of `HardwareAcceleratedCodecs` means nothing either way, only `="0"` is conclusive, and the
|
||||
log settles it.
|
||||
|
||||
Also on the page:
|
||||
|
||||
- **GID `992` is no longer presented as a number to expect.** Under the plugin the injected device
|
||||
arrives `root:root`, so the image's `cont-init.d` hook resolves GID **`0`** and adds `plex` to the
|
||||
**root group** — measured `Groups: 0 100 1000`, `id plex` → `groups=1000(plex),0(root),100(users)`.
|
||||
992 was the hostPath-era host `render` GID. The page now says so, marks the root-group membership as
|
||||
an inherent property of the plugin's `DeviceSpec` rather than a defect, and repeats that it is
|
||||
**not** a reason to reintroduce `supplementalGroups` (still a no-op: `s6-setuidgid` rebuilds the
|
||||
group list from `/etc/group`). The stale `Groups: 100 1000` claim in the escalation section is gone.
|
||||
- **A new troubleshooting entry for legitimate per-title fallbacks**, marked as a **hypothesis**.
|
||||
Between two fully hardware sessions, one logged `enabled, but no hardware decode accelerator found`
|
||||
and empty final decoder/encoder. UHD 630 decodes H.264, HEVC 8/10-bit and VP9 but **not AV1** and
|
||||
not 10-bit 4:2:2 HEVC, so the likely cause is the source file, not the platform — explicitly
|
||||
written as a lead to check first, **not** a confirmed diagnosis of that item.
|
||||
- The status is stated at the top of the page and in a rewritten provenance footer that separates the
|
||||
2026-10-02 confirmations from the superseded 2026-09-30/2026-10-01 hostPath-era readings.
|
||||
- `runbooks/README.md`'s index entry for the page was stale in the same two ways (it still described a
|
||||
`/dev/dri` hostPath and the dashboard-only proof) and is corrected to match.
|
||||
|
||||
Unchanged on purpose: the "STOP HERE IF PLEX IS `Pending`" section and the escape hatch (delete the
|
||||
`hardware:` block from `catalog/plex.yaml`, re-render) keep exactly their existing prominence — they
|
||||
are correct and they are the 2am material. The "no alert is possible" reasoning is also kept verbatim
|
||||
in substance: Plex declares no `metricsEndpoint`, renders no ServiceMonitor, and a fallback alert
|
||||
would be a rule evaluating against nothing.
|
||||
|
||||
**Documentation only.** No change to `catalog/`, `schemas/`, `apps/` or `platform/`.
|
||||
|
||||
## What I verified
|
||||
|
||||
- **Gate, in the pinned toolbox from this worktree** (`DEV_KUBECONFIG=/dev/null`, `ENGINE` shim
|
||||
bind-mounting the main checkout's `.git`): **rc 3 UNDETERMINED**, diff base
|
||||
`origin/main @ 948875aad37d`, with exactly the three expected skips and **no failures**. Green:
|
||||
`decisions-log`, `lint`, `l0`, `render-golden`, `l1`, `approval-policy`. Skipped:
|
||||
`spec-immutability` (spec repo unavailable → R-TEST-06 not verified), `gitea-settings`
|
||||
(`SETTINGS_API_TOKEN` unset), `conformance` (`CLUSTER_AVAILABLE!=true` → L2/L3 not run).
|
||||
Touched requirements: **R-OBS-04**, reported `UNVERIFIED` for lack of a cluster. `approval-policy`
|
||||
passed as `N/A` locally (`event='none'`, not a `pull_request`) — it evaluates for real on the PR.
|
||||
**rc 3 is not green**, it is the calibrated worktree result, and it is reported here as rc 3.
|
||||
- **The gate was run three times, with the same rc 3 / three-skip result on each committed tip** —
|
||||
including the final one. Worth recording from the first attempt: run *before* committing, with
|
||||
`HEAD == origin/main`, the touched-requirement set is `NOT DETERMINABLE (on-base)` and the
|
||||
R-TEST-03 coverage gate cannot be applied at all. Commit before gating from a fresh worktree branch.
|
||||
- **`runbooks/kustomization.yaml` still carries `- plex-quicksync=plex-quicksync.md`** in the
|
||||
`configMapGenerator` `files:` list (line 110) — checked explicitly, because without it the page
|
||||
merges and still 404s. `README=README.md` is likewise present, which is what serves the corrected
|
||||
index entry.
|
||||
- **This change touches no `sensitive_paths`.** `infra/gitea/approval-policy.json` lists `ci/**`,
|
||||
`.gitea/**`, `conformance/**`, `clusters/**`, `bootstrap/**`, `infra/gitea/**`,
|
||||
`platform/kyverno/**`, `platform-config/kustomization.yaml`, `.sops.yaml`, `**/*.sops.yaml`,
|
||||
`**/rbac.yaml`, `CLAUDE.md`. `runbooks/**` and `decisions/**` are not among them, and the
|
||||
`approval-policy` stage passed.
|
||||
- **No YAML was added or edited**, so `indent-sequences: consistent` (which blocked a sibling branch
|
||||
today) is not in play here; `lint` passed over the full tracked YAML set regardless.
|
||||
- **`DECISIONS-LOG.md` is untouched** — this handoff is its own file, per CLAUDE.md §7.
|
||||
- **The PR's own CI run was read, not assumed** (PR #39, run 208, task 261, head `1677dff5688b`).
|
||||
**It is RED, and not because of this branch.** Every stage this change could affect is green —
|
||||
`decisions-log`, `lint`, `l0`, `render-golden`, `l1`, `approval-policy` (`approval-policy` evaluated
|
||||
for real on the `pull_request` event and passed: no sensitive path touched). The single failure is
|
||||
**`gitea-settings`**, quoting the measured field:
|
||||
|
||||
[fail] required_approvals: DECLARED 1 but LIVE is 0
|
||||
[fail] Gitea has DRIFTED from infra/gitea/expected-branch-protection.json.
|
||||
[fail] MERGE GATE: BLOCKED
|
||||
|
||||
Every other declared setting matched (`block_admin_merge_override`, `block_on_rejected_reviews`,
|
||||
`dismiss_stale_approvals`, `enable_status_check`, `enable_push=false`,
|
||||
`status_check_contexts = ["gate / gate (pull_request)"]`, `runners online: 1`). So **live `main`
|
||||
currently requires ZERO approvals**, the declaration still says 1, and with
|
||||
`REQUIRE_GITEA_SETTINGS: "true"` in `.gitea/workflows/gate.yml` that drift **blocks every PR in
|
||||
the repo**, not just this one. Note the shape of it: the check that would notice approvals are
|
||||
unenforced is the check that is failing. Local branch `fix/admin-approval-override` (`616a497`,
|
||||
"fix(ci): let an administrator merge their own green PR") looks like the in-flight work that means
|
||||
to change the declaration. **I did not touch it** — `infra/gitea/**` is a sensitive path, needs a
|
||||
non-author approval, and is outside this branch's documentation-only scope. It is an operator
|
||||
decision: re-assert the protection, or change the declaration deliberately in its own PR.
|
||||
- The repository-wide grep for `992` to find what else still asserts it: remaining occurrences are in
|
||||
`catalog/plex.yaml`, `catalog/render.py`, `schemas/renderer-contract.md`,
|
||||
`platform/intel-gpu-plugin/README.md`, `platform/intel-gpu-plugin/kustomization.yaml`, and three
|
||||
older `decisions/` entries.
|
||||
|
||||
## What I did NOT verify
|
||||
|
||||
- **I did not re-measure anything on the live cluster in this session.** Every platform number on the
|
||||
page — the VAAPI/zero-copy log lines, `RW-OPEN-OK` as uid 1000, the `gpu.intel.com/i915` limits and
|
||||
requests on the pod spec, `Groups: 0 100 1000`, `id plex`, the 27 `Preferences.xml` attributes, the
|
||||
empty `/proc/*/fd` sweep — came to me as measurements taken earlier on 2026-10-02 and was
|
||||
transcribed, not re-run. Per CLAUDE.md §2b, treat the page as reporting those readings rather than
|
||||
as proof of current state; re-run the primary check before relying on it.
|
||||
- **The per-title fallback cause is unverified and labelled as such.** The source codec of the title
|
||||
that logged `no hardware decode accelerator found` was never read. The AV1 / 10-bit-4:2:2-HEVC
|
||||
explanation is inferred from UHD 630's capability set alone.
|
||||
- **Whether Plex's dashboard *ever* shows `Transcode (hw)` on this deployment is unknown.** It was
|
||||
observed silent during working hardware transcoding; whether that is a client/version quirk, a
|
||||
reporting bug, or something about this setup was not investigated. The page therefore demotes it
|
||||
rather than explaining it.
|
||||
- **The commands as written in the page were not executed from the page.** They are the forms that
|
||||
were run, re-wrapped for `dev.sh`; in particular the `exec -it deploy/plex -- sh` route for the argv
|
||||
check is noted as possibly lacking a TTY under `dev.sh run` and was not exercised in that shape.
|
||||
- **Stale `992` copies outside `runbooks/` were left alone**, deliberately — scope was documentation
|
||||
only, and `catalog/plex.yaml`, `catalog/render.py`, `schemas/renderer-contract.md` and
|
||||
`platform/intel-gpu-plugin/` all still carry the hostPath-era `Groups: 44 100 992 1000` reading as
|
||||
if current. `render.py --selftest` asserts around that comment, so correcting it is a real change
|
||||
with gate consequences, not a doc edit. **That is open work.** Older `decisions/` entries also cite
|
||||
992; those are historical records and should stay as written.
|
||||
- **CI on the PR.** The branch is pushed and the PR opened; I did not watch the Gitea run report, so
|
||||
nothing here should be read as "CI green". The only gate result I can quote is the local rc 3 above.
|
||||
+12
-1
@@ -161,7 +161,18 @@ a *green* one no longer needs a signature that cannot exist.
|
||||
**Security-relevant changes are still reviewed** — by `ci/approval-policy.sh` against
|
||||
`approval-policy.json` `sensitive_paths`, not by this field. An administrator waives that review
|
||||
only by posting `/approve-override <reason>` on the PR: reason mandatory, permission re-checked
|
||||
live, invalidated by a new commit, and recorded by account in the gate summary. Note that the
|
||||
live, invalidated by a new commit, and recorded by account in the gate summary.
|
||||
|
||||
> **Post the override as a PR *review* (type "Comment"), not in the plain comment box** — until
|
||||
> the CI token is re-minted. `SETTINGS_API_TOKEN` is `[read:repository, read:admin]`, and
|
||||
> `/issues/{n}/comments` needs Gitea's *issue* scope, so Gitea answers with an error **object**
|
||||
> instead of an array and the override is invisible. Measured on PR #38 (2026-10-02): a correct,
|
||||
> fresh, well-formed override in the comment box produced `approval-policy: FAIL` reading as "no
|
||||
> approving review" — a permission failure disguised as a missing one. `/pulls/{n}/reviews` is
|
||||
> under `read:repository` and worked in the same run. The stage now reads **both** sources, names
|
||||
> which one it used, and says plainly when the comments API is unreadable. `set-ci-token.sh`
|
||||
> requests `read:issue` as of 2026-10-02, so **re-run it** and the comment box works too — token
|
||||
> scopes cannot be widened in place. Note that the
|
||||
**agent account `gitea-admin` also reports permission `owner`**, so it can clear its own PRs that
|
||||
way; that is an accepted consequence of authorising by permission alone, and the route to
|
||||
narrowing it (an `override_admins` allowlist, already honoured by the code) is written down in
|
||||
|
||||
@@ -32,7 +32,8 @@
|
||||
"requires_reason": true,
|
||||
"authorized_permissions": ["admin", "owner"],
|
||||
"invalidated_by_new_commits": true,
|
||||
"_how": "Comment on the PR with the directive at the START of a line, followed by a reason: '/approve-override <why>'. ci/approval-policy.sh resolves the commenter's LIVE repo permission via /collaborators/{login}/permission and accepts the override only if it is in authorized_permissions. The overrider, the timestamp and the reason are printed in the gate log and the job summary.",
|
||||
"_how": "Post the directive at the START of a line, followed by a reason: '/approve-override <why>'. ci/approval-policy.sh resolves the poster's LIVE repo permission via /collaborators/{login}/permission and accepts the override only if it is in authorized_permissions. The overrider, the source, the timestamp and the reason are printed in the gate log and the job summary.",
|
||||
"_where_to_post_it": "EITHER a PR REVIEW body (type 'Comment') OR the ordinary PR comment box. Prefer the REVIEW: measured 2026-10-02 on PR #38, a well-formed override in the comment box was INVISIBLE to CI, because /issues/{n}/comments sits behind Gitea's *issue* token scope and SETTINGS_API_TOKEN is minted [read:repository, read:admin]. Gitea returned an error object rather than an array, and the stage reported 'no approving review' -- a permission problem wearing the costume of a missing override. /pulls/{n}/reviews is under read:repository and was readable in the same run, so a review body is the route that works on today's token. infra/gitea/set-ci-token.sh now also requests read:issue, which makes the comment box work -- but only after that script is RE-RUN, since an existing token's scopes cannot be widened in place. The stage accepts both sources and names which one it used, so neither route is load-bearing alone.",
|
||||
"_self_override_is_allowed": "The author may override their own PR if they hold an authorized permission. That is the entire point -- with one human there is no one else -- but it means the 'a change to the gate cannot be gated by the gate' protection is, for an administrator, advisory rather than structural. It is traded for an auditable record: the waiver is an explicit act, attributed, with a stated reason, in the PR's comment history and the gate output.",
|
||||
"_known_consequence_agent_accounts": "ACCEPTED, EYES OPEN (operator decision, 2026-10-01). Authorisation is by live permission ALONE, with no allowlist of human accounts. The agent account `gitea-admin` reports permission `owner` on this repo (measured 2026-10-01), so an AGENT can clear the sensitive-path review on its own PR by posting the directive. Agents author most PRs here, so this is the broad case, not the edge case. To narrow it later without changing any logic, add an `override_admins` allowlist here and require membership in addition to the permission check -- ci/approval-policy.sh reads both from this file.",
|
||||
"_staleness": "An override is dropped unless its comment is NEWER than the PR head commit, mirroring Gitea's own dismiss_stale_approvals. An override earned at commit A therefore does not silently cover commit B; push again and you must override again."
|
||||
|
||||
@@ -5,9 +5,23 @@
|
||||
# The token value is NEVER printed. It goes from the token API into the secrets API and
|
||||
# nowhere else -- not to stdout, not to a file, not into an agent transcript.
|
||||
#
|
||||
# Scope: `read:repository` + `read:admin` are what the check needs (the rule object, and the
|
||||
# runner list). Gitea token scopes are coarse; grant the least that works on your version and
|
||||
# confirm the check goes green rather than assuming the scope name.
|
||||
# Scope: `read:repository` + `read:admin` are what ci/gitea-settings.sh needs (the rule object,
|
||||
# and the runner list). `read:issue` is for ci/approval-policy.sh, and it was MISSING — see below.
|
||||
# Gitea token scopes are coarse; grant the least that works on your version and confirm the check
|
||||
# goes green rather than assuming the scope name.
|
||||
#
|
||||
# WHY read:issue IS HERE (added 2026-10-02, measured on PR #38). ci/approval-policy.sh reads the
|
||||
# PR's comments to find an admin `/approve-override`. /issues/{n}/comments sits behind the *issue*
|
||||
# scope, which this script did not request — so Gitea answered with an error OBJECT instead of an
|
||||
# array, and a perfectly well-formed override posted in the PR comment box was invisible to CI.
|
||||
# The failure did not look like a permission problem from the outside; it looked like the override
|
||||
# had not been posted. `/pulls/{n}/files` and `/pulls/{n}/reviews` are under read:repository, which
|
||||
# is why every other part of the same stage worked and this one did not.
|
||||
#
|
||||
# The stage now accepts an override from a REVIEW body as well, so it still functions on a token
|
||||
# WITHOUT read:issue. This scope is what makes the ordinary comment box work too — the thing
|
||||
# anybody would reach for first. **Re-run this script to pick it up; an existing token's scopes
|
||||
# cannot be widened in place.**
|
||||
set -euo pipefail
|
||||
HOST=gitea.home.keithjackson.dev; OWNER=Home-Network; REPO=home-network-k3s-impl
|
||||
# NOT GITEA_* — Gitea reserves that prefix for secret names and returns HTTP 400
|
||||
@@ -21,7 +35,7 @@ basic() { curl -sS --config <(printf 'user = "%s:%s"\n' "$U" "$P") "$@"; }
|
||||
|
||||
echo "==> minting token '${NAME}' as ${U}"
|
||||
TOKEN="$(basic -X POST -H 'Content-Type: application/json' \
|
||||
-d "{\"name\":\"${NAME}\",\"scopes\":[\"read:repository\",\"read:admin\"]}" \
|
||||
-d "{\"name\":\"${NAME}\",\"scopes\":[\"read:repository\",\"read:admin\",\"read:issue\"]}" \
|
||||
"https://${HOST}/api/v1/users/${U}/tokens" | jq -r '.sha1 // empty')"
|
||||
[ -n "$TOKEN" ] || { echo "token creation failed (check scopes for your Gitea version)" >&2; exit 1; }
|
||||
echo "==> got a ${#TOKEN}-character token (value not shown)"
|
||||
|
||||
+12
-8
@@ -153,14 +153,18 @@ Each affected runbook says so on its own page and gives the by-hand equivalent.
|
||||
plex.tv/claim — so it is a timed sequence, scripted as `infra/plex/claim-ship.sh`:
|
||||
token → `sops` → PR → gate → merge → **forced** `flux reconcile` → pod roll (~2m45s measured).
|
||||
Includes why a changed Secret does not restart anything, and why an expired token is harmless.
|
||||
- **`plex-quicksync`** — hardware transcoding on the host iGPU. GitOps puts `/dev/dri` in the pod
|
||||
(`hardware.gpu: intel-quicksync`); **a human still has to tick Settings → Transcoder → "Use hardware
|
||||
acceleration when available"**, and no environment variable can do it (the image supports no
|
||||
`PLEX_PREFERENCE*`, verified). The page names the real preference key (`HardwareAcceleratedCodecs`,
|
||||
read out of the binary), says which widely-cited key **does not exist**, and is explicit that the
|
||||
**only** proof is `Transcode (hw)` on the Plex dashboard — Plex renders no ServiceMonitor, so no
|
||||
alert could ever fire. Also: the device-cgroup failure to expect first and its device-plugin
|
||||
fallback, and why a `:latest` pull can silently break it.
|
||||
- **`plex-quicksync`** — hardware transcoding on the host iGPU. **Confirmed working 2026-10-02**
|
||||
(VAAPI decode *and* encode, zero-copy). GitOps requests `gpu.intel.com/i915` on the pod
|
||||
(`hardware.gpu: intel-quicksync`, satisfied by the `intel-gpu-plugin` DaemonSet — a `/dev/dri`
|
||||
hostPath was measured not to work: device-cgroup `EPERM`); **a human still has to tick Settings →
|
||||
Transcoder → "Use hardware acceleration when available"**, and no environment variable can do it
|
||||
(the image supports no `PLEX_PREFERENCE*`, verified). **The proof is `Plex Media Server.log`'s
|
||||
`final decoder: / final encoder:` line, not the Plex dashboard** — the dashboard showed nothing
|
||||
while hardware transcoding was working, so its silence proves nothing; the page carries the
|
||||
redacting grep (`X-Plex-Token` lives in that log), the WRONG log file to avoid, and two more
|
||||
documented false leads (an empty `/proc/*/fd` scan, and a missing `HardwareAcceleratedCodecs` key).
|
||||
Plex renders no ServiceMonitor, so no alert could ever fire. Also: **`Pending` stalls the whole app
|
||||
tier** and its one-field escape hatch, and why a `:latest` pull can silently break it.
|
||||
- **`oauth2-proxy-session`** — a gated UI loads but its saves quietly do nothing, and you are never
|
||||
re-prompted to log in. Tells the three stacked auth layers apart from the sidecar's own access log
|
||||
(the four lines that matter, including the upstream `-` that means the proxy answered a request the
|
||||
|
||||
+194
-55
@@ -1,5 +1,10 @@
|
||||
# plex-quicksync — making Plex actually use Intel Quick Sync, and proving it did
|
||||
|
||||
**Status: hardware transcoding is CONFIRMED WORKING on prod as of 2026-10-02** — VAAPI for both
|
||||
decode *and* encode, zero-copy, measured in Plex's own server log (see "How to verify"). The manual
|
||||
toggle has been ticked. This page is therefore now mostly a **verification and regression**
|
||||
procedure, and its old verification section was **wrong** — see the correction at the top of
|
||||
"How to verify".
|
||||
**Alert:** none, and none is possible — see "How to verify" below. This is an **operator procedure**,
|
||||
not an incident page. It is indexed here because the feature it completes is **inert until a human
|
||||
clicks one button**, and a switched-off feature that looks shipped is exactly what this page exists
|
||||
@@ -33,7 +38,7 @@ is in git**:
|
||||
| Half | Who does it | State |
|
||||
|---|---|---|
|
||||
| The render device is inside the container **and openable** | **GitOps.** `hardware.gpu: intel-quicksync` in `catalog/plex.yaml` renders `resources.limits: {gpu.intel.com/i915: 1}` on the app container. The `intel-gpu-plugin` DaemonSet (`platform/intel-gpu-plugin/`) injects the device **and the matching device-cgroup allow rule**. Flux applies it. | Done by merging that change. |
|
||||
| Plex is *told* to use it | **The operator, by hand, in the Plex web UI.** | **NOT done.** `HardwareAcceleratedCodecs` was **absent** from `Preferences.xml` when this page was written — the toggle has never been on. |
|
||||
| Plex is *told* to use it | **The operator, by hand, in the Plex web UI.** | **Done, 2026-10-02**, and confirmed from the **log** rather than from a preference key. `HardwareAcceleratedCodecs` is **still absent** from `Preferences.xml` with hardware transcoding working, so its absence proves nothing — see first-command #4. |
|
||||
|
||||
**There is no GitOps path to the second half, and that is verified rather than assumed.** A bounded
|
||||
grep across the running image's entire init surface — `/etc/cont-init.d`, `/etc/services.d`,
|
||||
@@ -79,8 +84,8 @@ also the only thing that produces the cgroup allow rule. There is no graceful-de
|
||||
The one improvement is that the loss is no longer silent.
|
||||
|
||||
What this does **not** do, so nobody reads more into a green gate than is there: it does not enable
|
||||
hardware transcoding (that is the manual toggle below), a green gate only proves the **manifest
|
||||
renders**, and it writes no `nodeSelector` — the resource itself constrains Plex to nodes the plugin
|
||||
hardware transcoding (that is the manual toggle below — done 2026-10-02), a green gate only proves
|
||||
the **manifest renders**, and it writes no `nodeSelector` — the resource itself constrains Plex to nodes the plugin
|
||||
advertises, which is the scheduler's doing and not a selector in this repo.
|
||||
|
||||
## Running the commands in this runbook
|
||||
@@ -172,17 +177,28 @@ cgroup rule that uid 0 could not buy its way past either. `RW-OPEN-DENIED` with
|
||||
visible is the **device-cgroup** failure — i.e. the plugin's allow rule did not arrive; see "Likely
|
||||
causes" #2.
|
||||
|
||||
**4. Is the toggle on?** Named key only — never the whole file:
|
||||
**4. Is the toggle on?** **You cannot answer this from `Preferences.xml` alone.** That mistake is on
|
||||
this page's record: it told readers to treat a missing `HardwareAcceleratedCodecs` as "never
|
||||
enabled", and on 2026-10-02 that key was **absent from all 27 attributes** while hardware
|
||||
transcoding was demonstrably working. Named keys only — never the whole file, it holds
|
||||
`PlexOnlineToken`:
|
||||
|
||||
```
|
||||
infra/devcontainer/dev.sh run bash -c \
|
||||
'kubectl --context k3s-prod -n plex exec deploy/plex -- \
|
||||
grep -o "HardwareAcceleratedCodecs=\"[01]\"" \
|
||||
"/config/Library/Application Support/Plex Media Server/Preferences.xml" || echo ABSENT'
|
||||
grep -o -e "HardwareAcceleratedCodecs=\"[01]\"" -e "HardwareDevicePath=\"[^\"]*\"" \
|
||||
"/config/Library/Application Support/Plex Media Server/Preferences.xml" || echo NEITHER-KEY'
|
||||
```
|
||||
|
||||
`ABSENT` ⇒ never enabled (the state this page was written in). `="0"` ⇒ explicitly off. `="1"` ⇒ on,
|
||||
and the remaining question is whether it *works* — go to "How to verify".
|
||||
| Reading | What it actually means |
|
||||
|---|---|
|
||||
| `HardwareDevicePath="…"` **present** | **Positive signal.** Plex writes this when hardware acceleration is configured; it was present on 2026-10-02 alongside a working VAAPI pipeline. |
|
||||
| `HardwareAcceleratedCodecs` **absent** | **Nothing. Neither on nor off.** Measured absent with the feature working (2026-10-02). Do not diagnose from it, and do not write it into the file to "fix" it. |
|
||||
| `HardwareAcceleratedCodecs="0"` | explicitly **off** — the only conclusive reading, and only in the negative. |
|
||||
| `HardwareAcceleratedCodecs="1"` | on. |
|
||||
|
||||
**The log settles it, this file does not.** Go to "How to verify"; the `hardware transcoding:
|
||||
enabled` and `final decoder/final encoder` lines are the verdict.
|
||||
|
||||
## The one manual step
|
||||
|
||||
@@ -191,8 +207,11 @@ Plex web UI → **Settings → Transcoder → "Use hardware acceleration when av
|
||||
|
||||
The underlying preference key is **`HardwareAcceleratedCodecs`**. That name was confirmed by
|
||||
extracting strings from the 23 MB `Plex Media Server` binary in the running container — **not** from
|
||||
recall and not from a forum post. Real neighbouring keys from the same binary, so you can recognise
|
||||
them and not invent them:
|
||||
recall and not from a forum post. **But do not expect to see it in `Preferences.xml` after ticking
|
||||
the box:** on 2026-10-02, with the toggle on and hardware transcoding working, the key was absent and
|
||||
`HardwareDevicePath` was what had been written. The key exists in the binary; Plex does not
|
||||
necessarily persist it. Real neighbouring keys from the same binary, so you can recognise them and
|
||||
not invent them:
|
||||
|
||||
| Key | What it is |
|
||||
|---|---|
|
||||
@@ -209,42 +228,121 @@ binary. Recorded explicitly so nobody spends an evening hunting for it or, worse
|
||||
Editing `Preferences.xml` by hand is **not** the procedure. It is app state on the replicated Longhorn
|
||||
tier, it is in no git commit, and the UI writes the key correctly. Use the UI.
|
||||
|
||||
## How to verify — and why there is no alert
|
||||
## How to verify — the log, not the dashboard
|
||||
|
||||
Start a playback that **forces a transcode** (a client that cannot direct-play the source, or pick a
|
||||
lower quality explicitly), then read Plex's own **Dashboard → Activity**:
|
||||
**Correction, 2026-10-02.** Until today this page said the Plex dashboard showing `Transcode (hw)`
|
||||
was "the *only* proof available, and nothing else will ever tell you". **That was false and it cost
|
||||
the operator real time:** hardware transcoding was fully working — VAAPI decode *and* encode, with
|
||||
zero-copy — while **Dashboard → Activity showed nothing of the sort**. A check that is silent while
|
||||
the feature works does not just fail to prove anything; it sends someone hunting a fault that does
|
||||
not exist. The authoritative check is **Plex's own server log**.
|
||||
|
||||
- **`Transcode (hw)`** on the session ⇒ hardware transcoding. **This is the proof.**
|
||||
- plain **`Transcode`** ⇒ software. The feature is not working, whatever the manifest says.
|
||||
### The primary check — run this one
|
||||
|
||||
**That dashboard is the *only* proof available, and nothing else will ever tell you.**
|
||||
`catalog/plex.yaml` declares no `metricsEndpoint`, so Plex renders **no ServiceMonitor at all** —
|
||||
there is no scrape, no transcode-mode metric, and therefore no alert that could fire. Do not open work
|
||||
to "alert on software-transcode fallback": with no metric behind it such a rule would be permanently
|
||||
inert, and this repo has already been burned at scale by rules that evaluated against nothing while
|
||||
looking like coverage (`runbooks/README.md`, "Alerts you should not trust the silence of"). If that
|
||||
gap is ever worth closing, the honest order is a real exporter first and a rule second.
|
||||
```
|
||||
./infra/devcontainer/dev.sh run bash -c 'kubectl --context k3s-prod -n plex exec deploy/plex -- \
|
||||
grep -aiE "hardware transcoding: (final|using)" \
|
||||
"/config/Library/Application Support/Plex Media Server/Logs/Plex Media Server.log" \
|
||||
| sed -E "s/(X-Plex-Token|token)=[^&[:space:]]*/\1=REDACTED/g" | tail -5'
|
||||
```
|
||||
|
||||
Corroborating, not a substitute — the server log on the config PVC:
|
||||
| Line | Verdict |
|
||||
|---|---|
|
||||
| `hardware transcoding: final decoder: vaapi, final encoder: vaapi` | **PASS.** Hardware in **both** directions. This is the pass condition. |
|
||||
| `hardware transcoding: using zero-copy transcoding` | Best case — frames never round-trip through the CPU. Observed on prod 2026-10-02. |
|
||||
| `hardware transcoding: final decoder: , final encoder: ` (both empty) | It **fell back to software** for that session. |
|
||||
| no matching line at all | No transcode has been attempted since the log rolled — force one (below), then look again. Absence is not a failure. |
|
||||
|
||||
The 2026-10-02 reading on prod, in full, for the shape to recognise:
|
||||
|
||||
```
|
||||
Codecs: hardware transcoding: testing API vaapi for device '/dev/dri/renderD128'
|
||||
(Intel CoffeeLake-S GT2 [UHD Graphics 630])
|
||||
hardware transcoding: using hardware decode accelerator vaapi
|
||||
hardware transcoding: zero-copy support present
|
||||
hardware transcoding: using zero-copy transcoding
|
||||
hardware transcoding: final decoder: vaapi, final encoder: vaapi
|
||||
```
|
||||
|
||||
To make a line appear, start a playback that **forces a transcode**: a client that cannot
|
||||
direct-play the source, or an explicitly lower quality. Direct Play / Direct Stream never reaches
|
||||
the transcoder and logs nothing.
|
||||
|
||||
### Three things to get right about that command
|
||||
|
||||
1. **Always keep the `sed`.** `Plex Media Server.log` carries API URLs containing
|
||||
**`X-Plex-Token=…`**, a live plex.tv credential. Any output that might land in a ticket, a
|
||||
transcript or a chat window must be redacted — CLAUDE.md §6, and a credential has already been
|
||||
leaked into a transcript in this repo exactly this way. The redacting form above is the form to
|
||||
copy; do not "simplify" the pipe out of it.
|
||||
2. **`Plex Transcoder Statistics.log` is the WRONG FILE.** The name is irresistible and someone will
|
||||
reach for it. It holds XML `<Variant .../>` session records — **not** ffmpeg/VAAPI output. There
|
||||
is no decoder/encoder verdict anywhere in it. The file you want is `Plex Media Server.log`.
|
||||
3. `grep -a` is deliberate: the log carries occasional non-UTF-8 bytes, and without `-a` GNU grep
|
||||
reports "binary file matches" and prints nothing — which reads exactly like "no hardware
|
||||
transcoding".
|
||||
|
||||
### Secondary and unreliable: the Plex dashboard
|
||||
|
||||
**Dashboard → Activity** *may* label a transcoding session `Transcode (hw)`. If it does, it agrees
|
||||
with the log and you are done. **If it does not, that proves nothing at all** — measured 2026-10-02:
|
||||
hardware transcoding active and zero-copy, dashboard silent. Never conclude "software transcoding"
|
||||
from the dashboard and never open an investigation on its say-so. Kept here only because it is the
|
||||
fastest glance available; it is a convenience, **not evidence**.
|
||||
|
||||
### Process-level, while a transcode is in flight: the transcoder's argv
|
||||
|
||||
This shows Plex *requesting* hardware, as distinct from reporting it afterwards. Get a shell in the
|
||||
pod (if `-it` has no TTY under `dev.sh run`, wrap each line in `sh -c "…"` instead):
|
||||
|
||||
```
|
||||
infra/devcontainer/dev.sh run bash -c \
|
||||
'kubectl --context k3s-prod -n plex exec deploy/plex -- \
|
||||
sh -c "grep -i -m20 -e hardware -e vaapi -e hwdevice \
|
||||
\"/config/Library/Application Support/Plex Media Server/Logs/Plex Media Server.log\""'
|
||||
'kubectl --context k3s-prod -n plex exec -it deploy/plex -- sh'
|
||||
```
|
||||
|
||||
Treat those strings as advisory: they are Plex's, not this repo's, and they move between versions.
|
||||
Absence of a match is **not** evidence of software transcoding; a VA-API device open is good evidence
|
||||
of hardware. The dashboard remains the verdict.
|
||||
then, inside the pod:
|
||||
|
||||
```
|
||||
ps -eo pid,user,comm | grep -i transcoder
|
||||
tr '\0' ' ' < /proc/<transcoder-pid>/cmdline | grep -oE '(-hwaccel[a-z_]*|vaapi|renderD[0-9]+)'
|
||||
```
|
||||
|
||||
`-hwaccel`, `-hwaccel_device`, `vaapi`, `renderD128` present ⇒ Plex is asking for hardware. Keep the
|
||||
`grep -oE`: the raw argv contains media paths and session identifiers you do not want in a
|
||||
transcript. This only works while a session is actually transcoding.
|
||||
|
||||
### Documented false lead: scanning `/proc/*/fd` for a holder of `renderD128`
|
||||
|
||||
**It returns nothing even when hardware transcoding is working.** Measured 2026-10-02: a
|
||||
`/proc/*/fd` sweep inside the pod found **no** process holding `/dev/dri/renderD128`, while the log
|
||||
showed `final decoder: vaapi, final encoder: vaapi` for sessions either side of the scan. Transcode
|
||||
sessions are **short** and the scan landed in a gap between two of them. An empty fd scan is a
|
||||
**timing artefact, not a finding** — do not report it as evidence of software transcoding, and do
|
||||
not escalate on it. Use the argv check above (which has the same timing constraint but tells you
|
||||
something when it does catch a session), or just read the log, which is retrospective and therefore
|
||||
does not care about timing.
|
||||
|
||||
### Why there is no alert — this half was always right
|
||||
|
||||
`catalog/plex.yaml` declares no `metricsEndpoint`, so Plex renders **no ServiceMonitor at all** —
|
||||
there is no scrape, no transcode-mode metric, and therefore no alert that could fire. Do not open
|
||||
work to "alert on software-transcode fallback": with no metric behind it such a rule would be
|
||||
permanently inert, and this repo has already been burned at scale by rules that evaluated against
|
||||
nothing while looking like coverage (`runbooks/README.md`, "Alerts you should not trust the silence
|
||||
of"). If that gap is ever worth closing, the honest order is a real exporter first and a rule
|
||||
second.
|
||||
|
||||
## Likely causes, most likely first
|
||||
|
||||
When the dashboard still says plain `Transcode`:
|
||||
When the **log** says it fell back to software — `final decoder: , final encoder: ` with both
|
||||
fields empty. (Not when the dashboard is quiet: the dashboard's silence means nothing, see "How to
|
||||
verify".)
|
||||
|
||||
1. **The toggle is off.** Overwhelmingly the most likely answer, and the reason this page exists.
|
||||
First-command #4 returns `ABSENT` or `="0"`. Remedy: the manual step above. Check this **before**
|
||||
investigating anything technical.
|
||||
1. **The toggle is off.** Historically the most likely answer and the reason this page exists. It was
|
||||
ticked on 2026-10-02 and should stay ticked, but a `/config` restore or a stray UI save can unset
|
||||
it. Remedy: the manual step above. **Do not diagnose this from a missing
|
||||
`HardwareAcceleratedCodecs`** — see first-command #4; only `="0"` is conclusive, and the log line
|
||||
`hardware transcoding: enabled` is the real signal that the toggle took.
|
||||
|
||||
2. **The device cannot be `open()`ed — the plugin's cgroup allow rule did not arrive.** Symptom:
|
||||
first-command #3 shows `renderD128` present and correctly owned, and the open returns
|
||||
@@ -276,13 +374,27 @@ When the dashboard still says plain `Transcode`:
|
||||
4. **The image changed under you.** See "The floating-image caveat". A `:latest` pull can remove the
|
||||
mechanism this feature leans on with no commit in this repo.
|
||||
|
||||
5. **The session was never transcoding.** Direct Play / Direct Stream never touches the transcoder, so
|
||||
`Transcode (hw)` cannot appear. Confirm the session actually says `Transcode` before treating it as
|
||||
a hardware failure.
|
||||
5. **The session was never transcoding.** Direct Play / Direct Stream never reaches the transcoder,
|
||||
so no `hardware transcoding:` line is written for it at all. An *absent* line is this, not a
|
||||
failure — confirm a transcode actually happened before treating anything as a hardware fault.
|
||||
|
||||
6. **The source needs a codec Quick Sync on this generation cannot handle** — e.g. AV1 decode, which
|
||||
Coffee Lake's UHD 630 has no engine for. Plex falls back to software *per stream*. Symptom: some
|
||||
sessions show `(hw)` and others do not. Not a fault.
|
||||
6. **The *source* codec may be one this iGPU cannot decode — HYPOTHESIS, not a confirmed
|
||||
diagnosis.** The symptom that points here is fallback on **particular titles** while other
|
||||
sessions are hardware. Measured on prod 2026-10-02, *between* two fully hardware sessions, one
|
||||
session logged:
|
||||
|
||||
```
|
||||
hardware transcoding: enabled, but no hardware decode accelerator found
|
||||
hardware transcoding: final decoder: , final encoder:
|
||||
```
|
||||
|
||||
The platform was demonstrably working either side of it, so the likely explanation is the **file**,
|
||||
not the cluster: UHD 630's media engine decodes H.264, HEVC 8-bit and 10-bit, and VP9 — but **not
|
||||
AV1**, and not 10-bit 4:2:2 HEVC. **This was NOT verified against that particular title.** It is
|
||||
inferred from the hardware's capability set and is written here as the lead to check first, not as
|
||||
a finding. So: fallback on *all* files is a platform problem (causes 1-4); fallback on *some*
|
||||
files, check the source codec before suspecting the device, and expect occasional legitimate
|
||||
per-title fallbacks to be normal rather than a fault.
|
||||
|
||||
## Plex will not schedule
|
||||
|
||||
@@ -360,19 +472,30 @@ infra/devcontainer/dev.sh run bash -c \
|
||||
```
|
||||
|
||||
`45-plex-hw-transcode-and-connected-tuner` still present ⇒ the group mechanism is still the one
|
||||
described here. Gone or renamed ⇒ stop and re-measure. In particular the "no `supplementalGroups`
|
||||
needed" conclusion in `catalog/plex.yaml` and `schemas/renderer-contract.md` is only true while that
|
||||
hook exists.
|
||||
described here: the hook reads the injected device's **own** ownership, which under the device plugin
|
||||
is `root:root`, so it resolves GID `0` (see "When to escalate or stop"). Gone or renamed ⇒ stop and
|
||||
re-measure. In particular the "no `supplementalGroups` needed" conclusion in `catalog/plex.yaml` and
|
||||
`schemas/renderer-contract.md` is only true while that hook exists.
|
||||
|
||||
## When to escalate or stop
|
||||
|
||||
- **Do not add `supplementalGroups` or a `podSecurityContext` to `catalog/plex.yaml`.** It would be
|
||||
**inert**, and it would mislead the next person. Measured in-container 2026-09-30: the image's
|
||||
`cont-init.d` hook stats each `/dev/dri` char device and `usermod -a -G`s the `plex` user into its
|
||||
group *inside* the container, and the service then execs via `s6-setuidgid plex`, which **replaces**
|
||||
the process group list from `/etc/group` rather than appending — `Plex Media Server` runs with
|
||||
`Groups: 100 1000`, root's gid `0` (which PID 1 does hold) gone. Anything Kubernetes supplies is
|
||||
**inert**, and it would mislead the next person. The image's `cont-init.d` hook stats each
|
||||
`/dev/dri` char device and `usermod -a -G`s the `plex` user into **that device's** group, inside
|
||||
the container; the service then execs via `s6-setuidgid plex`, which **rebuilds** the process group
|
||||
list from `/etc/group` rather than appending to it. Anything Kubernetes supplies is therefore
|
||||
discarded before Plex starts. `render.py --selftest` asserts that absence deliberately.
|
||||
- **GID `992` is a hostPath-era number — do not expect it, and do not read the root group as a
|
||||
defect.** Under the device plugin the injected `/dev/dri/renderD128` arrives **`root:root`**, so
|
||||
the hook resolves GID **`0`** and adds `plex` to the **root group**. Measured 2026-10-02:
|
||||
`Plex Media Server` runs with `Groups: 0 100 1000`, and `id plex` reports
|
||||
`groups=1000(plex),0(root),100(users)`. Under the old hostPath the device was `root:render`
|
||||
(GID 992 on these nodes) and the reading was `Groups: 44 100 992 1000`. **992 is now irrelevant**:
|
||||
if any page, comment or older handoff tells you to expect it, that text is stale and the system
|
||||
you are looking at is working. The root-group membership is an inherent property of the plugin's
|
||||
`DeviceSpec` — it injects the host device with its host ownership — **not** a defect, and **not** a
|
||||
reason to reintroduce `supplementalGroups`, which would still be a no-op for the reason in the
|
||||
bullet above.
|
||||
- **Do not reach for `privileged: true`.** Kyverno `restrict-host-namespaces` is Enforce and will
|
||||
reject it — and it would not help: `uid 0` failed the same `open()` in the 2026-09-30 measurement,
|
||||
because root does not bypass the cgroup device gate.
|
||||
@@ -390,7 +513,8 @@ hook exists.
|
||||
watch state (`policy: daily-offcluster`). The transcode toggle is one preference, not a reason to
|
||||
lose the DB.
|
||||
- **Stop and get the operator** if the device cannot be opened from the pod as uid 1000 (cause #2
|
||||
confirmed) *under the current mechanism*. The next step is a platform change in
|
||||
confirmed) *under the current mechanism* — note that this open **passed** on 2026-10-02
|
||||
(`RW-OPEN-OK` as uid 1000 via `s6-setuidgid plex`), so a failure now is a regression. The next step is a platform change in
|
||||
`platform/intel-gpu-plugin/`, with its own PR, not more poking at this workload.
|
||||
|
||||
## Known gaps, deliberately not fixed here
|
||||
@@ -413,10 +537,25 @@ Recorded so they are findable, and so nobody assumes they were overlooked:
|
||||
|
||||
---
|
||||
Source of truth: `runbooks/plex-quicksync.md`. Index: `/runbooks/`
|
||||
The node measurements (all three prod nodes) and the in-container image measurements cited here were
|
||||
taken **2026-09-30** against `plexinc/pms-docker` digest `sha256:e0ab27395614…`
|
||||
(`plexmediaserver 1.43.4.10903`). The `EPERM` open measurements, the `Groups: 44 100 992 1000`
|
||||
reading and the `gpu.intel.com/i915: 1` allocatable/DaemonSet readings were taken **2026-10-01**.
|
||||
Hardware transcoding has **still not been observed working**: the open test has not been re-run under
|
||||
the device-plugin mechanism (it cannot be, until that merges and Flux reconciles), and the Plex-side
|
||||
toggle is still unset.
|
||||
|
||||
**Hardware transcoding is CONFIRMED WORKING on prod, measured 2026-10-02.** From
|
||||
`Plex Media Server.log` in the running pod: API `vaapi` on `/dev/dri/renderD128`
|
||||
(`Intel CoffeeLake-S GT2 [UHD Graphics 630]`), `using hardware decode accelerator vaapi`,
|
||||
`zero-copy support present`, `using zero-copy transcoding`, and
|
||||
`final decoder: vaapi, final encoder: vaapi`. Also measured in the same pod: `RW-OPEN-OK` opening
|
||||
`/dev/dri/renderD128` as uid 1000 via `s6-setuidgid plex`; the pod spec carrying
|
||||
`{"limits":{"gpu.intel.com/i915":"1"},"requests":{"gpu.intel.com/i915":"1"}}`; `Groups: 0 100 1000`
|
||||
and `id plex` → `groups=1000(plex),0(root),100(users)`; and `HardwareDevicePath` present in
|
||||
`Preferences.xml` while `HardwareAcceleratedCodecs` was absent from all 27 attributes.
|
||||
|
||||
Earlier readings, kept for provenance and **superseded** where they conflict: the node measurements
|
||||
(all three prod nodes) and the in-container image measurements taken **2026-09-30** against
|
||||
`plexinc/pms-docker` digest `sha256:e0ab27395614…` (`plexmediaserver 1.43.4.10903`); the hostPath
|
||||
`EPERM` open results and the `Groups: 44 100 992 1000` / GID-992 group reading, **2026-10-01, under
|
||||
the hostPath mechanism that no longer exists**; and the `gpu.intel.com/i915: 1` allocatable and
|
||||
`intel-gpu-plugin` DaemonSet readings, **2026-10-01**.
|
||||
|
||||
What this page still does **not** establish: that any particular title's software fallback was caused
|
||||
by its source codec (cause 6 is an unverified hypothesis), and that Plex's dashboard *ever* labels a
|
||||
session `Transcode (hw)` on this deployment — it has not been observed doing so, which is exactly why
|
||||
the log is now the primary check.
|
||||
Reference in new issue
Block a user