Skip to content

Commit 2fc8493

Browse files
committed
Fix CAPI Machine name lookup, add Ready column, expand reconcile logging The reconciler had been constructing CAPI Machine names as {sm}-machine in two lookup paths while the create path (add_machine_to_cluster) writes the Machine with the bare SM name. Lookups silently 404'd in the field — status enrichment and pre-drain node lookup never matched the real Machine. Both call sites now use &name directly; the kind suffix is gone.
Signed-off-by: Erick Bourgeois <erick@jeb.ca>
1 parent 719747c commit 2fc8493

9 files changed

Lines changed: 269 additions & 24 deletions

File tree

.github/workflows/docs.yaml

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
name: Documentation
55

66
on:
7+
# Build-only triggers — exercise the docs pipeline so breakage is
8+
# caught at PR / merge time, but do NOT publish to GitHub Pages.
9+
# Deployment is gated on the `release` event below.
710
push:
811
branches:
912
- main
@@ -16,18 +19,28 @@ on:
1619
- 'docs/**'
1720
- 'src/**/*.rs'
1821
- '.github/workflows/docs.yaml'
22+
# Deploy trigger — fires when an operator publishes a GitHub Release.
23+
# The docs site published to Pages is what end users see, so we tie
24+
# publish cadence to the release cadence: every release ships matching
25+
# docs, and intermediate main commits don't churn the live site.
26+
release:
27+
types: [published]
1928
workflow_dispatch:
2029

2130
permissions:
2231
contents: read
2332
pages: write
2433
id-token: write
2534

26-
# Allow one concurrent deployment; cancel any in-progress run on the same
27-
# branch so a rapid series of pushes never leaves a stale deploy running.
35+
# Build runs on every PR / push and can race with itself — cancel
36+
# in-progress to keep CI from queuing redundant builds. Release deploys
37+
# happen at most once per release tag and must NEVER be cancelled by a
38+
# subsequent push (operators rely on the published release deploying);
39+
# the `cancel-in-progress` expression below disables cancellation only
40+
# for the release event.
2841
concurrency:
29-
group: "pages"
30-
cancel-in-progress: true
42+
group: pages-${{ github.event_name == 'release' && github.event.release.tag_name || github.ref }}
43+
cancel-in-progress: ${{ github.event_name != 'release' }}
3144

3245
env:
3346
CARGO_TERM_COLOR: always
@@ -160,19 +173,26 @@ jobs:
160173
npm install -g "linkinator@${LINKINATOR_VERSION}"
161174
linkinator docs/site/ --recurse --skip "rustdoc/.*" --verbosity error
162175
176+
# Pages setup + artifact upload only happen when the trigger is a
177+
# release publication. PR / push runs build the site (proving it
178+
# compiles cleanly) but never produce a deploy artifact.
163179
- name: Setup Pages
164-
if: github.ref == 'refs/heads/main'
180+
if: github.event_name == 'release'
165181
uses: actions/configure-pages@45bfe0192ca1faeb007ade9deae92b16b8254a0d # v6.0.0
166182

167183
- name: Upload Pages artifact
168-
if: github.ref == 'refs/heads/main'
184+
if: github.event_name == 'release'
169185
uses: actions/upload-pages-artifact@fc324d3547104276b827a68afc52ff2a11cc49c9 # v5.0.0
170186
with:
171187
path: docs/site
172188

173189
deploy:
174190
name: 🚀 Deploy to GitHub Pages
175-
if: github.ref == 'refs/heads/main'
191+
# Gated on the `release: published` event — a main push or PR build
192+
# validates the docs but does NOT publish them. The published site
193+
# tracks release cadence so end users see a stable, versioned doc
194+
# set rather than every intermediate main commit.
195+
if: github.event_name == 'release'
176196
needs: build
177197
runs-on: ubuntu-latest
178198
environment:

deploy/crds/scheduledmachine.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.12s
2+
Running `target/debug/crdgen`
13
apiVersion: apiextensions.k8s.io/v1
24
kind: CustomResourceDefinition
35
metadata:
@@ -17,6 +19,9 @@ spec:
1719
- jsonPath: .status.phase
1820
name: Phase
1921
type: string
22+
- jsonPath: .status.ready
23+
name: Ready
24+
type: boolean
2025
- jsonPath: .status.inSchedule
2126
name: InSchedule
2227
type: boolean
@@ -446,6 +451,14 @@ spec:
446451
unique across the cluster.
447452
nullable: true
448453
type: string
454+
ready:
455+
default: false
456+
description: |-
457+
True only when the machine has reached the `Active` phase. Surfaced as
458+
the `Ready` printer column for fast operator triage; any other phase
459+
(Pending, ShuttingDown, Inactive, Disabled, Terminated, Error) is
460+
reported as `False`.
461+
type: boolean
449462
type: object
450463
required:
451464
- spec

docs/src/reference/api.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,12 @@ Array of condition objects with the following fields:
196196

197197
(boolean) Whether the machine is currently within its scheduled time window.
198198

199+
#### ready
200+
201+
(boolean) `True` only when `phase` is `Active`. Surfaced as the `Ready` printer column
202+
for fast operator triage — any other phase (`Pending`, `ShuttingDown`, `Inactive`,
203+
`Disabled`, `Terminated`, `Error`) is reported as `False`.
204+
199205
#### message
200206

201207
(string) Human-readable message describing the current state.

src/bin/crddoc.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,14 @@ fn main() {
248248
println!();
249249
println!("(boolean) Whether the machine is currently within its scheduled time window.");
250250
println!();
251+
println!("#### ready");
252+
println!();
253+
println!(
254+
"(boolean) `True` only when `phase` is `Active`. Surfaced as the `Ready` printer column"
255+
);
256+
println!("for fast operator triage — any other phase (`Pending`, `ShuttingDown`, `Inactive`,");
257+
println!("`Disabled`, `Terminated`, `Error`) is reported as `False`.");
258+
println!();
251259
println!("#### message");
252260
println!();
253261
println!("(string) Human-readable message describing the current state.");

src/crd.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use std::collections::{BTreeMap, HashSet};
3333
shortname = "sm",
3434
status = "ScheduledMachineStatus",
3535
printcolumn = r#"{"name":"Phase","type":"string","jsonPath":".status.phase"}"#,
36+
printcolumn = r#"{"name":"Ready","type":"boolean","jsonPath":".status.ready"}"#,
3637
printcolumn = r#"{"name":"InSchedule","type":"boolean","jsonPath":".status.inSchedule"}"#,
3738
printcolumn = r#"{"name":"Enabled","type":"boolean","jsonPath":".spec.schedule.enabled"}"#,
3839
printcolumn = r#"{"name":"Schedule Days","type":"string","jsonPath":".spec.schedule.daysOfWeek"}"#,
@@ -487,6 +488,13 @@ pub struct ScheduledMachineStatus {
487488
#[serde(default)]
488489
pub in_schedule: bool,
489490

491+
/// True only when the machine has reached the `Active` phase. Surfaced as
492+
/// the `Ready` printer column for fast operator triage; any other phase
493+
/// (Pending, ShuttingDown, Inactive, Disabled, Terminated, Error) is
494+
/// reported as `False`.
495+
#[serde(default)]
496+
pub ready: bool,
497+
490498
/// Next scheduled activation time (RFC3339 format)
491499
#[serde(skip_serializing_if = "Option::is_none")]
492500
pub next_activation: Option<String>,

src/main.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -347,10 +347,11 @@ async fn main() -> Result<()> {
347347
.run(reconcile_scheduled_machine, error_policy, context)
348348
.for_each(|res| async move {
349349
match res {
350-
Ok(o) => {
350+
Ok((obj_ref, action)) => {
351351
info!(
352-
resource = o.0.name,
353-
namespace = ?o.0.namespace,
352+
resource = %obj_ref.name,
353+
namespace = ?obj_ref.namespace,
354+
next_action = ?action,
354355
"Reconciliation completed"
355356
);
356357
}

src/reconcilers/helpers.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -327,17 +327,19 @@ pub async fn handle_deletion(
327327
ReconcilerError::InvalidConfig("ScheduledMachine must be namespaced".to_string())
328328
})?;
329329
let name = resource.name_any();
330+
let current_phase = resource.status.as_ref().and_then(|s| s.phase.as_deref());
330331

331332
info!(
332333
resource = %name,
333334
namespace = %namespace,
334-
"Handling deletion"
335+
phase = current_phase.unwrap_or("(none)"),
336+
deletion_timestamp = ?resource.meta().deletion_timestamp,
337+
"DELETE: ScheduledMachine deletion requested — running finalizer cleanup"
335338
);
336339

337340
// Wrap machine removal in a hard timeout so a hung removal cannot block
338341
// namespace deletion or cluster upgrades indefinitely.
339342
let cleanup_timeout = Duration::from_secs(FINALIZER_CLEANUP_TIMEOUT_SECS);
340-
let current_phase = resource.status.as_ref().and_then(|s| s.phase.as_deref());
341343

342344
if let Some(phase) = current_phase {
343345
if matches!(phase, PHASE_ACTIVE | PHASE_SHUTTING_DOWN) {
@@ -435,7 +437,7 @@ pub async fn handle_deletion(
435437
info!(
436438
resource = %name,
437439
namespace = %namespace,
438-
"Finalizer removed, resource will be deleted"
440+
"DELETE: finalizer removed — Kubernetes will now delete the ScheduledMachine"
439441
);
440442

441443
Ok(Action::await_change())
@@ -482,10 +484,18 @@ pub async fn handle_kill_switch(
482484
info!(
483485
resource = %name,
484486
namespace = %namespace,
485-
"Kill switch active - removing machine immediately"
487+
phase = %phase,
488+
"KILL: removing CAPI Machine due to kill switch"
486489
);
487490

488491
remove_machine_from_cluster(&resource, &ctx.client, &namespace).await?;
492+
} else {
493+
info!(
494+
resource = %name,
495+
namespace = %namespace,
496+
phase = %phase,
497+
"KILL: kill switch set but machine not running — recording Terminated phase only"
498+
);
489499
}
490500
}
491501

@@ -735,6 +745,7 @@ pub async fn update_phase(
735745
message: Some(resolved_message.to_string()),
736746
conditions: vec![condition],
737747
in_schedule,
748+
ready: phase == PHASE_ACTIVE,
738749
..Default::default()
739750
};
740751

@@ -808,6 +819,7 @@ pub async fn update_phase_with_last_schedule(
808819
conditions: vec![condition],
809820
last_scheduled_time: Some(Utc::now().to_rfc3339()),
810821
in_schedule,
822+
ready: phase == PHASE_ACTIVE,
811823
..Default::default()
812824
};
813825

@@ -880,6 +892,7 @@ pub async fn update_phase_with_grace_period(
880892
message: Some(resolved_message.to_string()),
881893
conditions: vec![condition],
882894
in_schedule,
895+
ready: phase == PHASE_ACTIVE,
883896
..Default::default()
884897
};
885898

src/reconcilers/helpers_tests.rs

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,155 @@ mod tests {
854854
srv.await.unwrap();
855855
}
856856

857+
// ---- ready-field projection on status patch ----
858+
859+
#[tokio::test]
860+
async fn test_update_phase_active_sets_ready_true() {
861+
let (client, handle) = mock_client_pair();
862+
let ctx = make_test_context(client);
863+
864+
let srv = tokio::spawn(async move {
865+
let mut h = pin!(handle);
866+
let (_req, send) = h.next_request().await.expect("events call");
867+
send.send_response(
868+
Response::builder()
869+
.status(201)
870+
.header("content-type", "application/json")
871+
.body(Body::from(k8s_event_response_body()))
872+
.unwrap(),
873+
);
874+
let (req, send) = h.next_request().await.expect("patch_status call");
875+
let body = collect_json_body(req.into_body()).await;
876+
assert_eq!(body["status"]["phase"], "Active");
877+
assert_eq!(
878+
body["status"]["ready"], true,
879+
"ready must be True when phase is Active"
880+
);
881+
send.send_response(
882+
Response::builder()
883+
.status(200)
884+
.header("content-type", "application/json")
885+
.body(Body::from(sm_response_body("test-sm", "default", "Active")))
886+
.unwrap(),
887+
);
888+
});
889+
890+
update_phase(
891+
&ctx,
892+
"default",
893+
"test-sm",
894+
Some("Pending"),
895+
"Active",
896+
None,
897+
None,
898+
true,
899+
)
900+
.await
901+
.expect("update_phase should succeed");
902+
903+
srv.await.unwrap();
904+
}
905+
906+
#[tokio::test]
907+
async fn test_update_phase_inactive_sets_ready_false() {
908+
let (client, handle) = mock_client_pair();
909+
let ctx = make_test_context(client);
910+
911+
let srv = tokio::spawn(async move {
912+
let mut h = pin!(handle);
913+
let (_req, send) = h.next_request().await.expect("events call");
914+
send.send_response(
915+
Response::builder()
916+
.status(201)
917+
.header("content-type", "application/json")
918+
.body(Body::from(k8s_event_response_body()))
919+
.unwrap(),
920+
);
921+
let (req, send) = h.next_request().await.expect("patch_status call");
922+
let body = collect_json_body(req.into_body()).await;
923+
assert_eq!(body["status"]["phase"], "Inactive");
924+
assert_eq!(
925+
body["status"]["ready"], false,
926+
"ready must be False for any non-Active phase"
927+
);
928+
send.send_response(
929+
Response::builder()
930+
.status(200)
931+
.header("content-type", "application/json")
932+
.body(Body::from(sm_response_body(
933+
"test-sm", "default", "Inactive",
934+
)))
935+
.unwrap(),
936+
);
937+
});
938+
939+
update_phase(
940+
&ctx,
941+
"default",
942+
"test-sm",
943+
Some("Active"),
944+
"Inactive",
945+
None,
946+
None,
947+
false,
948+
)
949+
.await
950+
.expect("update_phase should succeed");
951+
952+
srv.await.unwrap();
953+
}
954+
955+
#[tokio::test]
956+
async fn test_update_phase_with_grace_period_shutting_down_sets_ready_false() {
957+
let (client, handle) = mock_client_pair();
958+
let ctx = make_test_context(client);
959+
960+
let srv = tokio::spawn(async move {
961+
let mut h = pin!(handle);
962+
let (_req, send) = h.next_request().await.expect("events call");
963+
send.send_response(
964+
Response::builder()
965+
.status(201)
966+
.header("content-type", "application/json")
967+
.body(Body::from(k8s_event_response_body()))
968+
.unwrap(),
969+
);
970+
let (req, send) = h.next_request().await.expect("patch_status call");
971+
let body = collect_json_body(req.into_body()).await;
972+
assert_eq!(body["status"]["phase"], "ShuttingDown");
973+
assert_eq!(
974+
body["status"]["ready"], false,
975+
"ready must be False during ShuttingDown"
976+
);
977+
send.send_response(
978+
Response::builder()
979+
.status(200)
980+
.header("content-type", "application/json")
981+
.body(Body::from(sm_response_body(
982+
"test-sm",
983+
"default",
984+
"ShuttingDown",
985+
)))
986+
.unwrap(),
987+
);
988+
});
989+
990+
update_phase_with_grace_period(
991+
&ctx,
992+
"default",
993+
"test-sm",
994+
Some("Active"),
995+
"ShuttingDown",
996+
None,
997+
None,
998+
false,
999+
)
1000+
.await
1001+
.expect("grace period update should succeed");
1002+
1003+
srv.await.unwrap();
1004+
}
1005+
8571006
// ---- Context::new — unit tests ----
8581007

8591008
#[tokio::test]

0 commit comments

Comments
 (0)