Dstack-TEE / Dstack-TEE/dstack
certbot: `renew --once` exits 0 when the renewed hook fails
- Dominant language
- Rust
- Stars
- 544
- Forks
- 96
- Avg merge
- 17h 57m
- Merged PRs (30d)
- 117
Description
Follow-up to #924.
`certbot renew --once` is the cron/one-shot entry point. #924 made it invoke the `renewed_hook` (correct — a one-shot renewal that skips the reload hook leaves the serving process on the old cert), but hook failures are only logged:
```rust
// dstack/certbot/src/bot.rs
match std::process::Command::new("/bin/sh").arg("-c").arg(hook).status() {
Ok(status) if status.success() => {}
Ok(status) => error!("renewed hook failed with status: {status}"),
Err(error) => error!("failed to run renewed hook: {error:?}"),
}
Ok(true)
```
So `renew_and_run_hook` returns `Ok(true)` regardless, and `certbot renew --once` exits 0 even when the hook never ran or exited non-zero. A cron job or systemd `OneShot` unit wrapping this sees success while the certificate on disk is new and the serving process is still holding the old one — exactly the failure that is supposed to be visible.
Swallowing the error is right for the daemon (the next interval retries), wrong for `--once` (there is no next interval).
## Proposal
Split the two semantics. Options, roughly in order of preference:
1. Have `renew_and_run_hook` return the hook outcome (e.g. `Result` carrying `hook_failed`) and let the `--once` path in `cli/src/main.rs` turn a hook failure into a non-zero exit, while `run()` keeps logging and continuing.
2. Add a `fail_on_hook_error: bool` parameter, set from the `once` flag.
Either way the daemon loop must keep its current behaviour: a failing hook should not abort the loop.
Contributor guide
Research direction
Start in dstack/certbot/src/bot.rs, tracing how renew_and_run_hook reports renewed-hook failures, then follow the --once path in cli/src/main.rs. Preserve the daemon loop's current logging-and-continuing behavior, but make --once return a non-zero exit when the hook does not run or exits unsuccessfully. Verify both semantics with the relevant certbot command paths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 64/100