moodlehq / moodle-docker

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

Bind exttests to port 9000 to support podman-compose #175

Closed dahrens closed 2 years ago

dahrens commented 2 years ago

Provides necessary changes as requested and provided in #172.

dahrens commented 2 years ago

Looks like the tests with the chrome selenium driver fail.

The Selenium or WebDriver server is not running. You must start it to run tests that involve Javascript.

I sit on podman and cannot get master running at all to check whether this was introduced by this PR. Therefore I started reproducing this locally on podman. I receive the same error, but from within my moodle webserver container curl -vv http://selenium:4444/session answers me, which indicates the services is up and running as expected and configured in the config.

Is it possible that we hit MDL-67659 here?

TBH I don't understand how/where the client is located inside of the moodle webserver to be able to compare the version ;)

dahrens commented 2 years ago

The more interesting part of the error message is:

The following debugging information is available:
. Could not open connection: Curl error thrown for http POST to /session with params: {"capabilities":{"firstMatch":[{"browserName":"chrome","goog:chromeOptions":{"args":["disable-web-security","unlimited-storage"]}}]},"desiredCapabilities":{"browserName":"chrome","platform":"ANY","tags":["7a7b2b6ecbc1","PHP 7.3.29"],"chromeOptions":{"args":["disable-web-security","unlimited-storage"]},"browser":"chrome","marionette":null,"ignoreZoomSetting":false,"name":"Behat feature suite"}}

Operation timed out after 30000 milliseconds with 0 bytes received

The url /session is misleading in the error message. php curl actually talks to http://selenium:4444/wd/hub/session as expected. The selenium container also receives the request and logs the following.

23:16:10.733 INFO [ActiveSessionFactory.apply] - Capabilities are: {
  "browser": "chrome",
  "browserName": "chrome",
  "chromeOptions": {
    "args": [
      "disable-web-security",
      "unlimited-storage"
    ]
  },
  "ignoreZoomSetting": false,
  "name": "Behat feature suite",
  "tags": [
    "8a67dc1f072d",
    "PHP 7.3.29"
  ]
}
23:16:10.733 INFO [ActiveSessionFactory.lambda$apply$11] - Matched factory org.openqa.selenium.grid.session.remote.ServicedSession$Factory (provider: org.openqa.selenium.chrome.ChromeDriverService)
Starting ChromeDriver 92.0.4515.107 (87a818b10553a07434ea9e2b6dccf3cbe7895134-refs/branch-heads/4515@{#1634}) on port 11519
Only local connections are allowed.
Please see https://chromedriver.chromium.org/security-considerations for suggestions on keeping ChromeDriver safe.
ChromeDriver was started successfully.

Nevertheless no answer shows up within the 30001 ms, as the error message says.

Sending the same request manually from within the webserver container leads to the same result:

curl -X POST -m 10 -d '{"capabilities":{"firstMatch":[{"browserName":"chrome","goog:chromeOptions":{"args":["disable-web-security","unlimited-storage"]}}]},"desiredCapabilities":{"browserName":"chrome","platform":"ANY","tags":["7a7b2b6ecbc1","PHP 7.3.29"],"chromeOptions":{"args":["disable-web-security","unlimited-storage"]},"browser":"chrome","marionette":null,"ignoreZoomSetting":false,"name":"Behat feature suite"}}' http://selenium:4444/wd/hub/session
curl: (28) Operation timed out after 10001 milliseconds with 0 bytes received

I give up at this point. Would be nice if someone can tell me whether this also happens with current master and MOODLE-DOCKER_BROWSER=chrome.

scara commented 2 years ago

Hi @dahrens, thanks for your hard work! 👍

Is it possible that we hit MDL-67659 here?

Guessing yes: better to vote that issue to get more attention on it 😉 !

Would be nice if someone can tell me whether this also happens with current master and MOODLE-DOCKER_BROWSER=chrome

Here it is:

This is the result using a VM running CentOS 7.9, docker-ce and docker-compose, under : root:

# export MOODLE_DOCKER_DB=pgsql
# export MOODLE_DOCKER_BROWSER=chrome
# export MOODLE_DOCKER_WWWROOT=/path/to/git/moodle/master
# export MOODLE_DOCKER_PHP_VERSION=8.0
# export MOODLE_DOCKER_APP_PATH=
# export MOODLE_DOCKER_APP_VERSION=
# export PHP=8.0
# export DB=pgsql
# export GIT=master
# export SUITE=behat
# export BROWSER=chrome
# tests/setup.sh
Pulling docker images
Pulling exttests  ... done
Pulling db        ... done
Pulling selenium  ... done
Pulling mailhog   ... done
Pulling webserver ... done
Starting up container
Creating network "moodle-docker_default" with the default driver
Creating moodle-docker_exttests_1 ... done
Creating moodle-docker_mailhog_1  ... done
Creating moodle-docker_db_1       ... done
Creating moodle-docker_selenium_1 ... done
Creating moodle-docker_webserver_1 ... done
Waiting for DB to come up
Waiting for Moodle app to come up
Waiting for DB to come up
Waiting for Moodle app to come up
Running: bin/moodle-docker-compose exec -T webserver php admin/tool/behat/cli/init.php
A new stable major version of Composer is available (2.1.5), run "composer self-update --2" to update to it. See also https://getcomposer.org/2
You are already using composer version 1.10.22 (1.x channel).
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Nothing to install or update
Generating autoload files
49 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
-->System
++ Success ++
[...]
-->logstore_standard
++ Success ++
Acceptance tests site installed
Creating Behat configuration ... done in 0.94 seconds.
Building theme CSS for boost [ltr] ... done in 1.13 seconds.
Building theme CSS for classic [ltr] ... done in 1.09 seconds.
Testing environment themes built
Acceptance tests environment enabled on http://webserver, to run the tests use:
vendor/bin/behat --config /var/www/behatdata/behatrun/behat/behat.yml
# tests/test.sh
Running: bin/moodle-docker-compose exec -T webserver php admin/tool/behat/cli/run.php --tags=@auth_manual
Running single behat site:
Moodle 4.0dev (Build: 20210728), dc437b51715eaee7e55998ff6eb52f751b19a306
Php: 8.0.8, pgsql: 11.12 (Debian 11.12-1.pgdg90+1), OS: Linux 3.10.0-1160.36.2.el7.x86_64 x86_64
Run optional tests:
- Accessibility: No
Server OS "Linux", Browser: "chrome"
Started at 01-08-2021, 17:20
...............

2 scenarios (2 passed)
15 steps (15 passed)
0m7.56s (49.92Mb)
# bin/moodle-docker-compose down
Stopping moodle-docker_webserver_1 ... done
Stopping moodle-docker_db_1        ... done
Stopping moodle-docker_selenium_1  ... done
Stopping moodle-docker_exttests_1  ... done
Stopping moodle-docker_mailhog_1   ... done
Removing moodle-docker_webserver_1 ... done
Removing moodle-docker_db_1        ... done
Removing moodle-docker_selenium_1  ... done
Removing moodle-docker_exttests_1  ... done
Removing moodle-docker_mailhog_1   ... done
Removing network moodle-docker_default

HTH, Matteo

dahrens commented 2 years ago

Is it possible that we hit MDL-67659 here?

Guessing yes: better to vote that issue to get more attention on it wink !

I read the commit diff later and it only offers to enhance the error message. The proposed usage of https://github.com/andrewnicols/chromedriver-wrapper mentioned in the comments already applies here - I went though it, when debugging my way down through the involved components.

Thanks a bunch for the verification that master works flawlessly - I was quite sure that the latest version of selenium/standalone-chrome:3 - built a few days ago - causes the issue. Will do a few tests with their debug build and try to get a deeper understanding of this mysterious exttests container.

Using a VM to have docker-ce and docker-compose up and running is a good idea as well.

scara commented 2 years ago

Hi @dahrens,

I read the commit diff later and it only offers to enhance the error message.

I mean: vote it to get that "improved message" in the upstream Moodle code. It could be of some help maybe even here or, for sure, there, eventually better than the current error message :wink:.

HTH, Matteo

dahrens commented 2 years ago

Left foot, right foot :)

The reason why there is no answer from the session API endpoint back to moodle behat run is, that chromedriver wants to connect to the chrome instance that should open the site, before it answers back. Using VNC to watch what happens, reveals that chrome tries to open the URL data:,. This does not work obviously.

After long timeout an error in the selenium logs indicates that as well: [SEVERE]: Timed out receiving message from renderer: 600.000

I have no clue how the URL is passed and what goes wrong there. Especially the relation to the changes of this PR is completely unclear to me.

A call that should resolve the environment returns the following - dunno if this is related:

seluser@3c0a01329888:/$ curl http://webserver/admin/tool/behat/tests/behat/fixtures/environment.php
{"moodleversion":"3.11.2 (Build: 20210729)","phpversion":"7.3.29","dbtype":"pgsql","dbversion":"11.12 (Debian 11.12-1.pgdg90+1)","os":"Linux 5.13.6-arch1-1 x86_64"}

The generated (?) behat configuration from within the webserver container located at /var/www/behatdata/behatrun/behat/behat.yml includes base_url: 'http://webserver', which I would expect to be the proper URL that chrome should access initially.

default:
  formatters:
    moodle_progress:
      output_styles:
        comment:
          - magenta
  suites:
    default:
      paths:
        - /var/www/html/admin/tool/policy/tests/behat/acceptances.feature
        [...]
      contexts:
        - behat_accessibility
        [...]
    classic:
      paths:
        - /var/www/html/theme/classic/tests/behat/courseadministrationmenu.feature
        - /var/www/html/theme/classic/tests/behat/pageadministrationmenu.feature
      contexts:
        - behat_accessibility
        [...]
  extensions:
    Behat\MinkExtension:
      base_url: 'http://webserver'
      goutte: null
      webdriver:
        wd_host: 'http://selenium:4444/wd/hub'
        browser: chrome
        capabilities:
          extra_capabilities:
            chromeOptions:
              args:
                - disable-web-security
                - unlimited-storage
    Moodle\BehatExtension:
      moodledirroot: /var/www/html
      steps_definitions:
        behat_accessibility: /var/www/html/lib/tests/behat/behat_accessibility.php
        [...]
stronk7 commented 2 years ago

Hi,

the problems that you're facing with selenium/chrome are a known regression introduced few days ago in the selenium images. See, for example, https://github.com/moodlehq/moodle-plugin-ci/issues/121 . I'm creating right now a new PR to make moodle-docker to use a previous version, known to work ok.

Stay tuned...

stronk7 commented 2 years ago

FYI, I've just created:

I'll comment here once #176 is merged, so you can rebase this PR on top of it, that should, hopefully, make GHA and Travis happy again.

Ciao :-)

dahrens commented 2 years ago

I'll comment here once #176 is merged, so you can rebase this PR on top of it, that should, hopefully, make GHA and Travis happy again.

No need for further efforts - I subscribed #176 and will rebase when it got merged.

Thanks a lot to let me know about it :)

dahrens commented 2 years ago

I executed at least one of the tests based on chrome locally and works :sweat_smile: hopefully this applies to all the tests :pray:

stronk7 commented 2 years ago

Hi,

this looks great, only a couple of comments:

  1. It's a pity that docker ps continues reporting the port 80 for the exttests container. But I bet that's taken from image metadata (from the PHP image) and cannot be changed. So nothing to do here.
  2. Only little detail I'd suggest is to rename the 2 new asset files, maybe prepending some exttests- to them. Or, alternatively, put them into a exttexts directory (instead of current web one). Only aiming to make it clearer that they "belong" to the exttest container and not to the webserver one.

Ciao :-)

scara commented 2 years ago

Hi Eloy,

docker ps continues reporting the port 80 for the exttests container. But I bet that's taken from image metadata (from the PHP image) and cannot be changed.

guessing from the actual EXPOSE in its Dockerfile.

What about the output of docker compose ps?

TIA, Matteo

stronk7 commented 2 years ago

What about the output of docker compose ps?

Same, so far. I think it's reported from the image definition (yes, original EXPOSE), because it's included in the metadata of the image, no matter it's effectively serving into another port.

$ cp base.yml compose.yml; docker-compose ps 
          Name                         Command               State           Ports         
-------------------------------------------------------------------------------------------
moodle-docker_db_1          docker-entrypoint.sh postgres    Up      5432/tcp              
moodle-docker_exttests_1    docker-php-entrypoint apac ...   Up      80/tcp                
moodle-docker_selenium_1    /opt/bin/entry_point.sh          Up      4444/tcp              
moodle-docker_webserver_1   docker-php-entrypoint apac ...   Up      127.0.0.1:8000->80/tcp

In any case, no problem my side, hardly somebody will need that (incorrect) information for anything.

dahrens commented 2 years ago

Thanks for the review @stronk7. I've adjusted the path and filenames.

Another option would be to change the port directly in exttests Dockerfile, which makes life easier here. Would be a breaking change to that container, which will cause action at everything that sits on it ... and it is published on docker hub ... so maybe better fiddle here with its apache config.

It is also worth to mention that people with existing setups need to adjust there config.php, when they pull this patch.

stronk7 commented 2 years ago

Yeah, changing the port @ exttests Dockerfile/image is no way. That very same image is used by other integrations like moodle-runner-ci, for example. So any solution has to be self-contained here.

I also looked if it was possible to override the image "metadata" port reported from compose, but it seems that is "by design" (there were some issues about that).

So, all right, I think once this passes all the tests... it can safely land. Thanks!

PS: Edited... and yeah, people will need to copy again the config.php file, I hope it's not a problem (we already have changed it in the past without too much problem).

stronk7 commented 2 years ago

Merged, thanks!