avast / pytest-docker

Docker-based integration tests
MIT License
423 stars 71 forks source link

Bring back the fallback when run without Docker #30

Closed petr-k closed 4 years ago

petr-k commented 5 years ago

Could you please bring back support for running without Docker & docker-compose removed in https://github.com/AndreLouisCaron/pytest-docker/commit/37901fba65759b1d4c7d78525b9590a6b96ce5a6? I was actually using that when running integration tests and it has significant value to me.

butla commented 5 years ago

The new version doesn't seem to be released yet. I was the one who proposed to delete that to simplify the code (which I want to refactor and extend later), because me and @AndreLouisCaron didn't see the use for this anymore. Also, I've never used that and I couldn't even figure why would anyone want to use this project if they don't have Docker... So can you explain your usecase / process? If it's still valid I could do a PR bringing it back and maybe describing it better.

petr-k commented 5 years ago

Sure. I was using this feature when running tests (which involve running docker-compose using your plugin) on a CI server that doesn't have anything installed besides Docker toolchain. That said, my objective is to execute those tests themselves inside a Docker container.

Even though you can have a docker-in-docker setup, it is rather involved to do and complicated things enormously. What I am doing instead is to package my application inside a Docker container using a docker-compose.yml and just override its entrypoint to call pytest in place of the normal main application file. But of course, since it now runs inside a Docker container without docker-compose installed inside, your plugin will complain. Allowing it to fallback and manually overriding hostname and port of the service (which does not change since it's being addressed within the docker-compose network) sorts out the issue.

I understand what I've just described might seem rather complicated, but I strongly believe it's a valid use case. I can prepare a simple example which might clarify might approach further.

butla commented 5 years ago

Ok, I pondered the fallback code some more after reading your answer and I think I'm getting it. @petr-k A working example I can run would be great, because I'd like to create a good, self-explanatory functional test for that feature if people are using it. And maybe document better when it's supposed to be used.

@AndreLouisCaron can you revert that commit?

butla commented 5 years ago

But then again... maybe doing that is not the best approach? @petr-k If I could see your example maybe I could advise on something better, that doesn't require configuring pytest-docker in a special way?

@AndreLouisCaron So maybe hold the revert for now :)

petr-k commented 5 years ago

Sure, but the high-level scenario is pretty simple: to be able to use pytest calling docker-compose, while the tests itself need to run inside a container.

petr-k commented 5 years ago

@butla I am curious as to what approach you'd take to achieve the above.

butla commented 5 years ago

@petr-k I'm fiddling with a docker-in-docker approach with the -v /var/run/docker.sock:/var/run/docker.sock approach from here and a CI image extending the application's image. It's not working for me at the moment, but I'll report my findings. Afterwards we may argue about what's the more elegant approach :D

Because from what I understand if you're using the fallback you need to explicitly map the port, e.g. 8080:8080 in docker-compose.yml, right? That's something I'm avoiding in my setups, so that I can have multiple Compose runs at the same time.

petr-k commented 5 years ago

/var/run/docker.sock:/var/run/docker.sock approach ...

That would work, except:

  1. Mounting /var/run/docker.sock might be disallowed in certain environments.
  2. More importantly, you still need to install docker client and docker-compose inside the image, which sucks.

Because from what I understand if you're using the fallback you need to explicitly map the port, e.g. 8080:8080 in docker-compose.yml, right?

No. Both the application (running pytest instead of its normal entrypoint) and its test-time dependencies (e.g. a PostgreSQL db) are being run via a single docker-compose up command, so they end up in the same network created by docker-compose, so ports can be still exposed at random numbers (or not exposed at all), so that you don't have any conflicts when running your tests on a CI server.

petr-k commented 5 years ago

See https://github.com/petr-k/pytest-docker-compose-ci

Please note that there's no actual code being tested, just some example "tests" that call PostgreSQL.

butla commented 5 years ago

OK, I see why the fallback is still valid. @AndreLouisCaron I think you should revert the commit in the end :) But it's good that we uncovered what is the exact usecase. I hope to document that better in the future and make the code more clear about this.

Still, having to run docker-compose yourself in addition to running pytest-docker seems clunky. For the environments that do support mounting docker.sock (@petr-k where you can't do that, BTW?) we could make it work automatically by:

@petr-k Why does adding the Docker and Compose in the image suck? From what I checked on python-alpine that makes the image go from 130 to 160 MBs, which isn't that terrible, I think. You are building an additional image for tests anyway.

With the approach I'd like the Dockerfile for the test image would look more or less like this:

FROM original_app_production_image

RUN apk add docker
RUN pip install docker-compose==1.21.2

# Dependencies of our test code, such as pytest and pytest-docker.
RUN pipenv sync --dev

ADD tests /app/tests

CMD ["pipenv", "run", "pytest", "-v", "tests/"]

Running the tests would look like this:

docker build -f ci/Dockerfile -t app_ci_tests . 
docker run --rm -v /var/run/docker.sock:/var/run/docker.sock app_ci_tests

But yeah, that's something for the future.

petr-k commented 5 years ago

Well, let's agree that clunkiness and suckiness is in the eyes of the beholder ;) I like the elegance of leveraging docker-compose's --exit-code-from.

I can only further clarify why I believe that approach is superior:

Yes, it also has downsides I can immediately see:

So yeah, to each his own I guess. I think the idea of autodetecting Docker, or having some kind of a simple switch is worth exploring. That said, this issue is just a suggestion. I understand that you don't want to support code you don't see use for. In that case, I'd have to move elsewhere or roll my own library, which I think would unnecessarily fragment the landscape.

petr-k commented 5 years ago

Let me say I'm very glad we've had this discussion, though! It's interesting to see how folks are tackling this (I'd say rather advanced) issue.

Btw, if you go the way you described in your example, won't you also have to handle the problem of determining the IP address where containers run as part of docker-compose are reachable? So I feel that you'd have to introduce some explicit support in your plugin to allow this scenario anyway.

butla commented 5 years ago

No, dude, what I said is that I see the validity of your way, and I told the maintainer to revert my changes :) You made a good point and I want to leave the fallback in. I just want to make it more uderstandable in the code in the future AND leave another option for people who wanna go my way. But they are both valid and have their tradeoffs. I think it'll be good to list both of those options in the documentation for people to use.

That is, after I have the chance to work on this project and finally get to refactoring/upgrading the core :) I hope I'll have more time in my upcoming job to polish, extend, and document this project :)

And not wanting to fragment the community, so that everyone has a great, versatile, shared, tool is precisely why I did so many maintenance commits here, even though my company used my fork of the project :) So I'm all with you.

And yeah, people need to have more public discussions about their advanced testing strategies. There's soo few good writeups about doing that in the post-lxc era :)

And what do you mean about IPs being reachable? Do some people actually use pytest-docker to run tests in distributed environments? If not, then the IP should be reachable. But If they do, that'd be very interesting :)