*: fix leaked shim caused by high IO pressure

[fuweid]

integration: add case to reproduce #7496

When the shim unmounts overlayfs rootfs, kernel will force syncfs if there is no volatile option. In order to reproduce the high IO pressure, this patch uses strace to delay the umount2 syscall.

• pkg/cri/server: fix leaked shim issue
• pkg/cri/sbserver: fix leaked shim issue for podsandbox mode

NOTE: I don't merge three commits into one because it's easy to backport to v1.6.

Fixes: #7496 #8931

integration: add ShouldRetryShutdown case based on #7496

Within current design, if the shim is killed before task-service.Delete API call, the callback on connect close will send 137 exit code because the callback doesn't have any context about container's exit code.

containerd/runtime/v2/shim.go

Lines 170 to 184 in 70a2c95

	if response != nil {
	pid = response.Pid
	exitStatus = response.Status
	exitedAt = response.Timestamp
	} else {
	exitStatus = 255
	exitedAt = time.Now()
	}
	events.Publish(ctx, runtime.TaskExitEventTopic, &eventstypes.TaskExit{
	ContainerID: id,
	ID: id,
	Pid: pid,
	ExitStatus: exitStatus,
	ExitedAt: protobuf.ToTimestamp(exitedAt),
	})

And the moby/moby can't handle duplicate exit event well. Let's say that the moby receives exit code 0 at first and then the duplicate exit event with different code 137 can override 0, as the #4769 described.

I think the best solution to avoid leaky issue is to redesign the task-service.Delete API, remove the async callback and then let the caller retry. Since the task in shim can't be restart, we can cache the exit code in container bundle so that the shim.Delete binary call and read it and return exit code correctly. However, it doesn't work with running shim server.

In order to prevent from regression like #4769, I add skipped
integration case as TODO item and we should rethink about how to handle
the task/shim lifecycle.

@mikebrow @dmcgowan @mxpv @AkihiroSuda @thaJeztah @laurazard

[cpuguy83]

LGTM

I have a feeling moby needs a similar patch.

[fuweid]

I have a feeling moby needs a similar patch.

Basically, yes. The leaky shim can be cleanup after containerd restart by the way.

Thanks for the review.