Add cross-platform support for running commands in Docker

[starlord-daniel]

Problem to be solved

testinfra docker backend should support the Windows Docker container platform testing in addition to the existing Linux functionality.

Currently, only Linux containers can be executed as a backend. This issues the lack of support for running Windows container backends.

Suggested Approach

Currently, if we look at backend/docker.py the implementation of the docker exec command is fixed using /bin/sh. This needs to be extendable for other platforms as e.g. powershell or others.

The suggested approach would be encapsulating the docker command as function, similar as this:

def docker_command(self, terminal, command):
    command_items = []
    if self.user is None:
        command_items = ["docker exec", "-u", self.name, terminal, "-c", command]
    else:
        command_items = ["docker exec", "-u", user, "-u", name, terminal, "-c", command]
    return " ".join(command_items)

def run(self, command, *args, **kwargs):
        cmd = self.get_command(command, *args)
        out = self.run_local(
            docker_command(self.user, self.name, terminal, cmd))
        out.command = self.encode(cmd)
        return out

This also requires updating the implementation of backend/base.py to be extendable in a similar fashion.

@philpep Let me know what you think, if this makes sense to you. I will add the PR introducing the suggested functionality.

See following Issues to similar problems:

• #532
• #516

[ssbarnea]

In fact it may also work with podman too. Feel free to experiment and propose a PR.

[aheumaier]

While working on a PR for Docker backend we skip touching the run() signature since this inherited from class BaseBackend: and means updating all child class signatures too. We think this would be a cleaner solution though like: def run(self, command, console, *args, **kwargs):

Any suggestions of a prefered solution ? @ssbarnea @philpep

[ssbarnea]

I think should not spare the opportunity to advertise my last weekend brew https://pypi.org/project/subprocess-tee/ -- .

I created it mainly for molecule to cover for the missing features of subprocess. See if you can get any inspiration out of it, maybe we endup with a generic run extension for dealing with processes run inside containers, with various backends.

I personally found a good idea to build on top of subprocess.run while keeping the API compatible (keeping return type and existing args, but adding new ones).

[philpep]

Hi @aheumaier

I don't have code fully in mind and known nothing about windows & powershell. But my advice would be either to

• add a shell or terminal parameter to existing docker backend
• add a new docker+windows backend.

Maybe adding a new backend is a better choice to handle windows because we could easily handle other issues specific to this backend (quoting, encoding etc..).

Then we could implement some auto-detection feature making "docker://host" backend would run some detection code to return an instance of the specific class (just like we do with modules).

[dariuszparys]

The current state of the project does not allow any Windows developer easily participate and contribute. There are a couple of challenges you face as a windows developer

1. Tests requirements don't install correctly on Windows only

If you execute tox you'll encounter errors during requirements installation. I attached a log file from a Python 3.6 environment run. py36-1.log

2. Tests don't run through on Windows / WSL2

You can use on WSL2 which allows you to run any linux distribution inside of windows. It also supports docker and all tests are passing with the exception of one test which is checking for the iptables and tries to communicate with systemd which is not present on WSL2.

3. No support for Windows containers

The current test run does not acknowledge multiple platforms. All containers are linux based and can run on the same host. In order to support Windows testing it would be necessary to provide a windows container which needs to run on a windows platform. There are several solutions for this, any CI pipeline system should be able to spawn multiple of those target platforms where containers can be created.

So even point 1. and 2. is not working to 100% for a windows developer, you always have the ability to develop the features for Windows on linux. Point 3 is the real blocker in my opinion. The testing part is a problem as it is currently the missing piece which blocks any meaningful PR for new functionality (as it won't be tested).

@philpep can you share your thoughts / plans on a possible testing roadmap to include windows containers or windows target build systems?

[ssbarnea]

I would not oppose adding support for Windows as github allows testing with Windows hosts but keep in mind: as far as I know none of the core developers is using Windows so you will be on your own.

I would personally not mind helping with reviews. Start by enabling one windows target on github pipeline and start adding fixes there.

Feel free to add Windows support but don't bother creating bugs for it, at least until we get the same level of coverage on CI.

[philpep]

Totally agree with @ssbarnea . First step is indeed to make some kind of windows CI working (cf https://github.com/pytest-dev/pytest-testinfra/issues/429). Then If you plan to work on this, don't assume I understand how windows works because I'm totally ignorant, so explain the more you can in commit messages.

About installing tests-requirements.txt, looks an issue with ansible installation. I would check if it's a bug or if ansible is supposed to be installable on windows. If not there a syntax to prevent the installation:

ansible>=2.8,<3; sys_platform != 'win32'

And tests suite can be modified to not run ansible related tests when ansible is not installed.

[PromoFaux]

In case it is of use to anyone, we're just using this to use bash as the shell, instead of the hardcoded sh on the host.run for docker backend:

https://github.com/pi-hole/pi-hole/commit/79c3de0d576e80257fca3649f39cc2d86a5ea70f

Not Ideal, but the scripts we're testing require bash (until we get around to converting them to remove bashisms... one day maybe)