[RFC] CI overhaul #622

umarcor · 2020-12-08T07:30:55Z

This PR shows a different approach to CI and how container images are defined and used.

Currently, all the Dockerfiles are generated in a single shell/bash script, using environment variables for setting versions. That feels unidiomatic. In this PR, Dockerfiles are defined in separate files, which can be edited without the need to escape due to bash constraints. Differences in procedures between versions of the OSs are handled through dockerfile stages. At the same time, version variables are handled through build arguments.

The current Dockerfile generation strategy cats several sections in a single Dockerfile for each target. Instead, this PR defines two images for each target: 'basic' and 'bazel'. The former contains all the dependencies, except Bazel. The latter is based on the former and includes Bazel.

The current build procedure executes a single docker build command, where all the dependencies are installed and Verible is built. Then, a dummy container is created just for extracting the artifact. Images and container are not saved/reused. In this PR, two docker build commands are executed, one for 'basic' and another one for 'bazel' images. Then, Verible is built in a temporary container.

The rationale is that those 'basic' and 'bazel' images do not need to be built each time. Those can be updated in an scheduled workflow. Then, the regular workflow would only need to pull one image and build Verilator in it. At the same time, any contributor can pull the image and build Verilator, without installing all the dependencies each time.

This was not implemented yet, because a registry needs to be used and I want to have feedback first. Nonetheless, the workflow in this PR shows three steps.

Instead of having multiple jobs trying to create a release, in this PR an additional job is defined, which is to be executed after all the build jobs finish. The additional job (named Release) downloads all the artifacts, creates a release and uploads all the assets at once. This job is only executed on branch master, and not for PRs.

/cc @mithro

googlebot · 2020-12-08T07:31:17Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2020-12-08T07:31:17Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

umarcor · 2020-12-08T10:08:59Z

@googlebot I signed it!

googlebot · 2020-12-08T10:09:13Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

google-cla · 2020-12-08T10:09:14Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

mithro

I like a lot of this pull request, need to take a little bit of a closer look however.

.github/workflows/verible-ci.yml

mithro · 2020-12-08T20:37:54Z

Can you rebase onto master?

umarcor · 2020-12-08T20:41:39Z

I can, but not today. It's not straightforward because install and success scripts are moved into the workflow. I'd rather rebase after #623 and #624.

mithro

I think the direction this is going makes sense.

mithro · 2020-12-08T20:41:10Z

.github/workflows/verible-ci.yml

+
+    - run: docker pull ${{ matrix.os }}
+
+    - name: Build container image 'basic'


I would call this base and maybe Creating "base" container image for ${{ matrix.os }} (to build Verible inside) and Adding "bazel" to "base" container image for ${{ matrix.os }}?

Images are now renamed to 'basic' and 'build'. I use 'base' for the images that are retrieved from Docker Hub.

.github/workflows/verible-ci.yml

mithro · 2020-12-08T20:42:38Z

.github/workflows/verible-ci.yml

+      run: source ./.github/settings.sh; ./releasing/docker-run.sh basic
+
+    - name: Build container image 'bazel'
+      run: source ./.github/settings.sh; ./releasing/docker-run.sh bazel


./releasing/docker-run.sh $OS prepare-image-add-bazel

.github/workflows/verible-ci.yml

mithro · 2020-12-08T20:46:51Z

releasing/centos.dockerfile

+
+RUN yum install -y bison flex
+
+# Build a newer version of flex


This is only needed for some versions of CentOS right?

Yes. That dockerfile has theree stages:

The first stage (L1-L12) is used regardless of the OS version.

The second stage (L14-L25) is executed on CentOS 8 only.

Others use the third stage right below (L27-L53).

The dockerfile was renamed to centos.basic.dockerfile.

mithro · 2020-12-08T20:48:08Z

releasing/centos.dockerfile

@@ -0,0 +1,52 @@
+ARG _VERSION_


BTW -- Ideally these commands would be in the README and pull into these files in some way (maybe using tuttest?).

Now, the list of supported base images is located in releasing/supported_bases.txt. Rather than having that content in the README (which requires parsing Markdown; and that's not fancy), I would just add a link from the README to that file.

umarcor · 2020-12-09T05:34:23Z

This PR is now rebased on top of master.

The failure on centos:8 is expected. It's fixed in #629.

However, I could not track which is the mistake that is making centos:7 fail:

https://github.com/umarcor/verible/runs/1521978326?check_suite_focus=true#step:6:225

...
ERROR: /wrk/centos-7/verible-v0.0-814-gc3f415c/bazel/BUILD:20:15: error executing shell command: '/bin/bash -c function cleanup_function() {
local ecode=$?
if [ $ecode -eq 0 ]; then
cleanup_on_success
else
cleanup_on_failure
fi
}
...

hzeller · 2021-01-29T01:37:53Z

Did this pull request fall of the radar ? @mithro you were looking at it last, any fresh comments ?

mithro · 2021-02-20T01:21:48Z

Can you rebase this onto master and then I think we can consider merging it?

umarcor · 2021-02-20T19:55:45Z

@mithro, as you see, centos7 is still failing. Let's merge #684 first.

codecov-commenter · 2021-08-31T03:50:56Z

Codecov Report

Merging #622 (a50953b) into master (b11c55c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #622   +/-   ##
=======================================
  Coverage   92.84%   92.84%           
=======================================
  Files         337      337           
  Lines       22500    22500           
=======================================
  Hits        20891    20891           
  Misses       1609     1609

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b11c55c...a50953b. Read the comment docs.

google-cla bot added the cla: no A contributor in the pull request has yet to sign the CLA with Google. label Dec 8, 2020

umarcor force-pushed the ci/rework branch 3 times, most recently from 2f8732a to bc28b3b Compare December 8, 2020 09:37

googlebot added cla: yes All contributors in pull request have signed the CLA with Google. and removed cla: no A contributor in the pull request has yet to sign the CLA with Google. labels Dec 8, 2020

umarcor force-pushed the ci/rework branch from bc28b3b to d51cc97 Compare December 8, 2020 10:13

mithro reviewed Dec 8, 2020

View reviewed changes

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

mithro reviewed Dec 8, 2020

View reviewed changes

umarcor mentioned this pull request Dec 8, 2020

ci: generate Release matrix dynamically #623

Closed

umarcor force-pushed the ci/rework branch from a616cb3 to 6347f8b Compare December 9, 2020 04:40

umarcor force-pushed the ci/rework branch from c3f415c to b3686e6 Compare December 9, 2020 15:35

umarcor requested a review from mithro December 10, 2020 03:44

umarcor force-pushed the ci/rework branch from b3686e6 to f4edc54 Compare December 23, 2020 12:30

This was referenced Feb 4, 2021

ci: cleanup #666

Merged

ci: use template dockerfiles #667

Merged

ci: move commands from scripts to workflow #670

Merged

umarcor force-pushed the ci/rework branch 2 times, most recently from 6d930d0 to 9ef429b Compare February 10, 2021 08:57

umarcor mentioned this pull request Feb 20, 2021

ci: add 'releasing/supported_bases.txt' #684

Merged

umarcor force-pushed the ci/rework branch 2 times, most recently from 28615f4 to 675f086 Compare February 20, 2021 19:43

umarcor mentioned this pull request Feb 25, 2021

releasing: move stages to dockerfiles #687

Merged

umarcor force-pushed the ci/rework branch 3 times, most recently from dc12df3 to af29971 Compare July 20, 2021 09:28

umarcor force-pushed the ci/rework branch from af29971 to df63410 Compare July 29, 2021 17:50

umarcor force-pushed the ci/rework branch from df63410 to 3490931 Compare August 31, 2021 03:19

umarcor mentioned this pull request Sep 2, 2021

Convert to Container Action chipsalliance/verible-linter-action#4

Merged

umarcor force-pushed the ci/rework branch from 3490931 to c8504e6 Compare September 18, 2021 12:40

umarcor force-pushed the ci/rework branch 2 times, most recently from a50953b to 19fc43b Compare January 13, 2022 16:40

releasing/docker: overhaul

52abb6e

umarcor force-pushed the ci/rework branch from 19fc43b to 52abb6e Compare January 13, 2022 17:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] CI overhaul #622

[RFC] CI overhaul #622

umarcor commented Dec 8, 2020 •

edited

Loading

googlebot commented Dec 8, 2020

google-cla bot commented Dec 8, 2020

umarcor commented Dec 8, 2020

googlebot commented Dec 8, 2020

google-cla bot commented Dec 8, 2020

mithro left a comment

mithro commented Dec 8, 2020

umarcor commented Dec 8, 2020

mithro left a comment

mithro Dec 8, 2020

umarcor Dec 9, 2020

mithro Dec 8, 2020

mithro Dec 8, 2020

umarcor Dec 9, 2020

umarcor Dec 9, 2020

mithro Dec 8, 2020

umarcor Dec 9, 2020

umarcor commented Dec 9, 2020 •

edited

Loading

hzeller commented Jan 29, 2021

mithro commented Feb 20, 2021

umarcor commented Feb 20, 2021

codecov-commenter commented Aug 31, 2021 •

edited

Loading


		- run: docker pull ${{ matrix.os }}

		- name: Build container image 'basic'


		RUN yum install -y bison flex

		# Build a newer version of flex

[RFC] CI overhaul #622

Are you sure you want to change the base?

[RFC] CI overhaul #622

Conversation

umarcor commented Dec 8, 2020 • edited Loading

googlebot commented Dec 8, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

google-cla bot commented Dec 8, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

umarcor commented Dec 8, 2020

googlebot commented Dec 8, 2020

google-cla bot commented Dec 8, 2020

mithro left a comment

Choose a reason for hiding this comment

mithro commented Dec 8, 2020

umarcor commented Dec 8, 2020

mithro 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

umarcor commented Dec 9, 2020 • edited Loading

hzeller commented Jan 29, 2021

mithro commented Feb 20, 2021

umarcor commented Feb 20, 2021

codecov-commenter commented Aug 31, 2021 • edited Loading

Codecov Report

umarcor commented Dec 8, 2020 •

edited

Loading

umarcor commented Dec 9, 2020 •

edited

Loading

codecov-commenter commented Aug 31, 2021 •

edited

Loading