CLO-188 orchestratord: report Materialize lifecycle transitions as events

[Alphadelta14]

Motivation

While #35829 addresses failure cases, this adds some additional information
about rollouts themselves upon success.

Description

Add Normal events upon rollout stages

Verification

Soon~

---

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[linear-code]

CLO-188

[github-actions]

All contributors have signed the CLA.
_{Posted by the CLA Assistant Lite bot.}

[Copilot]

Pull request overview

Adds Kubernetes events for Materialize rollout lifecycle transitions.

Changes:

• Publishes Normal or Warning events after status transitions persist.
• Shares the reconciliation event publisher with the Materialize controller.
• Adds orchestratord lifecycle-event coverage.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/orchestratord/src/controller/materialize.rs	Publishes lifecycle transition events.
src/orchestratord/src/bin/orchestratord.rs	Injects the shared event publisher.
src/orchestratord/src/reconcile.rs	Documents lifecycle event reporting.
test/orchestratord/mzcompose.py	Verifies successful rollout events.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[def-]

QA LLM Review

1. MEDIUM -- WaitingForApproval is filed as a Kubernetes Warning event

src/orchestratord/src/controller/materialize.rs:227

The condition-to-EventType mapping keys purely on condition.status == "False", and WaitingForApproval is written with status: "False" (materialize.rs:844-850). So a Materialize whose spec changed but whose rollout has not been requested yet -- a normal, user-driven steady state with the default WaitUntilReady strategy -- now files a Warning event on every such spec change, which kubectl get events --field-selector type=Warning and any event-exporter alerting pipeline will treat as a fault.

Details

That is the one reason in the enumerated vocabulary where the rule stated right above it does not hold. The comment says "A condition that went false is the environment failing to reach the state it was asked for" -- but WaitingForApproval is precisely the case where the environment has not been asked to reach a new state. RolloutTimeout and both FailedDeploy sites (materialize.rs:576, 618, 812) are genuine failures and correctly Warning; WaitingForApproval is the odd one out.

The mismatch is not caught by the new test: check_lifecycle_events asserts type == "Normal" only for Applying, ReadyToPromote, Promoting and Applied, and workflow_manually_promote never parks the resource in WaitingForApproval.

Fix -- classify on the reason rather than on the condition status, so the one non-failure False reason stays Normal:

type_: if condition.status == "False" && condition.reason != "WaitingForApproval" {
    EventType::Warning
} else {
    EventType::Normal
},

and adjust the comment to say that a False condition is a failure except for the approval gate, which is a False the user chose.

[jubrad]

Reads correct to me: the one-event-per-transition property genuinely follows from the needs_update guard, and publishing after replace_status means no event claims a transition that failed to persist.

Summary

1 consideration
1 nit

written by claude on behalf of @jubrad

[jubrad]

consider — Noted by claude:
wait_for_rollout_complete() polls the status, but the Applied event is published after replace_status returns, so the test can observe the final status before the event it asserts on exists. The window is one API round trip and will usually be closed by the 1s poll granularity, but it is a real race.

The sibling check just above uses retry(check, 300) for exactly this reason. Wrapping the body of check_lifecycle_events the same way would cost nothing and remove the flake:

retry(lambda: _check_lifecycle_events(name), 60)

written by claude on behalf of @jubrad

[jubrad]

nit — This is ~30 lines of doc and inline prose for ~20 lines of code, and a good part of it re-argues things that already have an owner. The third paragraph ("These do not aggregate the way repeated failures do...") restates Publisher's aggregation contract, which the Publisher doc explains at length; the inline comment at the let condition = ... line restates the guard three lines above it; and the second paragraph explains the relationship to Observed::publish_failure from the wrong end.

Per CLAUDE.md the doc comment should be the caller's contract, with reasoning at the decision point and each fact owned once. Something like a one-line summary plus "published only once the status is durable" would carry everything a caller needs, with the severity policy staying on transition_event_type where it already lives.

written by claude on behalf of @jubrad

[def-]

QA LLM Review

1. MEDIUM -- Failure events now bypass Publisher, restoring the pinned-note bug it was added to fix

src/orchestratord/src/reconcile.rs:324

Observed::publish_failure was rewritten to publish through self.recorder, a field Observed does not have (it holds publisher: Arc<Publisher>), so mz-orchestratord does not build. More than a slip: taken at its evident intent, sending reconciliation failures through one long-lived Recorder instead of Publisher puts a failing resource's event note back on whichever error came first, which is exactly what the parent commit added Publisher to prevent.

Details

cargo check -p mz-orchestratord at this commit: error[E0609]: no field 'recorder' on type '&Observed<Ctx>', reconcile.rs:324. The new free function reconcile::publish (reconcile.rs:426), which takes the &Recorder this call site wants, has no callers, so the refactor is half-applied in both directions.

Why the endpoint is wrong: Recorder::publish keys its aggregation cache on (type, action, reason, reportingController, reportingInstance, regarding, related) and, on a hit, clones the cached event and only bumps its series count, so a repeat keeps the first note (kube-runtime 3.1.0 events.rs:305-330). Every reconciliation failure of a resource shares that key. The entry expires six minutes after last use but each republish refreshes it, and the controller backoff tops out well inside that window, so one recorder pins the note for as long as the resource keeps failing: kubectl describe goes on reporting a missing secret after the real cause has become an invalid license key, under a count and timestamp that both look current. Publisher::recorder_for (reconcile.rs:532) exists to key on the note as well and hand out a fresh recorder when it changes.

Two more consequences of the same edit. Observed::record still calls self.publisher.forget(resource) on a clean pass (reconcile.rs:355), which would then have nothing to forget on the failure path it was written for. And the base branch's workflow_failure_events (test/orchestratord/mzcompose.py:3343) drives an environment through two different failures in a row and requires the note to follow, so resolving the build error by giving Observed a plain Recorder field fails that test rather than passing silently.

Fix: publish through self.publisher again and delete reconcile::publish, keeping Publisher::publish as the single entry point for both failures and transitions.

[Alphadelta14]

Closed in favor of #39030