fix(update_validation): poll system health instead of broken state wait

[JanZachmann]

Summary

• replace wait_for_system_running() (never observed a state transition: wrong property filter + uncached property; hung until the validation timeout) with a polling wait_for_system_healthy()
• fail validation fast on a degraded system state or crash-looping services; the cause becomes the reboot reason extra info
• crash loops are flagged by restart count only (default 3, configurable via CRASH_LOOP_RESTART_THRESHOLD) so a single transient restart cannot roll back a good update
• the health deadline is the remaining validation time minus a safety margin, so the descriptive error always wins the race against the generic validation timeout that decides the reboot reason
• new crash_loop health check (red when a service loops in auto-restart or has 3+ restarts while not active)
• update update_validation.md and README; bump to 0.45.0

[JanZachmann]

FYI @mlilien

[Copilot]

Pull request overview

This PR improves firmware update validation by replacing a broken “wait for running” mechanism with a polling-based system health check that can fail fast on degraded systems or crash-looping services. It also extends the standalone healthcheck/ scripts with new checks (system running, crash-loop, timesync, coredumps), updates documentation, and bumps the crate version.

Changes:

• Replace wait_for_system_running() with wait_for_system_healthy(deadline) and introduce crash-loop / degraded detection in src/systemd.
• Document the new validation behavior (system health deadline + crash-loop detection) and update README references.
• Add/adjust healthcheck scripts and configuration, and bump version to 0.45.0.

Reviewed changes

Copilot reviewed 11 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/twin/firmware_update/update_validation.rs	Switch validation to wait for “healthy” with an internal deadline.
src/twin/firmware_update/update_validation.md	Document system health polling deadline and crash-loop semantics.
src/systemd/mod.rs	Implement wait_for_system_healthy() plus unit/crash-loop evaluation and tests.
README.md	Mention healthcheck/ scripts and the new crash_loop check.
healthcheck/omnect_service_log.sh	Update script invocation name to omnect_health_services.sh.
healthcheck/omnect_service_log_analysis.json.template	Update referenced healthcheck script name.
healthcheck/omnect_health_timesync.sh	Add timesync healthcheck.
healthcheck/omnect_health_system_running.sh	Add system-running healthcheck.
healthcheck/omnect_health_services.sh	Add service-exit-log analysis healthcheck script.
healthcheck/omnect_health_crash_loop.sh	Add crash-loop detection healthcheck.
healthcheck/omnect_health_coredumps.sh	Add coredumps healthcheck.
healthcheck/omnect_health_checks.json.template	Update naming convention text; add crash_loop entry.
healthcheck/omnect_health_checks.json	Add crash_loop entry to default checks.
healthcheck/omnect_health_check.sh	Update invoked check-script prefix to omnect_health_.
Cargo.toml	Bump crate version to 0.45.0.
Cargo.lock	Bump locked crate version to 0.45.0.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/systemd/mod.rs:105

• wait_for_system_healthy(Duration::ZERO) can still sleep and take ~4s to return OK because the confirmation loop always requires HEALTHY_CONFIRMATION_POLLS polls, regardless of the provided deadline. This makes the deadline parameter surprising (0 does not mean “no waiting”) and can still allow the outer validation timeout to win when the remaining time is very small.

                healthy_polls += 1;
                if healthy_polls >= HEALTHY_CONFIRMATION_POLLS {
                    return Ok(());
                }

src/systemd/mod.rs:180

• CRASH_LOOP_RESTART_THRESHOLD currently accepts 0, which makes every non-active service match n_restarts >= threshold and therefore forces validation to fail on most systems. Treat 0 as invalid and fall back to the default (or require >= 1).

        match value.parse::<u32>() {
            Ok(value) => threshold = value,

[JoergZeidler]

Review of the system health validation.

Verification on my side: cargo fmt -- --check is clean. cargo clippy --features bootloader_grub and cargo test --features mock could not be run here, the azure-iot-sdk-sys build script aborts because azure-iot-sdk-dev.pc is not in PKG_CONFIG_PATH. So no compile or test evidence from me.

Two things I could not check from this repo:

• the claim that TEST_REBOOT_REASON_CHECK_EXTRA_INFO in the omnect-os CI does an exact match on the two extra info strings
• whether the meta-omnect recipe installs healthcheck/*.sh by wildcard or by an explicit list

The first three inline comments can end in a rollback of a good update, the rest are smaller.

[JoergZeidler]

Second round on 3db1212 and 4ec4b2e. All findings from the first round are addressed; I replied in each thread and resolved them, except the packaging one, which is still open.

Verification: cargo fmt -- --check is clean. cargo clippy --features bootloader_grub and cargo test --features mock still cannot run here (azure-iot-sdk-dev.pc is not in PKG_CONFIG_PATH), so the new watch_system_health tests are read, not run.

Five points below, the first two are the ones that matter.

[JoergZeidler]

Third round on b31c1bb, 9029c26, 4b6a329 and 2367e7a. All five findings from the last round are implemented, replied to in the threads and resolved, and the recipe question is answered.

Verification: cargo fmt -- --check is clean. cargo clippy --features bootloader_grub and cargo test --features mock still cannot run here (azure-iot-sdk-dev.pc is not in PKG_CONFIG_PATH), so the new code and tests are read, not compiled.

The first one follows from your own note about SubState and is the only one that can still roll back a good update.

[JoergZeidler]

Fourth round on f9ee5cc through fb6ec7c. All six findings from the last round are implemented, replied to in the threads and resolved.

Verification: cargo fmt -- --check is clean and bash -n on the health check script passes. cargo clippy --features bootloader_grub and cargo test --features mock still cannot run here (azure-iot-sdk-dev.pc is not in PKG_CONFIG_PATH), so the new proxy, ListedUnit and the string patterns are read, not compiled.

Two small points left, nothing that blocks.

One observation outside this PR: src/systemd/unit.rs:82 still filters on the typed ActiveState from list_units_by_patterns, so the same failure class lives on there. The pattern limits the reply to the wifi commissioning units, so the blast radius is much smaller, and it is not new here. Worth a follow up, not part of this change.

[JanZachmann]

The unit.rs observation from the last round is tracked in #209: active_units_by_patterns() still filters on the typed ActiveState, so the same failure class lives on there. Out of scope here, and the issue notes that the ListedUnit struct and the proxy from f9ee5cc can be reused, since ListUnitsByPatterns has the same return signature.

[JoergZeidler]

All findings from the four review rounds are addressed and the threads are resolved.

What I verified locally on 5e476fd: cargo fmt -- --check is clean, bash -n on the health check script passes, ListedUnit matches the ListUnits signature, and the SubState/ActiveState gaps that made the typed bindings brittle no longer reach the poll.

What I could not verify: cargo clippy --features bootloader_grub and cargo test --features mock do not run in my environment (azure-iot-sdk-dev.pc is not in PKG_CONFIG_PATH), so the code and the tests are read, not compiled. Please make sure both pass before merging.

One follow up outside this PR: src/systemd/unit.rs:82 still filters on the typed ActiveState from list_units_by_patterns, the same failure class this PR removed from the health poll, with a much smaller blast radius.