Skip to content

Migrate CI Pipeline to GitHub Actions#1284

Merged
jb3 merged 9 commits into
masterfrom
sebastiaan/features/move-ci-to-github-actions
Nov 15, 2020
Merged

Migrate CI Pipeline to GitHub Actions#1284
jb3 merged 9 commits into
masterfrom
sebastiaan/features/move-ci-to-github-actions

Conversation

@SebastiaanZ
Copy link
Copy Markdown
Contributor

I've migrated our Azure CI Pipeline to GitHub Actions. While the general workflow is the same, there are a few changes:

  • flake8 is no longer run by pre-commit, but rather by a separate action that adds annotations to the GH Action results page.

  • As we no longer have need for xml-formatted coverage files, the xmlrunner for unittest has been removed as a dependency. Instead, we now publish our coverage results to coveralls.io. Each Actions run will echo a unique jobs link to the output and our README.md has a badge that points to coveralls as well.

  • We use version 2 of docker's GitHub Action build-and-push flow, which is split over multiple steps instead of one.

  • I have changed the badges to GitHub Actions and coveralls.io badges.

Note: Because we accept PRs from forks, we need to be a bit careful with our secrets. While we do use the pull_request_target event, we should not expose secrets in steps that run code from the repository.

I've migrated our Azure CI Pipeline to GitHub Actions. While the general
workflow is the same, there are a few changes:

- `flake8` is no longer run by `pre-commit`, but rather by a separate
  action that adds annotations to the GH Action results page.

- As we no longer have need for xml-formatted coverage files, the
  xmlrunner for unittest has been removed as a dependency. Instead, we
  now publish our coverage results to coveralls.io.

- We use version 2 of docker's GitHub Action build-and-push flow, which
  is split over multiple steps instead of one.

- I have changed the badges to GitHub Actions and coveralls.io badges.

Note: Because we accept PRs from forks, we need to be a bit careful with
our secrets. While we do use the `pull_request_target` event, we should
not expose secrets in steps that run code from the repository.

Signed-off-by: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
@SebastiaanZ SebastiaanZ added a: CI Related to continuous intergration and deployment p: 2 - normal Normal Priority l: 1 - intermediate labels Nov 13, 2020
@SebastiaanZ SebastiaanZ requested a review from MarkKoz November 13, 2020 23:58
@SebastiaanZ SebastiaanZ requested a review from a team as a code owner November 13, 2020 23:58
@SebastiaanZ SebastiaanZ requested review from Senjan21 and removed request for a team November 13, 2020 23:58
@ghost ghost added the needs 2 approvals label Nov 13, 2020
Comment thread .github/workflows/lint-test-build.yml
Comment thread .github/workflows/lint-test-build.yml
Comment thread .github/workflows/lint-test-build.yml Outdated
Comment thread .github/workflows/lint-test-build.yml
Comment thread .github/workflows/lint-test-build.yml Outdated
Comment thread .github/workflows/lint-test-build.yml
@ghost ghost added s: waiting for author Waiting for author to address a review or respond to a comment and removed needs 2 approvals labels Nov 14, 2020
The dependency `coveralls` was installed directly in GitHub Actions, as
it's not required for local dev environments. However, it's a small
package and there's value in keeping all our dependency specifications
in one place. That's why I've moved it to the [dev] section of our
Pipfile.

Signed-off-by: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
By default, the Checkout Actions persists the credentials in the
environment. As our Actions will also run for PRs made from a fork, we
don't want to persist credentials in such a way.

I've also:
- Ported a comment on PIP_USER and pre-commit from the azure configs
- Removed unnecessary id for the pre-commit caching step

Signed-off-by: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
@ghost ghost added needs 2 approvals and removed s: waiting for author Waiting for author to address a review or respond to a comment labels Nov 14, 2020
jb3
jb3 previously requested changes Nov 14, 2020
Comment thread .github/workflows/lint-test-build.yml Outdated
Comment on lines +24 to +30
PIP_NO_CACHE_DIR: false
PIP_USER: 1
PIPENV_HIDE_EMOJIS: 1
PIPENV_IGNORE_VIRTUALENVS: 1
PIPENV_NOSPIN: 1
PRE_COMMIT_HOME: ${{ github.workspace }}/.cache/pre-commit-cache
PYTHONUSERBASE: ${{ github.workspace }}/.cache/py-user-base
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some brief documentation on these variables would be nice

@ghost ghost added s: waiting for author Waiting for author to address a review or respond to a comment and removed needs 2 approvals labels Nov 14, 2020
The `checkName` value of this action needs to have the same value as the name of the job.

Co-authored-by: Joe Banks <joseph@josephbanks.me>
@ghost ghost added needs 1 approval and removed s: waiting for author Waiting for author to address a review or respond to a comment labels Nov 14, 2020
The codeql analysis action we had proved to add little value to our test
suite and has been removed.

Signed-off-by: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
Signed-off-by: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
To make the transition easier, we push the Docker container to both
DockerHub and the GitHub Container Registry. I've also added a secondary
tag by short commit SHA.

Signed-off-by: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
We don't use DockerHub anymore; let's remove it!

Signed-off-by: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
@SebastiaanZ SebastiaanZ requested a review from jb3 November 15, 2020 01:37
Copy link
Copy Markdown
Contributor

@lemonsaurus lemonsaurus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Let's do it!

@ghost ghost removed the needs 1 approval label Nov 15, 2020
@lemonsaurus lemonsaurus dismissed jb3’s stale review November 15, 2020 13:58

Not legal voting age

The docker-compose file should pull the site container from the GitHub
Container Registry instead of DockerHub, as the latter will not receive
new container images.

Snekbox currently still pulls from DockerHub as it's not yet migrated to
GHCR.

Signed-off-by: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
@jb3 jb3 merged commit b40ce9e into master Nov 15, 2020
@jb3 jb3 deleted the sebastiaan/features/move-ci-to-github-actions branch November 15, 2020 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: CI Related to continuous intergration and deployment l: 1 - intermediate p: 2 - normal Normal Priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants