nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.37k stars 323 forks source link

Replicate (mostly) buildbot workflow for commits into master #1225

Closed oxpa closed 3 months ago

oxpa commented 5 months ago

Hello,

This pull request utilizes nginx self hosted runners to build unit and it's modules on a variety of distros and two archs (for the lack of others for now).

The proposed CI creates 252 jobs which can be a bit excessive and may require some optimisations: some jobs don't really do what they should do just skipping test. It's mostly true for python, java and php where each distro has a number of version available and I was too lazy to get a proper configuration for what plays where.

I also replicated configuration management system by creating setup.sh which installs packages from a number of lists. And I created those per-OS list. The list is a bit excessive and replicates current buildbot workers. So there is some room for optimisations there as well.

The existing CI is left active for pull requests as it provides relatively fast way of checking code for obvious problems.

ac000 commented 5 months ago

Some observations

1) Does it make sense to still rename ci.yml?, seeing as it isn't limited to pull-requests. 2) The last patch should just be folded into the first or 3) Do we even need those commits (first & last) now?

callahad commented 5 months ago

I agree with Andrew's comments above; LGTM once those are addressed.

oxpa commented 5 months ago

@ac000

  1. I'm happy to rename files further. See no reason to change their names but happy to rename it whatever you prefer.
  2. I expect the purll request to be squash-merged. See no reason to keep this history of commits
  3. Same as above: there should be a single commit in the repository.
callahad commented 5 months ago

Let's rename back to ci.yml and call this one ci-selfhosted.yml or something like that?

ac000 commented 5 months ago
2. I expect the purll request to be squash-merged. See no reason to keep this history of commits

If you keep the ci.yml change, then that should at least be a separate commit (with an explanation of why it's needed).

oxpa commented 5 months ago

@arbourd Running too may tests is my main concern, really. I failed to find an easy way of pinning os-lang-version triplet while this will reduce the amount of jobs significantly. I'm happy to adopt any solution (maybe just hardcode it in?). For now, I'm going to split the workflow into two parts, one per arch to avoid "no more than 256 jobs" limit. We can also test multiple modules at once (say, go, php, java) on a single run. But it's more complicated with multiple versions of a single module (java-11, java-17, java-21). I'm happy to hear any proposals here as well.

thresheek commented 5 months ago

Is it a such a good idea to create a job for every "$OS-$ARCH-$LANG"? I think if we're to reuse the buildbot approach, we should be fine with "$OS-$ARCH" and then run steps for different $LANGs... this will save us quite some overhead in spinning up the machines.

thresheek commented 3 months ago

I've come up with a slightly (much) easier workflow, not too dissimilar from what we currently use for njs (https://github.com/nginx/njs/blob/master/.github/workflows/ci.yml and https://github.com/nginx/ci-self-hosted/blob/main/.github/workflows/njs-buildbot.yml).

I'll follow up with a PR for that soon-ish.

callahad commented 3 months ago

Thanks for the heads up; closing in lieu of the new approach.