Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sandbox: fix the issue of failed to get the vmm master tid #9834

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

lifupan
Copy link
Member

@lifupan lifupan commented Jun 13, 2024

For kata container, the container's pid is meaning less to containerd/crio since the container's pid is belonged to VM, and containerd/crio couldn't use it. Thus we just return any tid of kata shim or hypervisor. But since the hypervisor had been stopped before deleting the container, and it wouldn't get the hypervisor's tid for some supported hypervisor, thus we'd better to return the kata shim's pid instead of hypervisor's tid.

Fixes: #9777
Fixes: #9374

For kata container, the container's pid is meaning less to
containerd/crio since the container's pid is belonged to VM,
and containerd/crio couldn't use it. Thus we just return any
tid of kata shim or hypervisor. But since the hypervisor had
been stopped before deleting the container, and it wouldn't
get the hypervisor's tid for some supported hypervisor, thus
we'd better to return the kata shim's pid instead of hypervisor's
tid.

Fixes: kata-containers#9777

Signed-off-by: Fupan Li <fupan.lfp@antgroup.com>
@katacontainersbot katacontainersbot added the size/tiny Smallest and simplest task label Jun 13, 2024
Copy link
Contributor

@pmores pmores left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks a lot @lifupan !

Just a technical remark, since this PR fixes also issue #9374 it would be great to add a Fixes line for that issue as well. Or we can handle #9374 manually afterwards, whatever is easier, let's just make sure that we don't leave the issue pollute the backlog.

Copy link
Contributor

@Apokleos Apokleos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx @lifupan LGTM

@Apokleos Apokleos merged commit 275c498 into kata-containers:main Jun 18, 2024
315 of 353 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/tiny Smallest and simplest task
Projects
None yet
4 participants