Merge pull request #4217 from norio-nomura/minimize-affects-by-restarting-guestagent

Minimize effects caused by restarting guestagent

Author: AkihiroSuda

cmd/lima-guestagent/daemon_linux.go

@@ -7,6 +7,8 @@ import (
 	"errors"
 	"net"
 	"os"
+	"os/signal"
+	"syscall"
 	"time"
 
 	"github.com/mdlayher/vsock"
@@ -26,6 +28,7 @@ func newDaemonCommand() *cobra.Command {
 		Short: "Run the daemon",
 		RunE:  daemonAction,
 	}
+	daemonCommand.Flags().String("runtime-dir", "/run/lima-guestagent", "Directory to store runtime state")
 	daemonCommand.Flags().Duration("tick", 3*time.Second, "Tick for polling events")
 	daemonCommand.Flags().Int("vsock-port", 0, "Use vsock server instead a UNIX socket")
 	daemonCommand.Flags().String("virtio-port", "", "Use virtio server instead a UNIX socket")
@@ -34,6 +37,13 @@ func newDaemonCommand() *cobra.Command {
 
 func daemonAction(cmd *cobra.Command, _ []string) error {
 	ctx := cmd.Context()
+	runtimeDir, err := cmd.Flags().GetString("runtime-dir")
+	if err != nil {
+		return err
+	}
+	if err := os.MkdirAll(runtimeDir, 0o755); err != nil {
+		return err
+	}
 	socket := "/run/lima-guestagent.sock"
 	tick, err := cmd.Flags().GetDuration("tick")
 	if err != nil {
@@ -66,11 +76,16 @@ func daemonAction(cmd *cobra.Command, _ []string) error {
 		tickerInst = ticker.NewCompoundTicker(simpleTicker, ebpfTicker)
 	}
 
-	agent, err := guestagent.New(ctx, tickerInst)
+	ctx, stop := signal.NotifyContext(ctx, syscall.SIGTERM)
+	defer stop()
+	go func() {
+		<-ctx.Done()
+		logrus.Debug("Received SIGTERM, shutting down the guest agent")
+	}()
+	agent, err := guestagent.New(ctx, tickerInst, runtimeDir)
 	if err != nil {
 		return err
 	}
-	defer agent.Close()
 
 	err = os.RemoveAll(socket)
 	if err != nil {
@@ -104,5 +119,6 @@ func daemonAction(cmd *cobra.Command, _ []string) error {
 		l = socketL
 		logrus.Infof("serving the guest agent on %q", socket)
 	}
-	return server.StartServer(l, &server.GuestServer{Agent: agent, TunnelS: portfwdserver.NewTunnelServer()})
+	defer logrus.Debug("exiting lima-guestagent daemon")
+	return server.StartServer(ctx, l, &server.GuestServer{Agent: agent, TunnelS: portfwdserver.NewTunnelServer()})
 }

cmd/lima-guestagent/install_systemd_linux.go

@@ -4,12 +4,14 @@
 package main
 
 import (
+	"bytes"
 	_ "embed"
 	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strings"
 
 	"github.com/sirupsen/logrus"
@@ -24,13 +26,18 @@ func newInstallSystemdCommand() *cobra.Command {
 		Short: "Install a systemd unit (user)",
 		RunE:  installSystemdAction,
 	}
+	installSystemdCommand.Flags().Bool("guestagent-updated", false, "Indicate that the guest agent has been updated")
 	installSystemdCommand.Flags().Int("vsock-port", 0, "Use vsock server on specified port")
 	installSystemdCommand.Flags().String("virtio-port", "", "Use virtio server instead a UNIX socket")
 	return installSystemdCommand
 }
 
 func installSystemdAction(cmd *cobra.Command, _ []string) error {
 	ctx := cmd.Context()
+	guestAgentUpdated, err := cmd.Flags().GetBool("guestagent-updated")
+	if err != nil {
+		return err
+	}
 	vsockPort, err := cmd.Flags().GetInt("vsock-port")
 	if err != nil {
 		return err
@@ -48,24 +55,42 @@ func installSystemdAction(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 	unitPath := "/etc/systemd/system/lima-guestagent.service"
+	unitFileChanged := true
 	if _, err := os.Stat(unitPath); !errors.Is(err, os.ErrNotExist) {
-		logrus.Infof("File %q already exists, overwriting", unitPath)
+		if existingUnit, err := os.ReadFile(unitPath); err == nil && bytes.Equal(unit, existingUnit) {
+			logrus.Infof("File %q is up-to-date", unitPath)
+			unitFileChanged = false
+		} else {
+			logrus.Infof("File %q needs update", unitPath)
+		}
 	} else {
 		unitDir := filepath.Dir(unitPath)
 		if err := os.MkdirAll(unitDir, 0o755); err != nil {
 			return err
 		}
 	}
-	if err := os.WriteFile(unitPath, unit, 0o644); err != nil {
-		return err
+	if unitFileChanged {
+		if err := os.WriteFile(unitPath, unit, 0o644); err != nil {
+			return err
+		}
+		logrus.Infof("Written file %q", unitPath)
+	} else if !guestAgentUpdated {
+		logrus.Info("lima-guestagent.service already up-to-date")
+		return nil
 	}
-	logrus.Infof("Written file %q", unitPath)
-	args := [][]string{
-		{"daemon-reload"},
-		{"enable", "lima-guestagent.service"},
-		{"start", "lima-guestagent.service"},
-		{"try-restart", "lima-guestagent.service"},
+	// unitFileChanged || guestAgentUpdated
+	args := make([][]string, 0, 4)
+	if unitFileChanged {
+		args = append(args, []string{"daemon-reload"})
 	}
+	args = slices.Concat(
+		args,
+		[][]string{
+			{"enable", "lima-guestagent.service"},
+			{"try-restart", "lima-guestagent.service"}, // try-restart: restart if running, otherwise do nothing
+			{"start", "lima-guestagent.service"},       // start: start if not running, otherwise do nothing
+		},
+	)
 	for _, args := range args {
 		cmd := exec.CommandContext(ctx, "systemctl", append([]string{"--system"}, args...)...)
 		cmd.Stdout = os.Stdout

cmd/lima-guestagent/lima-guestagent.TEMPLATE.service

@@ -2,7 +2,7 @@
 Description=lima-guestagent
 
 [Service]
-ExecStart={{.Binary}} daemon {{.Args}}
+ExecStart={{.Binary}} daemon {{.Args}} --runtime-dir="%t/%N"
 Type=simple
 Restart=on-failure
 OOMPolicy=continue

cmd/lima-guestagent/main_linux.go

@@ -11,12 +11,14 @@ import (
 
 	"github.com/lima-vm/lima/v2/cmd/yq"
 	"github.com/lima-vm/lima/v2/pkg/debugutil"
+	"github.com/lima-vm/lima/v2/pkg/osutil"
 	"github.com/lima-vm/lima/v2/pkg/version"
 )
 
 func main() {
 	yq.MaybeRunYQ()
 	if err := newApp().Execute(); err != nil {
+		osutil.HandleExitError(err)
 		logrus.Fatal(err)
 	}
 }

pkg/cidata/cidata.TEMPLATE.d/boot/25-guestagent-base.sh

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 # SPDX-FileCopyrightText: Copyright The Lima Authors
 # SPDX-License-Identifier: Apache-2.0
@@ -19,46 +19,66 @@ fi
 
 # Install or update the guestagent binary
 mkdir -p "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin
-install -m 755 "${LIMA_CIDATA_MNT}"/lima-guestagent "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent
+guestagent_updated=false
+if diff -q "${LIMA_CIDATA_MNT}"/lima-guestagent "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent 2>/dev/null; then
+	echo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}/bin/lima-guestagent is up-to-date"
+else
+	install -m 755 "${LIMA_CIDATA_MNT}"/lima-guestagent "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent
+	guestagent_updated=true
+fi
 
 # Launch the guestagent service
 if [ -f /sbin/openrc-run ]; then
-	# Convert .env to conf.d by wrapping values in double quotes.
-	# Split the variable and value at the first "=" to handle cases where the value contains additional "=" characters.
-	sed -E 's/^([^=]+)=(.*)/\1="\2"/' "${LIMA_CIDATA_MNT}/lima.env" >"/etc/conf.d/lima-guestagent"
-	# Install the openrc lima-guestagent service script
-	cat >/etc/init.d/lima-guestagent <<'EOF'
-#!/sbin/openrc-run
-supervisor=supervise-daemon
+	print_config() {
+		# Convert .env to conf.d by wrapping values in double quotes.
+		# Split the variable and value at the first "=" to handle cases where the value contains additional "=" characters.
+		sed -E 's/^([^=]+)=(.*)/\1="\2"/' "${LIMA_CIDATA_MNT}/lima.env"
+	}
+	print_script() {
+		# the openrc lima-guestagent service script
+		cat <<-'EOF'
+			#!/sbin/openrc-run
+			supervisor=supervise-daemon
 
-log_file="${log_file:-/var/log/${RC_SVCNAME}.log}"
-err_file="${err_file:-${log_file}}"
-log_mode="${log_mode:-0644}"
-log_owner="${log_owner:-root:root}"
+			log_file="${log_file:-/var/log/${RC_SVCNAME}.log}"
+			err_file="${err_file:-${log_file}}"
+			log_mode="${log_mode:-0644}"
+			log_owner="${log_owner:-root:root}"
 
-supervise_daemon_args="${supervise_daemon_opts:---stderr \"${err_file}\" --stdout \"${log_file}\"}"
+			supervise_daemon_args="${supervise_daemon_opts:---stderr \"${err_file}\" --stdout \"${log_file}\"}"
 
-name="lima-guestagent"
-description="Forward ports to the lima-hostagent"
+			name="lima-guestagent"
+			description="Forward ports to the lima-hostagent"
+
+			command=${LIMA_CIDATA_GUEST_INSTALL_PREFIX}/bin/lima-guestagent
+			command_args="daemon --debug=${LIMA_CIDATA_DEBUG} --vsock-port \"${LIMA_CIDATA_VSOCK_PORT}\" --virtio-port \"${LIMA_CIDATA_VIRTIO_PORT}\""
+			command_background=true
+			pidfile="/run/lima-guestagent.pid"
+		EOF
+	}
+	if [ "${guestagent_updated}" = "false" ] &&
+		diff -q <(print_config) /etc/conf.d/lima-guestagent 2>/dev/null &&
+		diff -q <(print_script) /etc/init.d/lima-guestagent 2>/dev/null; then
+		echo "lima-guestagent service already up-to-date"
+		exit 0
+	fi
 
-command=${LIMA_CIDATA_GUEST_INSTALL_PREFIX}/bin/lima-guestagent
-command_args="daemon --debug=${LIMA_CIDATA_DEBUG} --vsock-port \"${LIMA_CIDATA_VSOCK_PORT}\" --virtio-port \"${LIMA_CIDATA_VIRTIO_PORT}\""
-command_background=true
-pidfile="/run/lima-guestagent.pid"
-EOF
+	print_config >/etc/conf.d/lima-guestagent
+	print_script >/etc/init.d/lima-guestagent
 	chmod 755 /etc/init.d/lima-guestagent
 
 	rc-update add lima-guestagent default
-	rc-service lima-guestagent start
+	rc-service --ifstarted lima-guestagent restart # restart if running, otherwise do nothing
+	rc-service --ifstopped lima-guestagent start   # start if not running, otherwise do nothing
 else
 	# Remove legacy systemd service
 	rm -f "${LIMA_CIDATA_HOME}/.config/systemd/user/lima-guestagent.service"
 
 	if [ "${LIMA_CIDATA_VSOCK_PORT}" != "0" ]; then
-		sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd --debug="${LIMA_CIDATA_DEBUG}" --vsock-port "${LIMA_CIDATA_VSOCK_PORT}"
+		sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd --debug="${LIMA_CIDATA_DEBUG}" --guestagent-updated="${guestagent_updated}" --vsock-port "${LIMA_CIDATA_VSOCK_PORT}"
 	elif [ "${LIMA_CIDATA_VIRTIO_PORT}" != "" ]; then
-		sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd --debug="${LIMA_CIDATA_DEBUG}" --virtio-port "${LIMA_CIDATA_VIRTIO_PORT}"
+		sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd --debug="${LIMA_CIDATA_DEBUG}" --guestagent-updated="${guestagent_updated}" --virtio-port "${LIMA_CIDATA_VIRTIO_PORT}"
 	else
-		sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd --debug="${LIMA_CIDATA_DEBUG}"
+		sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd --debug="${LIMA_CIDATA_DEBUG}" --guestagent-updated="${guestagent_updated}"
 	fi
 fi

pkg/guestagent/api/server/server.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"net"
 
+	"github.com/sirupsen/logrus"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/keepalive"
 	"google.golang.org/protobuf/types/known/emptypb"
@@ -16,7 +17,7 @@ import (
 	"github.com/lima-vm/lima/v2/pkg/portfwdserver"
 )
 
-func StartServer(lis net.Listener, guest *GuestServer) error {
+func StartServer(ctx context.Context, lis net.Listener, guest *GuestServer) error {
 	server := grpc.NewServer(
 		grpc.InitialWindowSize(512<<20),
 		grpc.InitialConnWindowSize(512<<20),
@@ -26,7 +27,19 @@ func StartServer(lis net.Listener, guest *GuestServer) error {
 		grpc.KeepaliveParams(keepalive.ServerParameters{Time: 0, Timeout: 0, MaxConnectionIdle: 0}),
 	)
 	api.RegisterGuestServiceServer(server, guest)
-	return server.Serve(lis)
+	go func() {
+		<-ctx.Done()
+		logrus.Debug("Stopping the gRPC server")
+		server.GracefulStop()
+		logrus.Debug("Closing the listener used by the gRPC server")
+		lis.Close()
+	}()
+	err := server.Serve(lis)
+	// grpc.Server.Serve() expects to return a non-nil error caused by lis.Accept()
+	if opErr, ok := err.(*net.OpError); ok && opErr.Err.Error() == "use of closed network connection" {
+		return nil
+	}
+	return err
 }
 
 type GuestServer struct {
@@ -41,6 +54,7 @@ func (s *GuestServer) GetInfo(ctx context.Context, _ *emptypb.Empty) (*api.Info,
 
 func (s *GuestServer) GetEvents(_ *emptypb.Empty, stream api.GuestService_GetEventsServer) error {
 	responses := make(chan *api.Event)
+	// expects Events() to close the channel when stream.Context() is done or ticker stops
 	go s.Agent.Events(stream.Context(), responses)
 	for response := range responses {
 		err := stream.Send(response)