refactor: Break Exec and create in smaller reusable functions

Break the large Exec function and the create function into smaller
functions that cna be reused:
- split createUnikontainer into newUnikontainer (bundle parse and
  InitialSetup) and the reexec handshake
- make SetupNet a standalone function
- pull buildUnikernelCommand and execMonitor out of Exec

The rationale is to let the later port of libcontianer to use some of
this functionality directly instead of duplicating logic.

Signed-off-by: Charalampos Mainas <charalampos.mainas@gmail.com>

Author: cmainas

cmd/urunc/create.go

@@ -82,16 +82,15 @@ var createCommand = &cli.Command{
 	},
 }
 
-// createUnikontainer creates a Unikernel struct from bundle data,
-// initializes it's base dir and state.json,
-// setups terminal if required and spawns reexec process,
-// waits for reexec process to notify, executes CreateRuntime hooks,
-// sends ACK to reexec process
-func createUnikontainer(cmd *cli.Command, uruncCfg *unikontainers.UruncConfig) (err error) {
-	err = nil
+// newUnikontainer parses the bundle and performs the host-side preparation for
+// the monitor execution environment (Unikontainer, base directory, state and
+// monitor resources). It never returns for a container that is not a urunc
+// container: those are handed over to the real runc with an execve.
+func newUnikontainer(cmd *cli.Command, uruncCfg *unikontainers.UruncConfig) (*unikontainers.Unikontainer, error) {
 	containerID := cmd.Args().First()
-	if err = validateID(containerID); err != nil {
-		return err
+	err := validateID(containerID)
+	if err != nil {
+		return nil, err
 	}
 	metrics.SetLoggerContainerID(containerID)
 	metrics.Capture(m.TS00)
@@ -105,7 +104,7 @@ func createUnikontainer(cmd *cli.Command, uruncCfg *unikontainers.UruncConfig) (
 	if bundlePath == "" {
 		bundlePath, err = os.Getwd()
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
 
@@ -114,21 +113,33 @@ func createUnikontainer(cmd *cli.Command, uruncCfg *unikontainers.UruncConfig) (
 	if err != nil {
 		if errors.Is(err, unikontainers.ErrQueueProxy) ||
 			errors.Is(err, unikontainers.ErrNotUnikernel) {
-			// Exec runc to handle non unikernel containers
-			err = runcExec()
-			return err
+			// Exec runc to handle non urunc containers.
+			// It should never return.
+			return nil, runcExec()
 		}
-		return err
+		return nil, err
 	}
 	metrics.Capture(m.TS01)
 
 	err = unikontainer.InitialSetup()
 	if err != nil {
-		return err
+		return nil, err
 	}
-
 	metrics.Capture(m.TS02)
 
+	return unikontainer, nil
+}
+
+// createUnikontainer creates a Unikernel struct from bundle data, initializes
+// it's base dir and state.json, setups terminal if required and spawns reexec
+// process, waits for reexec process to notify, executes CreateRuntime hooks,
+// sends ACK to reexec process
+func createUnikontainer(cmd *cli.Command, uruncCfg *unikontainers.UruncConfig) (err error) {
+	unikontainer, err := newUnikontainer(cmd, uruncCfg)
+	if err != nil {
+		return err
+	}
+
 	// Create socket for nsenter
 	initSockParent, initSockChild, err := newSockPair("init")
 	if err != nil {

pkg/unikontainers/unikontainers.go

@@ -245,16 +245,17 @@ func (u *Unikontainer) SetRunningState() error {
 	return u.saveContainerState()
 }
 
-func (u *Unikontainer) SetupNet() (types.NetDevParams, error) {
-	networkType := u.getNetworkType()
+// SetupNet creates the sandbox's network device (tap) in the current network
+// namespace and returns its parameters; uid and gid own the tap device.
+func SetupNet(networkType string, uid, gid uint32) (types.NetDevParams, error) {
 	uniklog.WithField("network type", networkType).Debug("Retrieved network type")
 	netArgs := types.NetDevParams{}
 	netManager, err := network.NewNetworkManager(networkType)
 	if err != nil {
 		return netArgs, fmt.Errorf("failed to create network manager for %s type: %v", networkType, err)
 	}
 
-	networkInfo, err := netManager.NetworkSetup(u.Spec.Process.User.UID, u.Spec.Process.User.GID)
+	networkInfo, err := netManager.NetworkSetup(uid, gid)
 	if err != nil {
 		// TODO: Handle this case better. We do not need to show an error
 		// since there was no network in the container. Therefore, we
@@ -443,9 +444,6 @@ func monitorMemoryBytes(defaultMem uint, resources *specs.LinuxResources) uint64
 	return mem
 }
 
-// buildMonitorSpec assembles the base MonitorSpec: everything the monitor needs
-// that can be derived from the OCI spec, the container's annotations and the
-// monitor resources gathered during InitialSetup.
 func (u *Unikontainer) buildMonitorSpec(rootfsParams types.RootfsParams, monRes monitorResources) types.MonitorSpec {
 	var mSpec types.MonitorSpec
 
@@ -596,7 +594,7 @@ func (u *Unikontainer) Exec(metrics m.Writer) error {
 	}
 
 	// handle network
-	netArgs, err := u.SetupNet()
+	netArgs, err := SetupNet(u.getNetworkType(), u.Spec.Process.User.UID, u.Spec.Process.User.GID)
 	if err != nil {
 		uniklog.Errorf("failed to setup network: %v", err)
 		return err
@@ -646,20 +644,11 @@ func (u *Unikontainer) Exec(metrics m.Writer) error {
 	}
 
 	// unikernel
-	err = unikernel.Init(unikernelParams)
-	if errors.Is(err, unikernels.ErrUndefinedVersion) ||
-		errors.Is(err, unikernels.ErrVersionParsing) {
-		uniklog.WithError(err).Error("an error occurred while initializing the unikernel")
-	} else if err != nil {
-		return err
-	}
-
 	// build the unikernel command
-	unikernelCmd, err := unikernel.CommandString()
+	vmmArgs.Command, err = buildUnikernelCommand(unikernel, unikernelParams)
 	if err != nil {
 		return err
 	}
-	vmmArgs.Command = unikernelCmd
 
 	// pivot
 	_, err = findNS(u.Spec.Linux.Namespaces, specs.MountNamespace)
@@ -697,38 +686,57 @@ func (u *Unikontainer) Exec(metrics m.Writer) error {
 		return err
 	}
 
-	uniklog.Debug("calling vmm execve")
-	metrics.Capture(m.TS18)
-
-	// 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.
+	// Build the VMM command once and verify it can be constructed successfully, so
+	// we do not 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.
-	// TODO: The container can still be reported as running if the PreExec step
-	// (e.g., BPF/seccomp filter setup) fails after this point. We should find
-	// a way to handle that case as well.
+	// Notify urunc start that the monitor is ready to execute, only after the
+	// command builds so a container is never reported started when it cannot be.
 	err = u.SendMessage(StartSuccess)
 	if err != nil {
 		return err
 	}
 
+	return execMonitor(metrics, vmm, vmmArgs, execCmd)
+}
+
+// buildUnikernelCommand initializes the unikernel with the collected parameters
+// and returns its command line.
+func buildUnikernelCommand(unikernel types.Unikernel, params types.UnikernelParams) (string, error) {
+	err := unikernel.Init(params)
+	if errors.Is(err, unikernels.ErrUndefinedVersion) ||
+		errors.Is(err, unikernels.ErrVersionParsing) {
+		uniklog.WithError(err).Error("an error occurred while initializing the unikernel")
+	} else if err != nil {
+		return "", err
+	}
+
+	return unikernel.CommandString()
+}
+
+// execMonitor runs the monitor's pre-exec setup and finally execve's the monitor.
+// It does not return on success:
+//
+// TODO: The container can still be reported as running if the PreExec step
+// (e.g., BPF/seccomp filter setup) fails after the caller reported success. We
+// should find a way to handle that case as well.
+func execMonitor(metrics m.Writer, vmm types.VMM, execArgs types.ExecArgs, execCmd []string) error {
+	uniklog.Debug("calling vmm execve")
+	metrics.Capture(m.TS18)
 	// Perform any monitor-specific pre-exec setup (e.g., seccomp filters for HVT).
-	err = vmm.PreExec(vmmArgs)
+	err := vmm.PreExec(execArgs)
 	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
+	return syscall.Exec(vmm.Path(), execCmd, execArgs.Environment) //nolint: gosec
 }
 
 func setupUser(user specs.User) error {