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

[PROPOSAL] Remove some jobs from the libs CI #782

Closed
Andreagit97 opened this issue Dec 9, 2022 · 6 comments
Closed

[PROPOSAL] Remove some jobs from the libs CI #782

Andreagit97 opened this issue Dec 9, 2022 · 6 comments
Labels
kind/feature New feature or request

Comments

@Andreagit97
Copy link
Member

Andreagit97 commented Dec 9, 2022

Motivation

Our CI is every day slower and I think we can do something to improve it. Probably I'm missing some context for this reason i opened this issue before implementing something :)

  1. The first question is why do we run the run-e2e-tests-amd64 job 3 times? These are e2e IMHO they shouldn't test different building flavors 🤔 WDYT @Molter73 @FedeDP @LucaGuerra?

  2. build-modern-bpf-arm64 and build-modern-bpf-s390x are really time-consuming without a real benefit we cannot run bpf tests inside these jobs. My idea is to enrich build-libs-s390x and build-libs-arm64 jobs building also the modern probe here. In this way, we will have just 2 jobs that test the build on these 2 architectures. WDYT @hbrueckner @FedeDP ?

  3. Why do we run all our CI tests into debian-buster containers ? Why don't we use directly the ubuntu 22.04 machine?

  4. In many jobs we do make -j4 but I think we need to isolate only what we want to test instead of building all the possible targets 🤔 otherwise we are forced to add a lot of cmake options to avoid the build of some stuff in MINIMAL_DEPS for example

@FedeDP
Copy link
Contributor

FedeDP commented Dec 16, 2022

  1. I am ok with that ;) there should be no difference in build output between bundled and non bundled
  2. i am fine; it was something that we already proposed long time ago
  3. i tried to use the same images we were using on prow; we can change
  4. agree

@Molter73
Copy link
Contributor

The first question is why do we run the run-e2e-tests-amd64 job 3 times? These are e2e IMHO they shouldn't test different building flavors 🤔 WDYT @Molter73 @FedeDP @LucaGuerra?

Honestly, why not? What if the bundled dependency breaks something that the non-bundled ones don't catch or the other way around? I'm fine going down to only running it once, but then we need to really specify what we support, if we only support the bundled deps, then it needs to be firmly assessed and written in the repo, then we can go down to a single test.

An even better approach would be to re-use the sinsp-example binary built in the first CI steps, I've been playing around with GHA and I think I can get that implemented in the near future.

Why do we run all our CI tests into debian-buster containers ? Why don't we use directly the ubuntu 22.04 machine?

For the e2e tests, my original idea was that we can simply place the userspace component in a container, bundle it with different drivers and run it on different VMs, but I never got around to actually implementing that (yet).

In many jobs we do make -j4 but I think we need to isolate only what we want to test instead of building all the possible targets 🤔 otherwise we are forced to add a lot of cmake options to avoid the build of some stuff in MINIMAL_DEPS for example

Same as my first remark, I believe we can simply build everything once, then use upload-artifact and download-artifact to re-use the binaries that were built in a single workflow. Though, this will not work for the tests that run with address-santizer, since those need to be re-built with the tool itself.

@FedeDP
Copy link
Contributor

FedeDP commented Dec 16, 2022

An even better approach would be to re-use the sinsp-example binary built in the first CI steps, I've been playing around with GHA and I think I can get that implemented in the near future.

I would be fine with this; i agree, more tests are better than less tests; but it would be great to reduce CI time in any case ;)
If we can manage to reduce CI times while retaining all the tests, it is even better of course!

@Andreagit97
Copy link
Member Author

yeah, the idea here was to save some CI time

Honestly, why not? What if the bundled dependency breaks something that the non-bundled ones don't catch or the other way around?

That's a good point but building sinsp-example we should already be able to detect these issues 🤔 If we could at least reuse the sinsp-example it would be amazing!

I believe we can simply build everything once, then use upload-artifact and download-artifact to re-use the binaries that were built in a single workflow

💯

For the e2e tests, my original idea was that we can simply place the userspace component in a container, bundle it with different drivers and run it on different VMs, but I never got around to actually implementing that (yet).

Makes sense

build-modern-bpf-arm64 and build-modern-bpf-s390x are really time-consuming without a real benefit we cannot run bpf tests inside these jobs. My idea is to enrich build-libs-s390x and build-libs-arm64 jobs building also the modern probe here. In this way, we will have just 2 jobs that test the build on these 2 architectures. WDYT @hbrueckner @FedeDP ?

What about these? this would mitigate timing issues until we will have hosted runners

@poiana
Copy link
Contributor

poiana commented Mar 16, 2023

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

@FedeDP
Copy link
Contributor

FedeDP commented Mar 17, 2023

/remove-lifecycle stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants