Re: [EXTERNAL] Re: CI caching improvement
William, On 3/8/22 21:30, Tim Düsterhus wrote: - The action is also easily reusable by other projects. For testing my Adding to that: It's also easily reusable by the other workflows. We currently have the separate musl.yml workflow that does this: https://github.com/haproxy/haproxy/blob/5ce1299c643543c9b17b4124b299feb3caf63d9e/.github/workflows/musl.yml#L19-L20 Your patch proposal doesn't adjust that, but with the dedicated action it could automatically benefit from caching or any other improvements we make to the VTest installation without needing to touch all the .yml files separately. Here's an example run with an updated commit: https://github.com/TimWolla/haproxy/runs/5470838975?check_suite_focus=true https://github.com/TimWolla/haproxy/commit/cefe211b774f0393d4f78268a23036f32b74ee4b Best regards Tim Düsterhus
Re: [EXTERNAL] Re: CI caching improvement
William,
On 3/8/22 20:54, William Lallemand wrote:
Honestly I'm confused, it is overcomplicated in my opinion :(
I don't really see the benefits in creating a whole new repository
instead of the few lines in the yaml file.
I believe that having a non-trivial amount of logic in a YAML file will
ultimately result in a hard to understand configuration file.
As an example: YAML doesn't support any kind of syntax highlighting or
autocompletion.
We are talking about doing a new project for just the equivalent of a 5
lines shell script... which really don't need to be tested and
maintained outside of the project.
With your suggested diff you needed to change 4 different locations
within the vtest.yml, growing the file from 152 to 168 lines (+10%). And
none of those lines are specific to HAProxy itself!
By separating out the VTest installation logic all that's needed in
vtest.yml is the following:
- name: Install VTest
uses: haproxy/action-install-vtest@main
with:
branch: master
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
I think it's pretty obvious what that would do: It installs VTest and
when that step is finished you can simply use vtest.
How this action does this is left to the action, it just promises you to
do the right thing and then the HAProxy repository just needs to worry
about the HAProxy specific parts.
It's really the same like any other programming library. Just like
HAProxy uses libraries (e.g. mjson or SLZ) to perform some task, the CI
can do the same.
I feel like I'm missing something with my simple implementation, we are
already downloading all the SSL libraries, should we stop doing it this
way? What could be the problems with this?
I'd like to simplify the installation of the various SSL libs as well,
but I don't have a good proposal for that.
It seems like you want to do this in a strict github way, which is
probably convenient for a lot of usecase, but it just look really more
complicated that my first proposal.
It sure comes with a bit of initial set-up work, but I'm volunteering to
do that. I'm also volunteering the maintenance of that. As I've said:
This is nothing I just came up with, I'm using that for
haproxy-auth-request since July 2020 and I'm pretty happy with that.
The benefits of this dedicated repository are:
- Fixes and improvements VTest installation no longer require a testing
commit to the HAProxy repository. They can be developed in the dedicated
repository with a specialized CI for VTest installation.
- It uses a proper programming language instead of embedding bash in a YAML.
- The action is also easily reusable by other projects. For testing my
haproxy-auth-request repository I could remove the VTest installation
logic from action-install-haproxy and simply use the existing action.
This might also come in handy to test
https://github.com/haproxytech/haproxy-lua-oauth and other official
extensions.
Best regards
Tim Düsterhus
Re: [EXTERNAL] Re: CI caching improvement
On Tue, Mar 08, 2022 at 08:05:31PM +0100, Tim Düsterhus wrote: > > Let me make a competing proposal that: > > 1. Keeps the complexity out of the GitHub workflow configuration in > haproxy/haproxy. > 2. Still allows VTest caching. > > For my https://github.com/TimWolla/haproxy-auth-request repository I > have created a reusable GitHub Action to perform the HAProxy > installation similar to 'actions/checkout': > > https://github.com/TimWolla/action-install-haproxy/ > > I just spent a bit of time to fork that action and to make it VTest > specific: > > https://github.com/TimWolla/action-install-vtest/ > > The action receives the VTest branch or commit as the input and will > handle all the heavy lifting of downloading, compiling and caching VTest. > > The necessary changes to HAProxy then look like this: > > https://github.com/TimWolla/haproxy/commit/78af831402e354f22d67682be0f323dec9c26a52 > > This basically replaces the use of 'scripts/build-vtest.sh' by > 'timwolla/action-install-vtest@main', so the configuration in the > haproxy/haproxy repository is not getting any more complicated, as all > the heavy lifting is done in the action which can be independently > tested and maintained. > > If this proposal sounds good to you, then I'd like to suggest the following: > > 1. Willy creates a new haproxy/action-install-vtest repository in the > haproxy organization. > 2. Willy creates a new GitHub team with direct push access to that > repository. > 3. Willy adds me to that team, so that I can maintain that repository > going forward (e.g. to handle the Dependabot pull requests that keep the > JavaScript dependencies up to date). > > If that repository is properly set up, I'll send a patch to switch over > haproxy/haproxy to make use of that action. > > Best regards > Tim Düsterhus Tim, Honestly I'm confused, it is overcomplicated in my opinion :( I don't really see the benefits in creating a whole new repository instead of the few lines in the yaml file. We are talking about doing a new project for just the equivalent of a 5 lines shell script... which really don't need to be tested and maintained outside of the project. I feel like I'm missing something with my simple implementation, we are already downloading all the SSL libraries, should we stop doing it this way? What could be the problems with this? It seems like you want to do this in a strict github way, which is probably convenient for a lot of usecase, but it just look really more complicated that my first proposal. -- William Lallemand
