Support for Block CIMs #2261

ambarve · 2024-09-09T17:34:04Z

This PR adds support for creating, mounting and using block CIMs for running windows process isolated containers.
Individual commits provide additional details on what changes they bring in.

internal/hcs/schema2/cimfs.go

pkg/cimfs/cimfs.go

pkg/cimfs/cim_writer_windows.go

internal/winapi/cimfs.go

pkg/cimfs/cim_writer_windows.go

pkg/cimfs/cimfs.go

pkg/cimfs/cim_writer_windows.go

pkg/cimfs/mount_cim.go

pkg/cimfs/cim_writer_windows.go

internal/wclayer/cim/block_cim_writer.go

apurv15 · 2024-11-12T08:54:08Z

internal/wclayer/cim/process.go

@@ -34,10 +33,6 @@ func processBaseLayerHives(layerPath string) ([]pendingCimOp, error) {
 	}

 	hivesDirInfo := &winio.FileBasicInfo{
-		CreationTime:   windows.NsecToFiletime(time.Now().UnixNano()),


What would be the values of these Times when not set explicitly here?

I believe it will be January 1, 1601 as mentioned here.

internal/wclayer/cim/forked_cim_writer.go

pkg/cimfs/cimfs.go

pkg/cimfs/cim_writer_windows.go

pkg/cimfs/mount_cim.go

internal/wclayer/cim/block_cim_writer.go

kevpar · 2024-11-14T03:34:23Z

It looks like we leak stdFileWriter.root

internal/wclayer/cim/common.go

internal/layers/helpers.go

pkg/cimfs/cim_test.go

kevpar · 2024-12-13T10:36:09Z

With the recent quality focus, I would spend some time thinking about how to expand test coverage of this work. For instance, adding tests for parsing of WCOW layers seems like a really easy one.

ambarve · 2024-12-13T14:57:17Z

With the recent quality focus, I would spend some time thinking about how to expand test coverage of this work. For instance, adding tests for parsing of WCOW layers seems like a really easy one.

Yeah, I can add those. In a separate PR though, right? I don't know if we should keep this PR open until those tests get added & reviewed.

kevpar · 2024-12-13T19:39:39Z

With the recent quality focus, I would spend some time thinking about how to expand test coverage of this work. For instance, adding tests for parsing of WCOW layers seems like a really easy one.

Yeah, I can add those. In a separate PR though, right? I don't know if we should keep this PR open until those tests get added & reviewed.

I will leave it up to you to do in a future PR, but please try to do it soon.

kevpar

LGTM

Currently we have a map which maintains a mapping of CIM & containerd ID to the volume at which a CIM is mounted for the given container. This was required before the layer refactoring work when we needed to get the volume path from the layer cim path. However, this isn't needed anymore. As of now, this map doesn't provide much value and makes the code a bit complicated. Moreover, we will need to rewrite some of this code anyway when we do the work required for handling `shim delete` cleanups properly (containerd/containerd#9727). Signed-off-by: Amit Barve <[email protected]>

CimFS now supports a new format for storing CIMs, named BlockCIM. A block CIM format can store the entire CIM on a block device (like a VHD) or a file formatted like a block device. This commit adds Go wrappers for the new CimFS APIs that allow creation, merging and mounting of such Block CIMs. Some new flags required when creating and mounting these CIMs are added and some deprecated flags have been removed. New type has been introduced to represent a block CIM. Unit tests have been added to test the newly added CimFS functionality. Lastly, CimFS flags aren't a part of the hcs schema (only the CimMount request is), those flags are moved from the hcs/schema2 package to the cimfs package. Signed-off-by: Amit Barve <[email protected]>

This commit adds a layer writer that can be used for extracting an image layer tar into a Block CIM format. Existing forked CIM layer writer was renamed to a common base type `cimLayerWriter`. Forked CIM layer writer & Block CIM layer writer both now extend this common base type to write layers in that specific format. This commit also removes some code that used `time.Now()` as the default timestamps for some files that it creates within the layer CIM. These timestamps cause differences in the layer CIMs generated from the same layer tar. This change fixes that. Signed-off-by: Amit Barve <[email protected]>

This commit adds the ability to parse block CIM layer mounts and to mount the merged block CIMs to be used as a rootfs for a container. Signed-off-by: Amit Barve <[email protected]>

ambarve force-pushed the blocked_cim_import branch 3 times, most recently from ad3aed4 to 2afb976 Compare September 10, 2024 20:48

ambarve changed the title ~~Blocked cim import~~ Support for Block CIMs Sep 10, 2024

msscotb assigned msscotb and kevpar Sep 11, 2024

ambarve marked this pull request as ready for review September 11, 2024 18:17

ambarve requested a review from a team as a code owner September 11, 2024 18:17

kevpar reviewed Sep 16, 2024

View reviewed changes

internal/hcs/schema2/cimfs.go Outdated Show resolved Hide resolved

kevpar reviewed Sep 16, 2024

View reviewed changes

pkg/cimfs/cimfs.go Outdated Show resolved Hide resolved

kevpar reviewed Sep 16, 2024

View reviewed changes

pkg/cimfs/cim_writer_windows.go Show resolved Hide resolved