feat(uat): slot-aware daytime cluster-name convention (ADR-017) - #2081
Merged
Merged
Conversation
njhensley
added
the
theme/ci-dx
Contributor
📝 WalkthroughWalkthroughThe change adds validated reservation slugs and runtime slots to daytime UAT cluster naming. The run workflow resolves and forwards these values to AWS, GCP, and Azure workflows. Provider workflows use slug-and-slot names and retain legacy reservation-prefix matching. Registry fixtures, broker tests, contributor documentation, and ADR-017 define and verify the convention. Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/uat-aws.yaml:
- Around line 43-50: Validate inputs.slug against the registry slug format and
inputs.slot as numeric in every Validate inputs step before any guard, teardown
logic, DEPLOYMENT_ID, or prefix construction. Apply this in
.github/workflows/uat-aws.yaml lines 43-50, .github/workflows/uat-azure.yaml
lines 44-51, and .github/workflows/uat-gcp.yaml lines 43-50; malformed or blank
values must fail with an error rather than continue.
In @.github/workflows/uat-run.yaml:
- Around line 74-77: Validate inputs.slot in the resolve step before any
provider dispatch, accepting only the string value "0" and failing with an error
for every other value. Ensure this validation covers all daytime-up/daytime-down
forwarding paths, including the input declaration and dispatch blocks, so
unsupported slots cannot be provisioned or forwarded.
In `@docs/design/017-uat-cluster-name-convention.md`:
- Around line 22-24: Add the `text` language identifier to both fenced code
blocks in the UAT cluster naming documentation, including the block containing
the nightly/daytime examples and the additionally referenced block, so they
satisfy markdownlint MD040 while remaining illustrative output.
In `@pkg/uatbroker/registry_test.go`:
- Around line 267-300: The slug tests around the existing “bad-charset slug” and
“too-short slug” cases do not independently cover the length boundary. Add a
rejection case for the five-character slug “abcde”, plus valid cases for the
minimum-length “a1” and maximum-length “abcd” slugs, ensuring each exercises
slug validation without relying on character-set failures.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e0ab2520-9bed-4fdb-9402-8adb81b77bbe
📒 Files selected for processing (12)
.github/workflows/uat-aws.yaml.github/workflows/uat-azure.yaml.github/workflows/uat-gcp.yaml.github/workflows/uat-run.yamldocs/contributor/uat.mddocs/design/017-uat-cluster-name-convention.mdinfra/uat/reservations.yamlpkg/uatbroker/model.gopkg/uatbroker/registry.gopkg/uatbroker/registry_test.gotools/uat-broker/main.gotools/uat-broker/main_test.go
Comment on lines
+22
to
+24
| ``` | ||
| nightly: aicr-uat-<run_id> # run-scoped, per-run isolation | ||
| daytime: aicr-uat-day-<reservation>-<run_id> # ephemeral, reservation-scoped |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to both fenced code blocks.
markdownlint reports MD040 for both fences. Use text if the blocks are illustrative output.
Also applies to: 89-95
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/017-uat-cluster-name-convention.md` around lines 22 - 24, Add the
`text` language identifier to both fenced code blocks in the UAT cluster naming
documentation, including the block containing the nightly/daytime examples and
the additionally referenced block, so they satisfy markdownlint MD040 while
remaining illustrative output.
Source: Linters/SAST tools
njhensley
force-pushed
the
ci/uat-cluster-name-convention
branch
from
August 5, 2026 23:14
daa04e3 to
e837ed4
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/uat-aws.yaml:
- Around line 422-423: Replace the regex-based prefix checks in
.github/workflows/uat-aws.yaml lines 422-423 and 491-492,
.github/workflows/uat-azure.yaml lines 379-380 and 446-447, and
.github/workflows/uat-gcp.yaml lines 363-364 with literal prefix parsing that
also requires a digit suffix, or escape both prefixes before ERE matching.
Ensure malformed reservation names produce an error rather than being treated as
no match, and preserve guard and teardown discovery behavior.
In `@docs/contributor/uat.md`:
- Around line 158-165: Update the preceding cluster-discovery instructions in
the UAT documentation to use the new (slug, slot)-scoped prefix, matching the
AWS and GCP examples. If legacy clusters remain human-accessible during
migration, document their reservation-scoped prefix as a separate fallback
rather than the primary discovery method.
- Around line 10-14: Update the daytime cluster naming logic described in the
daytime deployment documentation to include github.run_attempt alongside
github.run_id in the generated name, ensuring reruns produce a distinct cluster
name and preserve the unique-per-provision guarantee. Update the documented name
pattern and any related prefix/discovery examples consistently.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e4c75ac4-8b8e-4c96-a809-43889ee47bf2
📒 Files selected for processing (12)
.github/workflows/uat-aws.yaml.github/workflows/uat-azure.yaml.github/workflows/uat-gcp.yaml.github/workflows/uat-run.yamldocs/contributor/uat.mddocs/design/017-uat-cluster-name-convention.mdinfra/uat/reservations.yamlpkg/uatbroker/model.gopkg/uatbroker/registry.gopkg/uatbroker/registry_test.gotools/uat-broker/main.gotools/uat-broker/main_test.go
Comment on lines
+10
to
+14
| - **On demand — handoff.** The [daytime human-access deployment](#daytime-human-access-deployment) is stood up with `lifecycle=daytime-up`: provision, deploy the stack, and **hold** (no teardown) under an ephemeral, `(slug, slot)`-scoped cluster name (`aicr-uat-day-<slug>-<slot>-<run-id>`, ADR-017) — unique per provision, so a re-provision never reuses the name the previous teardown just deleted (which collides with cloud deletion tombstones and stale terraform-state locks). This is **on demand — there is no morning cron**: an operator dispatches it when they want a cluster (see [Requesting a daytime cluster](#requesting-a-daytime-cluster)). DC2 owns the provision-and-hold mechanic; DC8 (`uat-daytime.yaml`) owns *which* flavor lands on *which* cloud and how access is shared. | ||
| - **Day — human use.** The daytime cluster is used outside CI — humans reach it [out-of-band](#daytime-human-access-deployment), never through the CI path. | ||
| - **Evening — teardown.** `uat-daytime.yaml` fires `lifecycle=daytime-down` on an evening cron — the **only** scheduled daytime edge — to tear the daytime cluster down and release the reservation **before** the next night batch. It is an unconditional safety net: it runs whether or not anyone stood a cluster up, so a manually-provisioned daytime cluster left running is reclaimed without anyone remembering to ask. It is a teardown *attempt*, not a guarantee — if the destroy itself fails, the [pre-batch guard](#pre-batch-guard) blocks the nightly batch rather than letting it race the still-held cluster. | ||
|
|
||
| The phases are independently scheduled (cron edges), not chained: the per-reservation lease — plus a [pre-batch guard](#pre-batch-guard) — keeps them from overlapping, so a crashed or overrunning phase never orphans the reservation. A hosted GitHub Actions job is capped at the runner's timeout (hours, not a whole working day), so a single lease-holding run cannot span the day; the lease only needs to cover the brief transition windows, and the steady-state daytime cluster's existence is tracked by its reservation-scoped name **prefix** (`aicr-uat-day-<reservation>-*`, discovered by a list-and-match scan) rather than a continuously held run. | ||
| The phases are independently scheduled (cron edges), not chained: the per-reservation lease — plus a [pre-batch guard](#pre-batch-guard) — keeps them from overlapping, so a crashed or overrunning phase never orphans the reservation. A hosted GitHub Actions job is capped at the runner's timeout (hours, not a whole working day), so a single lease-holding run cannot span the day; the lease only needs to cover the brief transition windows, and the steady-state daytime cluster's existence is tracked by its `(slug, slot)`-scoped name **prefix** (`aicr-uat-day-<slug>-<slot>-*`, discovered by a list-and-match scan) rather than a continuously held run. |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect: daytime cluster names use a value that changes on workflow re-runs.
rg -n -C 5 'aicr-uat-day-|run_id|run-id|run_attempt|run-attempt' \
.github/workflows/uat-run.yaml \
.github/workflows/uat-aws.yaml \
.github/workflows/uat-gcp.yaml \
.github/workflows/uat-azure.yamlRepository: NVIDIA/aicr
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- daytime DEPLOYMENT_ID definitions ---'
rg -n -A4 -B3 "DEPLOYMENT_ID:|format\\('aicr-uat-day|github\\.run_attempt|run_attempt" \
.github/workflows/uat-aws.yaml \
.github/workflows/uat-gcp.yaml \
.github/workflows/uat-azure.yaml
printf '%s\n' '--- guard and lifecycle control flow ---'
rg -n -A8 -B8 "Pre-batch guard|DAYTIME_FOUND|Resolve the ephemeral daytime cluster|Destroy Cluster|inputs\\.lifecycle|daytime-up|daytime-down" \
.github/workflows/uat-aws.yaml \
.github/workflows/uat-gcp.yaml \
.github/workflows/uat-azure.yaml \
| head -n 500
printf '%s\n' '--- documentation context ---'
sed -n '1,22p' docs/contributor/uat.mdRepository: NVIDIA/aicr
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
files = [
Path(".github/workflows/uat-aws.yaml"),
Path(".github/workflows/uat-gcp.yaml"),
Path(".github/workflows/uat-azure.yaml"),
]
pattern = re.compile(
r"format\('aicr-uat-day-\{0\}-\{1\}-\{2\}',\s*"
r"inputs\.slug,\s*inputs\.slot,\s*github\.run_id\)"
)
for path in files:
text = path.read_text()
matches = pattern.findall(text)
print(f"{path}: daytime_name_uses_github.run_id={bool(matches)}, "
f"occurrences={len(matches)}, uses_github.run_attempt={'github.run_attempt' in text}")
# A rerun changes the attempt but preserves the run ID. The provider expression
# therefore produces the same daytime name for both executions.
slug, slot, run_id = "h100", "0", "12345"
attempts = [1, 2]
names = [
f"aicr-uat-day-{slug}-{slot}-{run_id}"
for _attempt in attempts
]
print(f"rerun_attempts={attempts}")
print(f"generated_names={names}")
print(f"same_name_on_rerun={names[0] == names[1]}")
PYRepository: NVIDIA/aicr
Length of output: 617
🌐 Web query:
GitHub Actions official documentation GITHUB_RUN_ID same workflow run rerun GITHUB_RUN_ATTEMPT
💡 Result:
In GitHub Actions, GITHUB_RUN_ID and GITHUB_RUN_ATTEMPT serve different purposes when managing workflow executions and re-runs: GITHUB_RUN_ID This is a unique identifier for a specific workflow run within a repository [1]. Crucially, the GITHUB_RUN_ID remains constant for the entire lifecycle of that workflow run, even if you perform multiple re-runs [1][2]. GITHUB_RUN_ATTEMPT This is a unique number that tracks the specific attempt of a workflow run [1]. It begins at 1 for the initial attempt and increments by 1 with each subsequent re-run [1][3]. Combining these variables is a common practice for generating unique identifiers for artifacts, logs, or external resources that need to remain distinct across different attempts of the same workflow run [4][3]. You can access these as environment variables (e.g., $GITHUB_RUN_ID and
Citations:
- 1: https://docs.github.com/en/actions/reference/workflows-and-actions/variables
- 2: https://stackoverflow.com/questions/54310050/how-to-version-build-artifacts-using-github-actions
- 3: https://stackoverflow.com/questions/70927856/in-github-actions-how-can-i-uniquely-identify-the-workflow-under-execution
- 4: https://stackoverflow.com/questions/67789507/generate-unique-string-in-github-actions
- 5: https://stackoverflow.com/questions/75901304/access-env-variable-with-java-and-github-action
Include github.run_attempt in daytime cluster names.
All provider workflows use github.run_id, which remains unchanged on reruns. A rerun can therefore reuse the same cluster name and violate the “unique per provision” guarantee. Add github.run_attempt to the suffix or remove that guarantee.
🧰 Tools
🪛 LanguageTool
[style] ~12-~12: ‘whether or not’ might be wordy. Consider a shorter alternative.
Context: ...is an unconditional safety net: it runs whether or not anyone stood a cluster up, so a manuall...
(EN_WORDINESS_PREMIUM_WHETHER_OR_NOT)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/contributor/uat.md` around lines 10 - 14, Update the daytime cluster
naming logic described in the daytime deployment documentation to include
github.run_attempt alongside github.run_id in the generated name, ensuring
reruns produce a distinct cluster name and preserve the unique-per-provision
guarantee. Update the documented name pattern and any related prefix/discovery
examples consistently.
Contributor
|
@njhensley this PR now has merge conflicts with |
lockwobr
requested changes
Aug 6, 2026
lockwobr
left a comment
lockwobr
left a comment
Contributor
There was a problem hiding this comment.
Cross-review of e837ed45, with each finding independently re-checked against the pinned tree before being reported. Details are inline.
Nothing here is a merge blocker: the one finding with an operational failure mode is latent (no current registry row can trigger it), and the rest is documentation drift, most of it inside the migration window this PR opens.
Open questions
- Once the legacy
aicr-uat-day-<reservation>-leg is dropped, the guard's discovery key becomes purely(slug, slot)-scoped, so anightlyrun at the defaultslot=0would no longer be blocked by a leaked daytime cluster in a different slot on the same reservation. A nightly batch consumes the whole reservation, not one slot. Unreachable today sinceuat-run.yamlrejectsslot != 0, but the follow-up that removes the shim should decide the guard's slot scope deliberately rather than inheriting it. - Is the shim removal tracked anywhere? "Drop the legacy leg once none remain" appears in six places with no issue reference, and there is no TODO-with-issue or CI check, so nothing schedules the cleanup or flags it once the window closes.
docs/contributor/uat.mdnames ADR-017 in prose at lines 10, 94-97 and 185 but never links to../design/017-uat-cluster-name-convention.md, unlike how ADR-016 is referenced fromdocs/user/cli-reference.mdanddocs/integrator/recipe-development.md. Stylistic only.- GitHub applying the
workflow_dispatchdefault when a dispatcher omits-f slot=was reasoned from the workflow definitions, not executed. If defaults were not applied,inputs.slotwould be empty and the per-cloudSLOT_REpreflight fails closed, so the failure direction is safe either way. - The GKE 40-char / AKS 63 / EKS 100 caps are taken from the ADR text and were not confirmed against upstream cloud docs. The ADR's length-budget arithmetic is internally consistent.
Introduce a per-(slug, slot) daytime cluster-name convention to replace the cloud-scoped reservation-name discovery key: nightly: aicr-uat-<run_id> (unchanged) daytime: aicr-uat-day-<slug>-<slot>-<run_id> Captures the problem (cloud-scoped collisions, no slot axis, GKE 40-char length pressure), the convention, the length budget vs GKE/AKS/EKS caps, the slug/slot length policy, the per-(reservation, slot) discovery-key rationale, the consumer inventory, and the dual-prefix migration. Signed-off-by: Nathan Hensley <nhensley@nvidia.com>
The daytime cluster name embeds a discovery key the guard and evening
teardown scan across runs to find a held cluster. The reservation name
is long (pressuring GKE's 40-char cap) and only cloud-unique, so it
cannot distinguish two reservations one account holds in the same cloud.
Add a 'slug' field (^[a-z][a-z0-9]{1,3}$, registry-unique) as the compact,
account-stable discovery key for aicr-uat-day-<slug>-<slot>-<run_id>
(ADR-017):
- reservations.yaml: slug on every row (ah1/gh1/zh1/ag2/kh1)
- pkg/uatbroker: Reservation.Slug + load-time validation (non-empty,
unique, charset) in the same loop as the other fields
- tools/uat-broker: emit slug= in the reservations --name output so
uat-run.yaml exposes needs.resolve.outputs.slug
- tests: valid/empty/duplicate/bad-charset/too-short slug cases, the
committed-slug lock, and a slug= assertion on the resolver output
Signed-off-by: Nathan Hensley <nhensley@nvidia.com>
Consume the new registry slug and a runtime slot to name daytime clusters
aicr-uat-day-<slug>-<slot>-<run_id> (ADR-017), replacing the cloud-scoped
reservation-name discovery key:
- uat-run.yaml: workflow-level 'slot' input (default '0'); expose the
resolved slug; forward slug/slot into the aws/gcp/azure with: blocks
(not kind — the nvkind lane provisions no cloud cluster)
- uat-{aws,gcp,azure}.yaml: slug/slot workflow_call inputs; DEPLOYMENT_ID
daytime branch derives the new name; the pre-batch guard and teardown
resolve step key on the (slug, slot) prefix
- migration: guard + teardown scans are DUAL-PREFIX — they match both the
new prefix and the legacy aicr-uat-day-<reservation>- prefix so an
in-flight old-named daytime cluster is still discovered and torn down.
A transitional shim, removed once none remain. All existing fail-closed
behavior (list-error => exit 1, bounded retry, >1-match => fail loud)
and the guard-broad/resolve-anchored asymmetry are preserved
- docs/contributor/uat.md: lifecycle table, discovery-prefix prose,
reach-the-cluster examples, pre-batch-guard description, and the
add-a-reservation example updated to the new scheme + required slug
Signed-off-by: Nathan Hensley <nhensley@nvidia.com>
njhensley
force-pushed
the
ci/uat-cluster-name-convention
branch
from
August 7, 2026 15:46
e837ed4 to
78d5fbb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/uatbroker/registry.go`:
- Around line 150-154: Extend the reservation validation around seenSlug to
reject legacy-name/slug-slot prefix aliases: detect when one reservation name
matches another reservation’s slug followed by a hyphen and a one- or two-digit
slot, regardless of processing order, and return an invalid-request error. Add a
parsing test covering the ah1-0 name versus ah1 slug collision, ensuring
malformed or ambiguous alias checks fail explicitly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 93916b16-1dac-4cec-9b3f-90d5c9035aa2
📒 Files selected for processing (10)
.github/workflows/uat-aws.yaml.github/workflows/uat-azure.yaml.github/workflows/uat-gcp.yaml.github/workflows/uat-run.yamldocs/contributor/uat.mddocs/design/017-uat-cluster-name-convention.mdpkg/uatbroker/model.gopkg/uatbroker/registry.gopkg/uatbroker/registry_test.gotools/uat-broker/README.md
Comment on lines
+150
to
+154
| if prev, ok := seenSlug[res.Slug]; ok { | ||
| return errors.New(errors.ErrCodeInvalidRequest, | ||
| fmt.Sprintf("duplicate reservation slug %q (%s and %s)", res.Slug, prev, res.Name)) | ||
| } | ||
| seenSlug[res.Slug] = res.Name |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject legacy-name and slug-slot prefix aliases.
A reservation named ah1-0 passes namePattern. A different reservation can use slug ah1. Both then map to aicr-uat-day-ah1-0-.
During the migration window, the first reservation can block on or tear down the second reservation's new cluster. Reject a reservation name that equals another reservation's <slug>-<one-or-two-digit-slot> key. Add a parsing test for this collision.
As per coding guidelines, “malformed evaluator input, ambiguous negative checks, and override failures must produce errors rather than passing.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/uatbroker/registry.go` around lines 150 - 154, Extend the reservation
validation around seenSlug to reject legacy-name/slug-slot prefix aliases:
detect when one reservation name matches another reservation’s slug followed by
a hyphen and a one- or two-digit slot, regardless of processing order, and
return an invalid-request error. Add a parsing test covering the ah1-0 name
versus ah1 slug collision, ensuring malformed or ambiguous alias checks fail
explicitly.
Source: Coding guidelines
lockwobr
approved these changes
Aug 7, 2026
lockwobr
left a comment
lockwobr
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewed at 78d5fbb0. All eight findings from the previous round are addressed, and I've resolved those threads.
Verified against the current tree:
- The unvalidated reservation name in the
grep -Ealternation is closed at the data source withnamePattern(^[a-z]([a-z0-9-]*[a-z0-9])?$) inRegistry.Validate, rather than per-workflow. That covers all six scan sites by construction, and the lookup path fails closed before any cloud lane runs. The four added charset cases (ERE metacharacter, dot, uppercase, trailing hyphen) lock the invariant. - The daytime-down no-match log now names both the new and legacy prefixes in all three pipelines.
docs/contributor/uat.md: the "run-scoped clusters" discovery claim is corrected, the discovery prose is(slug, slot)-scoped, and the operator snippets carry the legacy alternative for the life of the shim.tools/uat-broker/README.mddocuments theslug=key.
The ADR-017 prose links raised as an open question were added too.
CI is green on this commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduce a slot-aware UAT daytime cluster-name convention —
aicr-uat-day-<slug>-<slot>-<run_id>— replacingaicr-uat-day-<reservation>-<run_id>, so the cross-run teardown/guard discovery key is scoped per(reservation, slot)and stays within GKE's 40-char cap with durable headroom (design: ADR-017).Motivation / Context
The daytime cluster name is a cross-run discovery key:
daytime-downand the pre-batch guard find a held cluster by scanning a stable prefix (the exact name isn't reconstructable from a different run). The oldaicr-uat-day-<reservation>-key was cloud-scoped (collides once an account holds >1 reservation), had no slot axis for the multi-slot end state, and burned GKE's 40-char budget on the verbose reservation name (~7 chars headroom, eroding as run_ids grow and reservation names lengthen).The new scheme uses a short registry
slug(2–4 chars) plus aslotindex, giving a per-(reservation, slot)discovery key that survives to the "multiple reservations per account, each multi-slot" end state, with ≥6 chars GKE headroom permanently at a 13-digit run_id.Fixes: N/A
Related: N/A
Type of Change
Component(s) Affected
docs/,examples/).github/workflows/uat-*.yaml), reservation registry + broker (infra/uat/reservations.yaml,pkg/uatbroker,tools/uat-broker)Implementation Notes
aicr-uat-<run_id>(unchanged); daytimeaicr-uat-day-<slug>-<slot>-<run_id>(slot0today).slug(new field, validated at load time: non-empty, unique,^[a-z][a-z0-9]{1,3}$):aws-h100→ah1,gcp-h100→gh1,azure-h100→zh1,aws-gb200→ag2,kind-h100→kh1. Emitted byuat-broker reservations --name→ threaded throughuat-run.yamlto the cloud lanes, same pattern asaccelerator.slotis a runtimeuat-run.yamlinput (default0), forwarded to aws/gcp/azure (not the kind lane, which provisions no cloud cluster) — forward-compatible with multi-slot without another rename.(slug, slot)prefix; fail-closed behavior preserved (list-error → block, bounded retry,>1-match → fail loud).<name>-rg) and EKS (100) have ample room.Testing
make qualify # CI-equivalent gate — run in CI on this PRThe implementation ran
go test -race ./pkg/uatbroker/... ./tools/uat-broker/...,golangci-lint, the brokerslug=emit check, and workflow lint before committing. Table-driven tests added for slug validation (valid / empty / duplicate / bad-charset) and theslug=output. Note:make qualifywas not run locally before push — CI is the first full gate.Risk Assessment
Rollout notes: The guard and teardown carry a dual-prefix migration shim — they match both the new
aicr-uat-day-<slug>-<slot>-and the legacyaicr-uat-day-<reservation>-prefixes, so any live old-named daytime cluster is still discovered and torn down by the nextdaytime-down(no manual pre-merge teardown needed). Once a cycle has passed with no old-named clusters remaining, drop the legacy leg (marked in-code as a transitional shim).Checklist
make testwith-race) — broker/tool packages; fullmake qualifydeferred to CImake lint) — changed Go packages + workflowsgit commit -S)