Hi @ruiwen-zhao. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

/assign @mikebrow

While working on this, I found a change of behavior introduced in #8007, which should have broken the test but didn't due to a bug in the test. I am reverting part of #8007 in this PR and fixing the bug in test as well.

cc @mxpv

sync'ed with @mxpv on Slack.

With current implementation, the PLEG events are generated before sandboxstore.Add is called. So the event cannot get sandbox status by either calling cri.PodSandboxStatus or Controller.Status.

The conclusion here is that we will move the event generations back to sbserver/, and will move them to podsandbox package after sandboxstore.Add is moved to Controller.

/ok-to-test

LGTM with a hold for the k8s PR being cherry picked to v1.26.. otherwise a v1.7 containerd would throw an exception on v1.26.. We'll need to mention in our 1.7 docs the requirement for the v1.26 patched release.

Thanks Mike. Created kubernetes/kubernetes#115552 to cherry pick.

LGTM with a hold for the k8s PR being cherry picked to v1.26.. otherwise a v1.7 containerd would throw an exception on v1.26.. We'll need to mention in our 1.7 docs the requirement for the v1.26 patched release.

@mikebrow kubernetes/kubernetes#115552 is now merged and will be released with 1.26.2. Can we get an approval on this PR?

Also the windows failure seems to be irrelevant

critest.exe : The term 'critest.exe' is not recognized as the name of a cmdlet, function, script file, or operable 
program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
At D:\a\_temp\[35](https://github.com/containerd/containerd/actions/runs/4129138934/jobs/7134499677#step:23:36)65d59b-178d-4c35-9457-7e4c1ead4ae5.ps1:11 char:1

Can we also get a re-test?

/test all

@ruiwen-zhao looks good but needs a rebase to pick up the main branch fix for a path issue with cri-tools .. they moved the location of the test executable .. sigh

@ruiwen-zhao looks good but needs a rebase to pick up the main branch fix for a path issue with cri-tools .. they moved the location of the test executable .. sigh

uh thanks Mike! Somehow i was under the impression that github would rebase my change for me before running the tests.

Rebased on top of main.

@ruiwen-zhao looks good but needs a rebase to pick up the main branch fix for a path issue with cri-tools .. they moved the location of the test executable .. sigh

uh thanks Mike! Somehow i was under the impression that github would rebase my change for me before running the tests.

Rebased on top of main.

it would for prow side k8s changes and any "latest" type project pulls, but not for the containerd/containerd changes as they are stowed in your branch

/test pull-containerd-sandboxed-node-e2e

@ruiwen-zhao I just merged the sandbox related change. Please rebase. Thanks

/test pull-containerd-sandboxed-node-e2e

All tests have passed and corresponding kubelet change is merged and cherry-picked.

This PR should be OK to be merged.