Skip to content

Commit ef895f5

Browse files
committed
docs: ADR-049 — clarify Option B's static-config cost and expand the KOM object-sourced-event enhancement
1 parent f7a0d74 commit ef895f5

1 file changed

Lines changed: 29 additions & 9 deletions

File tree

docs/designs/049-maintenance-request.md

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,37 @@ message = "Maintenance requested: node reboot"
5050
recommendedAction = "RESTART_VM"
5151
```
5252

53+
The catch is that **everything under `[policies.healthEvent]` is a static literal**. KOM assembles the published event from the *policy*, not from the object it is watching — only the node name is read from the object, via `nodeAssociation`. Two consequences follow, and they are the reason this is not simply "Option A for free":
54+
55+
- **One policy per distinct event, so publishing a new kind of fault means changing NVSentinel.** A policy emits exactly one event shape. Every new fault type the external system wants to raise needs its own policy block, which is a config change and a release *before* the requester can use it. An external system cannot introduce a fault type on its own schedule — it has to file a PR against NVSentinel and wait for a deploy. Over time that is a standing stream of config churn driven entirely by someone else's roadmap.
56+
- **No per-request detail.** Within a policy, `message` and `errorCode` are identical for every MR that matches it, and `HealthEventSpec` has no `metadata` map at all. A specific request cannot carry what makes it diagnosable — which CSP event, which maintenance window, which ticket — so an operator looking at a drained node sees the same generic text for every request of that type.
57+
5358
| | Option A (`lifecycle-manager`) | Option B (KOM policy) |
5459
|---|---|---|
55-
| New component / reconciler code | Yes | No — policy config only |
56-
| Clear on delete | Finalizer-guaranteed, retried until submitted | Best-effort; on publish failure KOM logs, drops match state, and the clear is lost |
57-
| Per-request event content | Yes — emitted as authored | No — `HealthEventSpec` is static TOML, and it has no `metadata` map |
58-
| New fault type using an existing action | Pure API call, no config | New policy + release, unless it fits an existing one |
59-
| Status on the MR | `HealthEventEmitted` condition | None — KOM writes no status to watched objects |
60-
| Constrains what requesters can inject | No | Yes — the policy set is an operator-controlled gate |
61-
| Survives restarts | Yes | Yes — match state is persisted in node annotations |
62-
63-
The per-request-content gap is closable with a contained KOM enhancement — sourcing event fields from the watched object (e.g. `fromField = "spec.healthEvent"`) — which would make Option B equivalent to Option A apart from the finalizer and status. The plumbing already carries the unstructured object to the evaluator.
60+
| New component / reconciler code to build | Yes | No — policy config only |
61+
| **Publishing a new kind of fault** | Create an MR; no NVSentinel change | Add a KOM policy and cut a release first |
62+
| **Per-request detail** (`message`, `errorCode`, metadata) | Taken from each MR, so every request is self-describing | Fixed in the policy — every MR matching it emits identical text |
63+
| Clearing the fault on delete | Finalizer holds the MR until the clear is submitted, and retries | Emitted after the fact; if the publish fails KOM drops its state and the clear is lost |
64+
| Status reported on the MR | `HealthEventEmitted` condition | None — KOM does not write to the objects it watches |
65+
| Who decides what a request may do | The requester (any event, any action) | The operator (the policy set is the gate) |
66+
| Survives a restart | Yes | Yes — match state is persisted in node annotations |
67+
68+
### Closing the gap: object-sourced event fields in KOM
69+
70+
Both consequences above trace to a single design choice — the event comes from the policy. An enhancement that lets a policy take the event from the watched object instead, e.g.:
71+
72+
```toml
73+
[policies.healthEvent]
74+
fromField = "spec.healthEvent" # publish the object's own event
75+
```
76+
77+
removes both at once. One policy then covers every MR regardless of what the requester puts in it, so a new fault type becomes a pure API call with no NVSentinel config change or release, and each request carries its own message, error codes, and metadata.
78+
79+
The change looks contained: the reconciler already holds the unstructured object where it calls the publisher — it evaluates the CEL `predicate` and `nodeAssociation` against it — so this is a new config field plus a branch in event construction, not new plumbing.
80+
81+
One wrinkle it has to settle: the **clearing event is published after the object is gone**, so its fields cannot be read from the object at that point. KOM currently caches only the node name (in `matchStates`, persisted to node annotations). Either the identity fields the clear depends on — `agent`, `checkName`, `nodeName` — stay policy-owned while only the descriptive fields are object-sourced, or KOM caches the emitted event alongside the match state.
82+
83+
With that in place, Option B differs from Option A only in the finalizer guarantee and MR status.
6484

6585
## Implementation
6686

0 commit comments

Comments
 (0)