search

kata-containers / kata-containers

Kata Containers is an open source project and community working to build a standard implementation of lightweight Virtual Machines (VMs) that feel and perform like containers, but provide the workload isolation and security advantages of VMs. https://katacontainers.io/
Apache License 2.0
5.08k stars 1k forks source link

sandbox: fix the issue of double initial_size_manager config #9836

Closed lifupan closed 1 week ago

lifupan commented 1 week ago

It shouldn't call the initial_size_manager's setup_config in the load_config since it had been called in the sandbox's try_init function.

Fixes: #9778

lifupan commented 1 week ago

lgtm thanks a lot @lifupan !

As for your question on the issue #9778: yes it does remove the double-add of memory limit so that's definitely a step ahead.

Now the VM is booted with default_memory + memory-limit (ie. 2G + 5G = 7G with reference to the concrete numbers I mentioned in the issue) and doesn't hotplug anything. I'd like to confirm that this is the expected and correct behaviour.

Originally I kind of expected memory-limit to be hot-plugged though thinking about it now, I can see how it makes sense to cold-plug it. I'm wondering under which circumstances memory is truly hot-plugged in runtime-rs ("truly" as in "resize_memory() is called with an argument that differs from the current VM memory"). Thanks in advance for any insights!

Hi @pmores

Generally speaking, the container's memory resource limit would be hotplugged since most of the time when creating the container, the VM had been launched. The containers created using the k8s/crictl are most of this case. But for case of created with ctr/nerdctl client, the container and the VM would be created/launch at the same stage, thus there's an optimization and calculated the total memory and did a cold plug.

pmores commented 1 week ago

Thanks @lifupan , that make a lot of sense and I do see container resources hotplugged.