ci: migrate ci setup from circle ci to github actions #464

galargh · 2024-07-05T07:09:48Z

In this PR, we migrate the CI from CircleCI to GitHub Actions.

The important difference in the GitHub Actions setup is that we're running the Linux ARM64 builds on self-hosted GitHub Actions runners.

Code style checks, builds, and tests have been tested in CI - see https://github.com/filecoin-project/filecoin-ffi/actions/runs/9857064865

Publishing will only be verified after the merge.

Before the merge, we have to stop building in CircleCI and change the Required workflows setting.

Once we verify the new workflow works as expected, we're going to remove the CircleCI configuration altogether.

As requested, the GitHub Actions release workflow supports releasing from an arbitrary historical refs using the workflow dispatch trigger with ref input.

…angci-lint

.github/workflows/ci.yml

laurentsenta · 2024-07-09T14:25:05Z

bls_test.go

 		var sigs []Signature
 		var pubks []PublicKey
 		for i := 0; i < size; i++ {
 			msg := Message(fmt.Sprintf("cats cats cats cats %d %d %d dogs", i, i, i))
-			msgs = append(msgs, msg)


We're touching a test, is a typo or expected? I guess that breaks some linting rules?

Yeah, the linting rules started failing. The issue made sense to me so I fixed it. I suspect we're using newer version of some linting tool or something like that.

https://hub.docker.com/layers/cimg/go/1.21.2-node/images/sha256-59b3fff1c4745e9195ea35eef2cfbee4cb0b562616869e304b581644020ad4b8 - golangci-lint v1.54.2 vs v1.56.2

but, I don't see any version of golangci-lint failing on this file. staticcheck does but I don't see it being used here?

btw I'm not objecting to this fix, just interested in why this is showing up for this diff

This was the failing run before I added this change - https://github.com/filecoin-project/filecoin-ffi/actions/runs/9805936223/job/27076653921

It does come from staticcheck as you pointed out. I wonder why we weren't running it in CircleCI.

It is listed in the linters list there, too. Honestly, I'm not sure why it wasn't flagging this issue.

~~make go-lint only runs golangci-lint. So how about we add staticcheck install to this workflow and to the Makefile too so we have it properly covered~~

Needs investigation, golangci-lint is invoking staticcheck, I wonder if we need it installed for that to happen or if there's some config we're missing. Maybe we should just import the lotus config and be done with it. I'll try and get to figuring this out unless someone has a chance before me.

It is running in GHA - that's why I had to add the fix - so I wonder how much time we want to spend investigating this.

apparently golangci-lint has "staticcheck" but not quite the same as "staticcheck", so this is even more confusing ..

it's a good fix, so not a big deal, it would just be god to understand this and maybe fix our linting to do a better job

not a blocker for this PR

rvagg · 2024-07-11T03:36:05Z

.github/actions/configure-environment/action.yml

+        wget -q https://developer.download.nvidia.com/compute/cuda/repos/ubuntu2204/sbsa/cuda-ubuntu2204.pin
+        sudo mv cuda-ubuntu2204.pin /etc/apt/preferences.d/cuda-repository-pin-600
+        wget -q https://developer.download.nvidia.com/compute/cuda/12.5.1/local_installers/cuda-repo-ubuntu2204-12-5-local_12.5.1-555.42.06-1_arm64.deb
+        sudo dpkg -i cuda-repo-ubuntu2204-12-5-local_12.5.1-555.42.06-1_arm64.deb
+        sudo cp /var/cuda-repo-ubuntu2204-12-5-local/cuda-*-keyring.gpg /usr/share/keyrings/
+        sudo apt-get update
+        sudo apt-get -y install cuda-toolkit-12-5
+        sudo mkdir -p /usr/lib/aarch64-linux-gnu/stubs


This is all new for GHA, is the arm64 image available to us different enough that there's no cuda available already?

We're using an almost clean Ubuntu image for Linux arm64 self-hosted runners. We can add installing a version of cuda-toolkit in the machine image to the self-hosted runners backlog if you think it'd be good to have a default available on a machine. One question I have is how much we care about the version of the toolkit.

I'd like to hear @vmx on this; it's his area, cuda and nvidia driver versions for me are just a source of frustration rather than something I know a whole lot about!

For development I recommend installing CUDA drivers from the NVIDIA website as the distro ones often don't work well (for whatever reason). Though for CI, I think using the distro packages could work (KISS). If it works for Linux, it should also work for ARM (I hope).

We should support old enough versions and for filecoin-ffi it's more of a sanity check. So whatever the CUDA version is, as long as the tests pass, we are good.

Got it; I'd move on with installing it on linux arm during the run first, and once we add A version of it to the base image, we'll come back here and remove that step.

.github/workflows/ci.yml

rvagg · 2024-07-11T04:14:02Z

Looks pretty close to circle config. Not at all clearer though! I worry a little about future maintainability of complex GHA configs like this, but I'm not sure what can be done in lieu of a better config language for GHA.

Co-authored-by: Rod Vagg <rod@vagg.org>

BigLep · 2024-07-11T20:28:37Z

For the record, this closes #463

rvagg · 2024-07-15T02:37:54Z

@vmx should be back this week and hopefully he can do a quick review on the relevant bits of this, particularly commenting on the cuda bits in the discussion above, and then we should be good to 🚢

.github/actions/configure-environment/action.yml

vmx · 2024-07-15T12:17:51Z

.github/actions/configure-environment/action.yml

+        sudo ln -s /usr/local/cuda-12.5/lib64/stubs/libcuda.so /usr/lib/aarch64-linux-gnu/stubs/libcuda.so.1
+        sudo ln -s /usr/local/cuda-12.5/lib64/stubs/libcuda.so /usr/lib/aarch64-linux-gnu/stubs/libcuda.so


I'm not sure if that's really needed. It looks a bit like a direct port from the CircleCI config. I'd expect the files to be accessible through /usr/local/cuda/lib64/stubs/libcuda.so (not that there's no specific cuda version in the path). And then later below, it should be enough to put the LD_LIBRARY_PATH to usr/local/cuda/lib64/stubs/ (if that's needed at all, perhaps it's already found automatically).

I've tried the suggestion in #472, but it doesn't seem to cut it, unfortunately.

vmx

I've left comments, but generally it looks good and as long it's green I think it's good to be merged. Hence I'm approving it.

BigLep · 2024-07-22T20:50:09Z

@galargh : anything blocking from getting this merged?

BigLep · 2024-07-23T17:31:58Z

@rvagg : per Slack, @galargh believes this is good to merge. Can you please handle?

galargh · 2024-07-24T15:31:49Z

Thanks for merging. I've now disabled CircleCI in this repo. Please ping me when you prepare to create a new tag here, we'll verify if publishing from GitHub Actions works as expected.

rvagg · 2024-07-25T06:35:08Z

#476 getting rid of circle config so it stops showing up in PRs; I assume this is the appropriate next step here.

Please ping me when you prepare to create a new tag here

Not sure how much we have queued that'll need it, maybe there's a new ref-fvm upgrade coming down the pipe but I don't think it's imminent.

rvagg · 2024-07-25T06:35:54Z

Of course we could just make a new tag, I don't think there'd be a problem with that now that we've got Lotus 1.28.0 done, a tag just for the sake of testing CI wouldn't be a bad thing I think. @rjan90 what do you think?

rjan90 · 2024-07-25T06:46:20Z

Of course we could just make a new tag, I don't think there'd be a problem with that now that we've got Lotus 1.28.0 done, a tag just for the sake of testing CI wouldn't be a bad thing I think. @rjan90 what do you think?

Agree that we can just create a tag for testing CI, but let us schedule it after the Mainnet upgrade on 2024-08-06, just to prevent any user confusion around which release to pull in for other teams depending on Filecoin-FFI for the coming nv23 upgrade.

galargh added 2 commits July 5, 2024 09:09

ci: migrate ci setup from circle ci to github actions

94c119f

fix: the env var setup for the ci workflow

53ba34b

galargh force-pushed the ipdx/ci-migration branch from 744787e to 53ba34b Compare July 5, 2024 07:22

galargh added 8 commits July 5, 2024 09:41

fix: use the correct casing for os checks

1b268ec

fix: gopath setting for the ci workflow

2c31e5b

fix: put go and rust bin on path

98715d9

fix: install cuda on linux arm, change xcode on macos and install gol…

ce16d25

…angci-lint

fix: install libmx

dedd25a

fix: install cuda on linux arm

2129b06

fix: remove unused variable from bls_test

b8c6e28

fix: link libcuda.so correctly

c167a6b

galargh force-pushed the ipdx/ci-migration branch from 0ae4895 to 984399d Compare July 5, 2024 11:48

fix: install pkg-config on linux

5df5d6f

galargh force-pushed the ipdx/ci-migration branch from 984399d to 5df5d6f Compare July 5, 2024 11:53

galargh added 7 commits July 5, 2024 14:06

fix: disable cargo term colors

540c302

fix: update library path on arm too

f1ad9f6

fix: symlink of cuda stubs

d3aad6d

fix: x64 -> ARM64

752f3c8

fix: do not run leak detector by default

353fc4b

chore: enabled triggering release from an arbitrary ref

2c7c643

chore: set DEBIAN_FRONTEND env variable to noninteractive

90e23dd

galargh changed the title ~~WIP: ci: migrate ci setup from circle ci to github actions~~ ci: migrate ci setup from circle ci to github actions Jul 9, 2024

galargh requested a review from laurentsenta July 9, 2024 13:23

laurentsenta reviewed Jul 9, 2024

View reviewed changes

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

laurentsenta reviewed Jul 9, 2024

View reviewed changes

laurentsenta approved these changes Jul 9, 2024

View reviewed changes

chore: remove comments that are not needed anymore

d0ed676

galargh marked this pull request as ready for review July 10, 2024 12:51

galargh requested review from BigLep and rvagg July 10, 2024 12:51

galargh requested a review from rjan90 July 10, 2024 12:52

rvagg reviewed Jul 11, 2024

View reviewed changes

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

rvagg reviewed Jul 11, 2024

View reviewed changes

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

Update .github/workflows/ci.yml

0e69123

Co-authored-by: Rod Vagg <rod@vagg.org>

vmx reviewed Jul 15, 2024

View reviewed changes

.github/actions/configure-environment/action.yml Show resolved Hide resolved

galargh mentioned this pull request Jul 15, 2024

[DX Streamline] Do not hardcode the versions of tools in the CI #470

Open

vmx reviewed Jul 15, 2024

View reviewed changes

vmx approved these changes Jul 15, 2024

View reviewed changes

galargh mentioned this pull request Jul 16, 2024

ci: add /usr/local/cuda/lib64/stubs to ld path #472

Closed

BigLep mentioned this pull request Jul 23, 2024

feat: automate the release process via CHANGELOG.md #473

Closed

rvagg merged commit d751003 into master Jul 24, 2024
14 of 15 checks passed

rvagg deleted the ipdx/ci-migration branch July 24, 2024 03:08

rvagg mentioned this pull request Jul 25, 2024

chore: remove circle CI config #476

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: migrate ci setup from circle ci to github actions #464

ci: migrate ci setup from circle ci to github actions #464

galargh commented Jul 5, 2024 •

edited

Loading

laurentsenta Jul 9, 2024 •

edited

Loading

galargh Jul 10, 2024

rvagg Jul 11, 2024

rvagg Jul 11, 2024

galargh Jul 11, 2024

galargh Jul 11, 2024

rvagg Jul 12, 2024 •

edited

Loading

galargh Jul 15, 2024

rvagg Jul 15, 2024

rvagg Jul 11, 2024

galargh Jul 11, 2024

rvagg Jul 12, 2024

vmx Jul 15, 2024

galargh Jul 15, 2024

rvagg commented Jul 11, 2024

BigLep commented Jul 11, 2024 •

edited

Loading

rvagg commented Jul 15, 2024

vmx Jul 15, 2024

galargh Jul 16, 2024

vmx left a comment

BigLep commented Jul 22, 2024

BigLep commented Jul 23, 2024

galargh commented Jul 24, 2024

rvagg commented Jul 25, 2024

rvagg commented Jul 25, 2024

rjan90 commented Jul 25, 2024

		sudo ln -s /usr/local/cuda-12.5/lib64/stubs/libcuda.so /usr/lib/aarch64-linux-gnu/stubs/libcuda.so.1
		sudo ln -s /usr/local/cuda-12.5/lib64/stubs/libcuda.so /usr/lib/aarch64-linux-gnu/stubs/libcuda.so

ci: migrate ci setup from circle ci to github actions #464

ci: migrate ci setup from circle ci to github actions #464

Conversation

galargh commented Jul 5, 2024 • edited Loading

laurentsenta Jul 9, 2024 • edited Loading

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

rvagg Jul 12, 2024 • edited Loading

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

rvagg commented Jul 11, 2024

BigLep commented Jul 11, 2024 • edited Loading

rvagg commented Jul 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmx left a comment

Choose a reason for hiding this comment

BigLep commented Jul 22, 2024

BigLep commented Jul 23, 2024

galargh commented Jul 24, 2024

rvagg commented Jul 25, 2024

rvagg commented Jul 25, 2024

rjan90 commented Jul 25, 2024

galargh commented Jul 5, 2024 •

edited

Loading

laurentsenta Jul 9, 2024 •

edited

Loading

rvagg Jul 12, 2024 •

edited

Loading

BigLep commented Jul 11, 2024 •

edited

Loading