Problem/Motivation

#3386076: GitLab CI integration for core landed with branch tests running by default on PHP 8.2 and MySQL 8 only.

DrupalCI allows us to configure on-commit and daily scheduled tests of selected combinations of branches, PHP versions and database drivers. This issue is to figure out how to implement that in GitLabCI.

GitLab allows us to run matrix builds in parallel; perhaps this is what we want here. But we should also consider that linting steps are likely only needed once per branch - they are PHP and database agnostic - and that unit tests should also be database agnostic, so ideally we only need to run those for each version of PHP we support.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3387055

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave’s picture

Currently we have the following jobs enabled in DrupalCI on commit:

  • PHP 8.1 / MySQL 5.7
  • PHP 8.2 / MySQL 8
  • PHP 8.2 / PostgreSQL 14.1
  • PHP 8.2 / SQLite 3.34

And the following set to run daily:

  • PHP 8.1 / MariaDB 10.3.22
  • PHP 8.1 / MySQL 5.7 with updated deps
  • PHP 8.1 / Postgres 14.1
  • PHP 8.1 / SQLite 3.27

Let's ignore the updated deps build for now and handle that in another followup.

After reading the GitLab matrix docs I'm not sure this is entirely suitable. I don't see how to easily split a matrix by push vs schedule events, and also we don't run all combinations of PHP with all combinations of databases.

Finally we have a complication with SQLite: it doesn't have a separate database image. For non-SQLite databases we define the image:

_DB_IMAGE: $_CONFIG_DOCKERHUB_ROOT/$_TARGET_DB_TYPE-$_TARGET_DB_VERSION:production

but for SQLite we define it as the PHP image:

_DB_IMAGE: $_CONFIG_DOCKERHUB_ROOT/php-$_TARGET_PHP-apache:production

There is no if/then or other logic that I can see that would let us define this dynamically based on the database type variable.

Therefore, I think the approach that will work for us is just defining each combination manually and specifying the rules for it; whether it should run in MRs by default or on demand, whether it should run on push, or whether it should run on a schedule.

catch’s picture

I think in general we should be fine if we can do something like this:

1. Run the default version of PHP with different database types for kernel, functional, and functional javascript tests.

2. Run the different supported versions of PHP with the latest version of MySQL for all test types.

3. Manually trigger arbitrary combinations if something weird comes up.

We can also slightly vary the CI yaml by branch, so 10.2 could treat PHP 8.2 as the default version and 10.1 could treat PHP 8.1 as the default version.

If we do that, are #1 and #2 a small enough amount of tests that we could just run all of them on commit? Mostly I'm wondering if we could drop the entire concept of scheduled tests for this.

Or... going in the opposite direction, instead of on-commit testing, if we're able to add a condition like 'must have been a commit since the last pipeline run', could we run the core pipeline once per hour or two hours? That would still catch HEAD broken situations very quickly, but would mean if someone commits three MRs in ten minutes, we're not running three pipelines then but only one at the end of the hour. But it'd need the condition of their having been a commit to make that less resource-intensive.

If either of these is doable, then we'd have a single suite of tests that run at whatever frequency, and it would be easy to see from both the YAML and the scheduling what's going to run when. The current mix of daily + on commit tests is a bit arbitrary and mostly to try to control budget + being due to monolithic test environments, not so much a feature.

longwave’s picture

Status: Active » Needs review

I've defined the eight combinations above in the MR; the MariaDB and SQLite options are commented out as before because I'm not sure they are working yet.

PHP 8.2 & MySQL 8 is the default combination so this has no rules section and should run by default.

I defined a run-on-commit section that runs on the push event or manually in MRs. I also defined a run-daily section that runs on the schedule event or also manually in MRs. We still have to define a schedule in GitLab for the latter to actually take effect.

I think we could then pass variables into the child template to skip some of the linting/tests in certain combinations?

longwave’s picture

> if someone commits three MRs in ten minutes, we're not running three pipelines then but only one at the end of the hour

The pipelines are interruptible which I believe means if a new commit comes in, existing pipelines are aborted and the newer commit restarts the pipeline from scratch. If we eventually start using the UI to merge and the merge train, that also means that each MR that is approved for merge is re-tested before being merged, so you can line up MRs to be merged in succession and the train stops if something fails. I think both of these are preferable to some kind of delayed branch testing.

catch’s picture

#7 is a good argument, and the MR looks less tricky than I thought it would.

longwave’s picture

To think of #4 another way: maybe the default job (PHP 8.2/MySQL 8) runs all linting and tests. But then for the other combinations (whether on commit, on schedule, or manually), we don't need to rerun the linting or unit tests; we only need to run the kernel, functional, functional JavaScript tests?

One problem is that we have defined all the lint steps as dependencies of the kernel, functional, functional JavaScript tests, so we can't skip lint steps in some combinations without changing this.

edit: we can use needs:optional to solve the last part of this I think.

fjgarlin’s picture

The _DB_IMAGE was made like that outside pipeline.yml because we couldn't use conditionals with the variables or when including services (ie: database).

I think having all the matrix in the .gitlab-ci.yml should be fine and then finetune the rules for when each job will run. I like the way this MR is going.

And yes, SQLite and MariaDB are still not working.

longwave’s picture

Re _DB_IMAGE it feels somewhat hacky but for SQLite we could define

    _TARGET_DB_TYPE: "php"
    _TARGET_DB_VERSION: "8.2-apache"

and then _DB_IMAGE can be consistently generated, we just have to map the "php" type to SQLite?!

fjgarlin’s picture

Happy with either, I guess we can accompany those two lines with a comment explaining that sqlite is included in the apache image.

We can't have _TARGET_DB_TYPE being "php" because we use that variable in more places in the "pipeline.yml".

catch’s picture

Just adding an explict note here that once this is committed and running different environments on commit, we should start switching off the respective environments on Drupal CI or move them down to daily.

andypost made their first commit to this issue’s fork.

catch’s picture

Status: Needs review » Reviewed & tested by the community

Looks great and pipeline is passing. We can't fully validate the scheduling rules etc. until it's merged. Unless there's more feedback I'll commit this tomorrow so we can keep going.

fjgarlin’s picture

+1 on the refactoring.

bbrala’s picture

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed 14c65a2 and pushed to 11.x. Thanks!

  • catch committed 14c65a2e on 11.x
    Issue #3387055 by longwave, andypost, fjgarlin: Configure GitLabCI...
catch’s picture

  • catch committed 2890d10b on 10.1.x
    Issue #3387055 by longwave, andypost, fjgarlin: Configure GitLabCI...
catch’s picture

Version: 11.x-dev » 10.1.x-dev

Also backported to 10.1.x

wim leers’s picture

Wow.

This is an incredible improvement. 🤩

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.