PHPLIB-565: Add MongoDB version axis to build matrix #780

alcaeus · 2020-09-02T13:52:39Z

This PR expands evergreen test coverage by testing against different driver versions, MongoDB server versions, and topologies. Summary:

The first matrix tests all PHP versions on all operating systems (dropping a few where we don't have binaries)
The second matrix tests all driver versions on all PHP versions. Uses Ubuntu 18.04 (since it offers all PHP versions) and only latest stable MongoDB server version
The third matrix tests all MongoDB server versions (3.0-latest dev) on PHP 7.4 and RHEL 7.0 (since that's the only distro that offers all server versions)

In all cases, we test against standalone, replica set, and sharded cluster topologies. The latter uses a replica set by default, but we override the provided file to make sure the replica set has secondaries, which is required for some tests. All other files are removed as they are not used.

Patch build: https://spruce.mongodb.com/version/5f4fa2e73e8e863a703f0b4e/tasks. Note that replica set builds fail intermittently due to segmentation faults (double free when closing cursors).

alcaeus · 2020-09-02T13:55:01Z

.evergreen/config.yml

@@ -174,7 +174,7 @@ functions:
      params:
        script: |
          ${PREPARE_SHELL}
-          MONGODB_VERSION=${VERSION} TOPOLOGY=${TOPOLOGY} AUTH=${AUTH} SSL=${SSL} STORAGE_ENGINE=${STORAGE_ENGINE} sh ${DRIVERS_TOOLS}/.evergreen/run-orchestration.sh
+          MONGODB_VERSION=${VERSION} ORCHESTRATION_FILE=${ORCHESTRATION_FILE} TOPOLOGY=${TOPOLOGY} AUTH=${AUTH} SSL=${SSL} STORAGE_ENGINE=${STORAGE_ENGINE} sh ${PROJECT_DIRECTORY}/.evergreen/run-orchestration.sh


Using our own file allows us to add logic to override the used file. Passing ORCHESTRATION_FILE as well allows configurations to select a specific file instead of relying on the filename being assembled by the logic included.

Was this borrowed from PHPC or is it new for PHPLIB (in which case it may be something you'd like to add back to PHPC)?

I didn't find ORCHESTRATION_FILE in https://github.com/mongodb/mongo-php-driver/blob/master/.evergreen/config.yml, so I assume this is new. Feel free to create a ticket if you think it's worth porting this over to PHPC down the line.

Created PHPC-1676 to track this.

alcaeus · 2020-09-02T13:55:27Z

.evergreen/config.yml

-        display_name: "1.8.0beta2"
-        variables:
-          DRIVER_VERSION: "1.8.0beta2"
+        display_name: "1.8-stable"


This was left over from earlier tests where we didn't have 1.8 stable yet. We can now omit the DRIVER_VERSION constant to always install the current stable release.

alcaeus · 2020-09-02T13:56:53Z

tests/Operation/AggregateFunctionalTest.php

-                if ($this->isShardedCluster()) {
-                    $this->assertObjectHasAttribute('shards', $result);
-
+                if (isset($result->shards)) {


This was necessary as the output of explain on a cluster with a single shard does not contain the shards key but rather looks like the result on a standalone.

tests/SpecTests/FunctionalTestCase.php

jmikola · 2020-09-02T15:26:36Z

.evergreen/config.yml

@@ -174,7 +174,7 @@ functions:
      params:
        script: |
          ${PREPARE_SHELL}
-          MONGODB_VERSION=${VERSION} TOPOLOGY=${TOPOLOGY} AUTH=${AUTH} SSL=${SSL} STORAGE_ENGINE=${STORAGE_ENGINE} sh ${DRIVERS_TOOLS}/.evergreen/run-orchestration.sh
+          MONGODB_VERSION=${VERSION} ORCHESTRATION_FILE=${ORCHESTRATION_FILE} TOPOLOGY=${TOPOLOGY} AUTH=${AUTH} SSL=${SSL} STORAGE_ENGINE=${STORAGE_ENGINE} sh ${PROJECT_DIRECTORY}/.evergreen/run-orchestration.sh


Was this borrowed from PHPC or is it new for PHPLIB (in which case it may be something you'd like to add back to PHPC)?

jmikola · 2020-09-02T15:28:45Z

.evergreen/run-tests.sh

@@ -22,9 +22,4 @@ OLD_PATH=$PATH
 PATH=/opt/php/${PHP_VERSION}-64bit/bin:$OLD_PATH

 # Run the tests, and store the results in a Evergreen compatible JSON results file


I assume "store the results in a Evergreen compatible JSON results file" doesn't happen here, nor in PHPC, which uses run-tests.php. Should we revise this comment?

I was going to look at the result file format and see if this makes test results easier to analyse. Going through the build log to see what's going on isn't really sustainable.

.evergreen/run-tests.sh

tests/SpecTests/FunctionalTestCase.php

.evergreen/orchestration/configs/sharded_clusters/basic.json

jmikola · 2020-09-02T18:35:58Z

Looking at the five errors in the patch build, four are related to:

1) MongoDB\Tests\SpecTests\ClientSideEncryptionSpecTest::testExternalKeyVault with data set #0 (false)
MongoDB\Driver\Exception\BulkWriteException: Bulk write failed due to previous MongoDB\Driver\Exception\EncryptionException: not all keys requested were satisfied
/data/mci/be31210cdc31fd73c4e4fedf4e369a8f/src/src/Operation/InsertOne.php:134
/data/mci/be31210cdc31fd73c4e4fedf4e369a8f/src/src/Collection.php:931
/data/mci/be31210cdc31fd73c4e4fedf4e369a8f/src/tests/SpecTests/ClientSideEncryptionSpecTest.php:346
Caused by
MongoDB\Driver\Exception\EncryptionException: not all keys requested were satisfied
/data/mci/be31210cdc31fd73c4e4fedf4e369a8f/src/src/Operation/InsertOne.php:134
/data/mci/be31210cdc31fd73c4e4fedf4e369a8f/src/src/Collection.php:931
/data/mci/be31210cdc31fd73c4e4fedf4e369a8f/src/tests/SpecTests/ClientSideEncryptionSpecTest.php:346

And one is the "double free or corruption" error (oddly no FLE error for that environment).

alcaeus · 2020-09-03T07:07:07Z

Yep, I'm not sure what the cause for the encryption errors is. @kevinAlbs are you aware of anything that could cause these "not all keys requested were satisfied" errors on replica sets? Note that these don't always show up.

kevinAlbs · 2020-09-03T12:53:30Z

If that only occurs on a replica set, my first thought was that either creating data keys was not using a majority write concern, or finding data keys was not using a majority read concern. I recall seeing that errors during development due to that.

However, the key vault collection does get write concern majority set: https://github.com/mongodb/mongo-c-driver/blob/1.16.0/src/libmongoc/src/mongoc/mongoc-client-side-encryption.c/#L1449-L1454

And obtaining keys does use read concern majority:
https://github.com/mongodb/mongo-c-driver/blob/1.16.0/src/libmongoc/src/mongoc/mongoc-crypt.c/#L356-L368

Looking through the patch build all of those appear on the external key vault test. I think this insert just needs a majority write concern: https://github.com/mongodb/mongo-php-library/blob/master/tests/SpecTests/ClientSideEncryptionSpecTest.php#L320 (see https://github.com/mongodb/mongo-c-driver/blob/master/src/libmongoc/tests/test-mongoc-client-side-encryption.c#L674-L683)

alcaeus · 2020-09-03T13:22:03Z

Thanks @kevinAlbs, that was the reason for the failures. Builds now pass, save for one double-free: https://spruce.mongodb.com/version/5f50e8d432f41749d8020f87/tasks 🎉

jmikola · 2020-09-03T22:35:23Z

@alcaeus did you re-kick the build such that the intermittent double-free error disappeared? If so, I'm OK to merge this as-is and revisit the double-free error in PHPC-1541.

alcaeus · 2020-09-04T05:44:59Z

@jmikola I restarted the build this morning to see if the failure is intermittent or more permanent. Merging as-is sounds good as we still haven't been able to reproduce this in real-life conditions.

Edit: looks like kicking it still helps :)

alcaeus added 8 commits September 2, 2020 15:25

Allow overriding default cluster configurations

5f1b857

Make phpunit output less verbose

cc7ca6b

Use latest stable driver version

b4720e7

Expand test matrix to test against all driver and server versions

46d5225

Add secondary servers to cluster replica set

ef0b082

Make explain command check more forgiving

695c128

Don't drop collections in admin database for spec tests

333f1d0

Test against all topologies by default

f9e6287

alcaeus self-assigned this Sep 2, 2020

alcaeus commented Sep 2, 2020

View reviewed changes

alcaeus requested a review from jmikola September 2, 2020 15:29

jmikola reviewed Sep 2, 2020

View reviewed changes

Use majority write concern when inserting external keyvault data

7cc227a

alcaeus requested a review from jmikola September 3, 2020 13:22

jmikola approved these changes Sep 6, 2020

View reviewed changes

alcaeus merged commit 8048e1a into mongodb:master Sep 7, 2020

alcaeus deleted the phplib-565 branch September 7, 2020 06:59

alcaeus mentioned this pull request Sep 30, 2020

PHPLIB-493: Unified test runner POC #783

Merged

		@@ -22,9 +22,4 @@ OLD_PATH=$PATH
		PATH=/opt/php/${PHP_VERSION}-64bit/bin:$OLD_PATH

		# Run the tests, and store the results in a Evergreen compatible JSON results file

PHPLIB-565: Add MongoDB version axis to build matrix #780

PHPLIB-565: Add MongoDB version axis to build matrix #780

Uh oh!

Conversation

alcaeus commented Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmikola commented Sep 2, 2020

Uh oh!

alcaeus commented Sep 3, 2020

Uh oh!

kevinAlbs commented Sep 3, 2020

Uh oh!

alcaeus commented Sep 3, 2020

Uh oh!

jmikola commented Sep 3, 2020

Uh oh!

alcaeus commented Sep 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

alcaeus commented Sep 2, 2020 •

edited

Loading

alcaeus commented Sep 4, 2020 •

edited

Loading