diff --git a/pkg/unikontainers/hypervisors/firecracker.go b/pkg/unikontainers/hypervisors/firecracker.go index a1ced99b..2787a813 100644 --- a/pkg/unikontainers/hypervisors/firecracker.go +++ b/pkg/unikontainers/hypervisors/firecracker.go @@ -20,7 +20,6 @@ import ( "os" "path/filepath" "strings" - "syscall" "github.com/urunc-dev/urunc/pkg/unikontainers/types" ) @@ -97,7 +96,7 @@ func (fc *Firecracker) Path() string { return fc.binaryPath } -func (fc *Firecracker) Execve(args types.ExecArgs, ukernel types.Unikernel) error { +func (fc *Firecracker) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) { // FIXME: Note for getting unikernel specific options. // Due to the way FC operates, we have not encountered any guest specific // options yet. However, we need to revisit how we can use guest specific @@ -189,12 +188,15 @@ func (fc *Firecracker) Execve(args types.ExecArgs, ukernel types.Unikernel) erro } FCConfigJSON, _ := json.Marshal(FCConfig) if err := os.WriteFile(JSONConfigFile, FCConfigJSON, 0o644); err != nil { //nolint: gosec - return fmt.Errorf("failed to save Firecracker json config: %w", err) + return nil, fmt.Errorf("failed to save Firecracker json config: %w", err) } vmmLog.WithField("Json", string(FCConfigJSON)).Debug("Firecracker json config") exArgs := strings.Split(cmdString, " ") - vmmLog.WithField("Firecracker command", exArgs).Debug("Ready to execve Firecracker") + return exArgs, nil +} - return syscall.Exec(fc.Path(), exArgs, args.Environment) //nolint: gosec +// PreExec performs pre-execution setup. Firecracker has no special pre-exec requirements. +func (fc *Firecracker) PreExec(_ types.ExecArgs) error { + return nil } diff --git a/pkg/unikontainers/hypervisors/hedge.go b/pkg/unikontainers/hypervisors/hedge.go index 436b4786..329f82db 100644 --- a/pkg/unikontainers/hypervisors/hedge.go +++ b/pkg/unikontainers/hypervisors/hedge.go @@ -50,7 +50,12 @@ func (h *Hedge) Path() string { return "" } -func (h *Hedge) Execve(_ types.ExecArgs, _ types.Unikernel) error { +func (h *Hedge) BuildExecCmd(_ types.ExecArgs, _ types.Unikernel) ([]string, error) { + return nil, fmt.Errorf("hedge not implemented yet") +} + +// PreExec performs pre-execution setup. Hedge is not fully implemented. +func (h *Hedge) PreExec(_ types.ExecArgs) error { return fmt.Errorf("hedge not implemented yet") } diff --git a/pkg/unikontainers/hypervisors/hvt.go b/pkg/unikontainers/hypervisors/hvt.go index ff1aa821..5b25d425 100644 --- a/pkg/unikontainers/hypervisors/hvt.go +++ b/pkg/unikontainers/hypervisors/hvt.go @@ -18,7 +18,6 @@ import ( "os/exec" "runtime" "strings" - "syscall" seccomp "github.com/elastic/go-seccomp-bpf" "github.com/urunc-dev/urunc/pkg/unikontainers/types" @@ -148,7 +147,7 @@ func (h *HVT) Ok() error { return nil } -func (h *HVT) Execve(args types.ExecArgs, ukernel types.Unikernel) error { +func (h *HVT) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) { hvtMem := BytesToStringMB(args.MemSizeB) cmdString := h.binaryPath + " --mem=" + hvtMem if args.Net.TapDev != "" { @@ -164,12 +163,16 @@ func (h *HVT) Execve(args types.ExecArgs, ukernel types.Unikernel) error { cmdString = appendNonEmpty(cmdString, " ", extraMonArgs.OtherArgs) cmdString += " " + args.UnikernelPath + " " + args.Command cmdArgs := strings.Split(cmdString, " ") + return cmdArgs, nil +} + +// PreExec performs pre-execution setup for HVT. +// HVT requires applying seccomp filters before syscall.Exec if Seccomp is enabled. +func (h *HVT) PreExec(args types.ExecArgs) error { if args.Seccomp { - err := applySeccompFilter() - if err != nil { + if err := applySeccompFilter(); err != nil { return err } } - vmmLog.WithField("hvt command", cmdString).Debug("Ready to execve hvt") - return syscall.Exec(h.binaryPath, cmdArgs, args.Environment) //nolint: gosec + return nil } diff --git a/pkg/unikontainers/hypervisors/qemu.go b/pkg/unikontainers/hypervisors/qemu.go index 3283df13..dd5b1380 100644 --- a/pkg/unikontainers/hypervisors/qemu.go +++ b/pkg/unikontainers/hypervisors/qemu.go @@ -18,7 +18,6 @@ import ( "fmt" "runtime" "strings" - "syscall" "github.com/urunc-dev/urunc/pkg/unikontainers/types" ) @@ -55,7 +54,7 @@ func (q *Qemu) Path() string { return q.binaryPath } -func (q *Qemu) Execve(args types.ExecArgs, ukernel types.Unikernel) error { +func (q *Qemu) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) { qemuMem := BytesToStringMB(args.MemSizeB) cmdString := q.binaryPath + " -m " + qemuMem + "M" cmdString += " -L /usr/share/qemu" // Set the path for qemu bios/data @@ -137,6 +136,10 @@ func (q *Qemu) Execve(args types.ExecArgs, ukernel types.Unikernel) error { exArgs := strings.Split(cmdString, " ") exArgs = append(exArgs, "-append", args.Command) - vmmLog.WithField("qemu command", exArgs).Debug("Ready to execve qemu") - return syscall.Exec(q.Path(), exArgs, args.Environment) //nolint: gosec + return exArgs, nil +} + +// PreExec performs pre-execution setup. QEMU has no special pre-exec requirements. +func (q *Qemu) PreExec(_ types.ExecArgs) error { + return nil } diff --git a/pkg/unikontainers/hypervisors/spt.go b/pkg/unikontainers/hypervisors/spt.go index d99fd3dd..26b7cde1 100644 --- a/pkg/unikontainers/hypervisors/spt.go +++ b/pkg/unikontainers/hypervisors/spt.go @@ -17,7 +17,6 @@ package hypervisors import ( "os/exec" "strings" - "syscall" "github.com/urunc-dev/urunc/pkg/unikontainers/types" ) @@ -60,7 +59,7 @@ func (s *SPT) Ok() error { return nil } -func (s *SPT) Execve(args types.ExecArgs, ukernel types.Unikernel) error { +func (s *SPT) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) { sptMem := BytesToStringMB(args.MemSizeB) cmdString := s.binaryPath + " --mem=" + sptMem if args.Net.TapDev != "" { @@ -76,6 +75,10 @@ func (s *SPT) Execve(args types.ExecArgs, ukernel types.Unikernel) error { cmdString = appendNonEmpty(cmdString, " ", extraMonArgs.OtherArgs) cmdString += " " + args.UnikernelPath + " " + args.Command cmdArgs := strings.Split(cmdString, " ") - vmmLog.WithField("spt command", cmdString).Debug("Ready to execve spt") - return syscall.Exec(s.binaryPath, cmdArgs, args.Environment) //nolint: gosec + return cmdArgs, nil +} + +// PreExec performs pre-execution setup. SPT has no special pre-exec requirements. +func (s *SPT) PreExec(_ types.ExecArgs) error { + return nil } diff --git a/pkg/unikontainers/types/types.go b/pkg/unikontainers/types/types.go index 3f434e51..28221ca2 100644 --- a/pkg/unikontainers/types/types.go +++ b/pkg/unikontainers/types/types.go @@ -25,7 +25,14 @@ type Unikernel interface { } type VMM interface { - Execve(args ExecArgs, ukernel Unikernel) error + // BuildExecCmd builds and validates the VMM command arguments without executing. + // This is used to verify the command can be built before reporting container as started. + // The returned slice contains the command path as the first element followed by arguments. + BuildExecCmd(args ExecArgs, ukernel Unikernel) ([]string, error) + // PreExec performs any monitor-specific setup that must happen after BuildExecCmd + // succeeds but before syscall.Exec is called. For example, HVT applies seccomp + // filters here. Most monitors can return nil (no-op). + PreExec(args ExecArgs) error Stop(int) error Path() string UsesKVM() bool diff --git a/pkg/unikontainers/unikontainers.go b/pkg/unikontainers/unikontainers.go index 85dc1e2a..ad462976 100644 --- a/pkg/unikontainers/unikontainers.go +++ b/pkg/unikontainers/unikontainers.go @@ -493,18 +493,33 @@ func (u *Unikontainer) Exec(metrics m.Writer) error { uniklog.Debug("calling vmm execve") metrics.Capture(m.TS18) - // metrics.Wait() - // TODO: We set the state to running and notify urunc Start that the monitor - // started, but we might encounter issues with the monitor execution. We need - // to revisit this and check if a failed monitor execution affects this approach. - // If it affects then we need to re-design the whole spawning of the monitor. - // Notify urunc start + + // Build the VMM command once and verify it can be constructed successfully. + // This ensures we don't report the container as started if command building fails. + execCmd, err := vmm.BuildExecCmd(vmmArgs, unikernel) + if err != nil { + uniklog.WithError(err).Error("failed to build VMM command") + return err + } + + // Notify urunc start that the monitor is ready to execute. + // We send this after BuildExecCmd succeeds to avoid reporting a container + // as started when the VMM command cannot be built. err = u.SendMessage(StartSuccess) if err != nil { return err } - return vmm.Execve(vmmArgs, unikernel) + // Perform any monitor-specific pre-exec setup (e.g., seccomp filters for HVT). + err = vmm.PreExec(vmmArgs) + if err != nil { + uniklog.WithError(err).Error("failed to perform pre-exec setup") + return err + } + + // Execute the VMM using the command we built earlier. + uniklog.WithField("command", execCmd).Debug("Ready to execve VMM") + return syscall.Exec(vmm.Path(), execCmd, vmmArgs.Environment) //nolint: gosec } func setupUser(user specs.User) error {