Skip to content

Commit b79d9da

Browse files
ebourgeoisebourgeois
committed
Phase 0 of the new SpotSchedule feature: ADR and CALM diagram update.
Signed-off-by: Erick Bourgeois <erick@jeb.ca>
1 parent 020d39f commit b79d9da

7 files changed

Lines changed: 623 additions & 3 deletions

File tree

‎.claude/CHANGELOG.md‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,54 @@ The format is based on the regulated environment requirements:
99

1010
---
1111

12+
## [2026-06-13 14:00] - ADR 0006/0007 + CALM: pluggable spot-schedule provider contract (Phase 0)
13+
14+
**Author:** Erick Bourgeois
15+
16+
### Changed
17+
- `docs/adr/0006-pluggable-spot-schedule-provider-contract.md`: NEW — accept a
18+
duck-typed external provider contract. `ScheduledMachine.spec.spotSchedule`
19+
(`apiVersion`/`kind`/`name`, same namespace) delegates the active/inactive
20+
decision to a CRD in the new `spotschedules.5spot.finos.org` group; 5-Spot
21+
reads only `status.active` (+ `Ready`), never the provider spec, never writes
22+
it. `spec.schedule` becomes optional (CEL: at least one of schedule/
23+
spotSchedule); both set ⇒ logical AND; `killSwitch` overrides. Unresolved
24+
references hold last-known state (fail-inactive when never resolved) and
25+
surface `SpotScheduleResolved=False` — no flapping. Event-driven dynamic
26+
per-GVK watch + reverse index. Providers are untrusted inputs (read-only,
27+
group-pinned RBAC; same-namespace only).
28+
- `docs/adr/0007-crd-multi-version-and-conversion.md`: NEW — CRDs are
29+
multi-version-capable (one Rust struct per served version, single
30+
`storage: true`), `conversion.strategy: None` with additive-only evolution;
31+
resolvers/watchers key off group+kind, never a pinned `apiVersion`. The first
32+
breaking change is the trigger for a webhook (superseding ADR). No CALM impact
33+
(generation/versioning policy). Applies to `ScheduledMachine` and the new
34+
provider group.
35+
- `docs/adr/README.md`: index rows for ADR 0006 and 0007.
36+
- `docs/architecture/calm/architecture.json`: NEW node
37+
`service-spot-schedule-provider` (external, untrusted) + data-asset
38+
`data-asset-spot-schedule-cr`; relationships `rel-spot-schedule-cr-stored-in-api`,
39+
`rel-provider-writes-spot-schedule-status`, `rel-controller-spot-schedule-watch`
40+
(read-only, group-pinned least-privilege control); flow
41+
`flow-spot-schedule-activation`. `make calm-validate` clean (0/0/0);
42+
`make calm-diagrams` re-rendered `docs/src/architecture/{system,flows}.md`.
43+
- `docs/src/reference/spot-schedule-contract.md`: NEW — provider contract
44+
skeleton (reference, status table, AND composition, unresolved behavior,
45+
versioning policy); added to `docs/mkdocs.yml` Reference nav.
46+
47+
### Why
48+
Phase 0 (ADD gate) of the spot-schedule provider roadmap: record and visualize
49+
the architecture before any code, so activation semantics (capital-markets
50+
calendars, PromQL emitters, change-freeze gates) can be implemented out-of-tree
51+
against a stable, versioned public contract. Team-approved to proceed
52+
2026-06-13.
53+
54+
### Impact
55+
- [ ] Breaking change
56+
- [ ] Requires cluster rollout
57+
- [ ] Config change only
58+
- [x] Documentation only
59+
1260
## [2026-06-10 11:30] - ADR 0005: remove spec.kata.destPath — fixed host path, H-1 gate closed
1361

1462
**Author:** Erick Bourgeois

‎docs/adr/0006-pluggable-spot-schedule-provider-contract.md‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
<!--
2+
Copyright (c) 2026 Erick Bourgeois, 5-Spot
3+
SPDX-License-Identifier: Apache-2.0
4+
-->
5+
# 0006 — Pluggable spot-schedule provider contract via `spec.spotSchedule` and the `spotschedules.5spot.finos.org` API group
6+
7+
- **Status:** Accepted
8+
- **Date:** 2026-06-13
9+
- **Deciders:** Erick Bourgeois, 5-Spot team
10+
- **Supersedes:** —
11+
- **Related:** ADR [0007](./0007-crd-multi-version-and-conversion.md) (the
12+
multi-version/conversion story this contract depends on);
13+
`ScheduleSpec` + `evaluate_schedule` (`src/crd.rs`, `src/reconcilers/helpers.rs`);
14+
`ObjectReference`-style refs already in `src/crd.rs`;
15+
the reclaim-agent Node-watch (`rel-controller-workload-kube-api`) as the
16+
event-driven watch shape to mirror; contract page
17+
`docs/src/reference/spot-schedule-contract.md`.
18+
19+
## Context
20+
21+
A `ScheduledMachine` decides "should this machine exist right now?" solely from
22+
the inline `spec.schedule` (`daysOfWeek` / `hoursOfDay` / `timezone` /
23+
`enabled`), evaluated by `evaluate_schedule()` in `src/reconcilers/helpers.rs`.
24+
That model is **closed**: every new activation semantic — exchange calendars,
25+
statutory holidays and early closes, change-freeze windows, metric-driven
26+
scale, a manual trading-desk drain — would force a new field on 5-Spot's CRD
27+
and new branches in its reconciler. The motivating case is capital markets:
28+
trading-floor compute must follow the *exchange session calendar*, which no
29+
`daysOfWeek`/`hoursOfDay` expression can encode.
30+
31+
**Options weighed:**
32+
33+
1. **Grow `spec.schedule`** with calendar/holiday/PromQL sub-objects. Rejected:
34+
unbounded surface area, 5-Spot would own every scheduling dialect forever,
35+
and operators still couldn't express semantics we didn't anticipate.
36+
2. **A plugin/script hook** (WASM, embedded expression language) evaluated
37+
in-process. Rejected: a sandbox and execution-safety burden inside a
38+
privileged controller, and still not a Kubernetes-native contract.
39+
3. **A duck-typed external provider resource** that owns the active/inactive
40+
decision; 5-Spot consumes it generically. **Chosen.** This is the exact
41+
shape Cluster API uses for `infrastructureRef` / `bootstrap.configRef`
42+
(a contract over `apiVersion`/`kind`/`name` reading a well-known status
43+
field) — a pattern 5-Spot already participates in from the *other* side, so
44+
it is idiomatic here and familiar to operators.
45+
46+
The cost of option 3 is the one recorded below: a provider is an **untrusted
47+
input** that can start and stop the machines that reference it, and an
48+
unresolved/late/unhealthy provider must never flap machines.
49+
50+
## Decision
51+
52+
### 1. `spec.spotSchedule` reference on `ScheduledMachine`
53+
54+
`src/crd.rs` is the source of truth; CRD YAML and API docs are regenerated
55+
(`regen-crds` → `regen-api-docs`), never hand-edited.
56+
57+
```rust
58+
// on ScheduledMachineSpec — schedule becomes optional (see §3)
59+
#[serde(skip_serializing_if = "Option::is_none")]
60+
pub spot_schedule: Option<SpotScheduleRef>,
61+
62+
pub struct SpotScheduleRef {
63+
pub api_version: String, // "spotschedules.5spot.finos.org/<version>"
64+
pub kind: String, // e.g. "CapitalMarketsSchedule"
65+
pub name: String, // object in the SAME namespace as the SM
66+
}
67+
```
68+
69+
`SpotScheduleRef` keeps the existing ref conventions (`deny_unknown_fields`,
70+
camelCase, schemars bounds). The referenced object **must live in the
71+
`ScheduledMachine`'s own namespace** — no `namespace` field, no cross-namespace
72+
references in this version.
73+
74+
### 2. The provider group and the duck-typed contract
75+
76+
The provider API group is **`spotschedules.5spot.finos.org`** — a sub-group of
77+
the existing `5spot.finos.org` group. A CEL `XValidation` on
78+
`spotSchedule.apiVersion` pins the **group** to exactly
79+
`spotschedules.5spot.finos.org` (any served version is accepted, per ADR 0007);
80+
references to any other group are rejected at admission.
81+
82+
5-Spot reads a **duck-typed `status`** and never the provider `spec`:
83+
84+
| Provider `status` field | Required | Meaning to 5-Spot |
85+
|---|---|---|
86+
| `active` (bool) | **yes** | the single source of truth: the machine should be up |
87+
| `conditions[type=Ready]` | recommended | provider health; `Ready=False` ⇒ **unresolved**, *not* inactive |
88+
| `observedGeneration` (int) | recommended | staleness detection |
89+
| `lastTransitionTime` (string) | recommended | observability / transition metrics |
90+
91+
Providers implement `spec` however they like (exchange calendars, PromQL,
92+
cron, a plain `enabled` toggle). The contract is published in
93+
`docs/src/reference/spot-schedule-contract.md`.
94+
95+
### 3. Composition with `spec.schedule` (AND), and `killSwitch` precedence
96+
97+
`spec.schedule` becomes `Option<ScheduleSpec>`. A CEL `XValidation` requires
98+
**at least one** of `schedule` / `spotSchedule` (existing SMs all set
99+
`schedule`, so this is non-breaking). When **both** are present the machine is
100+
active **iff the inline time window AND the provider both say active** — this
101+
supports "the market is open **and** only 09:00–17:00 local". `killSwitch`
102+
continues to override everything: `killSwitch=true` ⇒ inactive regardless of
103+
schedule or provider. Precedence, highest first:
104+
105+
```
106+
killSwitch > (schedule AND spotSchedule) > schedule-only / spotSchedule-only
107+
```
108+
109+
### 4. Unresolved references never flap machines
110+
111+
A reference is **Unresolved** when the provider CRD is not installed, the named
112+
object is absent, it has no `status.active`, or its `Ready` condition is
113+
`False`/missing. On Unresolved, 5-Spot:
114+
115+
- sets a `SpotScheduleResolved=False` condition on the `ScheduledMachine` with
116+
a precise reason (`ProviderCRDNotInstalled`, `ProviderNotFound`,
117+
`StatusActiveMissing`, `ProviderNotReady`),
118+
- increments `fivespot_spot_schedule_resolution_errors_total`, and
119+
- **holds the last known resolved state** — it does **not** tear down a running
120+
machine because its provider went briefly unreadable. If the reference has
121+
**never** resolved, the fail-safe default is **inactive** (no machine is
122+
created on the strength of a reference we cannot read).
123+
124+
This "hold-last-state, fail-inactive-when-never-resolved" rule is the sharpest
125+
edge of the contract and is tested on both branches.
126+
127+
### 5. Event-driven dynamic watch (no polling)
128+
129+
The controller maintains an in-memory **reverse index**
130+
`(group, version-agnostic kind, namespace, name) → {ScheduledMachine keys}`,
131+
updated on SM apply/delete. For each distinct **GVK** referenced, it lazily
132+
starts a dynamic watch (`Api<DynamicObject>` resolved via `kube::discovery`)
133+
and maps provider events back through the index to `reconcile_on` the affected
134+
`ScheduledMachine`s. Streams are stopped when the last referencing SM goes
135+
away. The index is **rebuilt from the SM list on controller start** — nothing
136+
is stored outside Kubernetes (5-Spot stays stateless). This mirrors the
137+
existing reclaim-agent Node-watch and honours the project's
138+
event-driven-not-polling rule; provider `status.active` transitions drive SM
139+
reconciles at watch latency, matching the inline-schedule path.
140+
141+
Hard edges handled (and tested): provider CRD installed *after* SMs reference
142+
it (discovery retry with backoff, surfaced as `Unresolved` meanwhile), provider
143+
CRD deleted while watched (stream error → re-resolve), controller restart
144+
(index rebuilt on boot).
145+
146+
### 6. Security boundary — providers are untrusted inputs
147+
148+
- Controller RBAC gains **`get;list;watch` only** on
149+
`spotschedules.5spot.finos.org` `*` — **no write** to any provider resource.
150+
- A reference provider's own controller gets `update;patch` on **its own**
151+
kind's `/status` subresource only (a separate Role, not 5-Spot's ClusterRole).
152+
- A compromised or buggy provider can flap the machines that *reference* it —
153+
the **same blast radius as a malicious edit to `spec.schedule.enabled`**, and
154+
bounded to SMs that opted in by naming it. Mitigations: same-namespace-only
155+
references, RBAC on the provider CRDs, audit via the `SpotScheduleResolved`
156+
condition + transition metrics, and a transition-rate alert. This actor and
157+
these mitigations are added to `docs/src/security/threat-model.md`.
158+
159+
## Consequences
160+
161+
- **Easier:** activation semantics evolve **out of tree** — capital-markets
162+
calendars, PromQL emitters, change-freeze gates ship as provider CRDs without
163+
touching 5-Spot's CRD or reconciler; 5-Spot gains **no** new write ability
164+
(read-only on providers); the inline `spec.schedule` stays the simple path.
165+
- **Harder / new burden:** a dynamic multi-GVK watch manager + reverse index to
166+
build and test; a published, versioned **public contract** (ADR 0007) we must
167+
not break; provider-flapping is a new failure mode requiring debounce-style
168+
thinking (a per-SM `minimumStateDuration` is noted as possible future work,
169+
not adopted here).
170+
- **Operator contract:** the referenced provider object **must pre-exist** in
171+
the SM's namespace and report `status.active`; absence/unreadiness is a
172+
loud `SpotScheduleResolved=False` status, never a silent machine teardown.
173+
- **Ruled out (this version):** cross-namespace / cross-cluster references;
174+
multiple `spotSchedule` refs per SM (no AND/OR ref-lists); 5-Spot ever
175+
writing provider status; a provider SDK/conformance kit; the Prometheus
176+
emitter implementation itself (a planned *consumer* of this contract).
177+
- **CALM impact:** **updated.** New external node
178+
`service-spot-schedule-provider`; a `data-asset-spot-schedule-cr`; a
179+
controller→provider **watch** relationship
180+
(`rel-controller-spot-schedule-watch`, read-only, group-pinned, least
181+
privilege control); and a flow `flow-spot-schedule-activation`
182+
(provider `status.active` transition → SM reconcile → activate/deactivate via
183+
the existing schedule flows). `make calm-validate` + `make calm-diagrams`
184+
before implementation.

‎docs/adr/0007-crd-multi-version-and-conversion.md‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
<!--
2+
Copyright (c) 2026 Erick Bourgeois, 5-Spot
3+
SPDX-License-Identifier: Apache-2.0
4+
-->
5+
# 0007 — CRD multi-version support with `None` conversion and additive-only evolution
6+
7+
- **Status:** Accepted
8+
- **Date:** 2026-06-13
9+
- **Deciders:** Erick Bourgeois, 5-Spot team
10+
- **Supersedes:** —
11+
- **Related:** ADR [0006](./0006-pluggable-spot-schedule-provider-contract.md)
12+
(introduces the public `spotschedules.5spot.finos.org` contract this policy
13+
governs); `src/crd.rs` (`#[kube(...)]` derive, source of truth); `crdgen`
14+
(`src/bin/crdgen.rs`); the `regen-crds` / `regen-api-docs` skills.
15+
16+
## Context
17+
18+
5-Spot's CRDs are served at a single version, `v1alpha1`, today
19+
(`5spot.finos.org/v1alpha1`, kind `ScheduledMachine`). ADR 0006 introduces a
20+
**second** API group, `spotschedules.5spot.finos.org`, whose `status.active`
21+
contract is **implemented by third parties** — it is a public API surface, not
22+
an internal type. Public contracts evolve: fields get added, semantics get
23+
clarified, an alpha graduates to beta to stable. We need a deliberate story for
24+
serving more than one version of a CRD **before** we generate the first one, so
25+
the contract can mature without breaking existing `ScheduledMachine`s or
26+
out-of-tree providers.
27+
28+
The constraints that shape the decision:
29+
30+
- **kube-rs derives one version per struct.** `#[kube(version = "…")]` on a
31+
`CustomResource` derive emits exactly one version. Serving N versions means N
32+
Rust structs and a **merged** CRD whose `spec.versions[]` lists all of them
33+
with exactly **one** `storage: true`.
34+
- **Conversion needs either compatibility or a webhook.** Kubernetes converts
35+
between served versions one of two ways: `conversion.strategy: None`, which
36+
works **only** when every served version round-trips losslessly through the
37+
stored object (the API server just relabels `apiVersion`); or a **conversion
38+
webhook**, which can transform fields but requires a TLS-served HTTPS endpoint
39+
plus a cert lifecycle (cert-manager / `caBundle` rotation).
40+
- **5-Spot runs no webhook server today.** The controller exposes only
41+
`/metrics` and `/healthz`. Standing up an admission/conversion webhook is a
42+
non-trivial new failure domain (TLS, availability, fail-open/closed posture).
43+
44+
**Options weighed:**
45+
46+
1. **Stay single-version, bump in place.** Rejected: breaks every stored object
47+
and every out-of-tree provider the moment the contract changes; unacceptable
48+
for a public API.
49+
2. **Multi-version with a conversion webhook from day one.** Rejected *for now*:
50+
pays the full webhook/TLS/cert cost before any breaking change exists to
51+
justify it.
52+
3. **Multi-version with `conversion.strategy: None` + additive-only
53+
evolution.** **Chosen.** Serve multiple versions that are all structurally
54+
round-trippable through the storage version; constrain changes to be
55+
additive (new optional fields, never a rename/retype/removal of a served
56+
field) so `None` conversion stays correct. The first genuinely *breaking*
57+
change is the documented trigger to introduce a webhook via a superseding
58+
ADR.
59+
60+
## Decision
61+
62+
1. **5-Spot CRDs are multi-version-capable.** Each served version of a CRD is a
63+
distinct Rust struct in `src/crd.rs` (e.g. `ScheduledMachine` at
64+
`v1alpha1`, a future `ScheduledMachineV1Beta1` at `v1beta1`), and `crdgen`
65+
emits a single CRD object per kind whose `spec.versions[]` lists every served
66+
version with exactly **one** marked `storage: true`. This policy applies to
67+
**both** `5spot.finos.org/ScheduledMachine` and the new
68+
`spotschedules.5spot.finos.org` provider group from ADR 0006.
69+
70+
2. **Conversion strategy is `None`; evolution is additive-only.** Generated
71+
CRDs set `spec.conversion.strategy: None`. Because the API server performs
72+
no field transformation under `None`, every served version **must** be
73+
round-trippable through the storage schema. We therefore constrain
74+
cross-version changes to be **additive** — new **optional** fields only;
75+
never rename, retype, remove, or change the meaning of a field that an older
76+
served version exposes. Defaulting is handled in the Rust types / reconciler,
77+
not by conversion.
78+
79+
3. **Resolvers and watchers are version-agnostic.** No code keys off a single
80+
hardcoded `apiVersion` string. The spot-schedule resolver and dynamic watch
81+
(ADR 0006 §5) key off **`group` + `kind`** and accept any *served* version of
82+
a provider; the CEL pin on `spotSchedule.apiVersion` validates the **group**,
83+
not a specific version.
84+
85+
4. **Generation and CI guards.** `crdgen` produces multi-version YAML; serde
86+
round-trip tests cover **every served version** of every kind; a CI/test
87+
guard asserts the **single-`storage: true`** invariant per CRD. CRD shape
88+
changes still start in `src/crd.rs` and flow through `regen-crds` →
89+
`regen-api-docs` (LAST), never hand-edited YAML.
90+
91+
5. **The webhook trigger is recorded, not built.** The first change that cannot
92+
be expressed additively (a true breaking change to a served version) is the
93+
trigger to introduce a conversion webhook. That is a **new architectural
94+
decision** — it gets its own ADR superseding the relevant part of this one,
95+
covering the webhook's TLS/cert lifecycle and fail posture. Until then,
96+
5-Spot ships **no** webhook.
97+
98+
## Consequences
99+
100+
- **Easier:** the public `spotschedules` contract (and `ScheduledMachine`
101+
itself) can grow new versions without breaking stored objects or out-of-tree
102+
providers, and without 5-Spot standing up a webhook server — no TLS, cert, or
103+
webhook-availability burden while the contract is young.
104+
- **Harder / constraints accepted:** cross-version changes are **disciplined to
105+
additive-only**; a rename/retype/removal is *not* a routine change — it forces
106+
the webhook ADR. Reviewers must police this on every CRD edit. Multiple Rust
107+
structs per kind add some duplication, and round-trip tests multiply by served
108+
version count.
109+
- **Operational invariant:** exactly one version per CRD is `storage: true`;
110+
the CI guard fails the build otherwise. Reading/writing always converges on
111+
the storage version with no lossy transformation.
112+
- **Ruled out (for now):** conversion webhooks; non-additive version changes;
113+
version-pinned resolver/watch code. Each is reintroduced only via a
114+
superseding ADR.
115+
- **CALM impact:** **none.** This is a generation/versioning policy for the CRD
116+
artifacts and the type layer — it changes neither the running system's
117+
topology (controller, agents, CAPI/provider resources, flows) nor any
118+
trust boundary. CALM models the system, not the CRD-version-evolution policy,
119+
so there is no node/relationship/flow to add.

0 commit comments

Comments
 (0)