Fix force deleting containers

[cmainas]

When we force delete running containers, urunc needs to stop the monitor process and then do the necessary cleanup. However, there is a race condition here. When urunc kills the monitor process it then checks if the process is still running and in the cases the process is still there, urunc returns with an error and can not finish deletion. For the time being, this does not hurt, since containerd will ask again urunc to delete the container and everything will run smoothly the next time. However, we get annoying log messages of failed deletion errors. Also, there have been cases where the state of the container gets deleted and as a result, we can not recover it.

Therefore, we need to fix this and properly handle the force deletion of containers.

[codesmith25103]

func (u *Unikontainer) Kill() error {
	vmmType := u.State.Annotations[annotHypervisor]
	vmm, err := hypervisors.NewVMM(hypervisors.VmmType(vmmType))
	if err != nil {
		return err
	}
	err = vmm.Stop(u.State.ID)
	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
		}
	}
	// 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
}

If I am correct, I think this is the function in unikontainers.go is causing the above issue. @cmainas ?

[cmainas]

Hello @codesmith25103 , looking a bit in the code, it seems there are many more issues that we need to address. Also, there might be much more reasons for the annoying logs, except of this issue.

However, one thing that needs to be taken care of is the enforcement of kill. Other container runtime make sure that the process is killed when they receive the kill command (e.g. https://github.com/opencontainers/runc/blob/77ead42c9f197e1db8494839de34277ce7d79bc5/delete.go#L16). However, in urunc we might not kill the process and exit. Therefore, a first step is to ensure that the process dies when kill is called.

[codesmith25103]

@cmainas I have opened a PR that eliminates the race condition and ensures the process is fully terminated when the kill command is called, before performing the necessary cleanup.
I’ll be happy to work on more issues to help.
I’m learning a lot while contributing

[cmainas]

Thank you @codesmith25103 a lot for your contributions.

It is very nice to hear that you want to help more and that you are learning through the process.