Fixes in the deletion path of the container

cmd/urunc/delete.go

@@ -16,7 +16,9 @@ package main
 
 import (
 	"context"
+	"errors"
 	"os"
+	"path/filepath"
 	"runtime"
 
 	"github.com/sirupsen/logrus"
@@ -54,6 +56,21 @@ status of "ubuntu01" as "stopped" the following will delete resources held for
 		// get Unikontainer data from state.json
 		unikontainer, err := getUnikontainer(cmd)
 		if err != nil {
+			if errors.Is(err, os.ErrNotExist) {
+				containerID := cmd.Args().First()
+				if containerID == "" {
+					return ErrEmptyContainerID
+				}
+				rootDir := cmd.String("root")
+				containerDir := filepath.Join(rootDir, containerID)
+				e := os.RemoveAll(containerDir)
+				if e != nil {
+					logrus.Errorf("remove %s: %v", containerDir, e)
+				}
+				if cmd.Bool("force") {
+					return nil
+				}
+			}
 			return err
 		}
 		if cmd.Bool("force") {

pkg/unikontainers/hypervisors/firecracker.go

@@ -69,8 +69,8 @@ type FirecrackerConfig struct {
 	NetIfs  []FirecrackerNet      `json:"network-interfaces"`
 }
 
-func (fc *Firecracker) Stop(_ string) error {
-	return nil
+func (fc *Firecracker) Stop(pid int) error {
+	return killProcess(pid)
 }
 
 func (fc *Firecracker) Ok() error {

pkg/unikontainers/hypervisors/hedge.go

@@ -33,7 +33,7 @@ func (h *Hedge) Ok() error {
 	return fmt.Errorf("hedge not implemented yet")
 }
 
-func (h *Hedge) Stop(_ string) error {
+func (h *Hedge) Stop(_ int) error {
 	return fmt.Errorf("hedge not implemented yet")
 }
 

pkg/unikontainers/hypervisors/hvt.go

@@ -120,10 +120,9 @@ func applySeccompFilter() error {
 	return nil
 }
 
-// Stop is an empty function to satisfy VMM interface compatibility requirements.
-// It does not perform any actions and always returns nil.
-func (h *HVT) Stop(_ string) error {
-	return nil
+// Stop kills the hvt process
+func (h *HVT) Stop(pid int) error {
+	return killProcess(pid)
 }
 
 // UsesKVM returns a bool value depending on if the monitor uses KVM

pkg/unikontainers/hypervisors/qemu.go

@@ -33,8 +33,8 @@ type Qemu struct {
 	binary     string
 }
 
-func (q *Qemu) Stop(_ string) error {
-	return nil
+func (q *Qemu) Stop(pid int) error {
+	return killProcess(pid)
 }
 
 func (q *Qemu) Ok() error {

pkg/unikontainers/hypervisors/spt.go

@@ -32,10 +32,9 @@ type SPT struct {
 	binary     string
 }
 
-// Stop is an empty function to satisfy VMM interface compatibility requirements.
-// It does not perform any actions and always returns nil.
-func (s *SPT) Stop(_ string) error {
-	return nil
+// Stop kills the spt process
+func (s *SPT) Stop(pid int) error {
+	return killProcess(pid)
 }
 
 // UsesKVM returns a bool value depending on if the monitor uses KVM

pkg/unikontainers/hypervisors/utils.go

@@ -15,8 +15,14 @@
 package hypervisors
 
 import (
+	"errors"
+	"fmt"
 	"runtime"
 	"strconv"
+	"syscall"
+	"time"
+
+	"golang.org/x/sys/unix"
 )
 
 func cpuArch() string {
@@ -60,3 +66,27 @@ func BytesToStringMB(argMem uint64) string {
 
 	return stringMem
 }
+
+func killProcess(pid int) error {
+	const timeout = 2 * time.Second
+	err := syscall.Kill(pid, unix.SIGKILL)
+	if err != nil {
+		return err
+	}
+	deadline := time.Now().Add(timeout)
+	for {
+		if err := syscall.Kill(pid, 0); err != nil {
+			if errors.Is(err, syscall.ESRCH) {
+				// process is dead
+				break
+			}
+			return fmt.Errorf("error checking if process with pid %d is alive: %w", pid, err)
+		}
+		if time.Now().After(deadline) {
+			return fmt.Errorf("timeout waiting for pid %d to die", pid)
+		}
+		time.Sleep(100 * time.Millisecond)
+	}
+
+	return nil
+}

pkg/unikontainers/types/types.go

@@ -26,7 +26,7 @@ type Unikernel interface {
 
 type VMM interface {
 	Execve(args ExecArgs, ukernel Unikernel) error
-	Stop(t string) error
+	Stop(int) error
 	Path() string
 	UsesKVM() bool
 	SupportsSharedfs(string) bool

pkg/unikontainers/unikontainers.go

@@ -25,11 +25,9 @@
 	"os/exec"
 	"path/filepath"
 	"runtime"
-	"strconv"
 	"strings"
 	"sync"
 	"syscall"
-	"time"
 
 	"github.com/urunc-dev/urunc/pkg/network"
 	"github.com/urunc-dev/urunc/pkg/unikontainers/hypervisors"
@@ -53,6 +51,7 @@
 
 var ErrQueueProxy = errors.New("this a queue proxy container")
 var ErrNotUnikernel = errors.New("this is not a unikernel container")
+var ErrNotExistingNS = errors.New("the namespace does not exist")
 
 // Unikontainer holds the data necessary to create, manage and delete unikernel containers
 type Unikontainer struct {
@@ -437,7 +436,11 @@
 	vmmArgs.Command = unikernelCmd
 
 	// pivot
-	withPivot := containsNS(u.Spec.Linux.Namespaces, specs.MountNamespace)
+	_, err = findNS(u.Spec.Linux.Namespaces, specs.MountNamespace)
+	// We just want to check if a mount namespace was define din the list
+	// Therefore, if there was no error and the mount namespace was found
+	// we can pivot.
+	withPivot := err != nil
 	err = changeRoot(rootfsParams.MonRootfs, withPivot)
 	if err != nil {
 		return err
@@ -492,153 +495,122 @@
 // Kill stops the VMM process, first by asking the VMM struct to stop
 // and consequently by killing the process described in u.State.Pid
 func (u *Unikontainer) Kill() error {
-	vmmType := u.State.Annotations[annotHypervisor]
+	// Try to join the Network namespace of the monitor before killing it.
+	// If we kill it there might be no process inside the namespace and hence
+	// the namespace gets destroyed.
+	err := u.joinSandboxNetNs()
+	if err != nil {
+		if errors.Is(err, ErrNotExistingNS) {
+			// There is no network namespace to join.
+			// Most probably the sandbox is dead and the namespace
+			// has been destroyed.
+			uniklog.Infof("could not find sandbox's network namespace: %v", err)
+			return nil
+		}
+		return fmt.Errorf("failed to join sandbox netns: %v", err)
+	}
+
 	// get a new vmm
+	vmmType := u.State.Annotations[annotHypervisor]
 	vmm, err := hypervisors.NewVMM(hypervisors.VmmType(vmmType), u.UruncCfg.Hypervisors)
 	if err != nil {
 		return err
 	}
-	err = vmm.Stop(u.State.ID)
+	err = vmm.Stop(u.State.Pid)
 	if err != nil {
 		return err
 	}
 
-	// Check if pid is running
-	if syscall.Kill(u.State.Pid, syscall.Signal(0)) == nil {
-		err = syscall.Kill(u.State.Pid, unix.SIGKILL)
-		if err != nil {
-			return err
-		}
-	}
-	const timeout = 2 * time.Second
-	deadline := time.Now().Add(timeout)
-	for {
-		if err := syscall.Kill(u.State.Pid, 0); err != nil {
-			if errors.Is(err, syscall.ESRCH) {
-				break // process is dead
-			}
-			return fmt.Errorf("error checking pid %d: %w", u.State.Pid, err)
-		}
-		if time.Now().After(deadline) {
-			return fmt.Errorf("timeout waiting for pid %d to die", u.State.Pid)
-		}
-		time.Sleep(100 * time.Millisecond)
-	}
-
-	// If PID is running we need to kill the process
-	// Once the process is dead, we need to enter the network namespace
-	// and delete the TC rules and TAP device
-	err = u.joinSandboxNetNs()
-	if err != nil {
-		uniklog.Errorf("failed to join sandbox netns: %v", err)
-		return nil
-	}
 	// TODO: tap0_urunc should not be hardcoded
 	err = network.Cleanup("tap0_urunc")
 	if err != nil {
 		uniklog.Errorf("failed to delete tap0_urunc: %v", err)
 	}
+
 	return nil
 }
 
 // Delete removes the containers base directory and its contents
 func (u *Unikontainer) Delete() error {
+	var dirs []string
+	var prefPath string
+
 	if u.isRunning() {
-		return fmt.Errorf("cannot delete running unikernel: %s", u.State.ID)
+		return fmt.Errorf("cannot delete running container: %s", u.State.ID)
+	}
+
+	// get a monitor instance of the running monitor
+	vmmType := u.State.Annotations[annotHypervisor]
+	vmm, err := hypervisors.NewVMM(hypervisors.VmmType(vmmType), u.UruncCfg.Hypervisors)
+	if err != nil {
+		return err
 	}
+
 	// Make sure paths are clean
 	bundleDir := filepath.Clean(u.State.Bundle)
 	rootfsDir := filepath.Clean(u.Spec.Root.Path)
 	if !filepath.IsAbs(rootfsDir) {
 		rootfsDir = filepath.Join(bundleDir, rootfsDir)
 	}
+	monRootfs := filepath.Join(bundleDir, monitorRootfsDirName)
 
-	// Check if we used a different directory for monitor's rootfs than the
-	// container's one.
-	withRootfsMount := false
-	withRootfsMount, err := strconv.ParseBool(u.State.Annotations[annotMountRootfs])
-	if err != nil {
-		withRootfsMount = false
-	}
-	annotBlock := u.State.Annotations[annotBlock]
-	// TODO: We might not need to remove all these directories.
-	if annotBlock == "" && withRootfsMount {
+	// TODO: We might not need to remove any of the directories and let
+	// the kernel cleanup the mounts and shim to remove directories.
+	// However, just to be on the safe side, we remove all the newly
+	// created directories from urunc. In order to check if we used the
+	// rootfs under the bundle directory or we create anew one, we can check
+	// if the monitorRootfsDirName directory exists under the bundle.
+	_, err = os.Stat(monRootfs)
+	if !os.IsNotExist(err) {
 		// Since there was no no block defined for the unikernel
 		// and we created a new rootfs for the monitor, we need to
 		// clean it up.
-		monRootfs := filepath.Join(bundleDir, monitorRootfsDirName)
-		err = os.RemoveAll(monRootfs)
-		if err != nil {
-			return fmt.Errorf("cannot remove %s: %v", monRootfs, err)
-		}
+		dirs = append(dirs, monitorRootfsDirName)
+		prefPath = bundleDir
 	} else {
 		// Otherwise remove the enw directories we created inside the
 		// container's rootfs.
-		cntrDev := filepath.Join(rootfsDir, "/dev")
-		err = os.RemoveAll(cntrDev)
-		if err != nil {
-			return fmt.Errorf("cannot remove /dev: %v", err)
-		}
-		cntrTmp := filepath.Join(rootfsDir, "/tmp")
-		err = os.RemoveAll(cntrTmp)
-		if err != nil {
-			return fmt.Errorf("cannot remove /tmp: %v", err)
-		}
-		cntrLib := filepath.Join(rootfsDir, "/lib")
-		err = os.RemoveAll(cntrLib)
-		if err != nil {
-			return fmt.Errorf("cannot remove /lib: %v", err)
-		}
-		cntrLib64 := filepath.Join(rootfsDir, "/lib64")
-		err = os.RemoveAll(cntrLib64)
-		if err != nil {
-			return fmt.Errorf("cannot remove /lib64: %v", err)
-		}
 		// We do not need to unmount anything here, since we rely on Linux
 		// to do the cleanup for us. This will happen automatically,
 		// when the mount namespace gets destroyed
-		cntrUsr := filepath.Join(rootfsDir, "/usr")
-		err = os.RemoveAll(cntrUsr)
-		if err != nil {
-			return fmt.Errorf("cannot remove /usr: %v", err)
+		dirs = []string{
+			"/lib",
+			"/lib64",
+			"/usr",
+			"/proc",
+			"/dev",
+			"/tmp",
 		}
+		dirs = append(dirs, vmm.Path())
+		prefPath = rootfsDir
 	}
+
+	err = rmMultipleDirs(prefPath, dirs)
+	if err != nil {
+		return err
+	}
+
 	return os.RemoveAll(u.BaseDir)
 }
 
-// joinSandboxNetns joins the network namespace of the sandbox (pause container).
+// joinSandboxNetns joins the network namespace of the sandbox
 // This function should be called only from a locked thread
 // (i.e. runtime. LockOSThread())
 func (u Unikontainer) joinSandboxNetNs() error {
-	var netNsPath string
-	// We want enter the network namespace of the container.
-	// There are two possibilities:
-	// 1. The unikernel was running inside a Pod and hence we need to join
-	//    the namespace of the pause container
-	// 2. The unikernel was running in its own network namespace (typical
-	//    in docker, nerdctl etc.). If that is the case, then when the
-	//    unikernel dies/exits the namespace will also die, since there will
-	//    not be any process in that namespace. Therefore, the cleanup will
-	//    happen automatically and we do not need to care about that.
-	// Therefore, focus only in the first case above.
-	for _, ns := range u.Spec.Linux.Namespaces {
-		if ns.Type == specs.NetworkNamespace {
-			if ns.Path == "" {
-				// We had to create the network namespace, when
-				// creating the container. Therefore, the namespace
-				// will die along with the unikernel.
-				return nil
-			}
-			err := checkValidNsPath(ns.Path)
-			if err == nil {
-				netNsPath = ns.Path
-			} else {
-				return err
-			}
-			break
+	netNsPath, err := findNS(u.Spec.Linux.Namespaces, specs.NetworkNamespace)
+	if err != nil && !errors.Is(err, ErrNotExistingNS) {
+		return err
+	}
+	// In case no path was specified for the network namespace it means,
+	// that we had to create a new one and therefore we can join it by
+	// using the pid of the monitor process.
+	if netNsPath == "" {
+		netNsPath = fmt.Sprintf("/proc/%d/ns/net", u.State.Pid)
+		err := checkValidNsPath(netNsPath)
+		if err != nil {
+			return err
 		}
 	}
-
 	uniklog.WithFields(logrus.Fields{
 		"path": netNsPath,
 	}).Debug("Joining network namespace")