Documentation Examples: Unable to start MicroVM

[RBSUS]

Describe the bug

A quick heads up - the example sandboxes aren't loading

I have also flagged here: https://github.com/codesandbox/codesandbox-client/issues/8074

For anyone looking, you can find the code for the examples here: https://github.com/TanStack/virtual/tree/beta/examples/react

Your minimal, reproducible example

https://codesandbox.io/embed/github/tanstack/virtual/tree/beta/examples/react/variable?autoresize=1&fontsize=14&theme=dark

Steps to reproduce

Expected behavior

Expected to be able to see the examples

How often does this bug happen?

None

Screenshots or Videos

No response

Platform

Windows

tanstack-virtual version

v3.30.1

TypeScript version

No response

Additional context

No response

Terms & Code of Conduct

• [X] I agree to follow this project's Code of Conduct
• [X] I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.

[danmihai1]

@ChengyuZhu6 please consider this change too: https://github.com/kata-containers/kata-containers/issues/8489

[ChengyuZhu6]

@ChengyuZhu6 please consider this change too: #8489

Sure. Thanks for your information.

[stevenhorsman]

Hey @ChengyuZhu6 - I have a widely comment about this PR - what is your plan regarding integration tests? We've had some discussion in the issue about test scenarios and that we probably need to add some more, so do you think that it is fair to have them as part of this PR, so we can test the code at the same time it's going in?

That definitely doesn't mean that it's on you to write all the tests as I think we need to re-think how we did them in CCv0 rather than just merge them straight over, so it will be on others (like myself, Wainer, Fabiano etc) to help/work on those, but do you think it's a fair request to deliver the feature and tests in the same PR?

[ChengyuZhu6]

Hey @ChengyuZhu6 - I have a widely comment about this PR - what is your plan regarding integration tests? We've had some discussion in the issue about test scenarios and that we probably need to add some more, so do you think that it is fair to have them as part of this PR, so we can test the code at the same time it's going in?

That definitely doesn't mean that it's on you to write all the tests as I think we need to re-think how we did them in CCv0 rather than just merge them straight over, so it will be on others (like myself, Wainer, Fabiano etc) to help/work on those, but do you think it's a fair request to deliver the feature and tests in the same PR?

Hey Steve, I think that’s fair and I agree that we should include the tests in the PR. As you know, the tests in CCv0 are currently in the tests repo. Do we have a plan to move them to the kata-containers repo? Please let me know if this has been planned already. I apologize for any oversight on my part.

[stevenhorsman]

Hey Steve, I think that’s fair and I agree that we should include the tests in the PR. As you know, the tests in CCv0 are currently in the tests repo. Do we have a plan to move them to the kata-containers repo? Please let me know if this has been planned already. I apologize for any oversight on my part.

No need to apologise. So the "plan" to move them to the kata-containers repo is to slowly drip them in as an when new features are added - for example this one as the first guest pull PR we want to move/re-write the basis guest pull tests (so no encrypted image/signed image support) over. As discussed on https://github.com/kata-containers/kata-containers/issues/8103 - we want to add some new tests for other variations - e.g. doing a runc pull follow by a kata-guest pull, doing pulls with the annotation off and then on etc.

The apologies are all mine that we haven't managed to nail down the definition of all the tests cases that we want both you are working with the dev code.

[wainersm]

hi @ChengyuZhu6 ! Thanks for working on this which is the foundation of the merge to main! I will be reviewing it today still...

[ChengyuZhu6]

@stevenhorsman @wainersm @fidencio I am currently working on migrating the test case Test can pull an unencrypted image inside the guest from tests repo to GHA. However, this is a new area for me. I would be very grateful if you could lend me your expertise on this.

[ChengyuZhu6]

@stevenhorsman @wainersm @fidencio @fitzthum @littlejawa In the PR, we used the variable GUEST_PULL to enable the guest pull feature by setting it to yes in the kata-agent. However, based on Fabiano’s suggestion, I have changed the variable name to PULL_TYPE. This way, we can use the same variable for both guest pull and host sharing features. For example, to enable guest pull, we can set PULL_TYPE to guest-pull. Similarly, to enable host sharing, we can set PULL_TYPE to host-sharing. I will implement host-sharing option for host sharing in PR .

Summarize briefly: to build the kata-agent with the guest-pull feature enabled, we can run the following command: sudo -E make PULL_TYPE=guest-pull

[littlejawa]

[...] Summarize briefly: to build the kata-agent with the guest-pull feature enabled, we can run the following command: sudo -E make PULL_TYPE=guest-pull

I think I'm confused with the use case (and I strongly suspect I'm missing something): we need to chose one or the other at build time? No way to support both, and let the decision come from the type of KataVirtualVolume we get? I understand why we'd want to disable all of that for kata-agent in a regular kata use case. But for kata-agent in a CoCo podvm, why make the distinction between guest pull and host sharing?

[ChengyuZhu6]

Split this PR in 2 different PRs.

One PR will be only related to changing the files under .github/workflows. There you add skeleton for the new items in the matrix, env-vars, and make sure those are no-op / skipped.

Once that PR is merged, we can actually start testing the code bits and the newly added tests.

Otherwise, this will get in without any test being actually exectued.

Thanks @fidencio. I have created a PR for test preparation.

[ChengyuZhu6]

I think I'm confused with the use case (and I strongly suspect I'm missing something): we need to chose one or the other at build time? No way to support both, and let the decision come from the type of KataVirtualVolume we get? I understand why we'd want to disable all of that for kata-agent in a regular kata use case. But for kata-agent in a CoCo podvm, why make the distinction between guest pull and host sharing?

@littlejawa Thank you for your questions. In theory, we can support both features by setting PULL_TYPE to host_and_guest or something similar when building the kata-agent. However, I do not recommend doing it now for several reasons.

First, the current nydus-snapshotter does not support both host-sharing and guest-pulling configurations. It can only work in either mode, not both. I think this is a limitation that can be overcome in future work, and I will work on it.

Second, enabling both features would make the rootfs bigger because of more dependencies. For example, the guest-pull feature requires the skopeo and umoci binaries to be included in the rootfs. The host-sharing feature requires the dm-setup binaries and related libraries to be included in the rootfs. Adding both binaries and libraries would increase the size of the rootfs. I am not sure that the bigger rootfs could work as expected.

Third, I think the containerd gap may cause a failure. The behavior of switching the mode from host-sharing to guest-pulling or from guest-pulling to host-sharing is like switching two remote snapshotters. This may confuse the containerd and lead to unexpected errors. There is an ongoing discussion about switching two remote snapshotters in this issue.

Last, the PR is focused on guest-pull, so we should only consider to enable guest-pull feature. If we want to support both features, we should create a new PR after both features are merged. I hope you understand my rationale.

[littlejawa]

@littlejawa Thank you for your questions. In theory, we can support both features by setting PULL_TYPE to host_and_guest or something similar when building the kata-agent. However, I do not recommend doing it now for several reasons.

[...]

Got it. And I agree with the current situation being kept as is, if only because of your last point : if we need it, we can always add it later :-)

I think my only concern is that the kata implementation is so related to containerd's implementation. The limitations you talk about for the snapshotter don't exist if we use crio for instance. I can picture crio being configured with the two possibilities available, and the user deciding which one to use for each workload. It is not there yet - I'm working on it - and for the moment, our only implementation is with containerd. But I feel that from kata's point of view, we should try to avoid assumptions on what the caller will be able to do.

Again, no change required. I just needed to ask this question for my own understanding :-)

[ChengyuZhu6]

Thanks for the great work. Though I am not quite familiar with kata, I am trying to give some constructive ideas upon the PR.

Thanks for your review @Xynnn007. I'll make some changes according to your comments.

[ChengyuZhu6]

I skimmed through this PR and I have several suggestions, but let me start with the most basic one that will allow things to actually be tested.

Split this PR in 2 different PRs.

One PR will be only related to changing the files under .github/workflows. There you add skeleton for the new items in the matrix, env-vars, and make sure those are no-op / skipped.

Once that PR is merged, we can actually start testing the code bits and the newly added tests.

Otherwise, this will get in without any test being actually exectued.

@ChengyuZhu6, thanks a lot for working on this!

Done.

[jodh-intel]

Hi, @ChengyuZhu6 - Please can you rebase to resolve the conflict?

[fitzthum]

@fidencio as I understand the pull-type is a limitation of the nydus-snapshotter itself. I agree that this is a pretty serious limitation, but if we are only supporting guest pulling for v0.9.0, I don't see it as a blocker here. We have talked about this problem a little bit in the past but I don't think we ever had an issue for it. It would be good to get a better picture of the upstream issues, as I recall they are kind of thorny.

[fidencio]

@fidencio as I understand the pull-type is a limitation of the nydus-snapshotter itself.

I do remember that the pull-type used on nydus is the one that it must be deployed, and that is a limitation. What I don't recall is why we'd bring that limitation to the agent as well, mainly at the build time.

... but if we are only supporting guest pulling for v0.9.0, I don't see it as a blocker here.

I don't think that's the case, peer-pods would still be using this after v0.9,0, AFAIR.

[fitzthum]

I don't think that's the case, peer-pods would still be using this after v0.9,0, AFAIR.

Ah I mean that in v0.9.0 we are only supporting guest pulling (I think?). We will support both after that.

[fidencio]

Ah I mean that in v0.9.0 we are only supporting guest pulling (I think?). We will support both after that.

I guess what's making me really confused here, @fitzthum, is that in the CCv0 branch we never had to have this feature in the agent as a build option. And I don't see why it'd be the case when moving this into main.

[fitzthum]

I guess what's making me really confused here, @fitzthum, is that in the CCv0 branch we never had to have this feature in the agent as a build option. And I don't see why it'd be the case when moving this into main.

Ok I think I misunderstood slightly. The question is not about guest pull vs host pull with nydus. It's about guest pulling vs standard kata host pulling. As you say in CCv0 we didn't have a build flag for this. We always included the image pulling code in the CCv0 agent, but we only used it is AA_KBC_PARAMS was set.

I think the idea in this PR is probably to avoid cluttering the non-CoCo kata agent with the image pull code, but I guess you are suggesting that we use the same agent binary for CoCo and Kata?

It is feasible to determine the behavior at runtime. The old approach of checking if AA_KBC_PARAMS is set is a little weird (maybe it would have been clearer if we'd had a separate flag to enable image pull). There has been some discussion of using InitData to setup an image-rs configuration file. This brings us back to the policy vs initdata discussion, but either way there is actually a bit of a snag. This setting is something we would definitely have to support on platforms that don't support initdata measurement. Maybe a kernel command line flag would be better after all? I'd have to think about it more.

[fidencio]

I think the idea in this PR is probably to avoid cluttering the non-CoCo kata agent with the image pull code, but I guess you are suggesting that we use the same agent binary for CoCo and Kata?

Yep, otherwise we'd have to have:

• kata-agent
• kata-agent-opa
• kata-agent-opa-guest-pull
• kata-agent-opa-host-pull

And for each kata-agent-opa-* we'd have to have:

• kata-containers-confidential-guest-pull.img
• kata-containers-confidential-guest-pull-initrd.img
• kata-containers-confidential-host-pull.img
• kata-containers-confidential-host-pull-initrd.img

And then test all the combinations of those, and this simply doesn't escale.

So, my suggestion here is, let's find a way, even if it's passing pull-type=foo either via policy or in the kernel command line (via agent.pull-type, which has to be introduced), and then make this a run time condition rathr and build time one.

It is feasible to determine the behavior at runtime. The old approach of checking if AA_KBC_PARAMS is set is a little weird (maybe it would have been clearer if we'd had a separate flag to enable image pull). There has been some discussion of using InitData to setup an image-rs configuration file. This brings us back to the policy vs initdata discussion, but either way there is actually a bit of a snag. This setting is something we would definitely have to support on platforms that don't support initdata measurement. Maybe a kernel command line flag would be better after all? I'd have to think about it more.

pull-type can be the name of the parameter passed, that would make things cleaner.

[fitzthum]

And then test all the combinations of those, and this simply doesn't escale.

I would expect to have at most one kata agent binary for CoCo. It would have OPA support and image-rs support (which would eventually support both host/guest pulling via the snapshotter). It's debatable whether this should also support the vanilla kata image pull.

For non-coco you might have two binaries depending on whether you want OPA. Neither one would have any of the image-rs stuff in it.

This doesn't seem untenable to me but I'm not against having a runtime flag.

We should maybe try to settle on some clearer names for these scenarios, mainly to avoid confusing the vanilla kata host pull with the nydus-snapshotter host pull.

[ChengyuZhu6]

Hi, @ChengyuZhu6 - Please can you rebase to resolve the conflict?

Done.

[ChengyuZhu6]

@fidencio Thank you for your comments. I would like to explain why I chose to build the agent with PULL_TYPE instead of building one agent for both Kata and CoCo. Then I will address your concerns.

I compared the compilation of an agent that supports guest pull with an agent that does not support guest pull. I found that the agent that supports guest pull requires additional downloads and compilation of a number of dependencies. You can see the details in the following two pictures. The first picture shows the compilation of an agent that does not support guest pull, and the second picture shows the compilation of an agent that supports guest pull. And the third picture show the size of these two kata-agent in bytes.

As you can see, the agent that supports guest pull will download and compile about two hundred more dependencies, which results in the agent that supports guest pull being twice the size of another agent. I am not sure whether this will cause any issues when merging the code into main. If there are no issues, I will make the changes to support direct compilation of agents that support guest-pull and kata-host-share.

If it is necessary to build an agent that supports guest-pull as an option, my approach in the PR is to export PULL_TYPE=guest-pull when building the confidential image/initrd that supports guest pull, and we would have kata-containers-confidential.img/kata-containers-confidential-initrd.img.

install_image_confidential() {
	export AGENT_POLICY=yes
	export MEASURED_ROOTFS=yes
	export PULL_TYPE=guest-pull
	install_image "confidential"
}
install_initrd_confidential() {
	export AGENT_POLICY=yes
	export MEASURED_ROOTFS=yes
	export PULL_TYPE=guest-pull
	install_initrd "confidential"
}

And build the agent tarball named kata-static-agent-confidential.tar.xz for coco. The agent built in this way will support both policy and guest pull.

install_agent_confidential() {
	export PULL_TYPE=guest-pull 
	install_agent_helper "yes"
}

[ChengyuZhu6]

cc @fitzthum

[ChengyuZhu6]

It's more work, but I do wonder if we should add all these agent options to https://github.com/kata-containers/kata-containers/blob/main/src/agent/README.md, or to a file like https://github.com/kata-containers/kata-containers/blob/main/src/runtime-rs/crates/shim/src/config.rs.in (but one with comments :) that we can point the README at.

@jodh-intel I have create a PR #9110 to add these agent options to README. I'll add proxy options to README when #9110 lands

[fidencio]

As you can see, the agent that supports guest pull will download and compile about two hundred more dependencies, which results in the agent that supports guest pull being twice the size of another agent. I am not sure whether this will cause any issues when merging the code into main. If there are no issues, I will make the changes to support direct compilation of agents that support guest-pull and kata-host-share.

This is definitely not optimal, but still better than having to select the agent pull-type at build time.

If you want to offer the option for users to only choose one specific pull-type, I'd be okay with that, as long as there's a pull-type=all option, and that's what we'll be using and shipping by default. Then if users want to optmise the size, they can do, but not at the cost of making our Ci more complex.

[ChengyuZhu6]

As you can see, the agent that supports guest pull will download and compile about two hundred more dependencies, which results in the agent that supports guest pull being twice the size of another agent. I am not sure whether this will cause any issues when merging the code into main. If there are no issues, I will make the changes to support direct compilation of agents that support guest-pull and kata-host-share.

This is definitely not optimal, but still better than having to select the agent pull-type at build time.

If you want to offer the option for users to only choose one specific pull-type, I'd be okay with that, as long as there's a pull-type=all option, and that's what we'll be using and shipping by default. Then if users want to optmise the size, they can do, but not at the cost of making our Ci more complex.

Done.

[ChengyuZhu6]

@stevenhorsman @liudalibj I modified the condition for the second and third tests in k8s-guest-pull-image.bats, so that they would pass. We can revert it if we use containerd 2.0 with the 'image pull per runtime class' feature in the future. Does that make sense to you?

[stevenhorsman]

@stevenhorsman @liudalibj I modified the condition for the second and third tests in k8s-guest-pull-image.bats, so that they would pass. We can revert it if we use containerd 2.0 with the 'image pull per runtime class' feature in the future. Does that make sense to you?

Hey Chengyu, I'm not sure if you were on the CoCo call ~last week~ yesterday, but I mentioned these test and the fact they would fail and that it would be a good point for us to assess them as a community and understand the limitation of the current solution. Given that in 0.8 we could just say we'd fall back to the legacy mode, which we don't have now I'd quite like them to be failing initially so that we can have the conversation wider about how we can document this limitation.