Skip to content

Introduce the Windows lcow diff/snaphotter#2583

Merged
dmcgowan merged 1 commit intocontainerd:masterfrom
jterry75:snapshotter_lcow
Aug 28, 2018
Merged

Introduce the Windows lcow diff/snaphotter#2583
dmcgowan merged 1 commit intocontainerd:masterfrom
jterry75:snapshotter_lcow

Conversation

@jterry75
Copy link
Copy Markdown
Contributor

Implements the Windows lcow differ/snapshotter responsible for managing
the creation and lifetime of lcow containers on Windows.

Signed-off-by: Justin Terry (VM) juterry@microsoft.com

@jterry75
Copy link
Copy Markdown
Contributor Author

Example Usage:

ctr.exe image pull --platform linux/amd64 --snapshotter windows-lcow docker.io/library/alpine:latest
ctr run --runtime io.containerd.runhcs.v1 --snapshotter windows-lcow --config "<path-to-bundle>\config.json" --rm docker.io/library/alpine:latest ar1 sh -c "echo Hello World!"

For now the --config is required because the oci section builds default platform specs based on the compiled GOOS target a subseqent PR will merge these to allow a Windows client to build a default Linux spec. But for now a --config with a Linux bundle works fine with ctr.exe

@jterry75
Copy link
Copy Markdown
Contributor Author

@crosbymichael - FYI

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating the scratch image only needs to be done once right? 🙀

How quick would this normally be when there is no contention? I am wondering if this is something that could be done when the snapshotter is created.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Takes ~10s on my machine. We certainly could do it at create time but everyone would then have to pay the price of this creation on Windows even if you weren't going to use LCOW right?

Copy link
Copy Markdown
Member

@dmcgowan dmcgowan Aug 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jterry75 ok, then it is in the right place, that is about 3 orders of magnitude longer than I was thinking when suggesting it on create :) This could be done asynchronously after create if we want to optimize this for creation time, but let's not worry about this now. Mentioning the time in a comment and possible solutions may be good here though.

@dmcgowan
Copy link
Copy Markdown
Member

This implementation is really clean, very encouraging!

I think something needs to added in the mount package to at least detect and error on this new mount type. I don't think there is anything else we can reasonably do there.

For compare, will this be implemented by runhcs.exe at some point? If buildkit is to support this, it will probably require more thought but this part looks straightforward to support.

Also appveyor's failure looks related

@jterry75
Copy link
Copy Markdown
Contributor Author

@dmcgowan - No prob will add a block to mount to avoid anything that contains an LCOW layer. That's a good idea.

For compare yes you are correct it will be runhcs.exe that does this. Given the mount's we will start a UVM in order to parse/create the tar in the guest and then stream that back to the content store.

Looking into CI

Implements the Windows lcow differ/snapshotter responsible for managing
the creation and lifetime of lcow containers on Windows.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
@crosbymichael
Copy link
Copy Markdown
Member

LGTM

@dmcgowan
Copy link
Copy Markdown
Member

LGTM

For notes, a couple follow ups that we might want to consider

  • Asynchronously create the scratch after create, maybe use sync.Once or something to create and set status
  • Always use ErrNotImplemented when encountering unknown mounts, this would apply to mount and differ. The intention with having multiple differs is the service would determine when the input is invalid when it cannot find an implementation which can handle the mounts.

@dmcgowan dmcgowan merged commit 29eab28 into containerd:master Aug 28, 2018
@jterry75 jterry75 deleted the snapshotter_lcow branch August 28, 2018 22:23
@jterry75
Copy link
Copy Markdown
Contributor Author

Will add these to my notes for a follow up

@olljanat
Copy link
Copy Markdown
Contributor

@jterry75 should those example commands still works in nowadays version?
Because for me it looks that they don't. Not either in latest 2.1.x or 1.7.x version.

Docker code still has LCOW v1 included as moby/moby#42170 is not merged and I would like to investigate if we can get LCOW v2 working as nowadays containerd integration with snapshotter exists.

So in theory as long Docker daemon.json looks like this:

{
  "default-runtime": "io.containerd.runhcs.v1",
  "features": {
    "containerd-snapshotter": true
  },
  "exec-opts": [
    "isolation=hyperv"
  ]
}

and code integrating with containerd would be updated to use same logic than CRI uses it should be possible to get Docker + LCOW v2 combination working.

@jterry75
Copy link
Copy Markdown
Contributor Author

@jterry75 should those example commands still works in nowadays version? Because for me it looks that they don't. Not either in latest 2.1.x or 1.7.x version.

Docker code still has LCOW v1 included as moby/moby#42170 is not merged and I would like to investigate if we can get LCOW v2 working as nowadays containerd integration with snapshotter exists.

So in theory as long Docker daemon.json looks like this:

{
  "default-runtime": "io.containerd.runhcs.v1",
  "features": {
    "containerd-snapshotter": true
  },
  "exec-opts": [
    "isolation=hyperv"
  ]
}

and code integrating with containerd would be updated to use same logic than CRI uses it should be possible to get Docker + LCOW v2 combination working.

Hey, I haven't worked on this for some time, but I don't think LCOW is officially supported publicly. Where are you getting the VM images and tooling to go along with containerd+shims?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants