silverstripe / github-actions-ci-cd

2 stars 2 forks source link

Agreeing on a general approach #1

Closed maxime-rainville closed 2 years ago

maxime-rainville commented 2 years ago

I reviewed Steve initial work ... before we start doing serious work on this and maybe encouraging other people to use this approach, we should agree on how we want to structure this CI/CD pipeline. Here are some key questions I would like us to answer.

Develop bottom-up or up-down

This is more of broad philosophical question. Essentially should we:

Workflow vs customs steps

GitHub Actions has two related concept that enable code reuse:

We need to figure out which parts of our build pipeline should be reusable workflow and which part should be in workflows.

One workflow to rule them all or several smaller workflow

The travis-shared-config combines everything in a single set up: behat test, PHPUnit test, Jest, linting, etc. Travis only allowed you to run one build per repo/branch.

GitHub actions allows you to define multiple workflows. So we could do something like:

Requiring third party steps

There's a wide array of pre-existing actions in the GitHub actions market place. We're practical, we should default to using pre-existing steps instead of re-inventing our owns.

What about templates?

GitHub allows you to create workflow templates that you can reuse across repos. We should identify which templates we might need up front.

Multi DB set up

To keep running our multi DB set up, we probably need a magic step that receives a DB type and magically set up the appropriate database.

Matrix strategy

In the past we tended to test specific PHP releases and recipe versions.

An alternative approach could be to test the lowest supported dependencies, latest stable dependencies and latest unstable dependencies.

It could also be interesting to see if we could dynamically detect the supported PHP versions of a module and test this and use that to install the appropriate PHP version for testing.

We probably need to have different strategies for recipe and modules. The sink is kind of its own special use case.

Moving dependencies to require-dev

Historically, we've manually required dependencies that are needed for a build to run within the build itself. This is weird has those would normally be considered dev dependencies.

We should re-examine this decision and see how this could allow us to simplify our build set up.

How do we handle different branches

Travis runs its build based on the .travis.yml file for the specific branch that is affected. GitHub actions runs build based on the default branch. This is a substantial difference. We should try to see how we will run builds for different minor release lines.

maxime-rainville commented 2 years ago

Develop bottom-up or up-down

I'm not confident with our level of familiarity with GitHub actions. I'd rather we start with simple actions and work our way up to more complex scenarios.

The current workflow we have in this repos seems to be doing some weird things. My instinct would be to spit it up in smaller more manageable chunks and validate the assumption in each chunks.

Workflow vs customs steps

I think we should start with building actions before building workflows. Those are easier to reuse and to recombine then workflows.

Workflows should handle things like building matrices and assembling common steps to perform bigger task.

For example, here's a step I built that run the unit test for a module: https://github.com/maxime-rainville/silverstripe-ci-phpunit/blob/master/action.yml

A module can use this step to perform more complex tasks: https://github.com/maxime-rainville/silverstripe-react/blob/master/.github/workflows/phpunit.yml

A lot of the steps in the workflow in this module should be spun off as custom actions ... or swap for marketplace actions other people have written.

One workflow to rule them all or several smaller workflow

My instinct would be to split our CI into a bunch of smaller workflows. The set up work for JEST, Behat or PHPUNit is substantially different. A lot of the shared config complexity derives from the fact that we are forced to do everything in one build.

What about templates?

This is highly dependent on how we want to split the CI built. I would wait until we have a pretty solid set up before we start looking at creating templates.

Multi DB set up

I'm thinking something like this:

    steps:
    - name: Run test suite
      id: db
      uses: silverstripe/ci-dbsetup
      with:
        db: mysql # Could pass matrix values here
        version: 8
    - name: Run unit tests
      run:  '....'
      env:
        DB: 
        SS_DATABASE_CLASS: ${{ steps.db.outputs.class }}
        SS_DATABASE_SERVER: ${{ steps.db.outputs.server }}
        SS_DATABASE_USERNAME: ${{ steps.db.outputs.username }}
        SS_DATABASE_PASSWORD: ${{ steps.db.outputs.password }}
        SS_DATABASE_NAME: ${{ steps.db.outputs.name }}

"Matrix strategy" and "How do we handle different branches"

I think splitting our workflows will make managing matrixes easier. Also, the fact that all branches run the same workflow will require us to detect what the requirements of the release line are.

I would rather we move to a scenario where we test the

This is example shows this approach working: https://github.com/maxime-rainville/silverstripe-react/blob/master/.github/workflows/phpunit.yml

Also we should have one workflow for recipes and one for modules.

I think the sink is its own special use case and should use its own custom workflow.

Other things

emteknetnz commented 2 years ago

I think one of the main requirements is that as much config as possible is stored in a single location so that we're able to roll-out global config changes easily. That was the driving reason behind using the travis shared config.

With this approach proposed above, it could be a little more central. For instance if this represents a module, we've got the php versions defined in the module, rather than in the shared config.

What this means is that to update the minimum version of PHP from 7.3 to 7.4, we'd need to update 80+ modules

I think we'd want to move the matrix to inside action.yml - that's probably not too a big a jump

That said, I'm not sure what the value of splitting a big central file into smaller files would be.

If we took my original approach and split the steps into separate yml files, on a module with behat + npm tests the before and after would looks like:

Before

name: Module CI

on:
  push:
  pull_request:

jobs:
  ci:
    uses: silverstripe/github-actions-ci-cd/.github/workflows/ci.yml@0.1.0
    with:
        run_endtoend: true
        run_js: true

After

name: Module CI

on:
  push:
  pull_request:

jobs:
  ci_phpunit:
    uses: silverstripe/github-actions-ci-cd/.github/workflows/phpunit.yml@0.1.0
  ci_phplinting:
    uses: silverstripe/github-actions-ci-cd/.github/workflows/phplinting.yml@0.1.0
  ci_phpcoverage:
    uses: silverstripe/github-actions-ci-cd/.github/workflows/phpcoverage.yml@0.1.0
  ci_endtoend:
    uses: silverstripe/github-actions-ci-cd/.github/workflows/endtoend.yml@0.1.0
  ci_js:
    uses: silverstripe/github-actions-ci-cd/.github/workflows/js.yml@0.1.0

Which is a bit more config in the module, and maybe a bit less central control? I'm not entirely sure what additional benefit is being added here?

I guess one benefit is it's a little tidier inside the jobs status, you would have less skipped steps which does add a bit of visual clutter

image

Though that's probably not really annoying in practice since you'd only look inside the job status if something failed, and github actions will automatically open and scroll to the step that failed, e.g.

image

maxime-rainville commented 2 years ago

Discussion has moved on to to other things