containers / podman

Podman: A tool for managing OCI containers and pods.
https://podman.io
Apache License 2.0
22.36k stars 2.31k forks source link

Run linting in parallel with building #23086

Closed cevich closed 4 days ago

cevich commented 6 days ago

Linting code changes with golangci-lint is a very slow and resource intensive process. However, it does not depend on compiling anything. This means it may run in parallel with the build tasks for a modest perceived runtime duration improvement.

Additionally, the former validation make targets that do require a build execute faster than CI is able to provision a VM, simply tack them onto the end of all build operations.

Does this PR introduce a user-facing change?

None
cevich commented 6 days ago

Validate "main_script" appears to have gone from ~5min down to ~1min (based on picking another random PR and comparing F40 validate task to this one).

In other words, this PR shaves ~4min off the "build -> validate -> build_success" chain as seen in the new task map:

cirrus yml

Another 1min reduction could be realized by moving some/all of the "pre-build" script into the new "lint" (or another new parallel) task.

cevich commented 5 days ago

Thanks, it is a good start but we can do better.

I'm going for an MVP here, so trying to make the smallest number of changes necessary. Certainly more work can/should be done in followup PRs.

the infra overhead is higher than the actual checks.

This is an important data-point. I think (if possible, I'm not sure yet) it should be handled in a followup PR though.

cevich commented 5 days ago

it should be handled in a followup PR though.

Nevermind, I think I see a way to do this. I'll rename the "lint" task to "validate-source", and the old "validate" to "validate-build". Both will make use of identically named (new) make targets. Then I'll append the "validate_build" target into the build runner.sh function (as Paul suggested).

Assuming that works, a followup PR can take care of relocating the "pre-build" script into the new "validate-source" task (will save ~1min).

Edit: Errr, correction - I'll just nuke the former validate task, we won't need it (it's moving into build).

cevich commented 5 days ago

Updated task-map:

cirrus yml

cevich commented 5 days ago

This should be ready for final review now.

openshift-ci[bot] commented 5 days ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, edsantiago, Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/containers/podman/blob/main/OWNERS)~~ [Luap99,edsantiago] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
cevich commented 4 days ago

this should safe as around 10 mins CI time

Yes, and as we close in on the target, every single minute (and then second) will become more important :grin:

cevich commented 4 days ago

I believe this should be ready to merge, no?

Luap99 commented 4 days ago

/lgtm

I am fine with it if you fix up @edsantiago comments in the next PR.

cevich commented 3 days ago

Since it's burred in comments, the comment fixup pr is https://github.com/containers/podman/pull/23112