fix(service): idle awaiting config instead of exit-restart loop when unconfigured - #10
Merged
Merged
Conversation
…unconfigured A missing/unparseable/invalid magpie.json, or one with platforms: [], made main() exit — bamboo's ServiceManager (max_attempts: 0) then respawned the process every ~1s forever. A freshly installed, not-yet-configured plugin is a normal state, not a crash. await_valid_config() now polls the config path every 30s (sleeping between checks, so it never busy-spins) until it parses, validates, and has at least one platform, logging once per idle-state change plus a low-rate 5-minute reminder. It races against shutdown_signal() so SIGTERM/Ctrl-C still exits immediately while idling. --check keeps the old fail-fast behavior since it's an interactive diagnostic, not the supervised loop. Closes #4.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the 1s restart hot-loop reported in #4 when magpie runs as a bamboo-plugin
servicesartifact:plugin_service_config/magpie/config.json): magpie used to log an ERROR and exitFAILURE. bamboo's ServiceManager restart policy (max_attempts: 0= unlimited) then respawned it every ~1s forever.platforms: [](config present, nothing enabled): magpie used to warn and exitSUCCESS— but the restart policy restarts clean exits too, producing the same 1s loop.FAILUREand crash-looped.All three are now treated as a normal "not yet configured" state: magpie stays alive and polls the config path every 30s until it parses, validates, and has at least one platform configured, then proceeds with normal startup.
Change summary
await_valid_config(path, poll_interval)— the idle loop. Each iteration does one synchronous check (classify_config) thentokio::time::sleep(poll_interval); it never busy-spins. This also preserves the old "accidental hot-reload" the issue calls out: editing the config file while idle is picked up on the next 30s poll, the same way an operator's edit used to land on the restart loop's next spawn.classify_config(path)— pure sync classifier (Ready(MagpieConfig)vsIdle(IdleReason)) so the missing-file / parse-error / validation-error / empty-platforms cases are unit-testable without going through the async poll loop.main()racesawait_valid_configagainst the existingshutdown_signal()(tokio::select!), so SIGTERM/Ctrl-C still interrupts a sleeping poll and exits immediately — graceful shutdown is unaffected while idling.--check(the interactiveGET /api/v1/execute/defaultssmoke test) intentionally keeps the old fail-fast behavior: it's an operator-invoked diagnostic, not the supervised service loop, so it should error immediately on a bad config rather than idle.tokio'stest-utilfeature as a dev-dependency fortokio::time::{pause, advance}in the new tests.Diff is scoped to
src/main.rs(startup/config handling) +Cargo.toml(test-only dependency) — no changes tobridge.rsor the resync/ask-park code (kept separate from the concurrent #9 work in that area).Test plan
cargo fmt --checkcargo clippy --all-targets— clean, no warningscargo build --testscargo test— 172 lib tests + 12 main.rs tests (incl. 6 new:classify_config_*missing/parse/validation/empty-platforms/ready,await_valid_config_does_not_busy_spin_and_picks_up_a_later_config,await_valid_config_resolves_immediately_when_already_valid)platforms: []→ same idle behavior, SIGTERM exits cleanlycargo tree -i aws-lc-sys— no match (native-tls pin unaffected by the new dev-dependency)Closes #4.
https://claude.ai/code/session_014iw5PBsSzDFHus1GfkAK4y