Skip to content

Commit 63b8a07

Browse files
ebourgeoisebourgeois
committed
Populate providerID/nodeRef in reconciler and add watchers
Signed-off-by: Erick Bourgeois <erick@jeb.ca>
1 parent 3af0e03 commit 63b8a07

7 files changed

Lines changed: 1253 additions & 40 deletions

File tree

‎.claude/CHANGELOG.md‎

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

1010
---
1111

12+
## [2026-04-19 15:00] - Harden OpenSSF Scorecard workflow: SPDX header + pin remaining action
13+
14+
**Author:** Erick Bourgeois
15+
16+
### Changed
17+
- `.github/workflows/scorecard.yaml`: Prepended the project's standard two-line SPDX/Copyright header (`Copyright (c) 2025 Erick Bourgeois, firestoned` + `SPDX-License-Identifier: Apache-2.0`) to match every other workflow in `.github/workflows/`. Pinned the previously unpinned `github/codeql-action/upload-sarif@v3` to the full commit SHA for v3.35.2 (`ce64ddcb0d8d890d2df4a9d1c04ff297367dea2a`) with an inline version comment — brings the last step into line with the other three actions in the file which were already SHA-pinned.
18+
19+
### Why
20+
OpenSSF Scorecard's own `Pinned-Dependencies` check flags float-tagged actions (`@v3`) as a supply-chain risk, so the scorecard workflow itself should score well on that check. The SPDX header is the repo-wide convention; every other workflow has it. Both changes are compliance housekeeping — no behavior change.
21+
22+
### Impact
23+
- [ ] Breaking change
24+
- [ ] Requires cluster rollout
25+
- [x] Config change only
26+
- [ ] Documentation only
27+
28+
---
29+
30+
## [2026-04-19 14:00] - Phases 3 + 4: Event-driven `.watches()` on CAPI Machine and Node
31+
32+
**Author:** Erick Bourgeois
33+
34+
### Changed
35+
- `src/reconcilers/helpers.rs`: Added two pure mapper functions used as secondary-watch reverse-maps on the Controller builder:
36+
- `machine_to_scheduled_machine(&DynamicObject) -> Vec<ObjectRef<ScheduledMachine>>` — reads the `5spot.eribourg.dev/scheduled-machine` label (constant `LABEL_SCHEDULED_MACHINE`) and emits at most one `ObjectRef`. Guards against missing/empty/whitespace label values, missing namespace, and imposter labels with similar prefixes.
37+
- `node_to_scheduled_machines<'a, I: IntoIterator<Item = &'a ScheduledMachine>>(&Node, I) -> Vec<ObjectRef<ScheduledMachine>>` — returns all SMs whose `status.nodeRef.name` matches `node.metadata.name`. Returns multiple refs on conflict so the reconciler can surface the issue. O(N) per Node event; documented as acceptable for current scale.
38+
- `src/reconcilers/mod.rs`: Re-exports both mappers alongside the existing `error_policy`, `evaluate_schedule`, `should_process_resource`.
39+
- `src/main.rs`: Extended the `Controller` builder with `.watches_with(...)` on CAPI `Machine` (`ApiResource` from GVK, label-filtered `watcher::Config`) and `.watches(...)` on `Node`. The Node mapper closure captures a clone of `controller.store()` (`reflector::Store<ScheduledMachine>`) and calls `state()` on each event to avoid out-of-band API lists.
40+
- `src/reconcilers/helpers_tests.rs`: Added 14 new unit tests — 7 for `machine_to_scheduled_machine` (positive match, missing label, no labels, empty value, whitespace value, missing namespace, wrong-prefix imposter) and 7 for `node_to_scheduled_machines` (single match, multi-match conflict, no match, empty list, SM-without-status, Node-without-name, empty `nodeRef.name`). Covers positive, negative, and exception paths per the project's 100%-coverage rule.
41+
42+
### Why
43+
Closes roadmap Phases 3 + 4 of `event-driven-watches-and-status-enrichment.md`. The `ScheduledMachine` controller previously only watched its own CR — all downstream state was polled during reconciles, which CLAUDE.md explicitly forbids ("ALWAYS use event-driven programming… as opposed to polling"). With these two watches, CAPI setting `status.nodeRef` or a Node being drained now enqueues an immediate reconcile via kube-rs's watch stream (<1s debounce) instead of waiting for the next periodic requeue. The label-based reverse map on Machines uses the label we already stamp on every child — zero new ownership semantics, no `controller: true` owner references introduced, no contention with CAPI's own controllers.
44+
45+
### Impact
46+
- [ ] Breaking change
47+
- [x] Requires cluster rollout — controller now opens additional watches (CAPI `Machine` with label selector; `Node` cluster-wide)
48+
- [ ] Config change only
49+
- [ ] Documentation only
50+
51+
### Operational note
52+
- RBAC: the controller already has `get`/`list`/`watch` on CAPI `Machine` (used for drain lookup) and needs `list`/`watch` on core `Node`. Verify `get,list,watch` on `nodes` is present in the ClusterRole before rollout; if not, add it in the same rollout.
53+
- Observability: the existing `reconcile_queue_depth` metric will show enqueues driven by Machine and Node events in addition to SM events.
54+
55+
---
56+
57+
## [2026-04-19 10:00] - Phase 2: Close test-coverage gap for async CAPI helpers
58+
59+
**Author:** Erick Bourgeois
60+
61+
### Changed
62+
- `src/reconcilers/helpers_tests.rs`: Added 14 `tower-test`-backed unit tests covering positive, negative, and exception paths for the three async helpers introduced in the previous Phase 2 entry:
63+
- `fetch_capi_machine` — 200 returns `Some`, 404 returns `Ok(None)`, 500 and 403 map to `CapiError`.
64+
- `patch_machine_refs_status` — both-fields patch asserts full body shape, single-field patches assert the other key is **omitted** (not null) so merge-patch never clears existing values, both-`None` case asserts **zero HTTP traffic**, 500 and 404 map to `KubeError`.
65+
- `get_node_from_machine` — success returns the node name, machine-404 and nodeRef-missing both return `Ok(None)`, 500 propagates as `CapiError`.
66+
67+
### Why
68+
Per durable user guidance: every function (public **and** private) must have unit tests covering the happy path, negative cases, and exception/error paths. The original Phase 2 change landed with tests only for the pure `extract_machine_refs` function — the three async helpers, which actually touch the Kubernetes API surface, were undertested. This entry closes that gap so the coverage floor now matches the project rule (not just CLAUDE.md's "public function" minimum).
69+
70+
### Impact
71+
- [ ] Breaking change
72+
- [ ] Requires cluster rollout
73+
- [ ] Config change only
74+
- [x] Documentation only (test-only change — no runtime behaviour modified)
75+
76+
---
77+
78+
## [2026-04-18 12:00] - Phase 2: Populate providerID and nodeRef from CAPI Machine
79+
80+
**Author:** Erick Bourgeois
81+
82+
### Changed
83+
- `src/reconcilers/helpers.rs`: Added three new helpers — `extract_machine_refs` (pure function pulling `providerID` + full `NodeRef` out of a CAPI Machine `DynamicObject`), `fetch_capi_machine` (typed 404 → `Ok(None)` wrapper around the dynamic GET), and `patch_machine_refs_status` (merge-patches both fields onto `ScheduledMachine.status`). Refactored `get_node_from_machine` to delegate to the first two helpers for the drain path.
84+
- `src/reconcilers/scheduled_machine.rs`: `handle_active_phase` happy path now fetches the CAPI Machine, extracts refs, and patches them onto the SM status. Failures are logged and ignored — status enrichment must never block reconciliation.
85+
- `src/reconcilers/helpers_tests.rs`: Added 6 TDD cases for `extract_machine_refs` covering fully populated, empty, providerID-only, nodeRef-without-uid, incomplete-nodeRef-returns-None, and malformed-providerID-ignored.
86+
87+
### Why
88+
Phase 2 of the event-driven watches + status enrichment roadmap. The schema landed in Phase 1; this change actually populates the new fields every time the reconciler visits an active machine, so `kubectl get sm -o jsonpath='{.status.providerID}{"\t"}{.status.nodeRef.name}'` returns meaningful values.
89+
90+
### Impact
91+
- [ ] Breaking change
92+
- [x] Requires cluster rollout — controller image carries the new reconcile logic
93+
- [ ] Config change only
94+
- [ ] Documentation only
95+
96+
---
97+
1298
## [2026-04-16 17:00] - Phase 1: Enrich ScheduledMachine status with providerID and full nodeRef
1399

14100
**Author:** Erick Bourgeois

‎.github/workflows/scorecard.yaml‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Copyright (c) 2025 Erick Bourgeois, firestoned
2+
# SPDX-License-Identifier: Apache-2.0
3+
#
4+
# This workflow uses actions that are not certified by GitHub. They are provided
5+
# by a third-party and are governed by separate terms of service, privacy
6+
# policy, and support documentation.
7+
8+
name: Scorecard supply-chain security
9+
on:
10+
# For Branch-Protection check. Only the default branch is supported. See
11+
# https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection
12+
branch_protection_rule:
13+
# To guarantee Maintained check is occasionally updated. See
14+
# https://github.com/ossf/scorecard/blob/main/docs/checks.md#maintained
15+
schedule:
16+
- cron: '24 10 * * 3'
17+
push:
18+
branches: [ "main" ]
19+
20+
# Declare default permissions as read only.
21+
permissions: read-all
22+
23+
jobs:
24+
analysis:
25+
name: Scorecard analysis
26+
runs-on: ubuntu-latest
27+
# `publish_results: true` only works when run from the default branch. conditional can be removed if disabled.
28+
if: github.event.repository.default_branch == github.ref_name || github.event_name == 'pull_request'
29+
permissions:
30+
# Needed to upload the results to code-scanning dashboard.
31+
security-events: write
32+
# Needed to publish results and get a badge (see publish_results below).
33+
id-token: write
34+
# Uncomment the permissions below if installing in a private repository.
35+
# contents: read
36+
# actions: read
37+
38+
steps:
39+
- name: "Checkout code"
40+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
41+
with:
42+
persist-credentials: false
43+
44+
- name: "Run analysis"
45+
uses: ossf/scorecard-action@f49aabe0b5af0936a0987cfb85d86b75731b0186 # v2.4.1
46+
with:
47+
results_file: results.sarif
48+
results_format: sarif
49+
# (Optional) "write" PAT token. Uncomment the `repo_token` line below if:
50+
# - you want to enable the Branch-Protection check on a *public* repository, or
51+
# - you are installing Scorecard on a *private* repository
52+
# To create the PAT, follow the steps in https://github.com/ossf/scorecard-action?tab=readme-ov-file#authentication-with-fine-grained-pat-optional.
53+
# repo_token: ${{ secrets.SCORECARD_TOKEN }}
54+
55+
# Public repositories:
56+
# - Publish results to OpenSSF REST API for easy access by consumers
57+
# - Allows the repository to include the Scorecard badge.
58+
# - See https://github.com/ossf/scorecard-action#publishing-results.
59+
# For private repositories:
60+
# - `publish_results` will always be set to `false`, regardless
61+
# of the value entered here.
62+
publish_results: true
63+
64+
# (Optional) Uncomment file_mode if you have a .gitattributes with files marked export-ignore
65+
# file_mode: git
66+
67+
# Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF
68+
# format to the repository Actions tab.
69+
- name: "Upload artifact"
70+
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
71+
with:
72+
name: SARIF file
73+
path: results.sarif
74+
retention-days: 5
75+
76+
# Upload the results to GitHub's code scanning dashboard (optional).
77+
# Commenting out will disable upload of results to your repo's Code Scanning dashboard
78+
- name: "Upload to code-scanning"
79+
uses: github/codeql-action/upload-sarif@ce64ddcb0d8d890d2df4a9d1c04ff297367dea2a # v3.35.2
80+
with:
81+
sarif_file: results.sarif

‎src/main.rs‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,24 @@ use std::sync::Arc;
1818
use anyhow::Result;
1919
use clap::Parser;
2020
use five_spot::constants::{
21-
DEFAULT_LEASE_DURATION_SECS, DEFAULT_LEASE_GRACE_SECS, DEFAULT_LEASE_NAME,
22-
DEFAULT_LEASE_NAMESPACE, DEFAULT_LEASE_RENEW_DEADLINE_SECS, DEFAULT_LEASE_RETRY_PERIOD_SECS,
23-
HEALTH_PORT, K8S_API_TIMEOUT_SECS, METRICS_PORT,
21+
CAPI_GROUP, CAPI_MACHINE_API_VERSION, CAPI_RESOURCE_MACHINES, DEFAULT_LEASE_DURATION_SECS,
22+
DEFAULT_LEASE_GRACE_SECS, DEFAULT_LEASE_NAME, DEFAULT_LEASE_NAMESPACE,
23+
DEFAULT_LEASE_RENEW_DEADLINE_SECS, DEFAULT_LEASE_RETRY_PERIOD_SECS, HEALTH_PORT,
24+
K8S_API_TIMEOUT_SECS, METRICS_PORT,
2425
};
2526
use five_spot::crd::ScheduledMachine;
2627
use five_spot::health::{start_health_server, HealthState};
28+
use five_spot::labels::LABEL_SCHEDULED_MACHINE;
2729
use five_spot::metrics::init_controller_info;
28-
use five_spot::reconcilers::{error_policy, reconcile_scheduled_machine, Context};
30+
use five_spot::reconcilers::{
31+
error_policy, machine_to_scheduled_machine, node_to_scheduled_machines,
32+
reconcile_scheduled_machine, Context,
33+
};
2934
use futures::StreamExt;
35+
use k8s_openapi::api::core::v1::Node;
3036
use kube::{
31-
api::ListParams,
37+
api::{ApiResource, GroupVersionKind, ListParams},
38+
core::DynamicObject,
3239
runtime::{watcher::Config, Controller},
3340
Api, Client,
3441
};
@@ -260,8 +267,32 @@ async fn main() -> Result<()> {
260267

261268
info!("Starting controller for ScheduledMachine resources");
262269

263-
// Run the controller
264-
Controller::new(scheduled_machines, Config::default())
270+
// Secondary watches — event-driven reactivity without polling.
271+
// 1. CAPI Machine (dynamic GVK) — filtered by the scheduled-machine label we
272+
// already stamp on every Machine we create. Reverse-mapped via that label.
273+
// 2. Kubernetes Node — name-matched against every SM's status.nodeRef.name
274+
// using a snapshot of the controller's own primary-resource Store.
275+
let machine_ar = ApiResource::from_gvk_with_plural(
276+
&GroupVersionKind::gvk(CAPI_GROUP, CAPI_MACHINE_API_VERSION, "Machine"),
277+
CAPI_RESOURCE_MACHINES,
278+
);
279+
let machines_api: Api<DynamicObject> = Api::all_with(client.clone(), &machine_ar);
280+
let nodes_api: Api<Node> = Api::all(client.clone());
281+
282+
let controller = Controller::new(scheduled_machines, Config::default());
283+
let sm_store = controller.store();
284+
285+
controller
286+
.watches_with(
287+
machines_api,
288+
machine_ar.clone(),
289+
Config::default().labels(LABEL_SCHEDULED_MACHINE),
290+
|machine: DynamicObject| machine_to_scheduled_machine(&machine),
291+
)
292+
.watches(nodes_api, Config::default(), move |node: Node| {
293+
let snapshot = sm_store.state();
294+
node_to_scheduled_machines(&node, snapshot.iter().map(std::convert::AsRef::as_ref))
295+
})
265296
.shutdown_on_signal()
266297
.run(reconcile_scheduled_machine, error_policy, context)
267298
.for_each(|res| async move {

0 commit comments

Comments
 (0)