State of CI

[tomjohnburton]

In the last couple of weeks, we have been making numerous changes to the way that the CI runs.
I believe that the requirements for each specific event have become somewhat unclear, and it would be great to agree on the state of things.

Below is what the CI currently does for each individual event:

Event	Tasks
1 Every PR to the main branches, develop & main
2 Every merge to develop or main branch
3 Every merge to a release or epic branch
4 Run overnight

Some discussion points:

1. Do we want to run test-single-node against every PR (Event 1)?
2. Do we want to run integration-tests on every merge to develop (Event 2)? Or even more frequently?

Definitions:
Protected branches - main, develop, epic/*, release/*
PR branches - anything that isn't protected

Let's refine this process once and for all 💪

[morelazers]

IMHO:

• PRs to develop, main, release/* and epic/* should run the workflows from 3 & 4
• Merges to develop, main, release/* and epic/* should run the workflows from 3 & 4

Would have been good to include the time taken in the screenshots of the steps, but I bet we lose a lot more time because we have four different workflows. Let's condense it into one (or maybe two), since all the code we merge (or want to merge) should build and run anyway.

[tomjohnburton]

Here is a workflow that outlines steps for PRs. Time 5m47s

And here is one that outlines steps for Merges. Time 11m5s

As you can see (ignoring the failure), I have dropped the integration-tests and building of the docker images when doing a PR. If our integration tests grow and take much more time, I don't personally see the sense in running them in a PR EXCEPT if you are actually introducing changes.

[tomjohnburton]

Already integration tests take nearly 6m extra to run. I think this is a bit much for just PRs

[tomjohnburton]

Actually I have an idea about how to speed that up. Will post here results

[tomjohnburton]

Will have to wait for integration tests to be fixed but initial results look good

[dandanlen]

I'm a little fuzzy on the difference between "PR to main" and "merge to main". What's the difference?

[dandanlen]

Genius !! No but seriously, I remember you posting about some tool that would do such thing a long time ago. Anyway...

You're right, found it! https://bors.tech/

From what I remember, bors creates a temporary branch off the base branch merges in the feature branch, tests the temporary branch, and then if that is ok, merges the temporary branch into base. That was a mouthful... Not sure what the benefit of this is over the naive git merge develop...

Not sure I understand all of this but looks sensible. Basically it would be great to know before a merge is made, whether the result will break anything.

At the cargo-build-and-test-validator-package step, we merge develop

What might be tricky here is that we need to merge in the base branch ie. it's not always develop.

[tomjohnburton]

Okay I pushed an update that allows this. We can extract the target from the github api and then merge

          # setup the github user
          git config --global user.email $( git log --format='%ae' $CIRCLE_SHA1^! )
          git config --global user.name $( git log --format='%an' $CIRCLE_SHA1^! )

          # if this is a PR, we extract the API URL to grab the PR as JSON from the Github API
          if [[ -n "$CIRCLE_PULL_REQUEST" ]]; then
          url="$(echo "$CIRCLE_PULL_REQUEST" | sed 's/https:\/\/github.com\//https:\/\/github.com\/repos\//' | sed 's/\/pull\//\/pulls\//')"
          target_branch=$(
          curl -H "Authorization: token $GITHUB_TOKEN" "$url" | jq '.base.ref' | tr -d '"'
          )
          echo "$target_branch"

          # this will fail if $target_branch is null
          # if it should pass, wrap this in `if [[ -n "$target_branch" ]]; then`
          git fetch && git merge origin/$target_branch --no-edit
          fi```

[tomjohnburton]

https://app.circleci.com/pipelines/github/chainflip-io/chainflip-backend/1230/workflows/82f2249c-3c91-491f-9a95-f12413b24933/jobs/4589?invite=true#step-102-24

[dandanlen]

LGTM.

git merge origin/$target_branch --no-edit

I assume this will cause the whole step to fail if there are merge conflicts?

[tomjohnburton]

Correct

[tomjohnburton]

Current workflow for PRs to protected branches looks like this

[tomjohnburton]

Hold is a manual approval step. So when you're ready to test your PR against develop you can approve the CI, it will merge your target branch, build and test

Check out this job and try it yourself
https://app.circleci.com/pipelines/github/chainflip-io/chainflip-backend/1237/workflows/f3ad618e-17cb-4afb-af36-78dcc4b354ce

[tomjohnburton]

Are the build- steps for deploying actual build artifacts?

Correct

[dandanlen]

Ok cool, got it thanks. Does the approval have to via the circle ci interface or can it be triggered automatically if the PR is approved in github 🤔

[tomjohnburton]

Not saying that is impossible but would require some custom coding

[dandanlen]

Ok, one for the future ;)