Skip to content

Commit 36243ea

Browse files
G4614claude
andcommitted
feat(health-check): expose --no-health-check CLI flag + REST escape hatch
boxlite-ai#613 makes the per-box health check task default-on so the new zombie-shim reaper (Issue boxlite-ai#523) has a watcher to piggy-back on. The schema field `BoxOptions.advanced.health_check: Option<HealthCheckOptions>` already lets Rust library users opt out via `None`, but CLI users (`boxlite run` / `create`) and REST/SDK clients (Python, Node, Go all route through `POST /v1/boxes`) had no way to express that choice — exactly the gap boxlite-ai#597 hit for `cap_overrides`. Surface plumbed end-to-end: - `ManagementFlags { no_health_check: bool }` — `--no-health-check` on `boxlite run` / `create`. When set, `apply_to` clears `opts.advanced.health_check = None`. - REST client-side `CreateBoxRequest` gets `health_check_disabled: Option<bool>`; `from_options` flips it to `Some(true)` only when the operator already disabled health check in the input options, so the wire form stays byte-identical for every other call. - Server-side `CreateBoxRequest` (in `cli/src/commands/serve/types.rs`, `#[serde(deny_unknown_fields)]`) gains the same field. The `build_box_options` mapper builds a manual `AdvancedBoxOptions` instead of falling through `..Default::default()`, then forces `health_check = None` iff the wire says so. Three-state semantics for `health_check_disabled`: - `Some(true)` → explicit disable; box runs with no watcher, no zombie reaping (operator takes responsibility — documented inline). - `Some(false)` → "use the server default" (= currently `Some(...)`). Same effect as omitting the field, but documents the intent on the wire. - absent → server default. Pre-boxlite-ai#613 clients keep the legacy POST body shape and get the new default-on behaviour without code changes. Seven unit tests cover the surface symmetrically: - CLI: `management_flags_no_health_check_clears_advanced_field` / `management_flags_no_health_check_unset_keeps_default_on` — flag-on clears, flag-off preserves; pre-asserts the baseline so a regression that broke the default-on schema would also fail. - REST client: `test_create_box_request_carries_health_check_disabled_on_the_wire` / `test_create_box_request_omits_health_check_disabled_when_default` — `None` in BoxOptions → `Some(true)` on the wire; `Some(default)` → field absent (backward-compat with pre-boxlite-ai#613 servers). - REST server: `build_box_options_health_check_disabled_true_clears_health_check` / `build_box_options_no_health_check_field_keeps_default_on` / `build_box_options_health_check_disabled_false_keeps_default_on` — three-state JSON → BoxOptions mapping, including the explicit "Some(false) means default" sentinel. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent cd27fcb commit 36243ea

4 files changed

Lines changed: 186 additions & 0 deletions

File tree

src/boxlite/src/rest/types.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ pub(crate) struct CreateBoxRequest {
102102
pub detach: Option<bool>,
103103
#[serde(skip_serializing_if = "Option::is_none")]
104104
pub security: Option<String>,
105+
/// Disable the per-box health check (Issue #523: also disables
106+
/// the async-death zombie reaper that piggybacks on health check).
107+
/// Omitted when absent so existing clients keep the default-on
108+
/// behaviour without changing their POST body shape.
109+
#[serde(skip_serializing_if = "Option::is_none")]
110+
pub health_check_disabled: Option<bool>,
105111
}
106112

107113
impl CreateBoxRequest {
@@ -128,6 +134,15 @@ impl CreateBoxRequest {
128134
Some(options.secrets.iter().map(CreateBoxSecret::from).collect())
129135
};
130136

137+
// Only set the wire field when the operator explicitly disabled
138+
// health check. `None` on the wire = "use server default" (which
139+
// is currently `Some(HealthCheckOptions::default())`).
140+
let health_check_disabled = if options.advanced.health_check.is_none() {
141+
Some(true)
142+
} else {
143+
None
144+
};
145+
131146
Self {
132147
name,
133148
image,
@@ -145,6 +160,7 @@ impl CreateBoxRequest {
145160
auto_remove: Some(options.auto_remove),
146161
detach: Some(options.detach),
147162
security: None, // TODO: map security preset
163+
health_check_disabled,
148164
}
149165
}
150166
}
@@ -481,6 +497,7 @@ mod tests {
481497
auto_remove: Some(true),
482498
detach: None,
483499
security: None,
500+
health_check_disabled: None,
484501
};
485502
let json = serde_json::to_string(&req).unwrap();
486503
assert!(json.contains("\"name\":\"mybox\""));
@@ -535,6 +552,52 @@ mod tests {
535552
);
536553
}
537554

555+
/// Wire serialization: when the operator disabled health check
556+
/// (`options.advanced.health_check = None`), `from_options` flips
557+
/// `health_check_disabled = Some(true)` on the wire body so the
558+
/// server side actually receives the intent.
559+
#[test]
560+
fn test_create_box_request_carries_health_check_disabled_on_the_wire() {
561+
use crate::runtime::advanced_options::AdvancedBoxOptions;
562+
use crate::runtime::options::{BoxOptions, RootfsSpec};
563+
let opts = BoxOptions {
564+
rootfs: RootfsSpec::Image("alpine:latest".into()),
565+
advanced: AdvancedBoxOptions {
566+
health_check: None,
567+
..Default::default()
568+
},
569+
..Default::default()
570+
};
571+
let req = CreateBoxRequest::from_options(&opts, None);
572+
assert_eq!(req.health_check_disabled, Some(true));
573+
let json = serde_json::to_string(&req).unwrap();
574+
assert!(
575+
json.contains("\"health_check_disabled\":true"),
576+
"explicit disable must appear on the wire; got: {json}"
577+
);
578+
}
579+
580+
/// Backward compat: a `BoxOptions` with the default health_check
581+
/// (i.e. `Some(HealthCheckOptions::default())`) omits the wire
582+
/// field entirely — old clients/servers that don't know about
583+
/// `health_check_disabled` keep behaving exactly as before.
584+
#[test]
585+
fn test_create_box_request_omits_health_check_disabled_when_default() {
586+
use crate::runtime::options::{BoxOptions, RootfsSpec};
587+
let opts = BoxOptions {
588+
rootfs: RootfsSpec::Image("alpine:latest".into()),
589+
// Don't touch `advanced` — uses default (health_check on).
590+
..Default::default()
591+
};
592+
let req = CreateBoxRequest::from_options(&opts, None);
593+
assert_eq!(req.health_check_disabled, None);
594+
let json = serde_json::to_string(&req).unwrap();
595+
assert!(
596+
!json.contains("health_check_disabled"),
597+
"wire form must omit the field when default health check is in effect; got: {json}"
598+
);
599+
}
600+
538601
#[test]
539602
fn test_create_box_request_from_options_disabled_network() {
540603
use crate::runtime::options::{BoxOptions, NetworkSpec, RootfsSpec};

src/cli/src/cli.rs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,12 +693,24 @@ pub struct ManagementFlags {
693693
/// Automatically remove the box when it exits
694694
#[arg(long)]
695695
pub rm: bool,
696+
697+
/// Disable the per-box health check (Issue #523: also disables the
698+
/// async-death zombie reaper that piggybacks on the health-check
699+
/// loop). Use when the box is so short-lived or low-priority that
700+
/// the 30 s background tick isn't worth its cost — typical case:
701+
/// very large fleets where the operator runs their own monitoring
702+
/// and accepts having to clean up zombies if a shim dies asynchronously.
703+
#[arg(long = "no-health-check")]
704+
pub no_health_check: bool,
696705
}
697706

698707
impl ManagementFlags {
699708
pub fn apply_to(&self, opts: &mut BoxOptions) {
700709
opts.detach = self.detach;
701710
opts.auto_remove = self.rm;
711+
if self.no_health_check {
712+
opts.advanced.health_check = None;
713+
}
702714
}
703715
}
704716

@@ -708,6 +720,52 @@ mod tests {
708720
use std::fs;
709721
use tempfile::TempDir;
710722

723+
/// `--no-health-check` is the operator-level escape hatch for the
724+
/// default-on health check introduced for zombie-shim reaping
725+
/// (Issue #523 / #613). Setting it must clear
726+
/// `BoxOptions.advanced.health_check` so the box-init pipeline
727+
/// doesn't spawn a watcher task.
728+
#[test]
729+
fn management_flags_no_health_check_clears_advanced_field() {
730+
let flags = ManagementFlags {
731+
name: None,
732+
detach: false,
733+
rm: false,
734+
no_health_check: true,
735+
};
736+
let mut opts = BoxOptions::default();
737+
// Sanity-check the baseline default: health check is on at the
738+
// schema level so `--no-health-check` actually has work to do.
739+
assert!(
740+
opts.advanced.health_check.is_some(),
741+
"baseline must be health-check-on; otherwise this test isn't measuring the override"
742+
);
743+
flags.apply_to(&mut opts);
744+
assert!(
745+
opts.advanced.health_check.is_none(),
746+
"--no-health-check must clear advanced.health_check"
747+
);
748+
}
749+
750+
/// Negative: leaving the flag off must leave the default-on
751+
/// behaviour intact, so absence of the flag and presence of the
752+
/// flag have distinguishable effects.
753+
#[test]
754+
fn management_flags_no_health_check_unset_keeps_default_on() {
755+
let flags = ManagementFlags {
756+
name: None,
757+
detach: false,
758+
rm: false,
759+
no_health_check: false,
760+
};
761+
let mut opts = BoxOptions::default();
762+
flags.apply_to(&mut opts);
763+
assert!(
764+
opts.advanced.health_check.is_some(),
765+
"without --no-health-check the schema default (Some) must win"
766+
);
767+
}
768+
711769
#[test]
712770
fn test_apply_env_vars_with_lookup() {
713771
let mut opts = BoxOptions::default();

src/cli/src/commands/serve/mod.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,16 @@ fn build_box_options(req: &CreateBoxRequest) -> Result<BoxOptions, boxlite::Boxl
656656
None => NetworkSpec::default(),
657657
};
658658

659+
// Resolve health_check policy:
660+
// - `health_check_disabled = Some(true)` → explicit None (operator opt-out)
661+
// - `Some(false)` / absent → keep the schema default (currently
662+
// `Some(HealthCheckOptions::default())`), so we let
663+
// `AdvancedBoxOptions::default()` win via `..Default::default()`.
664+
let mut advanced = boxlite::AdvancedBoxOptions::default();
665+
if req.health_check_disabled.unwrap_or(false) {
666+
advanced.health_check = None;
667+
}
668+
659669
Ok(BoxOptions {
660670
rootfs,
661671
cpus: req.cpus,
@@ -669,6 +679,7 @@ fn build_box_options(req: &CreateBoxRequest) -> Result<BoxOptions, boxlite::Boxl
669679
user: req.user.clone(),
670680
auto_remove: req.auto_remove.unwrap_or(false),
671681
detach: req.detach.unwrap_or(true),
682+
advanced,
672683
..Default::default()
673684
})
674685
}
@@ -1017,6 +1028,54 @@ mod tests {
10171028
assert!(constant_time_eq(b"", b""));
10181029
}
10191030

1031+
/// `POST /v1/boxes` body with `health_check_disabled: true` flips
1032+
/// `advanced.health_check` to `None` in the resulting BoxOptions —
1033+
/// the SDK→REST→server path's escape hatch for the default-on
1034+
/// behaviour introduced in Issue #523 / #613.
1035+
#[test]
1036+
fn build_box_options_health_check_disabled_true_clears_health_check() {
1037+
let json = r#"{"image": "alpine:latest", "health_check_disabled": true}"#;
1038+
let req: super::types::CreateBoxRequest =
1039+
serde_json::from_str(json).expect("must deserialize");
1040+
let opts = build_box_options(&req).expect("build_box_options");
1041+
assert!(
1042+
opts.advanced.health_check.is_none(),
1043+
"explicit health_check_disabled=true must clear advanced.health_check"
1044+
);
1045+
}
1046+
1047+
/// Backward compat: a request body that omits `health_check_disabled`
1048+
/// (i.e. every pre-#613 client) gets the server-side default, which
1049+
/// after #613 is `Some(HealthCheckOptions::default())`. The empty
1050+
/// body must not silently strip health check.
1051+
#[test]
1052+
fn build_box_options_no_health_check_field_keeps_default_on() {
1053+
let json = r#"{"image": "alpine:latest"}"#;
1054+
let req: super::types::CreateBoxRequest =
1055+
serde_json::from_str(json).expect("legacy body must still deserialize");
1056+
let opts = build_box_options(&req).expect("build_box_options");
1057+
assert!(
1058+
opts.advanced.health_check.is_some(),
1059+
"default-on health check must survive a request that doesn't mention it"
1060+
);
1061+
}
1062+
1063+
/// `Some(false)` is the explicit "I want the default" sentinel —
1064+
/// behaves identically to omitting the field. Documents the wire
1065+
/// contract so a confused client setting it to false doesn't get
1066+
/// the disabled behaviour.
1067+
#[test]
1068+
fn build_box_options_health_check_disabled_false_keeps_default_on() {
1069+
let json = r#"{"image": "alpine:latest", "health_check_disabled": false}"#;
1070+
let req: super::types::CreateBoxRequest =
1071+
serde_json::from_str(json).expect("must deserialize");
1072+
let opts = build_box_options(&req).expect("build_box_options");
1073+
assert!(
1074+
opts.advanced.health_check.is_some(),
1075+
"health_check_disabled=false must be treated as 'use default' (= on)"
1076+
);
1077+
}
1078+
10201079
/// Build an `ActiveExecution` backed by a stub `Execution` whose
10211080
/// stdout/stderr/result channels we control from the test.
10221081
fn make_test_active() -> (

src/cli/src/commands/serve/types.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ pub(super) struct CreateBoxRequest {
3939
pub auto_remove: Option<bool>,
4040
#[serde(default)]
4141
pub detach: Option<bool>,
42+
/// Disable the per-box health check (Issue #523: also disables the
43+
/// async-death zombie reaper that piggybacks on health check).
44+
/// `Some(true)` → disable; `Some(false)` / absent → server default
45+
/// (currently `Some(HealthCheckOptions::default())`).
46+
#[serde(default)]
47+
pub health_check_disabled: Option<bool>,
4248
}
4349

4450
#[derive(Deserialize)]

0 commit comments

Comments
 (0)