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

Parallelize CI workflows #100

Closed
2 tasks
zivnevo opened this issue Oct 19, 2023 · 8 comments · Fixed by #108
Closed
2 tasks

Parallelize CI workflows #100

zivnevo opened this issue Oct 19, 2023 · 8 comments · Fixed by #108
Labels
cicd CICD or build related

Comments

@zivnevo
Copy link
Collaborator

zivnevo commented Oct 19, 2023

Currently, time from opening a PR to getting full testing results is at least 10 minutes. This makes fix-and-retest iterations rather slow. Furthermore, time to get even partial results is too long (e.g., unit tests do not run until lint is done). Splitting work to jobs and running these jobs in parallel can speed up this process.

In particular:

  • In .github/workflows/pr-check.yml: unit-tests doesn't have to be dependent on static-checks-and-build. Moreover, make build runs in both jobs, which is redundant.
  • In .github/workflows/pr-e2e-test.yml: A first job can build docker images and upload them as an artifact. Then the 3 end-to-end tests can download this artifact and run their tests in parallel. See an example here
@zivnevo zivnevo added the cicd CICD or build related label Oct 19, 2023
@zivnevo
Copy link
Collaborator Author

zivnevo commented Oct 19, 2023

@orozery , any thoughts here?

@orozery
Copy link
Collaborator

orozery commented Oct 19, 2023

I think I disagree with an implicit assumption that you have that fix-and-retest should depend on CI.
My view is that you should run all tests on your dev machine, and verify those pass BEFORE pushing any changes.
I see the CI just as "evidence" for others that indeed everything passes.

@zivnevo
Copy link
Collaborator Author

zivnevo commented Oct 19, 2023

Thanks @orozery . I agree that on a day-to-day basis fix-and-retest shouldn't depend on CI.

However, this is complex project with many external dependencies. I'm afraid that expecting every developer to always have exactly the same environment as in the CI might be too much to ask for. From time-to-time a developer will have a slightly different version of Go/linter/kind/docker/make/Ubuntu/... and then checks may pass locally but fail on the CI. And now you have at least one iteration of fix-and-retest.

In addition, for the same reasons as above, I prefer to first open a Draft PR, see that CI passes, and only then ask for reviews. It will really help if I can wait only 5 minutes instead of 10.

Last point: what do we lose from parallelization? If we do it right, it may take exactly the same CPU time as the serial version.

@orozery
Copy link
Collaborator

orozery commented Oct 19, 2023

I actually don't like the current split of unit-tests. Indeed we waste cycles just for the additional setup+build.
I think we should have a single job for running static-check+build+unit-test.
To get the unit-tests, you will "pay" the price of the static check, which should be O(seconds).
The e2e testing on the other hand actually runs parallel to all today. This is OK for now, but IMO if the e2e testing gets too heavy (say, over 10-20 minutes), we should either run it only after unit-tests passed, or manually by the reviewer.

@zivnevo
Copy link
Collaborator Author

zivnevo commented Oct 19, 2023

Runtime for static checks doesn't seem to be negligible.
image

@orozery
Copy link
Collaborator

orozery commented Oct 19, 2023

I see variance between jobs.
e.g.: https://github.com/clusterlink-net/clusterlink/actions/runs/6530281043/job/17729292301
Though most of the time it is more O(m) than O(s).

Anyhow, I'm good with splitting to static-check, build+unit-test.
My problem is actually with the e2e tests.

@zivnevo
Copy link
Collaborator Author

zivnevo commented Oct 19, 2023

My problem is actually with the e2e tests.

How do you like then the proposal to parallelize the 3 e2e tests (second bullet in the first comment)?

@orozery
Copy link
Collaborator

orozery commented Oct 19, 2023

I'm actually working on a writing a suite that has a single k8s-kind setup that serves all tests.
I'm hoping this will allow tests each single e2e test to complete very fast (O(s)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cicd CICD or build related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants