refactor(fleet-deploy): collapse smoke companion to one trait, one method #299

Open
johnride wants to merge 1 commits from refactor/smoke-companion-minimal into feat/smoke-test-contract
Owner

Replaces the seven-file companion/smoke/ directory (Probe trait +
ProbeAttempt + ProbeOutcome + ProbeFailure + ProbeName + RetryPolicy +
run_probe + SmokeSuite + SmokeStage + SmokeReport + SmokeTest +
SmokeAssemblyError, ~1500 LOC) with a single companion/smoke.rs.

Net delta: +638 / -1578.

The earlier draft was cardinality matching gone overboard. For what
is, in the end, "after deploy, run an async function and surface a
pipeline report", we now ship:

  • SmokeTest<T> trait — one async method verify(&T) -> SmokeReport.
    Implementers write a regular async fn; no probe abstraction to
    learn, no suite builder, no policy value type.
  • SmokeReport { checks: Vec<CheckReport> } — the pipeline data the
    dashboard renders top-to-bottom. A report passes iff it has at
    least one check AND every check passed (a smoke impl that forgets
    to push checks fails loudly, not silently).
  • CheckReport { name, passed, detail } + pass / pass_with / fail
    constructors.
  • poll_until and tcp_reachable — free functions, not traits.
    Implementers call them inside verify when useful. A future
    http_healthy or k8s_pod_ready lives in this file as another
    async fn -> CheckReport, not as another trait impl.
  • deploy / deploy_with_smoke — free functions, returning
    DeployOutcome { interpret, smoke }. deploy_with_smoke blocks
    the deploy on smoke success (ADR-023 P4).

Per the PR #292 review: the SmokeTest's associated type is now
type Interpret: Interpret<T> (was type Score: Score<T>). One
smoke impl can cover every Score that shares an Interpret —
HelmChartScore + NatsHelmChartScore both verified by one
HelmChartSmokeTest. Pairing is convention-only in v0.3 because
Score::create_interpret still returns Box<dyn Interpret<T>>;
closing the loop is an additive type Interpret on Score, deferred.

Tests (12 in this module):

  • deploy runs interpret and returns the underlying Outcome
  • deploy_with_smoke runs smoke only after interpret succeeds
  • deploy_with_smoke returns SmokeFailed (with interpret preserved)
    when any check fails
  • deploy_with_smoke rejects an empty report — no silent pass-through
  • deploy_with_smoke skips smoke entirely when interpret fails
  • SmokeReport::passed semantics (nonempty + all pass)
  • poll_until pass-on-success, fail-on-budget
  • tcp_reachable pass against a real loopback listener
  • tcp_reachable fail with timeout against TEST-NET-1
Replaces the seven-file `companion/smoke/` directory (Probe trait + ProbeAttempt + ProbeOutcome + ProbeFailure + ProbeName + RetryPolicy + run_probe + SmokeSuite + SmokeStage + SmokeReport + SmokeTest + SmokeAssemblyError, ~1500 LOC) with a single `companion/smoke.rs`. Net delta: +638 / -1578. The earlier draft was cardinality matching gone overboard. For what is, in the end, "after deploy, run an async function and surface a pipeline report", we now ship: * `SmokeTest<T>` trait — one async method `verify(&T) -> SmokeReport`. Implementers write a regular async fn; no probe abstraction to learn, no suite builder, no policy value type. * `SmokeReport { checks: Vec<CheckReport> }` — the pipeline data the dashboard renders top-to-bottom. A report passes iff it has at least one check AND every check passed (a smoke impl that forgets to push checks fails loudly, not silently). * `CheckReport { name, passed, detail }` + `pass / pass_with / fail` constructors. * `poll_until` and `tcp_reachable` — free functions, not traits. Implementers call them inside `verify` when useful. A future `http_healthy` or `k8s_pod_ready` lives in this file as another `async fn -> CheckReport`, not as another trait impl. * `deploy` / `deploy_with_smoke` — free functions, returning `DeployOutcome { interpret, smoke }`. `deploy_with_smoke` blocks the deploy on smoke success (ADR-023 P4). Per the PR #292 review: the SmokeTest's associated type is now `type Interpret: Interpret<T>` (was `type Score: Score<T>`). One smoke impl can cover every Score that shares an Interpret — HelmChartScore + NatsHelmChartScore both verified by one HelmChartSmokeTest. Pairing is convention-only in v0.3 because `Score::create_interpret` still returns `Box<dyn Interpret<T>>`; closing the loop is an additive `type Interpret` on `Score`, deferred. Tests (12 in this module): - deploy runs interpret and returns the underlying Outcome - deploy_with_smoke runs smoke only after interpret succeeds - deploy_with_smoke returns SmokeFailed (with interpret preserved) when any check fails - deploy_with_smoke rejects an empty report — no silent pass-through - deploy_with_smoke skips smoke entirely when interpret fails - SmokeReport::passed semantics (nonempty + all pass) - poll_until pass-on-success, fail-on-budget - tcp_reachable pass against a real loopback listener - tcp_reachable fail with timeout against TEST-NET-1
johnride added 1 commit 2026-05-26 13:00:47 +00:00
refactor(fleet-deploy): collapse smoke companion to one trait, one method
All checks were successful
Run Check Script / check (pull_request) Successful in 2m24s
1d6fb40236
Replaces the seven-file `companion/smoke/` directory (Probe trait +
ProbeAttempt + ProbeOutcome + ProbeFailure + ProbeName + RetryPolicy +
run_probe + SmokeSuite + SmokeStage + SmokeReport + SmokeTest +
SmokeAssemblyError, ~1500 LOC) with a single `companion/smoke.rs`.

Net delta: +638 / -1578.

The earlier draft was cardinality matching gone overboard. For what
is, in the end, "after deploy, run an async function and surface a
pipeline report", we now ship:

* `SmokeTest<T>` trait — one async method `verify(&T) -> SmokeReport`.
  Implementers write a regular async fn; no probe abstraction to
  learn, no suite builder, no policy value type.
* `SmokeReport { checks: Vec<CheckReport> }` — the pipeline data the
  dashboard renders top-to-bottom. A report passes iff it has at
  least one check AND every check passed (a smoke impl that forgets
  to push checks fails loudly, not silently).
* `CheckReport { name, passed, detail }` + `pass / pass_with / fail`
  constructors.
* `poll_until` and `tcp_reachable` — free functions, not traits.
  Implementers call them inside `verify` when useful. A future
  `http_healthy` or `k8s_pod_ready` lives in this file as another
  `async fn -> CheckReport`, not as another trait impl.
* `deploy` / `deploy_with_smoke` — free functions, returning
  `DeployOutcome { interpret, smoke }`. `deploy_with_smoke` blocks
  the deploy on smoke success (ADR-023 P4).

Per the PR #292 review: the SmokeTest's associated type is now
`type Interpret: Interpret<T>` (was `type Score: Score<T>`). One
smoke impl can cover every Score that shares an Interpret —
HelmChartScore + NatsHelmChartScore both verified by one
HelmChartSmokeTest. Pairing is convention-only in v0.3 because
`Score::create_interpret` still returns `Box<dyn Interpret<T>>`;
closing the loop is an additive `type Interpret` on `Score`, deferred.

Tests (12 in this module):
- deploy runs interpret and returns the underlying Outcome
- deploy_with_smoke runs smoke only after interpret succeeds
- deploy_with_smoke returns SmokeFailed (with interpret preserved)
  when any check fails
- deploy_with_smoke rejects an empty report — no silent pass-through
- deploy_with_smoke skips smoke entirely when interpret fails
- SmokeReport::passed semantics (nonempty + all pass)
- poll_until pass-on-success, fail-on-budget
- tcp_reachable pass against a real loopback listener
- tcp_reachable fail with timeout against TEST-NET-1
stremblay approved these changes 2026-05-26 17:47:02 +00:00
@@ -0,0 +82,4 @@
async fn verify(&self, topology: &T) -> SmokeReport;
}
/// Aggregated smoke result. Dashboards render `checks` top-to-bottom.
Owner

SmokeTest companion is not specifically for "Dashboards"... Its to be able to have smoke testing done after deploying any kind of stuff. The "Dashboard" specificity which is everywhere in the comments in this file should be removed.

SmokeTest companion is not specifically for "Dashboards"... Its to be able to have smoke testing done after deploying any kind of stuff. The "Dashboard" specificity which is everywhere in the comments in this file should be removed.
@@ -0,0 +138,4 @@
}
}
pub fn fail(name: &'static str, detail: impl Into<String>) -> Self {
Owner

should maybe be named "fail_with" to be consistent with pass / pass_with as it needs "detail"

should maybe be named "fail_with" to be consistent with pass / pass_with as it needs "detail"
All checks were successful
Run Check Script / check (pull_request) Successful in 2m24s
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin refactor/smoke-companion-minimal:refactor/smoke-companion-minimal
git checkout refactor/smoke-companion-minimal
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: NationTech/harmony#299
No description provided.