Re: [PATCH-for-6.1] gitlab-ci: Extract OpenSBI job rules to reusable section

[Philippe Mathieu-Daudé]

On 7/22/21 5:49 PM, Daniel P. Berrangé wrote:
> On Tue, Jul 20, 2021 at 06:48:29PM +0200, Philippe Mathieu-Daudé wrote:
>> All jobs depending on 'docker-opensbi' job must use at most all
>> the rules that triggers it. The simplest way to ensure that
>> is to always use the same rules. Extract all the rules to a
>> reusable section, and include this section (with the 'extends'
>> keyword) in both 'docker-opensbi' and 'build-opensbi' jobs.
>>
>> The problem was introduced in commit c6fc0fc1a71 ("gitlab-ci.yml:
>> Add jobs to build OpenSBI firmware binaries"), but was revealed in
>> commit 91e9c47e50a ("docker: OpenSBI build job depends on OpenSBI
>> container").
>>
>> This fix is similar to the one used with the EDK2 firmware job in
>> commit ac0595cf6b3 ("gitlab-ci: Extract EDK2 job rules to reusable
>> section").
>>
>> Reported-by: Daniel P. Berrangé <berrange@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> Latent bug on CI, safe for 6.1.
>> ---
>>  .gitlab-ci.d/opensbi.yml | 28 +++++++++++++++++-----------
>>  1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml
>> index f66cd1d9089..d8a0456679e 100644
>> --- a/.gitlab-ci.d/opensbi.yml
>> +++ b/.gitlab-ci.d/opensbi.yml
>> @@ -1,10 +1,23 @@
>> -docker-opensbi:
>> - stage: containers
>> - rules: # Only run this job when the Dockerfile is modified
>> +# All jobs needing docker-opensbi must use the same rules it uses.
>> +.opensbi_job_rules:
>> + rules: # Only run this job when ...
>>   - changes:
>> +   # this file is modified
>>     - .gitlab-ci.d/opensbi.yml
>> +   # or the Dockerfile is modified
>>     - .gitlab-ci.d/opensbi/Dockerfile
>>     when: always
>> + - changes: # or roms/opensbi/ is modified (submodule updated)
>> +   - roms/opensbi/*
>> +   when: always
>> + - if: '$CI_COMMIT_REF_NAME =~ /^opensbi/' # or the branch/tag starts with 
>> 'opensbi'
>> +   when: always
>> + - if: '$CI_COMMIT_MESSAGE =~ /opensbi/i' # or last commit description 
>> contains 'OpenSBI'
>> +   when: always
> 
> In debugging why the acceptance jobs rnu despite their prerequisite
> jobs failing, I've realized we've been making a mistake in most of
> 'rules' sections.
> 
> 'when: always'  will make the job run regardless of status of any
> dependant jobs. IOW, if you have a 'needs: [....]', it is almost
> always wrong to use 'when: always'. Instead  we need 'when: on_success'

Indeed, when the first job fail, the second is marked "skipped" with
no "retry" option.

> So this patch needs to make that change, and likewise the edk2 patch
> with the same logic.
> 
> Alex has queued this one, but I don't see it in a PULL yet, so I
> guess we can just do a v2 of this.

Sure, thanks for updating me.