community.docker
community.docker copied to clipboard
Windows Container support
SUMMARY
This PR contains changes to docker.py
to support Windows Containers.
ISSUE TYPE
- Feature Pull Request
@felixfontein Thank for the quick reply on my PR! I changed the code and added a changelog fragment.
@felixfontein Why are the RHEL 8.2 tasks in the CI pipeline are failing. Is this relevant to my PR?
@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.
The RHEL will be fixed by #97.
Tests should now pass. Let's see :)
recheck
@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 ;)
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
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.
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.
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 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 :)
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.
@tmeckel do you have time to incorporate the latest review comments? It would be great if we could get this merged.
@felixfontein sorry for the delay. Found some time to work on the comments of @jborean93
@tmeckel ping
@felixfontein added comments. Awaiting answers from @jborean93
Docs Build 📝
This PR is closed and any previously published docsite has been unpublished.