Skip to content
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

Speedup CI #1578

Closed
5 of 7 tasks
ValarDragon opened this issue May 25, 2022 · 7 comments
Closed
5 of 7 tasks

Speedup CI #1578

ValarDragon opened this issue May 25, 2022 · 7 comments

Comments

@ValarDragon
Copy link
Member

ValarDragon commented May 25, 2022

Background

We should invest time in speeding up CI. Lots of time is lost in just waiting for CI to finish executing. If theres any reason we cannot do all of the above in github actions, imo we should investigate moving to a paid CI service, perhaps Circle, where they can all be done. Getting the feedback loop right for rapid merging multiple related PR's is important.

First where we stand on latencies. As of time of issue creation, we are blocked on:

  • CodeQL (Takes 10-15 minutes to complete) https://github.com/osmosis-labs/osmosis/runs/6593339656?check_suite_focus=true
  • Two simultaneous end to end tests (takes around 10 minutes to complete, 3.5 minutes of which is in docker building, and by unreliable visual inspection 1.5 minute seems to be in downloading go packages)
  • (Queued for awhile) the two go tests, each of which takes around 7 minutes to complete, 1.5 min of which I unreliably eyeball estimate to be spent in downloading go packages. A large chunk is also in CLI tests.

Furthermore, many github actions are stuck in 'queued', and take awhile before they begin execution, further increasing this dead time cost.

Components

Acceptance Criteria

  • CI completes in under 5 minutes
@ValarDragon
Copy link
Member Author

ValarDragon commented May 25, 2022

This repo has examples for how we can cache go package downloads in CI: https://github.com/mvdan/github-actions-golang

It also has the caching needed to get incremental builds, which would likely also help some of this CI time.

Seems like the caching for incremental go builds may also help with the docker image build time?

@p0mvn
Copy link
Member

p0mvn commented May 25, 2022

We can also run e2e upgrade tests on merge to main only:

if str := os.Getenv("OSMOSIS_E2E_SKIP_UPGRADE"); len(str) > 0 {

Upgrade testing takes a long time and it doesn't add much value running it on every PR / commit to PR. As long as we know that upgrade tests pass on main, that should be sufficient.

We should still continue running e2e tests on every PR, just the upgrade part can be skipped

@ValarDragon
Copy link
Member Author

oh thats a great idea. Perhaps we can make that variable also get run if theres any change in the upgrades sub folder as well?

@ValarDragon
Copy link
Member Author

ValarDragon commented May 25, 2022

For CodeQL, here are there docs on speeding it up: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/troubleshooting-the-codeql-workflow#the-build-takes-too-long

TL;DR:

  • if self hosting a runner, parallelize
  • Exclude many files from the building, e.g. all _test files. (And in our case, also likely CLI files)
  • Just do it on push to main or on a schedule, not on every PR commit

But to be honest, I don't really see much point in keeping it. I'd rather just use gosec, which is AST based code scanning

mergify bot pushed a commit that referenced this issue May 26, 2022
## What is the purpose of the change

Try adding a cache for go build data, to help speedup CI. Copied from mvdan here: https://github.com/mvdan/github-actions-golang/blob/master/.github/workflows/test.yml

cref #1578 

## Brief Changelog

- Adds go cache to CI

## Testing and Verifying

We will need to keep an eye on if there are edge cases that make this not work / give incorrect results, or things don't speedup. A bit hard to know this without it being in place though.

## Documentation and Release Note

  - Does this pull request introduce a new feature or user-facing behavior changes? yes
  - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? no (I don't think CI is in scope)
  - How is the feature or change documented? We need to figure out a strategy for if/how we want to document CI.
@p0mvn p0mvn mentioned this issue May 26, 2022
6 tasks
@ValarDragon
Copy link
Member Author

ValarDragon commented May 26, 2022

For docker build times, I think we can replace the make docker-build-debug step in test.yml with a direct docker build invocation, and use cache fields, e.g. https://github.com/docker/build-push-action/blob/master/docs/advanced/cache.md#cache-backend-api

cc @nikever

mergify bot pushed a commit that referenced this issue May 26, 2022
## What is the purpose of the change

Removes CodeQL. It takes forever, and I'm unconvinced its adding anything of value. We should just get Informal's gosec fixed, and use that imo.

If folks feel like CodeQL is useful, we can investigate speedup strategies in #1578 , but atm I'm not sure its helping us any.

## Documentation and Release Note

  - Does this pull request introduce a new feature or user-facing behavior changes? no
  - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? no, its CI
  - How is the feature or change documented? not applicable
@niccoloraspa
Copy link
Member

niccoloraspa commented May 27, 2022

Added the cache option in #1602
It gives a nice speed up but I would like to be even faster.

There are some redundant work between go_test step and e2e_test that could be improved.
I'm working on re-structuring a little bit the setup to avoid redoing work.

@ValarDragon
Copy link
Member Author

We've completed everything thats not test-e2e related!

Roman did a great job at summarizing a path to tackle for doing that in #1646 , nothing comes to my mind immediately for speedup steps beyond that. So going to close this in favor of 1646 for now!

Repository owner moved this from Needs Review 🔍 to Done ✅ in Osmosis Chain Development Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants
@niccoloraspa @ValarDragon @p0mvn and others