Fixing issue with setting proper fifo path

The machine configuration fifo paths originally pointed to somewhere on
the host. However, the fifo paths should be pointing to the chroot of
the jailer. This fix addresses that and renames the error that is
returned from the jailer handler as it was way too specific.

Signed-off-by: xibz <[email protected]>

Author: xibz

.gitignore

@@ -1,3 +1,4 @@
 firecracker
+jailer
 firecracker-*
 vmlinux

CHANGELOG.md

@@ -1,5 +1,6 @@
 # [ Unreleased ]
 
+* Fixes a bug where fifos were not working properly with jailer enabled (#96)
 * Fixes bug where context was not being used at all during startVM (#86)
 * Updates the jailer's socket path to point to the unix socket in the jailer's workspace (#86)
 * Fixes bug where default socketpath would always be used when not using jailer (#84).

handlers.go

@@ -115,6 +115,11 @@ var CreateLogFilesHandler = Handler{
 		logFifoPath := m.cfg.LogFifo
 		metricsFifoPath := m.cfg.MetricsFifo
 
+		if len(logFifoPath) == 0 || len(metricsFifoPath) == 0 {
+			// logging is disabled
+			return nil
+		}
+
 		if err := createFifos(logFifoPath, metricsFifoPath); err != nil {
 			m.logger.Errorf("Unable to set up logging: %s", err)
 			return err

jailer.go

@@ -322,7 +322,7 @@ func jail(ctx context.Context, m *Machine, cfg *Config) error {
 		WithStderr(stderr)
 
 	if stdin := cfg.JailerCfg.Stdin; stdin != nil {
-		builder.WithStdin(stdin)
+		builder = builder.WithStdin(stdin)
 	}
 
 	m.cmd = builder.Build(ctx)
@@ -375,21 +375,26 @@ func LinkFilesHandler(rootfs, kernelImageFileName string) Handler {
 
 			m.cfg.KernelImagePath = kernelImageFileName
 
-			for _, fifoPath := range []string{m.cfg.LogFifo, m.cfg.MetricsFifo} {
-				if fifoPath == "" {
+			for _, fifoPath := range []*string{&m.cfg.LogFifo, &m.cfg.MetricsFifo} {
+				if fifoPath == nil || *fifoPath == "" {
 					continue
 				}
-				fileName := filepath.Base(fifoPath)
+
+				fileName := filepath.Base(*fifoPath)
 				if err := linkFileToRootFS(
 					m.cfg.JailerCfg,
 					filepath.Join(rootfs, fileName),
-					fifoPath,
+					*fifoPath,
 				); err != nil {
 					return err
 				}
+
 				if err := os.Chown(filepath.Join(rootfs, fileName), *m.cfg.JailerCfg.UID, *m.cfg.JailerCfg.GID); err != nil {
 					return err
 				}
+
+				// update fifoPath as jailer works relative to the chroot dir
+				*fifoPath = fileName
 			}
 
 			return nil
@@ -412,14 +417,14 @@ func NewNaiveChrootStrategy(rootfs, kernelImagePath string) NaiveChrootStrategy
 	}
 }
 
-// ErrCreateMachineHandlerMissing occurs when the CreateMachineHandler is not
-// present in FcInit.
-var ErrCreateMachineHandlerMissing = fmt.Errorf("%s is missing from FcInit's list", CreateMachineHandlerName)
+// ErrRequiredHandlerMissing occurs when a required handler is not present in
+// the handler list.
+var ErrRequiredHandlerMissing = fmt.Errorf("required handler is missing from FcInit's list")
 
 // AdaptHandlers will inject the LinkFilesHandler into the handler list.
 func (s NaiveChrootStrategy) AdaptHandlers(handlers *Handlers) error {
-	if !handlers.FcInit.Has(CreateMachineHandlerName) {
-		return ErrCreateMachineHandlerMissing
+	if !handlers.FcInit.Has(CreateLogFilesHandlerName) {
+		return ErrRequiredHandlerMissing
 	}
 
 	handlers.FcInit = handlers.FcInit.AppendAfter(

jailer_test.go

@@ -111,7 +111,7 @@ func TestJailerBuilder(t *testing.T) {
 func TestJail(t *testing.T) {
 	m := &Machine{
 		Handlers: Handlers{
-			FcInit: HandlerList{}.Append(CreateMachineHandler),
+			FcInit: defaultFcInitHandlerList,
 		},
 	}
 	cfg := &Config{

machine.go

@@ -152,14 +152,6 @@ func (m *Machine) Logger() *log.Entry {
 	return m.logger.WithField("subsystem", userAgent)
 }
 
-// PID returns the machine's process ID
-func (m *Machine) PID() (int, error) {
-	if m.cmd == nil || m.cmd.Process == nil {
-		return 0, errors.New("firecracker process is not running")
-	}
-	return m.cmd.Process.Pid, nil
-}
-
 // NetworkInterface represents a Firecracker microVM's network interface.
 type NetworkInterface struct {
 	// MacAddress defines the MAC address that should be assigned to the network

machine_test.go

@@ -141,8 +141,8 @@ func TestJailerMicroVMExecution(t *testing.T) {
 	os.MkdirAll(jailerTestPath, 0777)
 
 	socketPath := filepath.Join(jailerTestPath, "firecracker", "TestJailerMicroVMExecution.socket")
-	logFifo := filepath.Join(testDataPath, "firecracker.log")
-	metricsFifo := filepath.Join(testDataPath, "firecracker-metrics")
+	logFifo := filepath.Join(tmpDir, "firecracker.log")
+	metricsFifo := filepath.Join(tmpDir, "firecracker-metrics")
 	defer func() {
 		os.Remove(socketPath)
 		os.Remove(logFifo)