ci: Port CI to GitHub Actions #780

jan-auer · 2020-09-18T07:47:00Z

Moves all CI jobs from Travis and AppVeyor to GitHub Actions. This comes with a few changes in the build pipeline:

Python linting and formatting is checked again
The macOS wheel now requires 10.15.0 since this is the minimum version offered by GitHub
Integration tests no longer run haproxy or gobetween. They can be added as service if they provide any value.

Due to the wheel change, this will require a changelog entry.

.github/workflows/ci.yml

jan-auer · 2020-09-18T09:19:39Z

For some reason, macOS is failing now, where it already passed in 312845f. Since it complains that serde_derive is missing, I'm assuming that caches are somehow broken.

error[E0463]: can't find crate for `serde_derive` which `serde` depends on
##[error] --> relay-redis/src/config.rs:1:5
  |
1 | use serde::{Deserialize, Serialize};
  |     ^^^^^ can't find crate

.github/workflows/build.yml

BYK · 2020-09-18T17:40:52Z

.github/workflows/ci.yml

+        with:
+          python-version: ${{ matrix.python-version }}
+
+      - name: Install Dependencies


Why not move this after the actions/cache step so it is also cached?

Btw protip: you can skip entire steps based on the action/cache step's output: getsentry/sentry-docs#2231

We're not caching toolchain installations, only cargo's own cache and the target folder. These are the expensive operations that take time. All the rest is usually so fast that caching doesn't really matter and I opted to reduce caches to the absolute minimum.

FWIW, you can see that release builds don't have caches at all, to ensure we're never working off a poisoned cache.

FWIW, you can see that release builds don't have caches at all, to ensure we're never working off a poisoned cache.

I don't think poitioned caches are a significant thread but even if they were, we can simply make the caching step conditional on this not being a release branch and speed up PR builds?

.github/workflows/ci.yml

.github/workflows/deploy.yml

BYK · 2020-09-18T18:24:59Z

.github/workflows/deploy.yml

+          git config --global user.name "$(git log -1 --pretty=format:%an $GITHUB_SHA)"
+          git config --global user.email "$(git log -1 --pretty=format:%ae $GITHUB_SHA)"
+          git clone https://getsentry-bot:[email protected]/getsentry/sentry-data-schemas


I think this part can/should be replaced with actions/checkout

Sounds good, I'll consider that in a follow-up PR. These jobs were just moved, so I would like to not touch them.

My plan is to make this a reusable action that we can use for all sorts of cross-committing activities we do in the organization.

jan-auer · 2020-09-18T20:43:20Z

This is ready now. Will disable AppVeyor and update status checks just before merging and then push the changelog to trigger the final build.

billyvg

lgtm!

billyvg · 2020-09-18T22:06:27Z

.github/workflows/build_binary.yml

+      - release/**
+
+jobs:
+  linux:


You could use a matrix here, but tbh what you have is probably more clear despite some duplication

Good point. I had a matrix at first, but ended up choosing this for the cleaner config. As soon as craft supports GitHub artifacts, the biggest chunk of duplication (zeus uploads) will be gone and this will be neat and clean. Would propose to revisit once we can move off Zeus.

untitaker

Do we really want to stop using the makefile entirely in CI? That creates a split between local dev and it.

untitaker · 2020-09-19T12:37:05Z

.github/workflows/build_binary.yml

+          ZEUS_API_TOKEN: ${{ secrets.ZEUS_API_TOKEN }}
+          ZEUS_HOOK_BASE: ${{ secrets.ZEUS_HOOK_BASE }}
+        run: |
+          npm install -D @zeus-ci/cli


out of curiosity, what was wrong with install -g and running the binary directly?

Good point, I can try that. Originally, I used just npx, and then noticed that it is reinstalling on every invocation. On Azure Pipelines, one cannot run global installations, but that may work on GitHub actions.

untitaker · 2020-09-19T12:38:50Z

.appveyor.yml

-on_success:
-  - zeus job update --status=passed || echo "%$APPVEYOR_REPO_BRANCH%" | findstr /V "release/">nul
-
-on_failure:


on_failure does not seem to be ported, is there no equivalent on gha?

There is: https://github.com/getsentry/onpremise/blob/21d1b6d42eee14d0de93ba77fe981f9f2a82447e/.github/workflows/test.yml#L38-L42

Also, we no longer need this, as we do not depend on Zeus for status checks. It is only an artifact storage, so it does not matter whether we report failures.

untitaker · 2020-09-19T12:40:56Z

.github/workflows/deploy.yml

+
+      - name: Deploy
+        if: github.ref == 'refs/heads/master'
+        uses: peaceiris/actions-gh-pages@v3


this has come up before but do we have a general policy wrt third-party github actions? since github tags are mutable as opposed to most package registries, and this has access to a token

Deferring to the policy-makers here. So far, there has been no push-back.

Note that this was not changed from master, just moved.

Maybe @billyvg has insights?

untitaker · 2020-09-19T12:45:51Z

Cargo.toml

@@ -2,5 +2,12 @@
 members = ["relay", "relay-*", "tools/*"]
 default-members = ["relay"]

+[profile.dev]


This seems like counterintuitive behavior. Do we want to start doing this across the org?

I think we should do this only where it shows significant benefits. I can imagine doing this in symbolicator, although there stack traces are actually useful for debugging in contrast to Relay. Repositories like symbolic or even smaller crates build fast enough so that such a hack is not necessary.

For the curious: This cuts build times in half, and reduces the size of the target folder by 45%.

jan-auer · 2020-09-21T07:12:39Z

Here are two test releases with craft:

Library run: https://github.com/getsentry/relay/runs/1142528497
Library artifacts: https://zeus.ci/gh/getsentry/relay/builds/7793
Binary run: https://github.com/getsentry/relay/runs/1142527218
Binary artifacts: https://zeus.ci/gh/getsentry/relay/builds/7795/artifacts

jan-auer · 2020-09-21T07:28:25Z

This will require getsentry/craft#124 for successful publishing with craft. Once we've released a patch release of craft, we need to bump the minimum craft requirement in craft.yml and then this can be merged.

* master: feat: Scrubbing of UTF-16 strings in minidumps (#742) meta: Update CI badges (#782) fix(pii): Fix issue where `$span` would not be recognized in Advanced Data Scrubbing (#781) ci: Port CI to GitHub Actions (#780) fix(setup): Log when reporting to Sentry is disabled (#779)

jan-auer added 2 commits September 18, 2020 09:41

ci: Run lints on release branches

8f96b8f

ci: Add default-feature tests to GHA

462f094

jan-auer requested a review from a team September 18, 2020 07:47

jan-auer marked this pull request as draft September 18, 2020 07:47

jan-auer added 4 commits September 18, 2020 09:50

ci: Fix workflow matrix config

94a585d

ci: Run correct test command

2ef460c

ci: Fix cache key for test job

312845f

ci: Start redis and test all features

bf2f6a7

untitaker approved these changes Sep 18, 2020

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

jan-auer added 3 commits September 18, 2020 10:45

ci: Split test jobs

9a86f37

ci: Expose Redis ports

0079cf4

ci: Clean out old jobs

910b144

jan-auer added 4 commits September 18, 2020 11:22

build: Disable debug information in debug builds

a6e7914

ci: Port integration tests to GHA

df73c86

ci: Simplify integration tests

73f8bc2

ci: Port python library tests to GHA

a779cc3

jan-auer force-pushed the build/remove-appveyor branch from d4ef88e to a779cc3 Compare September 18, 2020 10:48

jan-auer added 2 commits September 18, 2020 13:45

ci: Test Zeus uploads

9ad447f

ci: Fix actions syntax

92401eb

jan-auer force-pushed the build/remove-appveyor branch from fb1df50 to 92401eb Compare September 18, 2020 11:50

jan-auer added 9 commits September 18, 2020 13:59

ci: Simplify npx commands

0b71181

ci: Split up workflows

1d76e86

ci: Add builds for Linux, macOS and Windows

e835278

ci: Fix actions syntax

5a2e998

ci: Fix build jobs

e5d8bfd

ci: Fix build and debug bundling

db42ff8

ci: Clean up old builds

b3625d8

ci: Lint and style-check python code

1375bb0

ci: Fix windows release build uploads

2af0fde

jan-auer added 2 commits September 18, 2020 19:53

ci: Fix lint python dependencies

ea6c7f5

fix: Actual python formatting

37b4aa3

BYK reviewed Sep 18, 2020

View reviewed changes

jan-auer added 9 commits September 18, 2020 20:38

ci: Set kafka partition count to 1

bcc6519

ci: Port library builds to GHA

f2b3cc0

ci: Split build workflow

7ca2734

ci: Simplify docker builds

5c29000

ci: Build for macOS 10.15

f9ccb9a

ci: Install wheel for wheel builds

5805067

ci: Do not build manylinux with -it

3e35d67

ci: Try expression syntax

da94db2

ci: Disable release builds on PRs

0e1bc96

jan-auer marked this pull request as ready for review September 18, 2020 20:42

jan-auer requested a review from untitaker September 18, 2020 20:59

billyvg reviewed Sep 18, 2020

View reviewed changes

untitaker approved these changes Sep 19, 2020

View reviewed changes

jan-auer added 2 commits September 21, 2020 08:24

fix(craft): Remove Travis CI context for library releases

f026f29

meta: Changelog

7b97e4b

build: Bump macOS wheel version

0859f0d

untitaker mentioned this pull request Sep 21, 2020

fix(pii): Fix issue where $span would not be recognized in Advanced Data Scrubbing #781

Merged

build: Bump minimum craft requirement

bf2375f

jan-auer merged commit e5c85a6 into master Sep 21, 2020

jan-auer deleted the build/remove-appveyor branch September 21, 2020 08:35

jan-auer mentioned this pull request Sep 21, 2020

meta: Update CI badges #782

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: Port CI to GitHub Actions #780

ci: Port CI to GitHub Actions #780

jan-auer commented Sep 18, 2020 •

edited

Loading

jan-auer commented Sep 18, 2020

BYK Sep 18, 2020

BYK Sep 18, 2020

jan-auer Sep 18, 2020

BYK Sep 18, 2020

BYK Sep 18, 2020

jan-auer Sep 18, 2020

jan-auer commented Sep 18, 2020 •

edited

Loading

billyvg left a comment

billyvg Sep 18, 2020

jan-auer Sep 20, 2020

untitaker left a comment

untitaker Sep 19, 2020

jan-auer Sep 20, 2020

untitaker Sep 19, 2020

BYK Sep 20, 2020

jan-auer Sep 20, 2020

untitaker Sep 19, 2020

jan-auer Sep 20, 2020

jan-auer Sep 20, 2020

untitaker Sep 19, 2020

jan-auer Sep 20, 2020

jan-auer commented Sep 21, 2020 •

edited

Loading

jan-auer commented Sep 21, 2020

ci: Port CI to GitHub Actions #780

ci: Port CI to GitHub Actions #780

Conversation

jan-auer commented Sep 18, 2020 • edited Loading

jan-auer commented Sep 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jan-auer commented Sep 18, 2020 • edited Loading

billyvg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

untitaker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jan-auer commented Sep 21, 2020 • edited Loading

jan-auer commented Sep 21, 2020

jan-auer commented Sep 18, 2020 •

edited

Loading

jan-auer commented Sep 18, 2020 •

edited

Loading

jan-auer commented Sep 21, 2020 •

edited

Loading