feat: return non-zero exit code on activation errors#378
Conversation
jfroche
force-pushed
the
fix/final-exit-code
branch
2 times, most recently
from
c17ab02 to
ea4f13e
Compare
jfroche
force-pushed
the
fix/final-exit-code
branch
from
ea4f13e to
d189d6f
Compare
jfroche
force-pushed
the
fix/final-exit-code
branch
2 times, most recently
from
f94f70e to
5f099d0
Compare
jfroche
force-pushed
the
fix/final-exit-code
branch
from
5f099d0 to
6be53d9
Compare
jfroche
force-pushed
the
fix/final-exit-code
branch
2 times, most recently
from
1595d9a to
c97f004
Compare
There was a problem hiding this comment.
Partial review of activate.rs and etc_files.rs. LGTM for these two files. I'll do a second pass to review the rest.
Overall, this area do not spark joy ><
That being said, the issue is not coming from this PR but from the unclear semantics we already have towards partial system activation.
A partial activation currently results in a broken system that will loudly fail on reboot. We probably can do better, at least for the etc files linking. But that requires a bit of design planning.
Another issue: we're overriding the state with a partial state, losing track of the files we have backup in the past and which files have been managed by system-manager at some point.
In any case, I don't think this PR is meant to fix all that. I guess it's a nice addition and overalll makes sense.
| log::error!("Error during activation: {e:?}"); | ||
| } | ||
|
|
||
| // Only run daemon reload, userborn, tmpfiles, and services when etc files |
There was a problem hiding this comment.
That being said, the machine is in a broken state and the services will likely break on reboot.
We should probably log a scary message here for now.
Long term, we probably want to make sure the files can be all copied before trying to write them on the disk to prevent this situation from happening.
Alternatively, we could backup the previous files during the activation and roll back everything if something goes wrong.
| log::error!("Error during activation: {e:?}"); | ||
| } | ||
|
|
||
| // Only register services when etc files were fully applied, preserving |
There was a problem hiding this comment.
Same. This will break upon reboot. Not sure what the right semantic would be.
We probably need to create an issue and seriously think about the overall activation success/failure semantics and how to mitigate a failure.
jfroche
force-pushed
the
fix/final-exit-code
branch
from
c97f004 to
2a812b2
Compare
Use lazy_errors to accumulate errors during activation and return a non-zero exit code if any errors were encountered, while still applying as many changes as possible.
jfroche
force-pushed
the
fix/final-exit-code
branch
from
2a812b2 to
366f081
Compare
2 participants
Use lazy_errors to accumulate errors during activation and return a non-zero exit code if any errors were encountered, while still applying as many changes as possible.
We also disable the nix module by default to avoid issues with existing nix configuration file. It is possible to enable it again by setting
nix.enabletotrueandit will replace the existing
/etc/nix/nix.conffile with the one generated by nix module.