community.docker icon indicating copy to clipboard operation
community.docker copied to clipboard

Windows Container support

Open tmeckel opened this issue 3 years ago • 17 comments

SUMMARY

This PR contains changes to docker.py to support Windows Containers.

ISSUE TYPE
  • Feature Pull Request

tmeckel avatar Mar 01 '21 20:03 tmeckel

@felixfontein Thank for the quick reply on my PR! I changed the code and added a changelog fragment.

tmeckel avatar Mar 02 '21 10:03 tmeckel

@felixfontein Why are the RHEL 8.2 tasks in the CI pipeline are failing. Is this relevant to my PR?

tmeckel avatar Mar 02 '21 20:03 tmeckel

@tmeckel the RHEL issues are unrelated to this PR, there seems to be something wrong with the setup of the VMs. I've pinged someone to investigate.

felixfontein avatar Mar 02 '21 20:03 felixfontein

The RHEL will be fixed by #97.

felixfontein avatar Mar 02 '21 21:03 felixfontein

Tests should now pass. Let's see :)

felixfontein avatar Mar 02 '21 21:03 felixfontein

recheck

felixfontein avatar Mar 04 '21 19:03 felixfontein

@briantist @jborean93 I plan to create a new release of community.docker on Monday or Tuesday next week (so it can get included in Ansible 3.1.0). If you don't manage to review the PR until then, I can also create another release once this is merged, so no pressure ;)

felixfontein avatar Mar 05 '21 11:03 felixfontein

I plan to create a new release of community.docker on Monday or Tuesday next week (so it can get included in Ansible 3.1.0). If you don't manage to review the PR until then, I can also create another release once this is merged, so no pressure ;)

I'm not sure I'll be able to actually run this to try it out unfortunately, certainly not before your next release. I'll see if I get a chance to try it out at some point

briantist avatar Mar 06 '21 18:03 briantist

I will highly recommend that tests be added as soon as possible, there are a few private functions and edge cases with Windows that are hard to keep track off without tests actually testing those scenarios.

There's one big problem with Windows container tests: someone has to create and maintain them. Since Windows containers cannot run on Linux, this requires the tests to run on Windows. Our CI (AZP) theoretically does support that, but we've never run tests on Windows for any of the docker plugins and modules yet. So this is far from simple to set up.

felixfontein avatar Mar 11 '21 07:03 felixfontein

Totally understand that problem, at a quick glance you could do what we did for testing SSH against Windows hosts. Still spin up an AWS EC2 instance for Windows Server 2019 and install the Containers feature and a running container. I'm not sure if it's possible to even expose the Docker port over the internet like that so that might be a no go. Ultimately it's up to you as the maintainers what you would like to have. I just know this is going to be painful to support going forwards without some kind of tests around it.

jborean93 avatar Mar 11 '21 07:03 jborean93

If this were to be accepted, I have a CI pipeline for my organisation which would make use of these utilities for docker swarm creation and more. We use WinRM and Windows Server 2019 predominantly but resources to work with others since idempotent IasC is in it's infancy here.

I was actually on my way to find out the views of me PRing the community.docker.docker_swarm module to support Windows!

Therefore, watching this and stand ready to assist.

jimbo8098 avatar Mar 17 '21 12:03 jimbo8098

@jimbo8098 I'm happy to merge this (once all review comments are addressed) also without Windows tests. We (or in particularly I :-) ) don't mind Windows-specific PRs - as long as they don't break other platforms.

It would be great to have Windows tests in this repository as well; as long as we don't have them, Windows support in the modules is "use on your own risk, we try not to break it, but we won't notice in case we do". If your CI pipeline uses say the main branch of this collection, you would notice early if something breaks and you could create an issue (or even a PR to fix it), so that releases should usually work for Windows :)

felixfontein avatar Mar 17 '21 13:03 felixfontein

Just to add to my previous point, Our infrastructure is Windows Server 2019 hardware, not containers. I suspect the two should be the same in a CI environment if you are just building towards testing infra for this though some mechanisms of course might be a bit different. Will look to PR on docker_swarm for the moment but I'll be watching this.

jimbo8098 avatar Mar 17 '21 13:03 jimbo8098

@tmeckel do you have time to incorporate the latest review comments? It would be great if we could get this merged.

felixfontein avatar Apr 05 '21 10:04 felixfontein

@felixfontein sorry for the delay. Found some time to work on the comments of @jborean93

tmeckel avatar Apr 17 '21 19:04 tmeckel

@tmeckel ping

felixfontein avatar May 04 '21 06:05 felixfontein

@felixfontein added comments. Awaiting answers from @jborean93

tmeckel avatar May 05 '21 19:05 tmeckel

Docs Build 📝

This PR is closed and any previously published docsite has been unpublished.

github-actions[bot] avatar Nov 03 '23 07:11 github-actions[bot]