diff: implement windows layer support for linux #510
Conversation
Yes, but we would probably need to use labels since the differ and applier operations today are based off media type. Can you file an issue on containerd and we can discuss how it should be implemented in containerd and find someone interesting in implementing. |
source/containerimage/pull.go
Outdated
| if platform := p.Puller.Platform; platform != nil { | ||
| if platform.OS == "windows" && runtime.GOOS != "windows" { | ||
| ctx = winlayers.UseWindowsLayerMode(ctx) | ||
| layerTypeWindows = true |
There was a problem hiding this comment.
if platform.OS == "windows" && runtime.GOOS == "windows" then layerTypeWindows is false, which is confusing. I'm trying to think of a better name: layerNeedsTypeWindows, layerNeedsMarkTypeWindows or needsMarkTypeWindows ... Otherwise maybe just add a comment at definition.
source/containerimage/pull.go
Outdated
| pulled, err := p.Puller.Pull(ctx) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if pulled.ChainID == "" { | ||
| return nil, nil | ||
| } | ||
| return p.CacheAccessor.GetFromSnapshotter(ctx, string(pulled.ChainID), cache.WithDescription(fmt.Sprintf("pulled from %s", pulled.Ref))) | ||
| ref, err := p.CacheAccessor.GetFromSnapshotter(ctx, string(pulled.ChainID), cache.WithDescription(fmt.Sprintf("pulled from %s", pulled.Ref))) |
There was a problem hiding this comment.
nit: no need for Sprintf: "pulled from " + pulled.Ref
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
tonistiigi
force-pushed
the
win-differ-support
branch
from
9bb10fd to
bc76586
Compare
| return ocispec.Descriptor{}, errors.Wrapf(errdefs.ErrNotImplemented, "unsupported diff media type: %v", desc.MediaType) | ||
| } | ||
|
|
||
| var ocidesc ocispec.Descriptor |
There was a problem hiding this comment.
nit: variable of same type already defined in return value d. Rename d to ocidesc ?
| func addSecurityDescriptor(h *tar.Header) { | ||
| if h.Typeflag == tar.TypeDir { | ||
| // O:BAG:SYD:(A;OICI;FA;;;BA)(A;OICI;FA;;;SY)(A;;FA;;;BA)(A;OICIIO;GA;;;CO)(A;OICI;0x1200a9;;;BU)(A;CI;LC;;;BU)(A;CI;DC;;;BU) | ||
| h.PAXRecords[keySDRaw] = "AQAEgBQAAAAkAAAAAAAAADAAAAABAgAAAAAABSAAAAAgAgAAAQEAAAAAAAUSAAAAAgCoAAcAAAAAAxgA/wEfAAECAAAAAAAFIAAAACACAAAAAxQA/wEfAAEBAAAAAAAFEgAAAAAAGAD/AR8AAQIAAAAAAAUgAAAAIAIAAAALFAAAAAAQAQEAAAAAAAMAAAAAAAMYAKkAEgABAgAAAAAABSAAAAAhAgAAAAIYAAQAAAABAgAAAAAABSAAAAAhAgAAAAIYAAIAAAABAgAAAAAABSAAAAAhAgAA" |
There was a problem hiding this comment.
however obvious it can seem from the encoded string, it would be nice to tell readers that it is using base64. Apparently this is a base64 of raw binary stuff, not the human readable string in the comment.
split from #499 for easier review
The tar blobs containing the layers have a different structure in linux and windows. The current differ ignores that and hopes that the differ is compatible with the blob.
For example, linux blobs directly contain the rootfs for the container while windows container have a structure like
Files/,Hives/,UtilityVMwith theFilesdirectory containing the actual rootfs. Additionally, the records in the windows layer tarball need to contain specific PAX headers that are required for the files to be accessible from the container.In order to allow cross-build from one os to another, this PR enables additional features in the differ that enable it to apply and diff windows layers on linux, with existing linux snapshotters. On apply only contents of
Files/is extracted and on diff additional directory structure is added with the required header values.@dmcgowan @AkihiroSuda Do you think we could eventually add this functionality directly to containerd so that BuildKit does not need to override the differ/applier for this case?
Signed-off-by: Tonis Tiigi tonistiigi@gmail.com