Merge pull request #166 from michaeldwan/disable-signals-option

Configurable signal handling, fix signal cleanup

Author: xibz

machine.go

@@ -117,6 +117,10 @@ type Config struct {
 	// NetNS represents the path to a network namespace handle. If present, the
 	// application will use this to join the associated network namespace
 	NetNS string
+
+	// ForwardSignals is an optional list of signals to catch and forward to
+	// firecracker. If not provided, the default signals will be used.
+	ForwardSignals []os.Signal
 }
 
 // Validate will ensure that the required fields are set and that
@@ -303,6 +307,16 @@ func NewMachine(ctx context.Context, cfg Config, opts ...Opt) (*Machine, error)
 		cfg.VMID = randomID.String()
 	}
 
+	if cfg.ForwardSignals == nil {
+		cfg.ForwardSignals = []os.Signal{
+			os.Interrupt,
+			syscall.SIGQUIT,
+			syscall.SIGTERM,
+			syscall.SIGHUP,
+			syscall.SIGABRT,
+		}
+	}
+
 	m.machineConfig = cfg.MachineCfg
 	m.Cfg = cfg
 
@@ -481,20 +495,7 @@ func (m *Machine) startVMM(ctx context.Context) error {
 		close(errCh)
 	}()
 
-	// Set up a signal handler and pass INT, QUIT, and TERM through to firecracker
-	sigchan := make(chan os.Signal)
-	signal.Notify(sigchan, os.Interrupt,
-		syscall.SIGQUIT,
-		syscall.SIGTERM,
-		syscall.SIGHUP,
-		syscall.SIGABRT)
-	m.logger.Debugf("Setting up signal handler")
-	go func() {
-		if sig, ok := <-sigchan; ok {
-			m.logger.Printf("Caught signal %s", sig)
-			m.cmd.Process.Signal(sig)
-		}
-	}()
+	m.setupSignals()
 
 	// Wait for firecracker to initialize:
 	err = m.waitForSocket(time.Duration(m.client.firecrackerInitTimeout)*time.Second, errCh)
@@ -513,8 +514,6 @@ func (m *Machine) startVMM(ctx context.Context) error {
 			m.fatalErr = err
 		}
 
-		signal.Stop(sigchan)
-		close(sigchan)
 		close(m.exitCh)
 	}()
 
@@ -885,3 +884,31 @@ func (m *Machine) waitForSocket(timeout time.Duration, exitchan chan error) erro
 		}
 	}
 }
+
+// Set up a signal handler to pass through to firecracker
+func (m *Machine) setupSignals() {
+	signals := m.Cfg.ForwardSignals
+
+	if len(signals) == 0 {
+		return
+	}
+
+	m.logger.Debugf("Setting up signal handler: %v", signals)
+	sigchan := make(chan os.Signal, len(signals))
+	signal.Notify(sigchan, signals...)
+
+	go func() {
+		for {
+			select {
+			case sig := <-sigchan:
+				m.logger.Printf("Caught signal %s", sig)
+				m.cmd.Process.Signal(sig)
+			case <-m.exitCh:
+				break
+			}
+		}
+
+		signal.Stop(sigchan)
+		close(sigchan)
+	}()
+}

machine_test.go

@@ -23,6 +23,7 @@ import (
 	"net"
 	"os"
 	"os/exec"
+	"os/signal"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -1199,3 +1200,106 @@ func createValidConfig(t *testing.T, socketPath string) Config {
 		},
 	}
 }
+
+func TestSignalForwarding(t *testing.T) {
+	forwardedSignals := []os.Signal{
+		syscall.SIGUSR1,
+		syscall.SIGUSR2,
+		syscall.SIGINT,
+		syscall.SIGTERM,
+	}
+	ignoredSignals := []os.Signal{
+		syscall.SIGHUP,
+		syscall.SIGQUIT,
+	}
+
+	cfg := Config{
+		Debug:           true,
+		KernelImagePath: filepath.Join(testDataPath, "vmlinux"),
+		SocketPath:      "/tmp/TestSignalForwarding.sock",
+		Drives: []models.Drive{
+			{
+				DriveID:      String("0"),
+				IsRootDevice: Bool(true),
+				IsReadOnly:   Bool(false),
+				PathOnHost:   String(testRootfs),
+			},
+		},
+		DisableValidation: true,
+		ForwardSignals:    forwardedSignals,
+	}
+	defer os.RemoveAll("/tmp/TestSignalForwarding.sock")
+
+	opClient := fctesting.MockClient{}
+
+	ctx := context.Background()
+	client := NewClient(cfg.SocketPath, fctesting.NewLogEntry(t), true, WithOpsClient(&opClient))
+
+	fd, err := net.Listen("unix", cfg.SocketPath)
+	if err != nil {
+		t.Fatalf("unexpected error during creation of unix socket: %v", err)
+	}
+	defer fd.Close()
+
+	stdout := &bytes.Buffer{}
+	stderr := &bytes.Buffer{}
+	cmd := exec.Command(filepath.Join(testDataPath, "sigprint.sh"))
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
+	stdin, err := cmd.StdinPipe()
+	assert.NoError(t, err)
+
+	m, err := NewMachine(
+		ctx,
+		cfg,
+		WithClient(client),
+		WithProcessRunner(cmd),
+		WithLogger(fctesting.NewLogEntry(t)),
+	)
+	if err != nil {
+		t.Fatalf("failed to create new machine: %v", err)
+	}
+
+	if err := m.startVMM(ctx); err != nil {
+		t.Fatalf("error startVMM: %v", err)
+	}
+	defer m.StopVMM()
+
+	sigChan := make(chan os.Signal)
+	signal.Notify(sigChan, ignoredSignals...)
+	defer func() {
+		signal.Stop(sigChan)
+		close(sigChan)
+	}()
+
+	go func() {
+		for sig := range sigChan {
+			t.Logf("received signal %v, ignoring", sig)
+		}
+	}()
+
+	go func() {
+		for _, sig := range append(forwardedSignals, ignoredSignals...) {
+			t.Logf("sending signal %v to self", sig)
+			syscall.Kill(syscall.Getpid(), sig.(syscall.Signal))
+		}
+
+		// give the child process time to receive signals and flush pipes
+		time.Sleep(1 * time.Second)
+
+		// terminate the signal printing process
+		stdin.Write([]byte("q"))
+	}()
+
+	err = m.Wait(ctx)
+	require.NoError(t, err, "wait returned an error")
+
+	receivedSignals := []os.Signal{}
+	for _, sigStr := range strings.Split(strings.TrimSpace(stdout.String()), "\n") {
+		i, err := strconv.Atoi(sigStr)
+		require.NoError(t, err, "expected numeric output")
+		receivedSignals = append(receivedSignals, syscall.Signal(i))
+	}
+
+	assert.ElementsMatch(t, forwardedSignals, receivedSignals)
+}

testdata/sigprint.sh

@@ -0,0 +1,16 @@
+#!/bin/bash
+
+typeset -i sig=1
+while (( sig < 65 )); do
+    trap "echo '$sig'" $sig 2>/dev/null 
+    let sig=sig+1
+done
+
+>&2 echo "Send signals to PID $$ and type [q] when done."
+
+while :
+do 
+  read -n1 input
+  [ "$input" == "q" ] && break
+  sleep .1
+done