snap restart starts disabled services

This is documented like that:

By default, all services for a specified snap will be restarted:

$ sudo snap restart lxd
Restarted.

it says all, no conditionals. I understand it might be undesired. OTOH what should snap stop <snap> do, stop everything or ignore the disabled services that were set to manually running? snap start <snap>?

(DWIM behavior can be confusing in its own ways.)

This needs a bit more thinking.

I think there are the following cases with my opinion on what should happen for every situation:

1. svc is disabled, and running

`snap start SNAP` doesn't matter
`snap restart SNAP` should not stop the service and not start the service
`snap stop SNAP` should stop the service

2. svc is disabled, and not running

`snap start SNAP` should not start the service
`snap restart SNAP` should not stop the service and not start the service
`snap stop SNAP` doesn't matter

3. svc is enabled, and running

`snap start SNAP` doesn't matter
`snap restart SNAP` should stop the service and then start the service
`snap stop SNAP` should stop the service

4. svc is enabled, and not running
`snap START SNAP` should start the service
`snap restart SNAP` should stop the service and then start the service
`snap stop SNAP` doesn't matter

My reasoning for why `snap stop` (and also `snap start`) should behave differently from `snap restart` is that I view `snap restart` as the same as a system reboot, i.e. if I were to reboot my system instead of running `snap restart` then my disabled services don't magically start running.

I think the state of "enabling"/"disabling" a service affects whether it is automatically started/stopped (i.e. not running `snap start SNAP.svc` or `snap stop SNAP.svc`)

We discussed this further also with the devs that did the original design/implementation of this.

We think a reasonable set of changes to the current behavior (we would need to explore whether anybody is seriously relying on the corner aspects of it before changing though) would be:

* snap stop SNAP should stop all the running services of the snap (this seems to match your ideas as well)

* snap restart SNAP would restart all the running services of the snap (even if disabled), that's different than your thoughts, so I would like to understand what you think about this variant? or whether there's deeper use case from your original thought?

this is a bigger change from current behavior because it means a failed service would not be restarted by this, one would need to use the specific service variant for that

* snap start SNAP would start all the enabled services of the snap, this seems to match your expectations as well

What you described seems okay to us, as really the thing we want to not ever happen is to have a service manually disabled by a user start up again "inadvertently". The use case that's most important to us is:

* user disables a service (say SVC1), (and it's not running currently)
* user modifies some wide ranging config files, etc.
* user issues `snap restart` to restart all the services in the snap
* SVC1 does not start running

which under your proposal would happen.

I was under the thinking that snapd should never automatically start a "disabled" service (either running or not running), but I think I can be more strict in saying that snapd should never automatically start a "disabled and not running" service, as that's the use case that affects us the most negatively.

I posted the proposed behavior change also to the forum:

https://forum.snapcraft.io/t/command-line-interface-to-manipulate-services/262/47

I guess one case that hadn't been made clear in comment #3 but I can see from the forum post is that with the new behavior, an "enabled and not running" service would not get started with `snap restart SNAP`. I think that's wrong, because for example what about oneshot daemons and failed services? I think that both of those should still be started when issuing `snap restart`.

Again, though the critical thing for us is not starting "disabled and not running" services, so if there's a good reason to not also start "enabled and not running" services with `snap restart` it's not too bad for us.

To be clear these proposed changes are about when just the SNAP is specified and so implicitly all its services potentially, not the behavior when services are explicitly listed.

Fixed with https://github.com/snapcore/snapd/pull/10266

I just tested this with the edgexfoundry snap and it looks like the behavior with respect to "enabled/inactive" services is what was originally proposed in the forum post (i.e. these services are not started), not was was suggested by Ian in his response to the post, and in comment #6.

In the case of the edgexfoundry snap, this results in the services being in a non-functioning state, as the -setup services (which are all "enabled/inactive", of type "oneshot") are not started.

I verified this by grabbing all the systemd "Starting/Stopping Service..." messages in the journal after the restart was issued.

Also the 'snap tasks' output erroneously still lists all of the services as being restarted:

$ snap tasks 272
Status Spawn Ready Summary
Done today at 12:57 EDT today at 12:57 EDT Run service command "restart" for services ["app-service-configurable" "consul" "core-command" "core-data" "core-metadata" "device-virtual" "kong-daemon" "kuiper" "postgres" "redis" "security-bootstrap-redis" "security-proxy-setup" "security-secrets-setup" "security-secretstore-setup" "support-notifications" "support-scheduler" "sys-mgmt-agent" "vault"] of snap "edgexfoundry"

Note, this behavior also doesn't match the behavior of a reboot...

Tested with edgexfoundry snap (amd64/3066) and snapd 2.52 (13270) on Ubuntu 20.04.

You are right, Tony, we implemented the proposal from the forum -- AFAIK the only feedback we received was from other team members, so this is probably the reason why the decision on which solution to follow was taken internally. There is always room for changes, but we need to be careful on how such changes might impact snaps relying on the current behaviour.

The output from "snap tasks" is indeed misleading -- unless we change the actual behaviour, I'd suggest the output to be changed to something like

    Run service command "restart" on currently running services of snap "edgexfoundry"

Ideally we would also print the actual service names, but if that is too cumbersome we could go for this as an immediate solution (I've yet to investigate how hard that is).

But regarding your concrete problem: would it be possible for you to run the "snap restart" command on the individual service(s)? Because, if a service name is given (vs a snap name), then "snap restart" should restart the service regardless of its current status.