fabric use matrix to avoid copy my self in github action(pipeline as) code

Type of change

Improvement (improvement to code, performance, etc)

Description

use matrix to avoid copy my self in github action(pipeline as) code
use matrix add binary file compile checking or mac and windows os

Additional details

n/A

Related issues

n/A

Aug 27 '22 15:08 SamYuan1990

@denyeart , pre discussed on fabric contributor meeting, I opened this pr as a sample for metric usage. please help with pr review process.

I will squash this PR after a review, as I am not sure if split integration test into each condition as parameter, and the usage for make native is acceptable for maintainers or not.

Aug 27 '22 15:08 SamYuan1990

The changes for github action can be found at https://github.com/hyperledger/fabric/actions/runs/2939710490

Aug 27 '22 15:08 SamYuan1990

@denyeart , could you please ask some one take a look at this kind of idea? otherwise, I am going to close this pr.

Oct 02 '22 12:10 SamYuan1990

@SamYuan1990 Sorry missed this initially... I like the new approach.

Can you look into the native test failure though, both in this PR and at https://github.com/hyperledger/fabric/actions/runs/2939710490

Oct 03 '22 16:10 denyeart

https://github.com/hyperledger/fabric/actions/runs/2939710490

ok, if it looks worth to discuss, I am going to rebase and update.

Oct 04 '22 06:10 SamYuan1990

@denyeart , could you please help rerun azp check? and review this pr?

Oct 04 '22 08:10 SamYuan1990

Github Actions seems to handle many jobs well so I think it is fine to run the integration tests per directory now. It does make the pipeline simpler and more easily extensible, and the check output is more clear now since it shows the integration test directory.

What do you think @jcastrence

Oct 04 '22 14:10 denyeart

@denyeart At worst the matrix job can't take longer than the current workflow set up and it seems to be working fine so I don't see why we don't implement this strategy. I would however recommend using a more recent patch version of Go 1.18 and Ubuntu 22.04 as was suggested in this PR.

Oct 04 '22 21:10 jcastrence

I'm confused as to why this verify build run took 49 min, I thought that all the integration tests would be run in parallel?

Oct 04 '22 22:10 jcastrence

this verify build run

I suppose even if the jobs running in parallel, they still need to waiting for github agent ready to run the job.

Oct 05 '22 02:10 SamYuan1990

As for sample:

Requested labels: ubuntu-22.04
Job defined at: hyperledger/fabric/.github/workflows/verify-build.yml@refs/pull/3617/merge
Waiting for a runner to pick up this job...

Oct 05 '22 02:10 SamYuan1990

As for sample:

Requested labels: ubuntu-22.04
Job defined at: hyperledger/fabric/.github/workflows/verify-build.yml@refs/pull/3617/merge
Waiting for a runner to pick up this job...

This makes sense since GitHub Docs also says the number of concurrently running jobs is dependent on runner availability. With this in mind, I think it makes more sense to keep the faster integration tests grouped into the same jobs. We can still utilize the matrix strategy as it is definitely more readable and will be easier to maintain.

Alternatively, we can (and probably should) change the order of the integration test directory names as they appear in the strategy so that they are ordered slowest to fastest, since the order of the variables determines job creation priority.

Oct 05 '22 02:10 jcastrence

As for sample:
Requested labels: ubuntu-22.04
Job defined at: hyperledger/fabric/.github/workflows/verify-build.yml@refs/pull/3617/merge
Waiting for a runner to pick up this job...
This makes sense since GitHub Docs also says the number of concurrently running jobs is dependent on runner availability. With this in mind, I think it makes more sense to keep the faster integration tests grouped into the same jobs. We can still utilize the matrix strategy as it is definitely more readable and will be easier to maintain.

Alternatively, we can (and probably should) change the order of the integration test directory names as they appear in the strategy so that they are ordered slowest to fastest, since the order of the variables determines job creation priority.

let me try.

Oct 05 '22 03:10 SamYuan1990

I would change line 16 to GO_VER: 1.18 and line 21 to runs-on: ubuntu-22.04

updated with environment value to control go version CI.

Oct 05 '22 14:10 SamYuan1990

Ok, based on the delay and large number of runners, and overhead per runner, I have to agree with Julian that it would be better to continue to group the integration test executions into a smaller set. But if it can be done with a matrix that would be an improvement over the existing approach.

Oct 05 '22 17:10 denyeart

Ok, based on the delay and large number of runners, and overhead per runner, I have to agree with Julian that it would be better to continue to group the integration test executions into a smaller set. But if it can be done with a matrix that would be an improvement over the existing approach.

updated, so far keep the same group runner with previous.

Oct 06 '22 01:10 SamYuan1990

Thanks @SamYuan1990 !

Oct 06 '22 03:10 denyeart

@Mergifyio backport release-2.5

Oct 06 '22 03:10 denyeart

@Mergifyio backport release-2.4

Oct 06 '22 03:10 denyeart

@Mergifyio backport release-2.2

Oct 06 '22 03:10 denyeart