diff --git a/app/app.go b/app/app.go index 4e127a70..663e700e 100644 --- a/app/app.go +++ b/app/app.go @@ -182,8 +182,8 @@ func (c colimaApp) Stop(force bool) error { // the order for stop is: // container stop -> vm stop - // stop container runtimes if not a forceful shutdown - if c.guest.Running(ctx) && !force { + // stop container runtimes + if c.guest.Running(ctx) { containers, err := c.currentContainerEnvironments(ctx) if err != nil { log.Warnln(fmt.Errorf("error retrieving runtimes: %w", err)) @@ -196,7 +196,7 @@ func (c colimaApp) Stop(force bool) error { log := log.WithField("context", cont.Name()) log.Println("stopping ...") - if err := cont.Stop(ctx); err != nil { + if err := cont.Stop(ctx, force); err != nil { // failure to stop a container runtime is not fatal // it is only meant for graceful shutdown. // the VM will shut down anyways. diff --git a/cmd/kubernetes.go b/cmd/kubernetes.go index b4aabc8a..f40386cd 100644 --- a/cmd/kubernetes.go +++ b/cmd/kubernetes.go @@ -68,7 +68,7 @@ var kubernetesStopCmd = &cobra.Command{ return fmt.Errorf("%s is not enabled", kubernetes.Name) } - return k.Stop(ctx) + return k.Stop(ctx, false) }, } diff --git a/environment/container.go b/environment/container.go index c9f337e3..61bf0575 100644 --- a/environment/container.go +++ b/environment/container.go @@ -19,7 +19,8 @@ type Container interface { // Start starts the container runtime. Start(ctx context.Context) error // Stop stops the container runtime. - Stop(ctx context.Context) error + // If force is true, the runtime is killed immediately without graceful shutdown. + Stop(ctx context.Context, force bool) error // Teardown tears down/uninstall the container runtime. Teardown(ctx context.Context) error // Update the container runtime. diff --git a/environment/container/containerd/containerd.go b/environment/container/containerd/containerd.go index f38b572e..77460859 100644 --- a/environment/container/containerd/containerd.go +++ b/environment/container/containerd/containerd.go @@ -10,6 +10,7 @@ import ( "github.com/abiosoft/colima/cli" "github.com/abiosoft/colima/config" "github.com/abiosoft/colima/environment" + "github.com/abiosoft/colima/environment/guest/systemctl" ) // Name is container runtime name @@ -45,6 +46,7 @@ func newRuntime(host environment.HostActions, guest environment.GuestActions) en return &containerdRuntime{ host: host, guest: guest, + systemctl: systemctl.New(guest), CommandChain: cli.New(Name), } } @@ -56,8 +58,9 @@ func init() { var _ environment.Container = (*containerdRuntime)(nil) type containerdRuntime struct { - host environment.HostActions - guest environment.GuestActions + host environment.HostActions + guest environment.GuestActions + systemctl systemctl.Systemctl cli.CommandChain } @@ -85,7 +88,7 @@ func (c containerdRuntime) Start(ctx context.Context) error { a := c.Init(ctx) a.Add(func() error { - return c.guest.Run("sudo", "service", "containerd", "restart") + return c.systemctl.Restart("containerd.service") }) // service startup takes few seconds, retry at most 10 times before giving up. @@ -94,20 +97,20 @@ func (c containerdRuntime) Start(ctx context.Context) error { }) a.Add(func() error { - return c.guest.Run("sudo", "service", "buildkit", "start") + return c.systemctl.Start("buildkit.service") }) return a.Exec() } func (c containerdRuntime) Running(ctx context.Context) bool { - return c.guest.RunQuiet("service", "containerd", "status") == nil + return c.systemctl.Active("containerd.service") } -func (c containerdRuntime) Stop(ctx context.Context) error { +func (c containerdRuntime) Stop(ctx context.Context, force bool) error { a := c.Init(ctx) a.Add(func() error { - return c.guest.Run("sudo", "service", "containerd", "stop") + return c.systemctl.Stop("containerd.service", force) }) return a.Exec() } diff --git a/environment/container/docker/containerd.go b/environment/container/docker/containerd.go index fe0a33d2..7b89a799 100644 --- a/environment/container/docker/containerd.go +++ b/environment/container/docker/containerd.go @@ -37,7 +37,7 @@ func (d dockerRuntime) provisionContainerd(ctx context.Context) error { a.Add(func() error { // restart containerd service - return d.guest.Run("sudo", "service", "containerd", "restart") + return d.systemctl.Restart("containerd.service") }) return a.Exec() diff --git a/environment/container/docker/daemon.go b/environment/container/docker/daemon.go index 4c02c0ef..0b65b05b 100644 --- a/environment/container/docker/daemon.go +++ b/environment/container/docker/daemon.go @@ -119,10 +119,10 @@ func (d dockerRuntime) addHostGateway(conf map[string]any) error { } func (d dockerRuntime) reloadAndRestartSystemdService() error { - if err := d.guest.Run("sudo", "systemctl", "daemon-reload"); err != nil { + if err := d.systemctl.DaemonReload(); err != nil { return fmt.Errorf("error reloading systemd daemon: %w", err) } - if err := d.guest.Run("sudo", "systemctl", "restart", "docker"); err != nil { + if err := d.systemctl.Restart("docker.service"); err != nil { return fmt.Errorf("error restarting docker: %w", err) } return nil diff --git a/environment/container/docker/docker.go b/environment/container/docker/docker.go index 9434842f..edd168ee 100644 --- a/environment/container/docker/docker.go +++ b/environment/container/docker/docker.go @@ -9,6 +9,7 @@ import ( "github.com/abiosoft/colima/cli" "github.com/abiosoft/colima/config" "github.com/abiosoft/colima/environment" + "github.com/abiosoft/colima/environment/guest/systemctl" "github.com/abiosoft/colima/util" "github.com/abiosoft/colima/util/debutil" ) @@ -23,8 +24,9 @@ func init() { } type dockerRuntime struct { - host environment.HostActions - guest environment.GuestActions + host environment.HostActions + guest environment.GuestActions + systemctl systemctl.Systemctl cli.CommandChain } @@ -33,6 +35,7 @@ func newRuntime(host environment.HostActions, guest environment.GuestActions) en return &dockerRuntime{ host: host, guest: guest, + systemctl: systemctl.New(guest), CommandChain: cli.New(Name), } } @@ -80,7 +83,7 @@ func (d dockerRuntime) Start(ctx context.Context) error { a := d.Init(ctx) a.Retry("", time.Second, 60, func(int) error { - return d.guest.RunQuiet("sudo", "systemctl", "start", "docker.service") + return d.systemctl.Start("docker.service") }) // service startup takes few seconds, retry for a minute before giving up. @@ -102,17 +105,17 @@ func (d dockerRuntime) Start(ctx context.Context) error { } func (d dockerRuntime) Running(ctx context.Context) bool { - return d.guest.RunQuiet("service", "docker", "status") == nil + return d.systemctl.Active("docker.service") } -func (d dockerRuntime) Stop(ctx context.Context) error { +func (d dockerRuntime) Stop(ctx context.Context, force bool) error { a := d.Init(ctx) a.Add(func() error { if !d.Running(ctx) { return nil } - return d.guest.Run("sudo", "systemctl", "stop", "docker.service") + return d.systemctl.Stop("docker.service", force) }) // clear docker context settings diff --git a/environment/container/incus/incus.go b/environment/container/incus/incus.go index c085464d..0edaa1f0 100644 --- a/environment/container/incus/incus.go +++ b/environment/container/incus/incus.go @@ -12,6 +12,7 @@ import ( "github.com/abiosoft/colima/cli" "github.com/abiosoft/colima/config" "github.com/abiosoft/colima/environment" + "github.com/abiosoft/colima/environment/guest/systemctl" "github.com/abiosoft/colima/environment/vm/lima/limautil" "github.com/abiosoft/colima/util" "github.com/abiosoft/colima/util/debutil" @@ -23,6 +24,7 @@ func newRuntime(host environment.HostActions, guest environment.GuestActions) en return &incusRuntime{ host: host, guest: guest, + systemctl: systemctl.New(guest), CommandChain: cli.New(Name), } } @@ -51,8 +53,9 @@ func init() { var _ environment.Container = (*incusRuntime)(nil) type incusRuntime struct { - host environment.HostActions - guest environment.GuestActions + host environment.HostActions + guest environment.GuestActions + systemctl systemctl.Systemctl cli.CommandChain } @@ -134,7 +137,7 @@ func (c *incusRuntime) Provision(ctx context.Context) error { // Running implements environment.Container. func (c *incusRuntime) Running(ctx context.Context) bool { - return c.guest.RunQuiet("service", "incus", "status") == nil + return c.systemctl.Active("incus.service") } // Start implements environment.Container. @@ -148,13 +151,13 @@ func (c *incusRuntime) Start(ctx context.Context) error { if c.poolImported() { a.Add(func() error { - return c.guest.RunQuiet("sudo", "systemctl", "start", "incus.service") + return c.systemctl.Start("incus.service") }) } else { // pool not yet imported // restart incus to import pool a.Add(func() error { - return c.guest.RunQuiet("sudo", "systemctl", "restart", "incus.service") + return c.systemctl.Restart("incus.service") }) } @@ -201,7 +204,7 @@ func (c *incusRuntime) Start(ctx context.Context) error { } // Stop implements environment.Container. -func (c *incusRuntime) Stop(ctx context.Context) error { +func (c *incusRuntime) Stop(ctx context.Context, force bool) error { a := c.Init(ctx) a.Add(func() error { @@ -210,7 +213,7 @@ func (c *incusRuntime) Stop(ctx context.Context) error { }) a.Add(func() error { - return c.guest.RunQuiet("sudo", "incus", "admin", "shutdown") + return c.systemctl.Stop("incus.service", force) }) a.Add(c.unsetRemote) diff --git a/environment/container/kubernetes/kubernetes.go b/environment/container/kubernetes/kubernetes.go index 9879c978..3e70df3d 100644 --- a/environment/container/kubernetes/kubernetes.go +++ b/environment/container/kubernetes/kubernetes.go @@ -12,6 +12,7 @@ import ( "github.com/abiosoft/colima/environment" "github.com/abiosoft/colima/environment/container/containerd" "github.com/abiosoft/colima/environment/container/docker" + "github.com/abiosoft/colima/environment/guest/systemctl" ) // Name is container runtime name @@ -27,6 +28,7 @@ func newRuntime(host environment.HostActions, guest environment.GuestActions) en return &kubernetesRuntime{ host: host, guest: guest, + systemctl: systemctl.New(guest), CommandChain: cli.New(Name), } } @@ -38,8 +40,9 @@ func init() { var _ environment.Container = (*kubernetesRuntime)(nil) type kubernetesRuntime struct { - host environment.HostActions - guest environment.GuestActions + host environment.HostActions + guest environment.GuestActions + systemctl systemctl.Systemctl cli.CommandChain } @@ -152,7 +155,7 @@ func (c kubernetesRuntime) Start(ctx context.Context) error { } a.Add(func() error { - return c.guest.Run("sudo", "service", "k3s", "start") + return c.systemctl.Start("k3s.service") }) a.Retry("", time.Second*2, 10, func(int) error { return c.guest.RunQuiet("kubectl", "cluster-info") @@ -165,7 +168,7 @@ func (c kubernetesRuntime) Start(ctx context.Context) error { return c.provisionKubeconfig(ctx) } -func (c kubernetesRuntime) Stop(ctx context.Context) error { +func (c kubernetesRuntime) Stop(ctx context.Context, force bool) error { a := c.Init(ctx) a.Add(func() error { return c.guest.Run("k3s-killall.sh") @@ -173,7 +176,9 @@ func (c kubernetesRuntime) Stop(ctx context.Context) error { // k3s is buggy with external containerd for now // cleanup is manual - a.Add(c.stopAllContainers) + if !force { + a.Add(c.stopAllContainers) + } return a.Exec() } diff --git a/environment/guest/systemctl/systemctl.go b/environment/guest/systemctl/systemctl.go new file mode 100644 index 00000000..eeb25c01 --- /dev/null +++ b/environment/guest/systemctl/systemctl.go @@ -0,0 +1,52 @@ +package systemctl + +import "github.com/abiosoft/colima/environment" + +// Runner is the subset of environment.GuestActions that Systemctl requires. +// Using a narrow interface makes Systemctl easier to test and more loosely coupled. +type Runner interface { + Run(args ...string) error + RunQuiet(args ...string) error +} + +// compile-time check: environment.GuestActions satisfies runner. +var _ Runner = (environment.GuestActions)(nil) + +// Systemctl provides a typed wrapper for running systemctl commands in the guest VM. +type Systemctl struct { + runner Runner +} + +// New creates a new Systemctl instance backed by the given guest. +func New(guest Runner) Systemctl { + return Systemctl{runner: guest} +} + +// Start starts a systemd service. +func (s Systemctl) Start(service string) error { + return s.runner.Run("sudo", "systemctl", "start", service) +} + +// Restart restarts a systemd service. +func (s Systemctl) Restart(service string) error { + return s.runner.Run("sudo", "systemctl", "restart", service) +} + +// Stop stops a systemd service. If force is true, it is killed immediately without graceful shutdown. +func (s Systemctl) Stop(service string, force bool) error { + verb := "stop" + if force { + verb = "kill" + } + return s.runner.Run("sudo", "systemctl", verb, service) +} + +// Active returns whether a systemd service is currently active. +func (s Systemctl) Active(service string) bool { + return s.runner.RunQuiet("systemctl", "is-active", service) == nil +} + +// DaemonReload reloads the systemd manager configuration. +func (s Systemctl) DaemonReload() error { + return s.runner.Run("sudo", "systemctl", "daemon-reload") +} diff --git a/environment/guest/systemctl/systemctl_test.go b/environment/guest/systemctl/systemctl_test.go new file mode 100644 index 00000000..0465422f --- /dev/null +++ b/environment/guest/systemctl/systemctl_test.go @@ -0,0 +1,114 @@ +package systemctl + +import ( + "os" + "testing" +) + +// mockGuest records args passed to Run/RunQuiet and controls whether they succeed. +type mockGuest struct { + lastArgs []string + err error +} + +func (m *mockGuest) Run(args ...string) error { m.lastArgs = args; return m.err } +func (m *mockGuest) RunQuiet(args ...string) error { m.lastArgs = args; return m.err } + +func TestStart(t *testing.T) { + g := &mockGuest{} + s := New(g) + + if err := s.Start("docker.service"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + assertArgs(t, g.lastArgs, []string{"sudo", "systemctl", "start", "docker.service"}) +} + +func TestRestart(t *testing.T) { + g := &mockGuest{} + s := New(g) + + if err := s.Restart("containerd.service"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + assertArgs(t, g.lastArgs, []string{"sudo", "systemctl", "restart", "containerd.service"}) +} + +func TestStop(t *testing.T) { + tests := []struct { + name string + force bool + wantVerb string + }{ + {name: "graceful", force: false, wantVerb: "stop"}, + {name: "force", force: true, wantVerb: "kill"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := &mockGuest{} + s := New(g) + + if err := s.Stop("docker.service", tt.force); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + assertArgs(t, g.lastArgs, []string{"sudo", "systemctl", tt.wantVerb, "docker.service"}) + }) + } +} + +func TestActive(t *testing.T) { + tests := []struct { + name string + guestOK bool + want bool + }{ + {name: "active", guestOK: true, want: true}, + {name: "inactive", guestOK: false, want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := &mockGuest{} + if !tt.guestOK { + g.err = os.ErrProcessDone + } + s := New(g) + + got := s.Active("docker.service") + if got != tt.want { + t.Errorf("Active() = %v, want %v", got, tt.want) + } + + assertArgs(t, g.lastArgs, []string{"systemctl", "is-active", "docker.service"}) + }) + } +} + +func TestDaemonReload(t *testing.T) { + g := &mockGuest{} + s := New(g) + + if err := s.DaemonReload(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + assertArgs(t, g.lastArgs, []string{"sudo", "systemctl", "daemon-reload"}) +} + +// assertArgs fails the test if got and want differ. +func assertArgs(t *testing.T, got, want []string) { + t.Helper() + if len(got) != len(want) { + t.Errorf("args = %v, want %v", got, want) + return + } + for i := range want { + if got[i] != want[i] { + t.Errorf("args[%d] = %q, want %q (full: %v)", i, got[i], want[i], got) + } + } +}