moodlehq / moodle-docker

A docker environment for moodle developers
GNU General Public License v3.0
373 stars 244 forks source link

Container moodle-extests fails healthcheck when used with this repo #200

Closed terrycampbell closed 2 years ago

terrycampbell commented 2 years ago

The Problem

When launching a stack using this repo, the moodle-exttests container shows as unhealthy. image

The Cause

The healthcheck for moodle-exttests is

HEALTHCHECK --interval=5s CMD curl -f http://localhost/ || exit 1

i.e., it is simply trying to access the internal webserver at the default port. However, this repo sets the listening port of the container to 9000 (see assets/exttests/apache2_ports.conf, causing the curl request to fail.

Recommendation

Do not bother changing the listening port, keep it at 80 and eliminate the extests apache config files and modify base.yml to eliminate the volumes. Change the Moodle config file to set TEST_EXTERNAL_FILES_HTTP_URL = 'http://exttests'.

This will expose the exttests port 80 on the docker network created for the stack, at hostname exttests, and it will not conflict with port 80 of the webserver or any other container.

stronk7 commented 2 years ago

Hi @terrycampbell,

well spotted! I didn't remember that we were changing the port that way here, at moodle-docker! Will try to check why we decided to do that (maybe that's some old docker limitation or so).

Anyway, I'll try to fix this and #178 together, thanks for the report!

stronk7 commented 2 years ago

Ah, I see, this comes from #172 (and consequent PR), so, in order to support podman (that only allows one service by port, no matter they are networked):

https://github.com/containers/podman-compose/issues/224

I've seen in that issue (final comment) that, maybe, that was fixed some time ago. Just pinging to @dahrens , in case he can confirm if now it's possible for podman to have 2 containers internally using the same port or no.

Depending on that... we'll follow one solution (remove the port 9000) or another (remove the health-check from the image).

Ciao :-)

terrycampbell commented 2 years ago

Third option, change the healthcheck to use an Environment variable for the listening port and pass that in on launch.

Forth option, have the container listen on 9000 and 80 (do not expose 80). The loopback should allow the healthcheck to succeed. I will do some testing today.

Hmmm, my (admittedly cursory) reading on podman.io seems to indicate container-container communication is possible without publishing ports using the containers internal IP (and podmans dns feature should allow name use). Podman compose also states that containers can resolve themselves by container name, which sounds just like dockers internal dns capability.

I will be interested in finding out what you discover. Meanwhile I am going to install podman and do some testing for myself 😊

stronk7 commented 2 years ago

Yesterday I was thinking that, maybe... all we needed is to get the port from current configuration and done. Something like this may work for both standalone Docker instance and configured externally one:

cat /etc/apache2/ports.conf | grep '^Listen' | cut -d ' ' -f 2

Disclaimer, not tested!

And yes, from podman issues and docs, it seems that inter-container comms are ok, but I became lost reading yesterday.

stronk7 commented 2 years ago

Just to confirm, I've changed the container healthcheck to:

diff --git a/Dockerfile b/Dockerfile
index 516b8dd..f9c99d0 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -2,4 +2,4 @@ FROM php:7.1-apache

 COPY . /var/www/html

-HEALTHCHECK --interval=5s CMD curl -f http://localhost/ || exit 1
+HEALTHCHECK --interval=5s CMD curl -f http://localhost:$(cat /etc/apache2/ports.conf | grep '^Listen' | cut -d ' ' -f 2) || exit 1

And it seems to be working perfectly, both standalone and compose ones. Both are shown as healthily (curiously both say PORT 80, no matter the compose one is running @ 9000). I imagine that comes from the image meta-information.

$ docker ps | grep exttests
73f45a3f8600   moodlehq/moodle-exttests         "docker-php-entrypoi…"   10 seconds ago   Up 10 seconds (healthy)   80/tcp                   frosty_chandrasekhar
e51f5f067080   moodlehq/moodle-exttests         "docker-php-entrypoi…"   9 minutes ago    Up 8 minutes (healthy)    80/tcp                   moodle-docker_exttests_1

So, maybe, that's all we need and done. Thoughts?

terrycampbell commented 2 years ago

I agree that the change you propose should be adequate to address the immediate problem.

The base image does expose port 80, even if you are not listening on it. 9000 is used but there is no EXPOSE statement so it does not show in inspect.

I may circle back later with a proposal to eliminate the exttests apache config files and go back to using port 80 on exttests---simply because I like reducing complexity, but I would not do so without more complete testing on my part to prove it works.

Thanks for your prompt support.

--Terry

stronk7 commented 2 years ago

Great, @terrycampbell, so, for sure, any investigation about podman and see if we can remove the port 9000 "trick" is really welcome. Any time.

In the mean time, I'm going to move forward the change above and regenerate the exttests image with the new healthcheck, so we continue using it normally.

https://github.com/moodlehq/moodle-exttests/pull/23 should address that.

dahrens commented 2 years ago

The issue is not about podman itself, but about podman-compose and how it mimics docker-compose features into the podman world. podman, as kubernetes, has the conecpt of a pod, which is a collection of containers that share the same network stack. podman-compose maps all services of a docker compose file to one pod and therefore does not need to bother with networking at all. this has the sideffect, that all containers share the same network stack and therefore need dedicated ports.

stronk7 commented 2 years ago

Hi @dahrens ,

thanks for the feedback. Yeah, I was yesterday reading a bunch of information about podman (which I know near nothing about). But, for example, I saw that it was possible to create a pod that is not sharing the network (--share cgroup,ipc,uts only) and then go adding and running containers in that pod.

Also, I found a number of issues (some linked above) that suggested to use this or that option, for example --transform_policy=identity , but that's all I know (that is near nothing!).

So, that made me think if, maybe, the only problem is that the default pod in podman comes with shared network option and it's only a matter of not using that option. Or there is some magic switch (or light configuration) allowing to avoid the problem.

Ciao :-)

stronk7 commented 2 years ago

In any case, for now... with https://github.com/moodlehq/moodle-exttests/pull/23 implemented, I think we can safely continue using current moodle-docker configuration and keep both docker and podman people happy, so not much of a trouble!

stronk7 commented 2 years ago

Ok,

so the new moodle-exttests image has just been generated, so all we need is to pull it and done!

I think this can be closed now, thanks all!

dahrens commented 2 years ago

Yes, I tested those settings as well. Nothing really worked. Maybe this has changed meanwhile, but I don't think so.

I'm quite sure that podman play kube will be a nice solution in the future, but this also lacks a lot of features right now.

Glad to see that you found a solution for now. :+1:

terrycampbell commented 2 years ago

Follow up: I created a simple docker-compose file that launches two containers derived from php:7.3-apache. Both are listening on and exposing port 80 (ports are not published to the host). I added a unique file to each container and then verified webserver can call webserver2 at port 80 and vice versa. I may be missing something, but the conclusion I draw from this is that any number of containers can expose the same port without issue, and container-container communication will resolve by service name. If so, then we can easily access the exttests container at port 80 from the webserver container, and would not need to listen on port 9000---allowing us to delete the exttests configuration files and revert to the base exttests image without change.

Since the problem has already been addressed this clearly has low priority, but simplifying code is always a nice thing. Perhaps something to consider in the future.

docker-compose.yml

version: "3"
services:
  webserver1:
    image: "php:7.3-apache"
    volumes:
      - "./000-default.conf:/etc/apache2/sites-enabled/000-default.conf"
      - "./webserver1.txt:/var/www/html/webserver1.txt"
  webserver2:
    image: "php:7.3-apache"
    volumes:
      - "./000-default.conf:/etc/apache2/sites-enabled/000-default.conf"
      - "./webserver2.txt:/var/www/html/webserver2.txt"

Test Results


# Launch the stack
terry@TERRY-RYZEN:/mnt/c/Users/Terry/podman$ podman-compose up -d
['podman', '--version', '']
using podman version: 3.4.2
{'default'} {'default'}
** excluding:  set()
['podman', 'network', 'exists', 'podman_default']
podman run --name=podman_webserver1_1 -d --label io.podman.compose.config-hash=123 --label io.podman.compose.project=podman --label io.podman.compose.version=0.0.1 --label com.docker.compose.project=podman --label com.docker.compose.project.working_dir=/mnt/c/Users/Terry/podman --label com.docker.compose.project.config_files=docker-compose.yml --label com.docker.compose.container-number=1 --label com.docker.compose.service=webserver1 -v /mnt/c/Users/Terry/podman/000-default.conf:/etc/apache2/sites-enabled/000-default.conf -v /mnt/c/Users/Terry/podman/webserver1.txt:/var/www/html/webserver1.txt --net podman_default --network-alias webserver1 php:7.3-apache
1fb23f804fe2c8b6cb6ece51211b5b488afd96dab0870d1582abc153aa593144
exit code: 0

['podman', 'network', 'exists', 'podman_default']
podman run --name=podman_webserver2_1 -d --label io.podman.compose.config-hash=123 --label io.podman.compose.project=podman --label io.podman.compose.version=0.0.1 --label com.docker.compose.project=podman --label com.docker.compose.project.working_dir=/mnt/c/Users/Terry/podman --label com.docker.compose.project.config_files=docker-compose.yml --label com.docker.compose.container-number=1 --label com.docker.compose.service=webserver2 -v /mnt/c/Users/Terry/podman/000-default.conf:/etc/apache2/sites-enabled/000-default.conf -v /mnt/c/Users/Terry/podman/webserver2.txt:/var/www/html/webserver2.txt --net podman_default --network-alias webserver2 php:7.3-apache
d111f2a1e5697ac796fe3938e80b0b92c5f40496b2c544aac77201898921d2d1
exit code: 0

# Access webserver2:80- from webserver1
terry@TERRY-RYZEN:/mnt/c/Users/Terry/podman$ podman-compose exec webserver1 curl http://webserver2/webserver2.txt
['podman', '--version', '']
using podman version: 3.4.2
{'default'} {'default'}
podman exec --interactive --tty podman_webserver1_1 curl http://webserver2/webserver2.txt
I am webserver2
exit code: 0

# Access webserver1:80 from webserver2
terry@TERRY-RYZEN:/mnt/c/Users/Terry/podman$ podman-compose exec webserver2 curl http://webserver1/webserver1.txt
['podman', '--version', '']
using podman version: 3.4.2
{'default'} {'default'}
podman exec --interactive --tty podman_webserver2_1 curl http://webserver1/webserver1.txt
I am webserver1
exit code: 0
dahrens commented 2 years ago

Yes, it looks as if it was really addressed in the past weeks. Thanks for pointing that out @terrycampbell. I currently have issues with moodle-docker containers not being able to talk to each other at all (using podman and podman-compose). Looks like they dropped the whole idea of the pod and rely on podman-dnsname now... Need some time to check this out.

stronk7 commented 2 years ago

Hi,

great research @terrycampbell !

maybe we can create a new issue here, linking it to this one (already closed).

So, whenever it's confirmed that current podman versions work with multiple containers using same port, we can remove the 9000 "hack"? Not in a hurry, but better to have it registered in new issue.

terrycampbell commented 2 years ago

I will work up an issue and a PR to review and test.

BTW: Has anyone worked on adding podman and podman-compose features to the moodle-docker repo? E.g., as an alternate configuration setting to use podman versus Docker?

From: Eloy Lafuente @.> Sent: Friday, January 28, 2022 2:59 AM To: moodlehq/moodle-docker @.> Cc: Terry Campbell @.>; Mention @.> Subject: Re: [moodlehq/moodle-docker] Container moodle-extests fails healthcheck when used with this repo (Issue #200)

Hi,

great research @terrycampbellhttps://github.com/terrycampbell !

maybe we can create a new issue here, linking it to this one (already closed).

So, whenever it's confirmed that current podman versions work with multiple containers using same port, we can remove the 9000 "hack"? Not in a hurry, but better to have it registered in new issue.

— Reply to this email directly, view it on GitHubhttps://github.com/moodlehq/moodle-docker/issues/200#issuecomment-1024012747, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADM5KJQBRGI3JNNVV7EW5LDUYJLE7ANCNFSM5M3KQLIA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.Message ID: @.***>

dahrens commented 2 years ago

BTW: Has anyone worked on adding podman and podman-compose features to the moodle-docker repo? E.g., as an alternate configuration setting to use podman versus Docker?

This question was raised in #172 as well.

scara commented 2 years ago

Hi @terrycampbell,

BTW: Has anyone worked on adding podman and podman-compose features to the moodle-docker repo? E.g., as an alternate configuration setting to use podman versus Docker?

AFAIK none.

I regularly use podman and skopeo but for the Moodle Docker Toolbox where I think docker-compose is still the right choice to quickly get DEVs on board with a containers based Dev Toolbox like this nice project.

BTW I keep looking at podman based "compose" (including portainer) to cover what I think docker (engine) to be still a strength here at the moment.

An example of the learning curve: VOLUME should be explicitly set in the podman cli comand whilst docker (engine) honors the persistence thanks to that directive regardless an explicit --volume. Another one: rootless podman could requires USER revisions there and there.

HTH, Matteo

terrycampbell commented 2 years ago

Good point on rootless. Fully leveraging these alternate platforms may require some “hacks” to some of the base containers.

BTW: I am working on a PowerShell module to provide the same features as this project in a cross-platform single codebase. As part of that I am adding a feature to emit an .env and docker-compose file so I can leverage VSCode Open in Container feature. i.e., create a configuration file, invoke a PowerShell function that creates the .env and docker-compose and then just use base docker/docker-compose commands. Perhaps someone else has worked on this?

From: Matteo Scaramuccia @.> Sent: Friday, January 28, 2022 8:18 AM To: moodlehq/moodle-docker @.> Cc: Terry Campbell @.>; Mention @.> Subject: Re: [moodlehq/moodle-docker] Container moodle-extests fails healthcheck when used with this repo (Issue #200)

Hi @terrycampbellhttps://github.com/terrycampbell,

BTW: Has anyone worked on adding podman and podman-compose features to the moodle-docker repo? E.g., as an alternate configuration setting to use podman versus Docker?

AFAIK none.

I regularly use podman and skopeo but for the Moodle Docker Toolbox where I think docker-compose is still the right choice to quickly get DEVs on board with a containers based Dev Toolbox like this nice project.

BTW I keep looking at podman based "compose" (including portainer) to cover what I think docker (engine) to be still a strength here at the moment.

An example of the learning curve: VOLUME should be explicitly set in the podman cli comand whilst docker (engine) honors the persistence thanks to that directive regardless an explicit --volume. Another one: rootless podman could requires USER revisions there and there.

HTH, Matteo

— Reply to this email directly, view it on GitHubhttps://github.com/moodlehq/moodle-docker/issues/200#issuecomment-1024266612, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADM5KJW2GIJ6WQQVISPH5KDUYKQTFANCNFSM5M3KQLIA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.Message ID: @.**@.>>