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é <berra...@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <phi...@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.
