many: support killing running snap apps #14160

ZeyadYasser · 2024-07-05T12:15:11Z

This is needed in preparation for the new snap remove --terminate flag.

This PR introduces a new /v1/app-control userd endpoint that currently only support killing snap apps. This endpoint will be invoked by snapd during removal of snaps when the --terminate flag is used.

Calling client.AppsKill would do the following for each user session:

Find running snap apps transient scope units (matching pattern snap.<snap-name>.*.scope)
Send SIGKILL signal to those running snap apps

Looking up snap's transient units and killing them is delegated to systemd by calling systemctl.

This PR is part 3.1 of implementing force removal of snaps according to SD174 - Force removal of snaps spec.

For context:

zyga

I had a quick scroll through the code and most of it looks good.

I have one conceptual question. What happens when after we kill all the apps, the user runs some more. In the grand scheme of things, it seems that some sort of inhibition is needed to be able to kill apps to force remove them and prevent further processes from spawning. I suspect it can be handled with the current removal logic but I wanted to double-check.

zyga · 2024-07-10T11:11:08Z

usersession/agent/rest_api.go

+
+func appKill(inst *client.AppInstruction, sysd systemd.Systemd) Response {
+	// TODO: Use inst.Reason as a hint to explain to users what is happening
+	if inst.Signal != 9 {


Nitpick, any reason not to use syscall.SIGKILL here?

Good point, updated to use syscall.Signal type, Thanks!

ZeyadYasser · 2024-07-10T11:26:05Z

it seems that some sort of inhibition is needed to be able to kill apps to force remove them and prevent further processes from spawning. I suspect it can be handled with the current removal logic but I wanted to double-check.

@zyga You are absolutely correct, there is a PR #14126 out for remove inhibition linked in the description. My bad for not making it more obvious.

pedronis

thank you

ZeyadYasser · 2024-07-12T08:07:35Z

usersession/client/client.go

+// for the user session rest api to control apps.
+type AppInstruction struct {
+	Action string   `json:"action"`
+	Snaps  []string `json:"snaps,omitempty"`


@pedronis should this stay per-snap or be more specific and be per-app? If we don't worry about running hooks I think we can scope it down to an app list.

We could go even more generic and allow passing a list of security tag patterns?

listing apps would be more consistent with ServiceInstruction, OTOH nothing stops us to add that granularity when it makes sense. For kill this seems the right granularity to me

This method will be used later to list all snap apps' transient units using the pattern "<snap-name>.*.scope". Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>

This helper will be used to find running snap apps that need to be force killed. Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>

@zyga

The new /v1/app-control endpoint currently only supports the "kill" action. This action finds and kills all running apps for the requested snaps. This endpoint will be invoked by snapd during removal of snaps when the --terminate flag is used. Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com> snap: move kill reason to snap package Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com> usersession: use syscall.Signal type for AppInstruction.Signal Thanks @zyga Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>

This is an alternative implmentation to the userd approach in canonical#14160. The userd approach will not work in the absense of user session daemon (i.e. ubuntu-core) or if a snap is started as root. The alternative algorithm kills the running apps in snapd directly in their cgroups. Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>