servicer-labs / servicer

A CLI to simplify service management on systemd
https://servicer.dev
MIT License
427 stars 9 forks source link

fest: reload service #5

Closed flexwie closed 1 year ago

flexwie commented 1 year ago

adds command to reload a service configuration

flexwie commented 1 year ago

@secretshardul please let me know if this is missing something to be merged :)

secretshardul commented 1 year ago

Looks like reload works only when active_state is failed.

hp@hp:~/Documents/servicer-labs/servicer$ sudo ./target/debug/servicer reload hello-world
thread 'main' panicked at 'Failed to reload service hello-world.ser.service: MethodError(OwnedErrorName(ErrorName(Str(Owned("org.freedesktop.systemd1.JobTypeNotApplicable")))), Some("Job type reload is not applicable for unit hello-world.ser.service."), Msg { type: Error, sender: UniqueName(Str(Borrowed(":1.9"))), reply-serial: 6, body: Signature("s") })', src/handlers/handle_reload_service.rs:51:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

From the docs-

ReloadUnit(), RestartUnit(), TryRestartUnit(), ReloadOrRestartUnit(), or ReloadOrTryRestartUnit() may be used to restart and/or reload a unit. These methods take similar arguments as StartUnit(). Reloading is done only if the unit is already running and fails otherwise.

I'm updating the code to ensure we don't call dbus unless the active_state is failed.

    if active_state == "failed" {
        reload_service(&manager_proxy, &full_service_name).await;
        println!("service reloaded: {name}");

    } else {
        eprintln!("No-op. Service state of {full_service_name} is {active_state}");
    };

Question, do you have a specific need for reload? It's not a good idea to port the entire systemd API. As pointed above reload / restart comes in 5 flavours.

flexwie commented 1 year ago

I'm assuming your test is failing because the hello-world example has no ExecReload target. Normally it shouldn't require a failed state to reload the configuration of a service.

I use reload a lot in web server configurations like caddy or nginx.

Sorry for not checking in with you beforehand, I added this for my use cases and thought someone else might find it useful as well! I agree that not the whole API should be blindly ported. Maybe we can settle on TryReloadOrRestart as a sensible default? That way units that support it get reloaded correctly and others simply restart to grab a new configuration.