Skip to content

settings: an operator can check before the agent runs - #43

Merged
github-actions[bot] merged 1 commit into
developfrom
feat/is-this-cluster-configured-right
Sep 12, 2026
Merged

github-actions[bot] merged 1 commit into
developfrom
feat/is-this-cluster-configured-right

Conversation

@traviswu-bigstack

Copy link
Copy Markdown
Contributor

What

advisor-agent config check — ADR 0016 slice 2. Prints what each of the five
per-cluster settings resolved to and exits non-zero if any is broken, without
starting anything.

$ advisor-agent config check -dir /etc/cube/advisor-agent
action level: operate
consent: never
cube-cos-api access: datacenter cube-combined at http://10.32.1.200:8082
instance profile: flavor m1.large, image ubuntu-24.04, network tenant-net
OpenStack credential: loaded for project advisor-lab
action level operate; serving 6 tool(s)
checked: every file parses, its mode is safe and its values are well-formed. Not
checked: that these ids exist on this cluster, that the credential redeems, or
that the api answers — the first create and the first read are what prove those.

Why

There are five settings now — action level, consent, cube-cos-api access,
instance profile, OpenStack credential — and the only way to check one was to
restart the agent and read its startup lines. Some settings only fail at use:
a transposed flavour id is syntactically fine and invisible until a person has
approved a VM that then fails.

Reviewer notes

It calls configure, the function run calls. Not a parallel validator —
a second reader of the settings list is this design's own defect one level up,
and the one nobody ran would drift from the one that decides. The per-setting
lines are the same settingState.line values run prints; summaryLine is now
formatted in one place and used by both, so a check and the run it predicts
cannot describe one registry differently.

Absent is not broken. Every setting but the action level is optional, and a
cluster that has not opted in is in a state rather than a mistake. An
unconfigured directory exits 0 and says "not configured" per setting.

A broken setting exits 6, not 1. Correcting a file and reporting a bug want
different actions, and main.go's own comment says an installer that can only
see non-zero has to parse messages, "and then the messages become an interface
nobody meant to define". exitFailed stays for a check that could not run.

No semantic validation, deliberately. ADR 0016 drew the line at syntactic at
load, semantic at use. Checking that a flavour id exists needs the credential
and a network call, and a checker that sometimes talks to a cloud is a different
tool with different failure modes — one an operator learns to ignore the first
time it fails because the cloud was busy, and one that is useless during an
air-gapped bring-up before the cloud answers. A passing check therefore says
what it did not check, because "every setting is fine" reads as a stronger claim
than it is.

-probes is accepted so the tool count answers for the agent the operator
will actually start: with it the same directory reports 8 tools rather than 6.
Building a ProbeRunner validates the probe list and touches nothing else; the
sweeper that does touch scratch is started by run, separately, never here.

-dir defaults to the production directory, and pointing it at a staging
copy is how an operator checks before promoting one.

Output goes to stdout, failures to stderr. In run these lines are log
context; here they are the answer, and an operator may pipe them.

main's switch moved into dispatch so the tests reach the subcommand the
way an operator does. This matters: a subcommand tested only through its own
function can be deleted from the switch and still pass its own tests. Removing
case "config" fails four tests.

Breaks proved, each watched to fail a named test then restored:

break failed
a broken setting returns exitOK TestConfigCheckReportsABrokenSettingAndExitsNonZeroexit = 0, want 6
case "config" removed from dispatch four tests, all exit = 2
config.go reaches internal/agent TestConfigCheckStartsNothing — "a check that can dial or serve is no longer a check"

None failed to fail.

TestConfigCheckStartsNothing is enforcement, not a guarantee: it reads
this file's imports, so a helper in another file could still reach the tunnel.
It catches the change that would actually be written, and the behavioural half —
a directory with settings but no identity exits 0, where run would refuse — is
asserted beside it.

discardAuditor() in settings_test.go now calls checkAuditor(), the one
config check uses, so the tests and that command do not disagree about how to
build a registry with no audit log.

Docs

kb/cube-ai-advisor/architecture/read-only-plane-end-to-end.md, with the other
slices.

ADR 0016 slice 2. There are five per-cluster settings now, and the only way to
check one was to restart the agent and read its startup lines. `config check`
calls configure -- the function run calls -- prints what each setting resolved
to, and exits non-zero if any is broken.

It is not a parallel validator. A second reader of the settings list is this
design's own defect one level up: the one nobody ran would drift from the one
that decides. The startup lines are the same states run prints, and the summary
line is formatted in one place so a check and the run it predicts cannot
describe one registry differently.

Absent is not broken. Every setting but the action level is optional, and a
cluster that has not opted in is in a state rather than a mistake, so an
unconfigured directory exits zero. A broken setting exits 6 rather than 1:
correcting a file and reporting a bug want different actions, and an installer
that can only see non-zero has to parse messages to tell them apart.

A passing check says what it did not check. The line ADR 0016 drew is syntactic
at load, semantic at use -- whether a flavour id exists needs the credential and
a network call, and a checker that sometimes talks to a cluster is one an
operator learns to ignore the first time it fails because the cloud was busy.

main's switch moves into dispatch so the tests reach the subcommand the way an
operator does. A subcommand tested only through its own function can be deleted
from the switch and still pass; removing the case here fails four tests.

Signed-off-by: Travis Wu <travis.wu@bigstack.co>
@traviswu-bigstack
traviswu-bigstack marked this pull request as ready for review September 12, 2026 10:38
@traviswu-bigstack traviswu-bigstack added the done Apply to a PR to trigger the fast-forward merge label Sep 12, 2026
@github-actions
github-actions Bot merged commit c4ebc55 into develop Sep 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

done Apply to a PR to trigger the fast-forward merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant