Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicit support for Windows containers #117

Closed
baronfel opened this issue Aug 12, 2022 · 7 comments
Closed

Explicit support for Windows containers #117

baronfel opened this issue Aug 12, 2022 · 7 comments
Labels
Area: Library Tasks and stories around the image and layer manipulation library Area: Task

Comments

@baronfel
Copy link
Member

baronfel commented Aug 12, 2022

When building a container for Windows we'll likely need:

  • different WorkingDirectory
  • different Entrypoints (though this may be handled with the better WorkingDirectory)
  • potentially different layer tarball shape (include registry hives and files differently, don't include drive letter root?)
@baronfel
Copy link
Member Author

baronfel commented Aug 15, 2022

Testing this requires specifying an explicit base image name, a good example is mcr.microsoft.com/dotnet/aspnet:7.0-nanoserver-ltsc2022. Right now this fails during ContentStore.PathForDescriptor because a mediaType of application/vnd.docker.image.rootfs.foreign.diff.tar.gzip is passed in.

@Danielku15
Copy link
Contributor

@baronfel
I made today various extensions and got a first version of Windows Containers running: https://github.com/Danielku15/sdk-container-builds/tree/fix-windows-packaging

The branch builds on top of #323 which is a prerequisite. Here the relevant commit/diff.

The main changes I added are similar as described in the initial post of this issue:

  1. Detection of Windows through RID.
  2. Reworked Layer.tar packaging as Windows layers are different:
    1. Drive Letter is removed from the container path
    2. All application files are placed within a Files/ subdirectory
    3. An empty Hives/ folder is added (required for Windows Layers, can hold Registry Hive Diffs but the Hives themselves are optional)
  3. Ensure we embed the Security Identifiers as PAX Extended Attributes into the Tar (otherwise the ContainerUser doesn't have access to the working dir)
  4. Set default WorkingDirectory differently based RID
  5. Remove Working Directory from entry point as it is not needed and causes troubles on the Path concat.

When writing some unit tests for the Layer structure I ran into a bug in the TarReader and likely the GitHub Actions need extension to run certain tests on a Windows environment. So overall it is rather a functional PoC than something that can be merged as it is.

Example run of the latest state. There you can see a publish of the same console app (with nothing special defined beside the nuget installed) being published first as linux image, then as windows image and both are running outputting the OS information.
image

@baronfel
Copy link
Member Author

baronfel commented Feb 6, 2023

This is great stuff! You're right that GitHub actions' windows runners don't include Docker (due to licensing) - this is why all of the RID-permutation tests are commented out save linux-x64. I'm not sure if MSTest has the equivalent of conditional theories or facts - I would really love to lean into making a data-driven XUnit Fact for example that could probe the environment and determine to skip a test or not.

The changes to TAR layout overall in that diff look good to me, and of course tests are always a great help to bring confidence in changes :)

@Danielku15
Copy link
Contributor

@baronfel What do you think would be the minimal implementations to ship an experimental support in one of the next releases? I am thinking of preparing some PR which brings in some essential changes for an early access feature.

Personally I am concerned about the automatic testing strategy. Some things we can likely test against "best knowledge" or "specification" (e.g. the layer structure) but not against a real docker/container env.

@baronfel
Copy link
Member Author

@Danielku15 I would be very happy to bring in anything along the lines of the delta you had on your other branch - we're starting work on absorbing this repo into the .NET SDK, and part of that work is moving to dedicated Azure DevOps runners for CI, and those runners do have Docker available on Windows machines/Windows machines that support running containers. When we do that we'd 'light up' specific test runs to verify that Windows containers are able to be authored by this tooling, and that Windows Container Runtimes are able to run the Windows Containers generated from this tooling. Does that make sense?

@Danielku15
Copy link
Contributor

Great then I will soon-ish start preparing a PR with at least the essential changes and some basic unit tests around the layer structure (to ensure this part doesn't break). I will not invest much at this point into an integration level test which would touch a container runtime.

@baronfel
Copy link
Member Author

Going to close this as @Danielku15 has implemented support for Windows Containers back in Februrary. Thanks @Danielku15!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Library Tasks and stories around the image and layer manipulation library Area: Task
Projects
None yet
Development

No branches or pull requests

2 participants