Implement early cloning for UVM#787
Implement early cloning for UVM#787ambarve wants to merge 7 commits intouser/ambarve/fix_multivhd_wcow_bugfrom
Conversation
inside the UVM. We generate a unique mount point (inside UVM) for every such mount that is requested by a container. However, this breaks when two container try to mount the same VHD because then we generate two unique paths for the same VHD and try to mount it at two different locations inside the UVM. This change fixes that issue for WCOW containers.
|
I'm not sure I understand why your PR is towards |
|
@katiewasnothere This PR is mostly created for feedback purposes, I don't think we will actually merge this PR. |
ambarve
force-pushed
the
user/ambarve/fix_multivhd_wcow_bug
branch
from
de42338 to
13588ad
Compare
| // TODO (ambarve): The third return value of this function (the template id) | ||
| // is only needed until we implement the late cloning part after that this return | ||
| // value can be removed. | ||
| func createPod(ctx context.Context, events publisher, req *task.CreateTaskRequest, s *specs.Spec) (shimPod, error, string) { |
There was a problem hiding this comment.
The error is always the last return value by convention. Even if you plan to remove this later it's best to follow this convention.
| PseudoOplocks: true, | ||
| TakeBackupPrivilege: true, | ||
| NoLocks: true, | ||
| PseudoDirnotify: true, |
There was a problem hiding this comment.
Are these settings we always want on or just when we're running with cloning?
|
|
||
| uvm.scsiLocations[0][0].hostPath = doc.VirtualMachine.Devices.Scsi["0"].Attachments["0"].Path | ||
|
|
||
| fullDoc, err := mergemaps.MergeJSON(doc, ([]byte)(opts.AdditionHCSDocumentJSON)) |
There was a problem hiding this comment.
We should take a moment to remove this merging code (as a separate commit that we can PR immediately). As far as I know it's not used anywhere and it just makes things more complicated.
|
|
||
| // LoadTemplateConfig loads a persisted template config from the registry that matches | ||
| // `templateID`. If not found returns `regstate.NotFoundError` | ||
| func LoadPersistedUVMConfig(ID string) (*PersistedUVMConfig, error) { |
| return err | ||
| } | ||
|
|
||
| srcVhdPath := templateConfig.VirtualMachine.Devices.Scsi["0"].Attachments["0"].Path |
There was a problem hiding this comment.
I wonder if it would be cleaner to store a shim-specific JSON struct just with what you need (in this case, just the UVM sandbox VHDX path) instead of the whole HCS configuration. This is fine for now, but for late clone you'll need to be more clever anyway, since with late clone we will perform hot adds on the template VM to start up the container, but the clone VM will need to have all the configuration passed up front. So you'll need some way to keep track of which resources have been added to the UVM and be able to transform them to HCS configuration transformations... and in some of those cases you'll need to clone the resources before applying them.
I wonder whether Danny's resource interface should have a Clone method (for copying/diffing VHDs) and MarshalJSON and UnmarshalJSON methods (for transferring the list to the new shim) so that we can unify this clone stuff with the resource stuff we already have. Maybe these could be on an extended UVMResource interface or something that only gets used for resources at the UVM level. Just a thought.
ambarve
force-pushed
the
user/ambarve/fix_multivhd_wcow_bug
branch
from
f776086 to
76286f9
Compare
|
@ambarve is this PR still relevant? Can we close it out? |
4 participants
This PR features a set of changes that implement early cloning for WCOW UVMs.
In order to support cloning of Pods with containers created inside them (late cloning) first we need to be able to clone the Pods (i.e the UVMs). This PR adds the set of functions required for creating a pod as a template which can be later used for creating clones. Currently we don't have any good way of reporting these template pods to cotainerd or CRI so the tests don't work well yet.
(created this PR towards
fix_multivhd_wcow_bugbranch so that changes from that branch don't show up in the diff)