Incompatibility with CRI-O

[dkennetzoracle]

Summary

A race condition with crio and dranet causes pod's networking namespace to be destroyed before firing the NRI StopPodSandbox hook to dranet.

What should happen

1. Pod deletion request
2. CRI-O calls NRI StopPodSandbox hook
3. dranet opens the pod's netns via netns.GetFromPath(containerNsPath) (nri_hooks.go:372)
4. dranet calls nsDetachNetdev / nsDetachRdmadev` to move NICs + RDMA devices back to root namespace
5. CRI-O tears down the network namespace

What actually happens in CRI-O 1.34

1. Pod deletion requested
2. CRI-O tears down the network namespace first
3. CRI-O calls NRI StopPodSandbox
4. dranet tries netns.GetFromPath(containerNsPath) -> fails because the namespace file descriptor no longer exists at that path
5. nsDetachNetdev and nsDetachRdmadev both error out
6. But StopPodSandbox swallows errors (logs then, returns nil) so the pod shutdown continues
7. The physical NICs and RDMA link devices that were moved into the now-destroyed namespace are orphaned

Current process to get NICs back

After this happens, the NICs are now disappeared from both the host and the ResourceSlice so rdma: false.

Manually, on each node, it is required to unbind and re-bind the NICs.

Proposed Solution (open to suggestions here)

As a short-term fix, it may be possible to anchor a file-descriptor to that path in the pod so that CRI-O does not remove it / tear it down. Then, the logic to move stuff back from the pod to the container should work fine.

[BenTheElder]

Do we have a tracking issue with CRI-O to discuss this possibly being a CRI-O bug? Or is this possibly fixed already in 1.35.x?
Probably there will be other tools leveraging NRI that will run into any behavioral inconsistency like this.

[dkennetzoracle]

Do we have a tracking issue with CRI-O to discuss this possibly being a CRI-O bug? Or is this possibly fixed already in 1.35.x? Probably there will be other tools leveraging NRI that will run into any behavioral inconsistency like this.

Not yet, let me look to see if this has been fixed 1.35 and I'll post back.

[gauravkghildiyal]

Looks like CRI-O at HEAD tears down network namesapce before NRI invocation, the opposite of containerd does. @samuelkarp is there any behavior NRI as an API requires runtimes to implement, because depending on what stage the NRI hook is invoked, the expectation could be completely different. (I couldn't find any explicit requirements hence asking)

I'm not sure that we explicitly verified this with CRI-O, but the expectation has been that even if DRANET isn't able to perform cleanup of the network namespace, once it gets deleted by the container runtime, the interfaces should get moved back to the host namespace. @dkennetzoracle can you help point out which part isn't happening automatically which DRANET may have done if not for this CRI-O issue (I'm trying to ascertain whether even with CRI-O issue resolved, DRANET does what you need or not)

[dkennetzoracle]

@gauravkghildiyal - sure, they get moved back via the StopPodSandbox, specifically under the following conditions (which we had). Quite a few interesting necessities here:

1. CRI-O cleans up the pod netns
2. StopPodSandbox is called which calls stopPodSandbox (local) which calls np.netdb.GetPodNetNs(podKey(pod)) and nsDetachRdmadev(ns, config.RDMADevice.LinkDev)
3. if !np.rdmaSharedMode && config.RDMADevice.LinkDev != "" { - critical because we were running in exclusive mode for rdma, so this path is hit, which calls nsDetachRdmadev
4. The first thing both of these functions do is GetFromPath but the path no longer exists so nsDetachNetdev and nsDetachRdmadev fail because the namespace file is already gone.
5. They can't open a handle to enter the namespace, so they can't issue the netlink commands to move the devices back.
6. Physical NIC: The kernel's default_device_exit_batch() moves it back to init_net automatically — but in DOWN state, possibly renamed (it is recoverable via unbind / rebind)
7. RDMA device (exclusive mode): The RDMA subsystem does NOT have the same automatic return-to-init behavior. The RdmaLinkSetNsFd() call at rdmadevice.go:75 never executes, and the RDMA device is orphaned.

I also looked into CRI-O and there does seem to be a bug (or maybe it is intended and just an incompatibility):

in server/sandbox_stop_linux.go, stopPodSandbox() executes operations in this order

1. s.networkStop(cox, sb) - CNI teardown
2. sb.RemoveManagedNamespaces() critical - unmounts + deletes the netns file
3. s.nri.stopPodSandbox(cox, sb) - NRI hook fires last after the unmount happens

The same inverted hook order exists in server/sandbox_remove.go, removePodSandbox()

RemoveManagedNamespaces() (in internal/config/nsmgr/types_linux.go:77-95) calls unix.Unmount(fp, MNT_DETACH) then os.RemoveAll(fp). The netns file is gone before the NRI hook fires.

However, in containerd the order is (correct):

internal/cri/server/sandbox_stop.go:

1. Stop containers
2. Stop sandbox container
3. c.nri.StopPodSandbox(cox, &sandbox) - NRI hook fires first
4. Network namespace teardown

The order has existed forever in crio.

[aojea]

Is this related containerd/containerd#11331?

tagging some crio folks @mrunalp @haircommander

s.networkStop(cox, sb) - CNI teardown
sb.RemoveManagedNamespaces() critical - unmounts + deletes the netns file
s.nri.stopPodSandbox(cox, sb) - NRI hook fires last after the unmount happens

one of the discussions during last kubecon on the OCI meeting was the lack of support for the Pod concept, as everything is container based in the OCI spec, so is not unsurprising things are not consistent.

The NRI https://github.com/containerd/nri lifecycle has 3 pods hooks based on the CRI API https://github.com/kubernetes/cri-api/blob/861ac7bf9e55c4d8079b86033708ccdf7e9d2d55/pkg/apis/services.go#L70-L78

        // RunPodSandbox creates and starts a pod-level sandbox. Runtimes should ensure
	// the sandbox is in ready state.
	RunPodSandbox(ctx context.Context, config *runtimeapi.PodSandboxConfig, runtimeHandler string) (string, error)
	// StopPodSandbox stops the sandbox. If there are any running containers in the
	// sandbox, they should be force terminated.
	StopPodSandbox(ctx context.Context, podSandboxID string) error
	// RemovePodSandbox removes the sandbox. If there are running containers in the
	// sandbox, they should be forcibly removed.
	RemovePodSandbox(ctx context.Context, podSandboxID string) error

My mental model for the Pod hooks is as follows:

• StopPodSandbox does not perform resource deletion, it just stops whatever is running on the sandbox, so it is reasonable to expect the plugin receives the existing sandbox resources in this hook and expect them to be available

• RemovePodSandbox is called after the sandbox resources has been removed

So IMHO namespace deletion has to happen AFTER StopPodSandbox has been called

[haircommander]

nothing comes to mind as a reason to do it this way in CRI-O. I do think it should be codified in the NRI API if that's an expectation, but I'm totally open to moving it

[aojea]

nothing comes to mind as a reason to do it this way in CRI-O. I do think it should be codified in the NRI API if that's an expectation, but I'm totally open to moving it

let me open an issue in nri repo, so we discuss there and then runtimes can just implement what the nri api expects

[BenTheElder]

let me open an issue in nri repo, so we discuss there and then runtimes can just implement what the nri api expects

Thanks, that makes more sense 👍

[aojea]

containerd/nri#286