-
Notifications
You must be signed in to change notification settings - Fork 839
Migrate CircleCI workflows to GitHub Actions (1/3) #3302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| # GitHub Actions CI/CD | ||
|
|
||
| The purpose of this workflow is to run all continuous integration (CI) and continuous deployment (CD) jobs when needed while respecting their internal dependencies. The continuous integration jobs serve to ensure new code passes linting, unit tests and integration tests before reaching the master branch. The continuous deployment jobs serve to deploy the latest version of the code to cortex and the website when merged with master. | ||
|
|
||
| ## Contributing | ||
|
|
||
| If you wish to add a new CI or CD job, add it to the existing current test-build-deploy workflow and make sure it does not prevent any of the other jobs from passing. If you wish to change any of the build or testing images, update it in all sections are containers are often reused. If you wish to add an entirely new workflow, create a new yml file with separate triggers and filters. In all cases, clearly document any changes made to the workflows, images and dependencies below. | ||
|
|
||
| ## Test, Build and Deploy | ||
|
|
||
| test-build-deploy.yml specifies a workflow that runs all Cortex continuous integration and continuous deployment jobs. The workflow is triggered on every pull request and commit to master, however the CD jobs only run when changes are merged onto master . The workflow combines both CI and CD jobs, because the CD jobs are dependent on artifacts produced the CI jobs. | ||
|
|
||
|
|
||
| ## Specific Jobs | ||
|
|
||
| | Job | Description | Type | | ||
| |------------------------|-------------------------------------------------------------------------------------------------------------------------------|------| | ||
| | lint | Runs linting and ensures vendor directory, protos and generated documentation are consistent. | CI | | ||
| | test | Runs units tests on Cassandra testing framework. | CI | | ||
| | integration-configs-db | Integration tests for database configurations. | CI | | ||
| | integration | Runs integration tests after upgrading golang, pulling necessary docker images and downloading necessary module dependencies. | CI | | ||
| | build | Builds and saves an up-to-date Cortex image and website. | CI | | ||
| | deploy_website | Deploys the latest version of Cortex website to gh-pages branch. Triggered within workflow. | CD | | ||
| | deploy | Deploys the latest Cortex image. | CD | | ||
|
|
||
| ## Job Dependency Graph | ||
|
|
||
| Internal dependencies between jobs illustrated below. Jobs run concurrently where possible but do not start until all jobs they depend on have completed successfully. | ||
|
|
||
|
|
||
|  | ||
|
|
||
| ### Key Details | ||
|
|
||
| **Naming Convention** | ||
|
|
||
| Each step in a job has a clear name that encapsulates the purpose of the command. The convention we are using is each word in the name should be capitalized except articles and prepositions. This creates consistent labeling when looking at the progress of the current workflow on GitHub. | ||
|
|
||
| ```yaml | ||
| - name: Checkout Repo | ||
| # commands | ||
| - name: Get Dependencies | ||
| # commands | ||
| ``` | ||
|
|
||
| **Checkout Version** | ||
|
|
||
| Current build-image ships with an older version of Git which breaks github/actions@v2 so we are using actions/checkout@v1 for all jobs until the quay image is updated to ship with a more recent version of Git. | ||
|
|
||
| ```yaml | ||
| - name: Checkout Repo | ||
| uses: actions/checkout@v1 | ||
| ``` | ||
|
|
||
| **Symbolic Link to Expected Workspace** | ||
|
|
||
| A significant number of commands in the Makefile are hardcoded with an assumed file structure of the CI container. To ensure paths specified in previous commands don’t break, a symlink was created from the hardcoded “expected” working directory `/go/src/github.com/cortexproject/cortex` to the actual working directory `$GITHUB_WORKSPACE`. | ||
|
|
||
| ```yaml | ||
| - name: Sym link expected path to github workspace | ||
| run: | | ||
| mkdir -p /go/src/github.com/cortexproject/cortex | ||
| ln -s $GITHUB_WORKSPACE/* /go/src/github.com/cortexproject/cortex | ||
| ``` | ||
|
|
||
| **Sharing Artifacts Between Jobs** | ||
|
|
||
| As of October 2020, GitHub Actions do not persist between different jobs in the same workflow. Each job is run on a fresh virtual environment (https://docs.github.com/en/free-pro-team@latest/actions/learn-github-actions/introduction-to-github-actions#runners). As such, we need to upload and download artifacts to share data between jobs. | ||
|
|
||
| | Artifact | Stored In | Used By | Purpose of Storing Artifact | | ||
| |-------------------------------|-----------|---------------------------------------------|-----------------------------| | ||
| | website public | build | deploy_website | share data between jobs | | ||
| | Docker Images | build | deploy, integration, integrations-config-db | share data between jobs | | ||
| | Frontend Protobuf | build | | long term storage | | ||
| | Caching Index Client Protobuf | build | | long term storage | | ||
| | Ring Protobuf | build | | long term storage | | ||
| | Rules Protobuf | build | | long term storage | | ||
|
|
||
| *Note:* Docker Images are zipped before uploading as a workaround. The images contain characters that are illegal in the upload-artifact action. | ||
| ```yaml | ||
| - name: Compressing Images | ||
| run: tar -zcvf images.tar.gz /tmp/images | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if compression gives us anything here. We can use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The compression was in fact unnecessary. We removed it in the subsequent migration PR: #3341 |
||
| - name: Cache Images | ||
| uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: Docker Images | ||
| path: ./images.tar.gz | ||
| ``` | ||
| **Tags** | ||
|
|
||
| As of Oct 2020, GitHub [does not support](https://github.202132.xyzmunity/t/using-regex-for-filtering/16427/2) regex for tag filtering. The regex /^v[0-9]+(\.[0-9]+){2}(-.+|[^-.]*)$/ was approximated using the available GitHub [filter patterns](https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet) | ||
| ```yaml | ||
| tags: | ||
| - v[0-9]+.[0-9]+.[0-9]+** | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| name: ci | ||
| on: | ||
| push: | ||
| branches: [master] | ||
| tags: | ||
| - v[0-9]+.[0-9]+.[0-9]+** # Tag filters not as strict due to different regex system on Github Actions | ||
AzfaarQureshi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| pull_request: | ||
| jobs: | ||
| lint: | ||
| runs-on: ubuntu-latest | ||
| container: | ||
| image: quay.io/cortexproject/build-image:update-golang-1.14.9-eb0c8d4d2 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think we can use something similar to
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There exists a "defaults" field for GitHub actions, but it automatically applies to all jobs in the workflow. The integration and integration-configs-db jobs that will be added to the workflow with PR 2/3 of the migration do not use the default container so using a default would require explicitly removing the default container from those jobs by setting them to None. We would like to reuse setup commands and default containers with something like commands in CircleCI, but this feature is not available yet on GitHub Actions. Status: https://github.202132.xyzmunity/t/reusing-sharing-inheriting-steps-between-jobs-declarations/16851
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GHA doesn't support yaml anchors yet because of some security reasons but they are actively working on it
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible to use env variable instead?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a blocker for getting this PR merged. We can discuss it separately. |
||
| steps: | ||
| - name: Checkout Repo | ||
| uses: actions/checkout@v1 | ||
| - name: Sym Link Expected Path to Workspace | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This linking is explained in README, apart from doing review, who reads that? I'd include the explanations from README directly into this file.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment explaining this within the code itself in the subsequent PR to address this. |
||
| run: | | ||
| mkdir -p /go/src/github.com/cortexproject/cortex | ||
| ln -s $GITHUB_WORKSPACE/* /go/src/github.com/cortexproject/cortex | ||
|
Comment on lines
+16
to
+19
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A significant number of commands in the Makefile were hardcoded with an assumed file structure of the CI container. make sure previous commands don’t break a symlink was created instead from the hardcoded “expected” working directory |
||
| - name: Lint | ||
| run: make BUILD_IN_CONTAINER=false lint | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we set
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But |
||
| - name: Check Vendor Directory | ||
| run: make BUILD_IN_CONTAINER=false mod-check | ||
| - name: Check Protos | ||
| run: make BUILD_IN_CONTAINER=false check-protos | ||
| - name: Check Generated Documentation | ||
| run: make BUILD_IN_CONTAINER=false check-doc | ||
| - name: Check White Noise. | ||
| run: make BUILD_IN_CONTAINER=false check-white-noise | ||
|
|
||
| test: | ||
| runs-on: ubuntu-latest | ||
| container: | ||
| image: quay.io/cortexproject/build-image:update-golang-1.14.9-eb0c8d4d2 | ||
| services: | ||
| cassandra: | ||
| image: cassandra:3.11 | ||
| env: | ||
| JVM_OPTS: "-Xms1024M -Xmx1024M" | ||
| ports: | ||
| - 9042:9042 | ||
| steps: | ||
| - name: Checkout Repo | ||
| uses: actions/checkout@v1 | ||
| - name: Sym Link Expected Path to Workspace | ||
| run: | | ||
| mkdir -p /go/src/github.com/cortexproject/cortex | ||
| ln -s $GITHUB_WORKSPACE/* /go/src/github.com/cortexproject/cortex | ||
| - name: Run Tests | ||
| run: CASSANDRA_TEST_ADDRESSES=cassandra:9042 make BUILD_IN_CONTAINER=false test | ||
|
|
||
| build: | ||
| runs-on: ubuntu-latest | ||
| container: | ||
| image: quay.io/cortexproject/build-image:update-golang-1.14.9-eb0c8d4d2 | ||
| steps: | ||
| - name: Checkout Repo | ||
| uses: actions/checkout@v1 | ||
| - name: Install Docker Client | ||
| run: | | ||
| set -x | ||
| VER="17.03.0-ce" | ||
| curl -L -o /tmp/docker-$VER.tgz https://download.docker.com/linux/static/stable/x86_64/docker-$VER.tgz | ||
| tar -xz -C /tmp -f /tmp/docker-$VER.tgz | ||
| mv /tmp/docker/* /usr/bin | ||
| - name: Sym Link Expected Path to Workspace | ||
| run: | | ||
| mkdir -p /go/src/github.com/cortexproject/cortex | ||
| ln -s $GITHUB_WORKSPACE/* /go/src/github.com/cortexproject/cortex | ||
| - name: Build Image | ||
| run: | | ||
| touch build-image/.uptodate | ||
| make BUILD_IN_CONTAINER=false | ||
| - name: Build Website | ||
| run: | | ||
| touch build-image/.uptodate | ||
| make BUILD_IN_CONTAINER=false web-build | ||
| - uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: website public | ||
| path: website/public/ | ||
| - uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: Frontend Protobuf | ||
| path: pkg/querier/frontend/frontend.pb.go | ||
| - uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: Caching Index Client Protobuf | ||
| path: pkg/chunk/storage/caching_index_client.pb.go | ||
| - uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: Ring Protobuf | ||
| path: pkg/ring/ring.pb.go | ||
| - uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: Cortex Protobuf | ||
| path: pkg/ingester/client/cortex.pb.go | ||
| - uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: Rules Protobuf | ||
| path: pkg/ruler/rules/rules.pb.go | ||
|
Comment on lines
+78
to
+101
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you figured out if we actually need to upload these artifacts? I know we do it in CircleCI but I have the feeling they're unused (at least I don't remember if we still use them anywhere).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, we included it because this PR here made it seem like there was an explicit decision to save these files. Has the situation changed? But you are right, those artifacts are not used anywhere else within CI
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since then we have added these files into Git, so I think #1224 is not relevant anymore.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (In #1399)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also think we don't need to store these artifacts anymore. |
||
| - name: Save Images | ||
| run: | | ||
| mkdir /tmp/images | ||
| ln -s /tmp/images ./docker-images | ||
| make BUILD_IN_CONTAINER=false save-images | ||
| - name: Zip Images | ||
| run: tar -zcvf images.tar.gz /tmp/images | ||
| - name: Upload Images Artifact | ||
| uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: Docker Images | ||
| path: ./images.tar.gz | ||
|
Comment on lines
+107
to
+113
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As of Oct 2020 GitHub Actions does not persist between different jobs in the same workflow. Each job is run on a fresh virtual environment. As such, we need to upload and download artifacts to share data between jobs. Docker Images are zipped before uploading as a workaround. The images contain characters that are illegal in the upload-artifact action. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks suspicious... why create many links instead of one: