nexedi / slapos

mentioned in merge request !313

The background story is !313

--zserver argument does not clearly document it, but it also support a ip address, in that case a random port is picked. I suggest we also apply this change to erp5 master branch to make it more obvious: jerome/erp5@8915aaf4

Added 1 commit:

• 9b7c2933 - component/tesseract: Fix compilation errors for gcc 4.7 / debian 7.1

This cause some problems for the testsuite, because the URL becomes something like http://erp5-catalog-0:55488/ , which is not localhost anymore, so we have problems like this test. Firefox says "The operation is insecure" and chrome this error message:

This is because localhost was a secure origin (as defined in https://goo.gl/Y0ZkNV ), but erp5-catalog-0 is not. As a result, the service worker registration from gadget_officejs_bootloader_js is rejected with DOMException: Only secure origins are allowed (see: https://goo.gl/Y0ZkNV).

So it seems we need either:

• keep on using localhost ( either by reconsidering !313 or by adding a host mapping so that zope and firefox use localhost for the partition IP )
• use a https frontend

Chrome supports a --unsafely-treat-insecure-origin-as-secure=http://erp5-catalog-0:55488/ ( ref) that we could use to workaround this, but firework does not seem to allow this.

/cc @vpelletier @Nicolas @rafael @alain.takoudjou @romain @vincentB @yusei @seb @luke

• use a https frontend

• use a local https proxy (IOW, generate a certificate for local apache or whatever the proxy is)

Although this requires also adding the authoring CA in the browser cert store, it would be much cleaner than exposing the test to a frontend.

Of course, it's not an instant solution, but I doubt using a frontend just for https wrapping to an automated test would fly: tests should not be accessible from the outside, and they should rely on as few "external" machines as possible during runtime (we see the instability still caused by the shared cloudooo cluster which although not systematic is still noticeable, and I see no reason to think any other remote service would go perfectly fine).

• find how the browser manages its domain whitelist and whitelist the partition's own IP or domain name (btw, erp5-catalog-0 is not the best domain to pick, although I guess it was just picked because it comes alphabetically first and the lookup key is juste the IP - is it really required to have a domain name at all here ?). This is one (non-trivial) step less than my above proposal

use a local https proxy (IOW, generate a certificate for local apache or whatever the proxy is)

Yes, I was also thinking that this "frontend" could be a "local frontend".

Although this requires also adding the authoring CA in the browser cert store,

Apparently selenium knows how to tell firefox to tolerate invalid certificates https://www.hskupin.info/2017/01/23/using-selenium-and-webdriver-to-interact-with-insecure-ssl-pages-in-firefox/ . This and a local https frontend might be enough.

it would be much cleaner than exposing the test to a frontend.

Even if this approach is not clean, I think there have already been some work to make testnodes use a public accessible frontend ( to run tests on saucelabs or something ). Anybody knows more about this ? I guess it was not for erp5 test suite because I could not find the code.

I noticed testnode requests an external frontend which proxy to testnode's ip on port 9081 and also includes an apache with zope virtualhost, but I don't understand ... this seem to be listening on [%(ip)s]%(port)s and proxy to the same [%(ip)s]:%(port)s, which cannot work. Apparently this was introduced for performance test ( 717efa34 ), but I guess this is not used for anything, is it ?

find how the browser manages its domain whitelist and whitelist the partition's own IP or domain name

This is easy with chrome (--unsafely-treat-insecure-origin-as-secure command line flag), but apparently not so easy with firefox. I don't think we have plans to switch to running zelenium tests with chrome.

(btw, erp5-catalog-0 is not the best domain to pick, although I guess it was just picked because it comes alphabetically first and the lookup key is juste the IP - is it really required to have a domain name at all here ?).

It's picked by this gethostbyaddr if I understood correctly ( and because firefox also runs in the same userhosts wrapper, it understands this hostname )

Yes, I was also thinking that this "frontend" could be a "local frontend".

To me "local frontend" means "request an instance of a frontend software release, then a slave instance in it". But the backend included in the partition could be enough (and we want HTTPS there too anyway).

Added 17 commits:

• f67bd37e...235c3bca - 16 commits from branch nexedi:master
• 87187af8 - erp5: runUnitTest start zserver on the partition IP

Added 2 commits:

• 4307a5b8 - 1 commit from branch nexedi:master
• ca91499e - erp5: start runUnitTest's zserver on the partition IP

Added 1 commit:

• 25773f20 - erp5: start runUnitTest's zserver on the partition IP

Added 1 commit:

• 1b335691 - component/firefox: version up 53.0.3

Added 5 commits:

• 182a56ee - component/apache: fix documentation of backend template
• fe7b8939 - component/apache: extend backend template with direct zope rewrite rules
• ba978dc0 - erp5: start runUnitTest's zserver on the partition IP
• 127e2c36 - erp5: make expected errors obvious
• 5a6a4c01 - component/firefox: version up 53.0.3

Added 2 commits:

• b055643c - erp5: start runUnitTest's zserver on the partition IP
• 57070dfc - component/firefox: version up 53.0.3

Changed title: WIP: erp5: make runUnitTest start zserver on the partition IP → WIP: erp5: start test runner's zserver on partition IP and access it through an apache https proxy

mentioned in merge request erp5!729

We reached a first milestone where tests are running here. This test suite uses another distributor in Nexedi ERP5 for which test nodes build seleniumrunner on this branch, so that it use this newer firefox.

Some Functional tests have already passed, including erp5_officejs_ui_test:testFunctionalOfficeJSoOoText which before was failing with "operation is insecure", so this looks working.

The following changes have been made:

• backend apache has new entries with rewriterules for test runners.
• runUnitTest is modified to listen on partition IP and to tell firefox to use URL from apache.
• runTestSuite is generated with several pre-assigned ports for runUnitTest
• firefox is updated. Maybe it will solve the problems sometimes happening with indexeddb, or maybe it will reveal new problems.

This is still WIP, there are some known problems below and I want to confirm all test results. If you want to start reviewing, please also read commit messages. There are also changes on ERP5 in erp5!729 .

The known problems:

• this apache is publicly available from IPv6 while running tests, so anybody on the internet can access the running tests. This seem a big problem. At the same time, it can be cool to access runUnitTest instance remotely without having to use ssh port forward. I think we recently changed to generate a password dynamically in some conditions. I have to study this.
• instanciation needs to run several times because of the circular dependencies in arguments, but nothing is here to say if the instance is ready to run tests or not. If I understood correctly, a promise is missing for this.
• runTestSuite needs to pre-allocate enough addresses so that it can start as many runTestSuite that erp5testnode will pass as --node_quantity. For now, we allocate as many test ports as mariadb.test-database-amount but we should probably introduce a new instance parameter for this.
• runUnitTest only starts an http zserver, starting the webdav server here print some Can't bind to 10.0.12.198:2202 errors. Not sure if this cause some failing test of if anyone uses this ( @jm maybe ?)
• I think that geckodriver and marionette are listening on 127.0.0.1. I don't know if we can tell them to use the partition IP and let them choose a free port dynamically. The testnodes I'm using probably here both resolve localhost by DNS, so maybe there will be problems we don't see.
• we just tell firefox to ignore SSL errors, but we could add the certificate to the certificate store instead. Selenium allows to pass a profile directory as firefoxOptions and we can use certutils to add trusted certificates ( see this snippet or this stackoverflow question). How to access the certificate is also an issue. I think we should postpone this after caucase integration is more advanced.

cc @xiaowu.zhang this seem related to what you are doing recently

mentioned in merge request !383

About the upgrade of Firefox, I've just pushed the firefox52 branch, which I did a while ago. Please look at it first.

Added 4 commits:

• 66e3ae4d - component/apache: extend backend template with direct zope rewrite rules
• 86a14420 - erp5: start runUnitTest's zserver on the partition IP
• e1603a12 - component/firefox: version up 53.0.3
• 3eb31079 - erp5: promise to wait for apache testrunner URL to be returned

Added 1 commit:

• 95184a91 - erp5: promise to wait for apache testrunner URL to be returned

About the upgrade of Firefox, I've just pushed the firefox52 branch, which I did a while ago. Please look at it first.

Thanks. I see this firefox52 branch contain changes to simplify the build scripts and increase the version. Here, we just increase the version, so if there are conflicts it will be easy. Anyway, I can take a look at firefox52 while I have this test node which can easily run a different firefox.

Added 8 commits:

• 979cba3b - erp5: promise to wait for apache testrunner URL to be returned
• 5909c07c - jstestnode: switch to buildout.hash.cfg
• ad2e1b78 - seleniumrunner: switch to buildout.hash.cfg
• c418f59a - component/firefox: simplify profile
• 7eee7b77 - component/firefox: use .format string subsitutions
• 67f516a2 - component/firefox: Don't discard firefox's stderr
• 7732f921 - component/firefox: version up firefox 53.0.3
• 91ff5b11 - component/firefox: version up geckodriver 0.17.0

@jm I included a first version of the updated patches from "firefox52". It's a bit early to review, I'll extract it in an independent merge request if it test pass.

Added 7 commits:

• cfe81b60 - seleniumrunner: switch to buildout.hash.cfg
• 38d18c76 - component/firefox: simplify profile
• 7539e7d1 - component/firefox: use .format string subsitutions
• 5ea464e2 - component/firefox: Don't discard firefox's stderr
• a2f3b27b - component/firefox: download firefox over https
• bf373571 - component/firefox: version up firefox 53.0.3
• a1ba920e - component/firefox: version up geckodriver 0.17.0

mentioned in merge request !387

Added 51 commits:

• a1ba920e...e21900ff - 45 commits from branch nexedi:master
• d13e4167 - erp5: fix typos in README
• adbb8865 - erp5: new testrunner instance parameter
• d556a317 - component/apache: switch to buildout.hash.cfg
• 82de3e07 - component/apache: fix documentation of backend template
• 20ab41c1 - component/apache: extend backend template with direct zope rewrite rules
• 967afae0 - erp5: Rework testrunner's HTTP server

Added 5 commits:

• 6423b41a - erp5: new test-runner instance parameter
• fcbb72c9 - component/apache: switch to buildout.hash.cfg
• 00a2c9f2 - component/apache: fix documentation of backend template
• 1d5e138d - component/apache: extend backend template with direct zope rewrite rules
• e110a7fb - erp5: Rework testrunner's HTTP server

I pushed updated patches. I still have to merge this with !387 and make sure the test pass, but I think this is very close to the final version, so don't hesitate to review and comment.

Here's an update on the known problems:

• this apache is publicly available from IPv6 while running tests, so anybody on the internet can access the running tests. This seem a big problem. At the same time, it can be cool to access runUnitTest instance remotely without having to use ssh port forward. I think we recently changed to generate a password dynamically in some conditions. I have to study this.

A simpler approach was to have this apache listen only on ipv4, which are local on our tests infrastructure, so good enough for now.

For the records, I started to change all tests to always generate random passwords, but it's a big change and I'm still not sure how to proceed. One remaining issue is with functional tests who needs to login as specific users. I guess we'll need to create the user in setup and find a way to pass them to tests, because scripts which creates a manager user are also a problem. The current state is left at https://lab.nexedi.com/jerome/erp5/commits/feat/passwords-in-tests , but I'm not sure I'll continue this. It would be needed if we want to use runUnitTest on saucelabs.

Also this affect also tests for customer configurations, so it's just safer to just use IPv4 for now.

One suggestion from @vpelletier I did not investigate yet is to have testnodes have firewalls (with firewalld component that was integrated recently) so that this port is restricted at firewall level.

• instanciation needs to run several times because of the circular dependencies in arguments, but nothing is here to say if the instance is ready to run tests or not. If I understood correctly, a promise is missing for this.

A new promise-test-runner-apache-url was added for this.

• runTestSuite needs to pre-allocate enough addresses so that it can start as many runTestSuite that erp5testnode will pass as --node_quantity. For now, we allocate as many test ports as mariadb.test-database-amount but we should probably introduce a new instance parameter for this.

From a suggestion from @vpelletier this is now different. We have new parameters "how many parallel tests this instance can run" (test-runner.node-count) and "how many supplementary databases each test needs" ( test-runner.extra-database-count) . Instead of configuring "I need a total of 15 databases", we configure "I need to be able to run 3 tests in parallel, each test requiring 4 extra databases" which generates 3 * (4 +1) databases.

The old mariadb.test-database-amount is still understood for compatibility, also its side effect of not generating test runner when requesting 0 test databases.

This was done as a preliminary commit, 6423b41a

Obviously, we pre-allocated testrunner.node-count addresses for zservers and frontends.

• runUnitTest only starts an http zserver, starting the webdav server here print some Can't bind to 10.0.12.198:2202 errors. Not sure if this cause some failing test of if anyone uses this ( @jm maybe ?)

I did simply erp5@e5d9c4f2 . I think it's good enough for now.

• I think that geckodriver and marionette are listening on 127.0.0.1. I don't know if we can tell them to use the partition IP and let them choose a free port dynamically. The testnodes I'm using probably here both resolve localhost by DNS, so maybe there will be problems we don't see.

ignored for now. This will be another MR if needed.

• we just tell firefox to ignore SSL errors, but we could add the certificate to the certificate store instead. Selenium allows to pass a profile directory as firefoxOptions and we can use certutils to add trusted certificates ( see this snippet or this stackoverflow question). How to access the certificate is also an issue. I think we should postpone this after caucase integration is more advanced.

ignored for now. BTW, we have certutils in component/nss/buildout.cfg

New problems:

• ERP5/SlapOS version incompatibilities: every time we introduce a new required argument to testrunner scripts, we have issues that new slapos cannot run old ERP5, because runTestSuite does not accept this new argument. To prevent this problem, the argument is passed as an environment variable in runTestSuite slapos generated wrapper (it still exists as a command line argument, to be defined / handled in a central place and to keep --help being an exhaustive documentation). runUnitTest wrapper just pass as argument, because it's only used by developers and developers can fix this easily if it's ever a problem ( it was a problem only in test nodes if I understood correctly ). This is more visible on the ERP5 side of this MR erp5!729 (diffs)

• Apparently the tests sometimes timeout on apache. For example this test. In that case where we don't consume resource on shared frontend, I don't really see the point of having apache defaults 300 second timeout and I'm considering using a much larger Timeout, maybe still coupled with RequestReadTimeout to protect from buggy / malicious clients (if this is really enough to replace mod_antiloris . This will be for later if this appear to really be a problem.

Added 17 commits:

• e110a7fb...28bf2a43 - 11 commits from branch nexedi:master
• 97aafb85 - erp5: fix typos in README
• bde30f7a - erp5: new test-runner instance parameter
• 510fd172 - component/apache: switch to buildout.hash.cfg
• 6c263629 - component/apache: fix documentation of backend template
• d595fe26 - component/apache: extend backend template with direct zope rewrite rules
• 579c9bc1 - erp5: Rework testrunner's HTTP server

Changed title: WIP: erp5: start test runner's zserver on partition IP and access it through an apache https proxy → WIP: erp5: make testrunner access zserver through apache

Rebased on master, now that firefox was updated to 52 in !387 , acceptInsecureCerts should be supported.

Tests are OK all zelenium tests are OK, there's just this facebook test failure that's no related.

This adds some entries to the JSON schema, please check the naming is OK.

About this test-runner name and what can be inside: I'm thinking that in the future we can extend test-runner to add any extra things we need for testing, for example:

• selenium.server-url ( http://saucelabs or a self hosted selenium server ) and selenium.desired-capabilities ( {'browserName': 'firefox', 'version': '60' } https://github.com/SeleniumHQ/selenium/wiki/DesiredCapabilities ) , similar to !383

• URL of a sftp server ( see erp5!680 (comment 60566) )

• connection information for a ldap server

• or maybe something more generic: a list of software URLs / instance parameters to ask testnode to request a slapos instance and pass parameters to runTestSuite.

All this is for maybe later, maybe later, but I was thinking of a name that could include this kind of things.

Unmarked this merge request as a Work In Progress

Added 2 commits:

• 02f9e50f - erp5: instance-zope: fix indentation in jinja template
• bb227a11 - erp5: instance-zope: allocate zope ports before test ports

I added another change to keep zope running on the same ports. It's not critical, because we make frontends to the apache and the zope ports are just internal to the cluster, but it's better to keep the same ports I guess.

I added another change to keep zope running on the same ports. It's not critical

I consider it as important, so thanks for implementing this. Ports are the interface of an existing instance, and I remember someone ( @cedric.leninivin I believe) having rather big problems because some ports changed on an instance (it way maybe a webrunner, and in my recollection he basically lost access because frontend did not follow the change for whatever reason... it was years ago, I may be misremembering).

I believe a few commits would benefit from being squashed before merge, but I did not check in details.

Added 5 commits:

• e4d1ea03 - erp5: new test-runner instance parameter
• 8a8379f0 - component/apache: switch to buildout.hash.cfg
• 7d63b3e6 - component/apache: fix documentation of backend template
• 41413606 - component/apache: extend backend template with direct zope rewrite rules
• 2a457867 - erp5: Rework testrunner's HTTP server

I believe a few commits would benefit from being squashed before merge, but I did not check in details.

I squashed together

• 02f9e50f erp5: instance-zope: fix indentation in jinja template
• bb227a11 erp5: instance-zope: allocate zope ports before test ports

with

• 579c9bc1 erp5: Rework testrunner's HTTP server ( and it became 2a457867 )

I also amended the schema to remove the deprecated mariadb.test-database-amount argument.

The paragraph below was not added to the README, but let's keep it here in case we want to extend user documentation of this software release.Even though the schema should describes everything, having some textual documentation and also some examples requests paramters could be nice (not necessaraly in the README)

---

Running test suite

All zope partitions will contain runTestSuite and runUnitTest script, unless the instance was requested with test-runner.enabled set to false.

The following scripts will be created:

• runUnitTest runs one test in a temporary instance. Developers can use this script to easily run one individual test.
• runTestSuite runs a full test suite, optionally by running several tests in parallel. This will be used by the test infrastructure to run all ERP5 tests. The instance needs to set a value for test-runner.node-count large enough for the test node.

While most of ERP5 tests use the default mariadb database, some tests needs more databases connections, for example to test the use of an external datawarehouse or test hot reindexing features of ERP5. When requesting, set extra-database-count to the number of extra database connections needed for the tests.

https://nexedi.erp5.net/test_result_module/20180910-622AB5DF

• I think that geckodriver and marionette are listening on 127.0.0.1. I don't know if we can tell them to use the partition IP and let them choose a free port dynamically. The testnodes I'm using probably here both resolve localhost by DNS, so maybe there will be problems we don't see.

ignored for now. This will be another MR if needed.

It's needed. For now, this is done by monkey patching selenium erp5!742 (in a quick workaround way)

mentioned in commit erp5@0a09fe10

Status changed to merged

mentioned in commit bddfdad2

2018-09-11 09:02:08,200 testsuite    : INFO     runTestSuite : $ runUnitTest --verbose '--erp5_sql_connection_string=erp5_test_6@10.0.89.206:2099 testuser_6 testpassword6' --xvfb_bin=/srv/slapgrid/slappart1/srv/slapos/soft/f13a9cb77381284f3c6cc02ae22a06a3/parts/xserver/bin/Xvfb --firefox_bin=/srv/slapgrid/slappart1/srv/slapos/soft/f13a9cb77381284f3c6cc02ae22a06a3/parts/firefox/firefox-slapos '--extra_sql_connection_string_list=erp5_test_7@10.0.89.206:2099 testuser_7 testpassword7,erp5_test_8@10.0.89.206:2099 testuser_8 testpassword8' --bt5_path=None --instance_home=unit_test.3 testCrmReports
2018-09-11 09:02:08,988 testsuite    : INFO     runTestSuite : option --zserver_frontend_url not recognized

As we can see on some tests, the approach for backward compatibility was not correct. Unlike what I wrote earlier, runTestSuite call the generated runUnitTest, so this approach does not really solve the ERP5/SlapOS version incompatibilities and users of this SlapOS also need a recent ERP5 to run tests. Because now most of our test nodes have the localhost resolution problem, it's anyway required to update both erp5 and SlapOS.

mentioned in merge request erp5!742

mentioned in merge request !420

mentioned in commit erp5@b955dbe0

mentioned in merge request !422

mentioned in commit b47625ae

mentioned in merge request !440

mentioned in commit 47c9518e

mentioned in commit 3e1c76f5

mentioned in commit 8635c753

mentioned in commit c3d5f3ef

mentioned in commit b2478b38

this fails when testrunner is not enabled !

See !440 for a fix

/cc @tatuya

mentioned in merge request !548

mentioned in commit c79055ed