[PATCH v2] gitlab-ci: Fix Avocado cache usage

[PATCH v2] gitlab-ci: Fix Avocado cache usage

[Philippe Mathieu-Daudé]

In commit 6957fd98dc ("gitlab: add avocado asset caching") we
tried to save the Avocado cache (as in commit c1073e44b4 with
Travis-CI) however it doesn't work as expected. For some reason
Avocado uses /root/avocado_cache/ which we can not select later.

Manually generate a Avocado config to force the use of the
current directory.

We add a new 'build-acceptance-cache' job that runs first,
(during the 'build' stage) to create/update the cache.

The cache content is then pulled (but not updated) during the
'test' stage.

See:
- https://docs.gitlab.com/ee/ci/caching/
- https://avocado-framework.readthedocs.io/en/latest/guides/writer/chapters/writing.html#fetching-asset-files

Reported-by: Thomas Huth <thuth@redhat.com>
Fixes: 6957fd98dc ("gitlab: add avocado asset caching")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Since v1:
- add a specific 'build-acceptance-cache' job

Thomas prefers to use a different cache for each job.
Since I had this patch ready, I prefer to post it as
v2 and will work on a v3 using Thomas suggestion.

Supersedes: <20200723200318.28214-1-f4bug@amsat.org>
Based-on: <20200724073524.26589-1-f4bug@amsat.org>
          "tests: Add 'fetch-acceptance' rule"
---
 .gitlab-ci.yml | 61 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 52 insertions(+), 9 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 362e5ee755..a8d8a7e849 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -8,11 +8,9 @@ stages:
   - build
   - test
 
-# We assume GitLab has it's own caching set up for RPM/APT repositories so we
-# just take care of avocado assets here.
-cache:
-  paths:
-    - $HOME/avocado/data/cache
+# We assume GitLab has it's own caching set up for RPM/APT repositories
+cache: &global_cache
+  policy: pull
 
 include:
   - local: '/.gitlab-ci.d/edk2.yml'
@@ -47,11 +45,52 @@ include:
     - find . -type f -exec touch {} +
     - make $MAKE_CHECK_ARGS
 
-.post_acceptance_template: &post_acceptance
+.acceptance_template: &acceptance_definition
+  cache:
+    # inherit all global cache settings
+    <<: *global_cache
+    key: acceptance_cache
+    paths:
+      - $CI_PROJECT_DIR/avocado_cache
+    policy: pull
+  before_script:
+    - JOBS=$(expr $(nproc) + 1)
+    - mkdir -p ~/.config/avocado
+    - echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
+    - echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >> ~/.config/avocado/avocado.conf
   after_script:
     - cd build
     - python3 -c 'import json; r = json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for t in r["tests"] if t["status"] not in ("PASS", "SKIP")]' | xargs cat
-    - du -chs $HOME/avocado/data/cache
+    - du -chs $CI_PROJECT_DIR/avocado_cache
+
+build-acceptance-cache:
+  stage: build
+  cache:
+    # inherit all global cache settings
+    <<: *global_cache
+    key: acceptance_cache
+    paths:
+      - $CI_PROJECT_DIR/avocado_cache
+    policy: pull-push
+  variables:
+    # any image should work
+    IMAGE: ubuntu2004
+    CONFIGURE_ARGS: --disable-user --disable-system
+      --disable-docs --disable-tools
+    MAKE_CHECK_ARGS: fetch-acceptance
+  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
+  before_script:
+    - mkdir -p ~/.config/avocado
+    - echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
+    - echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >> ~/.config/avocado/avocado.conf
+  script:
+    - mkdir build
+    - cd build
+    - ../configure --disable-user --disable-system --disable-docs --disable-tools
+    # ignore "asset fetched or already on cache" error
+    - make fetch-acceptance || true
+  after_script:
+    - du -chs $CI_PROJECT_DIR/avocado_cache
 
 build-system-ubuntu-main:
   <<: *native_build_job_definition
@@ -76,13 +115,15 @@ check-system-ubuntu-main:
 
 acceptance-system-ubuntu-main:
   <<: *native_test_job_definition
+  <<: *acceptance_definition
   needs:
     - job: build-system-ubuntu-main
       artifacts: true
+    - job: build-acceptance-cache
+      artifacts: false
   variables:
     IMAGE: ubuntu2004
     MAKE_CHECK_ARGS: check-acceptance
-  <<: *post_acceptance
 
 build-system-fedora-alt:
   <<: *native_build_job_definition
@@ -107,13 +148,15 @@ check-system-fedora-alt:
 
 acceptance-system-fedora-alt:
   <<: *native_test_job_definition
+  <<: *acceptance_definition
   needs:
     - job: build-system-fedora-alt
       artifacts: true
+    - job: build-acceptance-cache
+      artifacts: false
   variables:
     IMAGE: fedora
     MAKE_CHECK_ARGS: check-acceptance
-  <<: *post_acceptance
 
 build-disabled:
   <<: *native_build_job_definition
-- 
2.21.3

Re: [PATCH v2] gitlab-ci: Fix Avocado cache usage

[Wainer dos Santos Moschetta]

Hi Philippe,

On 7/24/20 4:42 AM, Philippe Mathieu-Daudé wrote:
> In commit 6957fd98dc ("gitlab: add avocado asset caching") we
> tried to save the Avocado cache (as in commit c1073e44b4 with
> Travis-CI) however it doesn't work as expected. For some reason
> Avocado uses /root/avocado_cache/ which we can not select later.
>
> Manually generate a Avocado config to force the use of the
> current directory.
>
> We add a new 'build-acceptance-cache' job that runs first,
> (during the 'build' stage) to create/update the cache.
>
> The cache content is then pulled (but not updated) during the
> 'test' stage.
>
> See:
> - https://docs.gitlab.com/ee/ci/caching/
> - https://avocado-framework.readthedocs.io/en/latest/guides/writer/chapters/writing.html#fetching-asset-files
>
> Reported-by: Thomas Huth <thuth@redhat.com>
> Fixes: 6957fd98dc ("gitlab: add avocado asset caching")
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Since v1:
> - add a specific 'build-acceptance-cache' job
>
> Thomas prefers to use a different cache for each job.
> Since I had this patch ready, I prefer to post it as
> v2 and will work on a v3 using Thomas suggestion.
>
> Supersedes: <20200723200318.28214-1-f4bug@amsat.org>
> Based-on: <20200724073524.26589-1-f4bug@amsat.org>
>            "tests: Add 'fetch-acceptance' rule"
> ---
>   .gitlab-ci.yml | 61 ++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 52 insertions(+), 9 deletions(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 362e5ee755..a8d8a7e849 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -8,11 +8,9 @@ stages:
>     - build
>     - test
>   
> -# We assume GitLab has it's own caching set up for RPM/APT repositories so we
> -# just take care of avocado assets here.
> -cache:
> -  paths:
> -    - $HOME/avocado/data/cache
> +# We assume GitLab has it's own caching set up for RPM/APT repositories
> +cache: &global_cache
> +  policy: pull
>   
>   include:
>     - local: '/.gitlab-ci.d/edk2.yml'
> @@ -47,11 +45,52 @@ include:
>       - find . -type f -exec touch {} +
>       - make $MAKE_CHECK_ARGS
>   
> -.post_acceptance_template: &post_acceptance
> +.acceptance_template: &acceptance_definition

What if you:

- Keep the post_acceptance section which defines the common after_script 
only.

- Create the acceptance_definition as you did, with before_script only. 
This way it doesn't need to repeat the logic in build-acceptance-cache 
job definition.


> +  cache:
> +    # inherit all global cache settings
> +    <<: *global_cache
> +    key: acceptance_cache
> +    paths:
> +      - $CI_PROJECT_DIR/avocado_cache
> +    policy: pull

Isn't this policy inherited from global settings already?

> +  before_script:
> +    - JOBS=$(expr $(nproc) + 1)
> +    - mkdir -p ~/.config/avocado
> +    - echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
> +    - echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >> ~/.config/avocado/avocado.conf
>     after_script:
>       - cd build
>       - python3 -c 'import json; r = json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for t in r["tests"] if t["status"] not in ("PASS", "SKIP")]' | xargs cat
> -    - du -chs $HOME/avocado/data/cache
> +    - du -chs $CI_PROJECT_DIR/avocado_cache
> +
> +build-acceptance-cache:
> +  stage: build
> +  cache:
> +    # inherit all global cache settings
> +    <<: *global_cache
> +    key: acceptance_cache
> +    paths:
> +      - $CI_PROJECT_DIR/avocado_cache
> +    policy: pull-push
> +  variables:
> +    # any image should work
> +    IMAGE: ubuntu2004
> +    CONFIGURE_ARGS: --disable-user --disable-system
> +      --disable-docs --disable-tools
> +    MAKE_CHECK_ARGS: fetch-acceptance
> +  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
> +  before_script:
> +    - mkdir -p ~/.config/avocado
> +    - echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
> +    - echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >> ~/.config/avocado/avocado.conf
> +  script:
> +    - mkdir build
> +    - cd build
> +    - ../configure --disable-user --disable-system --disable-docs --disable-tools
Use the CONFIGURE_ARGS variable here, or not define it.
> +    # ignore "asset fetched or already on cache" error
> +    - make fetch-acceptance || true

Likewise for MAKE_CHECK_ARGS.

Regards,

Wainer

> +  after_script:
> +    - du -chs $CI_PROJECT_DIR/avocado_cache
>   
>   build-system-ubuntu-main:
>     <<: *native_build_job_definition
> @@ -76,13 +115,15 @@ check-system-ubuntu-main:
>   
>   acceptance-system-ubuntu-main:
>     <<: *native_test_job_definition
> +  <<: *acceptance_definition
>     needs:
>       - job: build-system-ubuntu-main
>         artifacts: true
> +    - job: build-acceptance-cache
> +      artifacts: false
>     variables:
>       IMAGE: ubuntu2004
>       MAKE_CHECK_ARGS: check-acceptance
> -  <<: *post_acceptance
>   
>   build-system-fedora-alt:
>     <<: *native_build_job_definition
> @@ -107,13 +148,15 @@ check-system-fedora-alt:
>   
>   acceptance-system-fedora-alt:
>     <<: *native_test_job_definition
> +  <<: *acceptance_definition
>     needs:
>       - job: build-system-fedora-alt
>         artifacts: true
> +    - job: build-acceptance-cache
> +      artifacts: false
>     variables:
>       IMAGE: fedora
>       MAKE_CHECK_ARGS: check-acceptance
> -  <<: *post_acceptance
>   
>   build-disabled:
>     <<: *native_build_job_definition

Re: [PATCH v2] gitlab-ci: Fix Avocado cache usage

[Philippe Mathieu-Daudé]

On 7/24/20 5:52 PM, Wainer dos Santos Moschetta wrote:
> Hi Philippe,
> 
> On 7/24/20 4:42 AM, Philippe Mathieu-Daudé wrote:
>> In commit 6957fd98dc ("gitlab: add avocado asset caching") we
>> tried to save the Avocado cache (as in commit c1073e44b4 with
>> Travis-CI) however it doesn't work as expected. For some reason
>> Avocado uses /root/avocado_cache/ which we can not select later.
>>
>> Manually generate a Avocado config to force the use of the
>> current directory.
>>
>> We add a new 'build-acceptance-cache' job that runs first,
>> (during the 'build' stage) to create/update the cache.
>>
>> The cache content is then pulled (but not updated) during the
>> 'test' stage.
>>
>> See:
>> - https://docs.gitlab.com/ee/ci/caching/
>> -
>> https://avocado-framework.readthedocs.io/en/latest/guides/writer/chapters/writing.html#fetching-asset-files
>>
>>
>> Reported-by: Thomas Huth <thuth@redhat.com>
>> Fixes: 6957fd98dc ("gitlab: add avocado asset caching")
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> Since v1:
>> - add a specific 'build-acceptance-cache' job
>>
>> Thomas prefers to use a different cache for each job.
>> Since I had this patch ready, I prefer to post it as
>> v2 and will work on a v3 using Thomas suggestion.
>>
>> Supersedes: <20200723200318.28214-1-f4bug@amsat.org>
>> Based-on: <20200724073524.26589-1-f4bug@amsat.org>
>>            "tests: Add 'fetch-acceptance' rule"
>> ---
>>   .gitlab-ci.yml | 61 ++++++++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 52 insertions(+), 9 deletions(-)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index 362e5ee755..a8d8a7e849 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -8,11 +8,9 @@ stages:
>>     - build
>>     - test
>>   -# We assume GitLab has it's own caching set up for RPM/APT
>> repositories so we
>> -# just take care of avocado assets here.
>> -cache:
>> -  paths:
>> -    - $HOME/avocado/data/cache
>> +# We assume GitLab has it's own caching set up for RPM/APT repositories
>> +cache: &global_cache
>> +  policy: pull
>>     include:
>>     - local: '/.gitlab-ci.d/edk2.yml'
>> @@ -47,11 +45,52 @@ include:
>>       - find . -type f -exec touch {} +
>>       - make $MAKE_CHECK_ARGS
>>   -.post_acceptance_template: &post_acceptance
>> +.acceptance_template: &acceptance_definition
> 
> What if you:
> 
> - Keep the post_acceptance section which defines the common after_script
> only.
> 
> - Create the acceptance_definition as you did, with before_script only.
> This way it doesn't need to repeat the logic in build-acceptance-cache
> job definition.

See below [*].

> 
> 
>> +  cache:
>> +    # inherit all global cache settings
>> +    <<: *global_cache
>> +    key: acceptance_cache
>> +    paths:
>> +      - $CI_PROJECT_DIR/avocado_cache
>> +    policy: pull
> 
> Isn't this policy inherited from global settings already?

Uh, bug! I had it right and messed when cleaning before posting...
This one is "pull-push" (while global_cache is pull).

> 
>> +  before_script:
>> +    - JOBS=$(expr $(nproc) + 1)
>> +    - mkdir -p ~/.config/avocado
>> +    - echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
>> +    - echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >>
>> ~/.config/avocado/avocado.conf
>>     after_script:
>>       - cd build
>>       - python3 -c 'import json; r =
>> json.load(open("tests/results/latest/results.json"));
>> [print(t["logfile"]) for t in r["tests"] if t["status"] not in
>> ("PASS", "SKIP")]' | xargs cat
>> -    - du -chs $HOME/avocado/data/cache
>> +    - du -chs $CI_PROJECT_DIR/avocado_cache
>> +
>> +build-acceptance-cache:
>> +  stage: build
>> +  cache:
>> +    # inherit all global cache settings
>> +    <<: *global_cache
>> +    key: acceptance_cache
>> +    paths:
>> +      - $CI_PROJECT_DIR/avocado_cache
>> +    policy: pull-push
>> +  variables:
>> +    # any image should work
>> +    IMAGE: ubuntu2004
>> +    CONFIGURE_ARGS: --disable-user --disable-system
>> +      --disable-docs --disable-tools
>> +    MAKE_CHECK_ARGS: fetch-acceptance
>> +  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
>> +  before_script:
>> +    - mkdir -p ~/.config/avocado
>> +    - echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
>> +    - echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado_cache']" >>
>> ~/.config/avocado/avocado.conf
>> +  script:
>> +    - mkdir build
>> +    - cd build
>> +    - ../configure --disable-user --disable-system --disable-docs
>> --disable-tools
> Use the CONFIGURE_ARGS variable here, or not define it.
>> +    # ignore "asset fetched or already on cache" error
>> +    - make fetch-acceptance || true
> 
> Likewise for MAKE_CHECK_ARGS.

[*] The point here is to not call 'make -j"$JOBS"'. Using
variables for the same script seems over complicated IMO.

> 
> Regards,
> 
> Wainer
> 
>> +  after_script:
>> +    - du -chs $CI_PROJECT_DIR/avocado_cache
>>     build-system-ubuntu-main:
>>     <<: *native_build_job_definition
>> @@ -76,13 +115,15 @@ check-system-ubuntu-main:
>>     acceptance-system-ubuntu-main:
>>     <<: *native_test_job_definition
>> +  <<: *acceptance_definition
>>     needs:
>>       - job: build-system-ubuntu-main
>>         artifacts: true
>> +    - job: build-acceptance-cache
>> +      artifacts: false
>>     variables:
>>       IMAGE: ubuntu2004
>>       MAKE_CHECK_ARGS: check-acceptance
>> -  <<: *post_acceptance
>>     build-system-fedora-alt:
>>     <<: *native_build_job_definition
>> @@ -107,13 +148,15 @@ check-system-fedora-alt:
>>     acceptance-system-fedora-alt:
>>     <<: *native_test_job_definition
>> +  <<: *acceptance_definition
>>     needs:
>>       - job: build-system-fedora-alt
>>         artifacts: true
>> +    - job: build-acceptance-cache
>> +      artifacts: false
>>     variables:
>>       IMAGE: fedora
>>       MAKE_CHECK_ARGS: check-acceptance
>> -  <<: *post_acceptance
>>     build-disabled:
>>     <<: *native_build_job_definition
>