Skip to content

Commit

Permalink
sandbox/cgroup: support killing running snap apps
Browse files Browse the repository at this point in the history
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.

- many: address review comments (thanks @zyga)
  - add contexts support to FreezeSnapProcesses
  - add comment on possible race when killing processes in a v2 cgroup
  - for cgroups V2, only thaw on error
  - for cgroups V1, add comment explaining why we always need to thaw
  - update KillSnapProcesses to only send SIGKILL
  - replace deprecated os.IsNotExist with errors.Is
- trim whitespace when reading freezer.state

Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
  • Loading branch information
ZeyadYasser authored and soumyaDghosh committed Dec 6, 2024
1 parent 1d23706 commit 3ccdd9c
Show file tree
Hide file tree
Showing 10 changed files with 546 additions and 90 deletions.
4 changes: 3 additions & 1 deletion cmd/snap-update-ns/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package main

import (
"context"
"fmt"

"github.com/snapcore/snapd/cmd/snaplock"
Expand Down Expand Up @@ -79,7 +80,8 @@ func (upCtx *CommonProfileUpdateContext) Lock() (func(), error) {
// introduce a symlink that would cause us to mount something other
// than what we expected).
logger.Debugf("freezing processes of snap %q", instanceName)
if err := cgroup.FreezeSnapProcesses(instanceName); err != nil {
// TODO: Ideally we should use signal.NotifyContext
if err := cgroup.FreezeSnapProcesses(context.TODO(), instanceName); err != nil {
// If we cannot freeze the processes we should drop the lock.
lock.Close()
return nil, err
Expand Down
3 changes: 2 additions & 1 deletion cmd/snap-update-ns/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package main_test

import (
"bytes"
"context"
"os"
"path/filepath"

Expand Down Expand Up @@ -55,7 +56,7 @@ func (s *commonSuite) TestInstanceName(c *C) {
func (s *commonSuite) TestLock(c *C) {
// Mock away real freezer code, allowing test code to return an error when freezing.
var freezingError error
restore := cgroup.MockFreezing(func(string) error { return freezingError }, func(string) error { return nil })
restore := cgroup.MockFreezing(func(context.Context, string) error { return freezingError }, func(string) error { return nil })
defer restore()
// Mock system directories, we use the lock directory.
dirs.SetRootDir(s.dir)
Expand Down
3 changes: 2 additions & 1 deletion cmd/snap-update-ns/system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package main_test

import (
"bytes"
"context"
"os"
"path/filepath"

Expand All @@ -46,7 +47,7 @@ func (s *systemSuite) TestLockCgroup(c *C) {

var frozen []string
var thawed []string
happyFreeze := func(snapName string) error {
happyFreeze := func(ctx context.Context, snapName string) error {
frozen = append(frozen, snapName)
return nil
}
Expand Down
1 change: 1 addition & 0 deletions overlord/snapstate/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,7 @@ func (f *fakeSnappyBackend) StartServices(svcs []*snap.AppInfo, disabledSvcs *wr
}

func (f *fakeSnappyBackend) StopServices(svcs []*snap.AppInfo, reason snap.ServiceStopReason, meter progress.Meter, tm timings.Measurer) error {
meter.Notify("stop-services")
f.appendOp(&fakeOp{
op: fmt.Sprintf("stop-snap-services:%s", reason),
path: svcSnapMountDir(svcs),
Expand Down
8 changes: 6 additions & 2 deletions sandbox/cgroup/cgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,13 @@ func init() {
func pickVersionSpecificImpl() {
switch probeVersion {
case V1:
pickFreezerV1Impl()
FreezeSnapProcesses = freezeSnapProcessesImplV1
ThawSnapProcesses = thawSnapProcessesImplV1
KillSnapProcesses = killSnapProcessesImplV1
case V2:
pickFreezerV2Impl()
FreezeSnapProcesses = freezeSnapProcessesImplV2
ThawSnapProcesses = thawSnapProcessesImplV2
KillSnapProcesses = killSnapProcessesImplV2
}
}

Expand Down
29 changes: 29 additions & 0 deletions sandbox/cgroup/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package cgroup

import (
"context"
"syscall"
"time"

"github.com/godbus/dbus"
Expand Down Expand Up @@ -135,3 +136,31 @@ func (iw *inotifyWatcher) MonitorDelete(folders []string, name string, channel c
}

var NewInotifyWatcher = newInotifyWatcher

func MockFreezeSnapProcessesImplV1(fn func(ctx context.Context, snapName string) error) (restore func()) {
return testutil.Mock(&freezeSnapProcessesImplV1, fn)
}

func MockThawSnapProcessesImplV1(fn func(snapName string) error) (restore func()) {
return testutil.Mock(&thawSnapProcessesImplV1, fn)
}

func MockSyscallKill(fn func(pid int, sig syscall.Signal) error) (restore func()) {
return testutil.Mock(&syscallKill, fn)
}

func MockFreezeOneV2(fn func(ctx context.Context, dir string) error) (restore func()) {
return testutil.Mock(&freezeOneV2, fn)
}

func MockThawOneV2(fn func(dir string) error) (restore func()) {
return testutil.Mock(&thawOneV2, fn)
}

func MockFreezePulseDelay(t time.Duration) (restore func()) {
return testutil.Mock(&freezePulseDelay, t)
}

func MockOsReadFile(fn func(name string) ([]byte, error)) (restore func()) {
return testutil.Mock(&osReadFile, fn)
}
Loading

0 comments on commit 3ccdd9c

Please sign in to comment.