diff --git a/cmd/urunc/delete.go b/cmd/urunc/delete.go index 1154403a..c9f0da01 100644 --- a/cmd/urunc/delete.go +++ b/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") { diff --git a/pkg/unikontainers/hypervisors/firecracker.go b/pkg/unikontainers/hypervisors/firecracker.go index b47c2527..d950428e 100644 --- a/pkg/unikontainers/hypervisors/firecracker.go +++ b/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 { diff --git a/pkg/unikontainers/hypervisors/hedge.go b/pkg/unikontainers/hypervisors/hedge.go index bbca6d26..b58b4801 100644 --- a/pkg/unikontainers/hypervisors/hedge.go +++ b/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") } diff --git a/pkg/unikontainers/hypervisors/hvt.go b/pkg/unikontainers/hypervisors/hvt.go index 12bedf71..e758fa69 100644 --- a/pkg/unikontainers/hypervisors/hvt.go +++ b/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 diff --git a/pkg/unikontainers/hypervisors/qemu.go b/pkg/unikontainers/hypervisors/qemu.go index bad9b5c4..814eac2f 100644 --- a/pkg/unikontainers/hypervisors/qemu.go +++ b/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 { diff --git a/pkg/unikontainers/hypervisors/spt.go b/pkg/unikontainers/hypervisors/spt.go index ca210067..c2e9eaec 100644 --- a/pkg/unikontainers/hypervisors/spt.go +++ b/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 diff --git a/pkg/unikontainers/hypervisors/utils.go b/pkg/unikontainers/hypervisors/utils.go index 6e5872d4..5b78c17f 100644 --- a/pkg/unikontainers/hypervisors/utils.go +++ b/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 +} diff --git a/pkg/unikontainers/types/types.go b/pkg/unikontainers/types/types.go index a6bc38f5..0c1915ae 100644 --- a/pkg/unikontainers/types/types.go +++ b/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 diff --git a/pkg/unikontainers/unikontainers.go b/pkg/unikontainers/unikontainers.go index 194d83bb..1b4d7adc 100644 --- a/pkg/unikontainers/unikontainers.go +++ b/pkg/unikontainers/unikontainers.go @@ -25,11 +25,9 @@ import ( "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 uniklog = logrus.WithField("subsystem", "unikontainers") 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 @@ func (u *Unikontainer) Exec(metrics m.Writer) error { 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 @@ func setupUser(user specs.User) error { // 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") diff --git a/pkg/unikontainers/utils.go b/pkg/unikontainers/utils.go index 2bb5de4a..9316f460 100644 --- a/pkg/unikontainers/utils.go +++ b/pkg/unikontainers/utils.go @@ -193,7 +193,7 @@ func remove(s []string, i int) []string { func checkValidNsPath(path string) error { // only set to join this namespace if it exists if _, err := os.Lstat(path); err != nil { - return fmt.Errorf("namespace path: %w", err) + return ErrNotExistingNS } // do not allow namespace path with comma as we use it to separate // the namespace paths @@ -279,14 +279,18 @@ func fileExists(fpath string) bool { } // containsNS checks of the container's configuration contains a specific namespace -func containsNS(namespaces []specs.LinuxNamespace, nsType specs.LinuxNamespaceType) bool { +func findNS(namespaces []specs.LinuxNamespace, nsType specs.LinuxNamespaceType) (string, error) { for _, ns := range namespaces { if ns.Type == nsType { - return true + err := checkValidNsPath(ns.Path) + if err != nil { + return "", err + } + return ns.Path, nil } } - return false + return "", fmt.Errorf("Namespace %s was not found", string(nsType)) } // findQemuDataDir tries to find the location of data and BIOS files for Qemu. @@ -319,3 +323,14 @@ func findQemuDataDir(basename string) (string, error) { return qdPath, nil } + +func rmMultipleDirs(prefixPath string, dirs []string) error { + for _, d := range dirs { + path := filepath.Join(prefixPath, d) + if err := os.RemoveAll(path); err != nil { + return fmt.Errorf("cannot remove %s: %w", d, err) + } + } + + return nil +} diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 4f16f33f..c5763181 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -843,7 +843,7 @@ func TestCrictl(t *testing.T) { TestFunc: pingTest, }, { - Image: "harbor.nbfc.io/nubificus/urunc/nginx-firecracker-unikraft:latest", + Image: "harbor.nbfc.io/nubificus/urunc/nginx-firecracker-unikraft-initrd:latest", Name: "Firecracker-unikraft-nginx", Devmapper: false, Seccomp: true,