Simplified greenboot health-check #100
Conversation
|
You have successfully added a new devskim configuration |
|
I getting: The grub variable boot_success is not set. |
say-paul
force-pushed
the
greenboot-rs-continued
branch
from
025530b
to
eacb3d5
Compare
say-paul
force-pushed
the
greenboot-rs-continued
branch
3 times, most recently
from
5dba90c
to
665c669
Compare
say-paul
force-pushed
the
greenboot-rs-continued
branch
4 times, most recently
from
c836f58
to
b4b06c6
Compare
its working now |
|
Would it be possible to break up the one monolithic commit into multiple commits? It would ease the PR review process and make backing out changes easier. |
Signed-off-by: Sayan Paul <[email protected]>
Accepetd params: healtch-check , rollback Signed-off-by: Sayan Paul <[email protected]>
Signed-off-by: Sayan Paul <[email protected]>
greenboot-healtcheck: healthcheck which runs on every boot. greenboot-rollback: rollback(if healtcheck fails) which is tagged with systemd-update-done.service that runs, only after a system is updated. Signed-off-by: Sayan Paul <[email protected]>
Makefile params: build,install,check,rpm spec - bump from v0.15 series which is bash version of greenboot to v0.16.0 Signed-off-by: Sayan Paul <[email protected]>
CI runs formatting spellcheck and unit tests.
packit build rpm for fedora and centos
Signed-off-by: Sayan Paul <[email protected]>
Signed-off-by: Sayan Paul <[email protected]>
devcontainer has all the necessary libaries installed to run test and build greenboot. Signedff-by: Sayan Paul <[email protected]>
say-paul
force-pushed
the
greenboot-rs-continued
branch
from
40eee34
to
3a09a4b
Compare
I have modified this too. |
| "/etc/motd.d/boot-status", | ||
| format!("Greenboot {state}.").as_bytes(), | ||
| )?; | ||
| Ok(()) |
There was a problem hiding this comment.
if the last statement already returns a Result, then there is no need for the Ok below, just map the error to the type that we are returning
Signed-off-by: Sayan Paul <[email protected]>
saving 2 Bytes of ram!! Signed-off-by: Sayan Paul <[email protected]>
say-paul
dismissed
nullr0ute’s stale review
moving the log related comments to issue #149
| bail!("countdown ended, check greenboot-rollback status") | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
can't you compress it to:
if !force && if let Some(t) = get_boot_counter()? && t <= 0 {
I don't like 3 nested if(s) - so, up to others for style too...
There was a problem hiding this comment.
not exactly whats suggested..
but able to reduce the nested loop in : ee8bd08
| } | ||
| } | ||
| bail!("Rollback not initiated"); | ||
| } |
There was a problem hiding this comment.
this can be better rewritten as:
pub fn handle_rollback() -> Result<()> {
let boot_counter = get_boot_counter()?;
if boot_counter <= 0 {
log::info!("Greenboot will now attempt to rollback");
Command::new("rpm-ostree").arg("rollback").status()?;
return Ok(());
}
bail!("Rollback not initiated");
}
| set_grub_var("boot_success", 0)?; | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
pub fn set_boot_status(success: bool) -> Result<()> {
if success {
set_grub_var("boot_success", 1)?;
unset_boot_counter()?;
return Ok(());
}
set_grub_var("boot_success", 0)?
}
src/main.rs
Outdated
| #[clap(subcommand)] | ||
| command: Commands, | ||
| } | ||
| #[derive(Debug, Deserialize)] | ||
| ///config params for greenboot |
There was a problem hiding this comment.
this bothers me personally 😛 consistency with spaces after /// - the code is a mix afaics
Signed-off-by: Sayan Paul <[email protected]>
This will implement the the existing greenboot health check functionality:
Below are the functionality that will be implemented as part of this PR: