diff mbox series

[1/2] gitlab-ci: don't use before_script in job templates

Message ID 3c87e90584519c73eeb1ae2dd5c52fe60d1572a2.1675805584.git.yann.morin.1998@free.fr
State Accepted
Headers show
Series gitlab-ci: fix pipelines with the newer docker image (branch yem/pipelines) | expand

Commit Message

Yann E. MORIN Feb. 7, 2023, 9:33 p.m. UTC
When gitlab prepares a job to run, it checks out the repository with a
non-root user, and spawns a container that runs as root, with some UID
mapping that makes the files be owned by root in the container. However,
our pipelines run as a nont-root user.

Commit bde165f7ad (.gitlab-ci.yml: update Docker image to use) updated
the docker image that is used to run in our pipelines.

That new image includes a git version that is stricter about the
ownership of the git tree it is acting in: git aborts in error when the
user running it does not own the repository.

We use `git ls-tree` quite a lot in our check-{flake8,package,symbols}
rules, so they all fail (in various ways).

To fix this, we either need to fix the ownership or tell git to ignore
the situation. In either case, we'll need to run a scriptlet before all
our jobs.

Gitlab-ci allows to provide a global before_script, that is inherited by
all jobs. However, some of our jobs already declare a before_script, and
that would shadow the global before_Scrikpt.

There is no technical reason to do our before_script separately from
the actual script, so we move the code from the before_scripts to the
corresponding scripts.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Romain Naour <romain.naour@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 support/misc/gitlab-ci.yml.in | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

Comments

Romain Naour Feb. 7, 2023, 9:47 p.m. UTC | #1
Hello Yann,

Le 07/02/2023 à 22:33, Yann E. MORIN a écrit :
> When gitlab prepares a job to run, it checks out the repository with a
> non-root user, and spawns a container that runs as root, with some UID
> mapping that makes the files be owned by root in the container. However,
> our pipelines run as a nont-root user.
> 
> Commit bde165f7ad (.gitlab-ci.yml: update Docker image to use) updated
> the docker image that is used to run in our pipelines.
> 
> That new image includes a git version that is stricter about the
> ownership of the git tree it is acting in: git aborts in error when the
> user running it does not own the repository.
> 
> We use `git ls-tree` quite a lot in our check-{flake8,package,symbols}
> rules, so they all fail (in various ways).

is flake8 has been updated too? flake8 may trigger new check-package warning now.
> 
> To fix this, we either need to fix the ownership or tell git to ignore
> the situation. In either case, we'll need to run a scriptlet before all
> our jobs.
> 
> Gitlab-ci allows to provide a global before_script, that is inherited by
> all jobs. However, some of our jobs already declare a before_script, and
> that would shadow the global before_Scrikpt.

before_Scrikpt/before_scripts

> 
> There is no technical reason to do our before_script separately from
> the actual script, so we move the code from the before_scripts to the
> corresponding scripts.
> 

Reviewed-by: Romain Naour <romain.naour@smile.fr>

Best regards,
Romain


> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Romain Naour <romain.naour@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  support/misc/gitlab-ci.yml.in | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/support/misc/gitlab-ci.yml.in b/support/misc/gitlab-ci.yml.in
> index 0ccf36665e..9c1faf0d5f 100644
> --- a/support/misc/gitlab-ci.yml.in
> +++ b/support/misc/gitlab-ci.yml.in
> @@ -23,9 +23,8 @@
>          - utils/check-symbols
>  
>  .defconfig_check:
> -    before_script:
> +    script:
>          - DEFCONFIG_NAME=$(echo ${CI_JOB_NAME} | sed -e 's,_check$,,g')
> -    script:
>          - echo "Configure Buildroot for ${DEFCONFIG_NAME}"
>          - make ${DEFCONFIG_NAME}
>          - support/scripts/check-dotconfig.py .config configs/${DEFCONFIG_NAME}
> @@ -44,10 +43,9 @@
>        }
>  
>  .defconfig_base:
> -    before_script:
> +    script:
>          - DEFCONFIG_NAME=${CI_JOB_NAME}
>          - OUTPUT_DIR=output
> -    script:
>          - echo "Configure Buildroot for ${DEFCONFIG_NAME}"
>          - make ${DEFCONFIG_NAME}
>          - ./support/scripts/check-dotconfig.py .config ./configs/${DEFCONFIG_NAME}
> @@ -72,13 +70,12 @@
>              - runtime-test.log
>  
>  .runtime_test_base:
> -    before_script:
> -        - TEST_CASE_NAME=${CI_JOB_NAME}
>      # Keep build directories so the rootfs can be an artifact of the job. The
>      # runner will clean up those files for us.
>      # Multiply every emulator timeout by 10 to avoid sporadic failures in
>      # elastic runners.
>      script:
> +        - TEST_CASE_NAME=${CI_JOB_NAME}
>          - echo "Starting runtime test ${TEST_CASE_NAME}"
>          - ./support/testing/run-tests -o test-output/ -d test-dl/ -k --timeout-multiplier 10 ${TEST_CASE_NAME}
>      artifacts:
> @@ -91,9 +88,8 @@
>  
>  .test_pkg:
>      stage: build
> -    before_script:
> +    script:
>          - OUTPUT_DIR=${CI_JOB_NAME}
> -    script:
>          - echo "Configure Buildroot for ${OUTPUT_DIR}"
>          - make O=${OUTPUT_DIR} syncconfig
>          - make O=${OUTPUT_DIR} savedefconfig
Yann E. MORIN Feb. 7, 2023, 9:55 p.m. UTC | #2
Romain, All,

On 2023-02-07 22:47 +0100, Romain Naour spake thusly:
> Le 07/02/2023 à 22:33, Yann E. MORIN a écrit :
> > When gitlab prepares a job to run, it checks out the repository with a
> > non-root user, and spawns a container that runs as root, with some UID
> > mapping that makes the files be owned by root in the container. However,
> > our pipelines run as a nont-root user.
> > 
> > Commit bde165f7ad (.gitlab-ci.yml: update Docker image to use) updated
> > the docker image that is used to run in our pipelines.
> > 
> > That new image includes a git version that is stricter about the
> > ownership of the git tree it is acting in: git aborts in error when the
> > user running it does not own the repository.
> > 
> > We use `git ls-tree` quite a lot in our check-{flake8,package,symbols}
> > rules, so they all fail (in various ways).
> 
> is flake8 has been updated too? flake8 may trigger new check-package warning now.

Yes, there is a newer flake8 too, but there is no new error detected:

    https://gitlab.com/ymorin/buildroot/-/pipelines/770406457

> > 
> > To fix this, we either need to fix the ownership or tell git to ignore
> > the situation. In either case, we'll need to run a scriptlet before all
> > our jobs.
> > 
> > Gitlab-ci allows to provide a global before_script, that is inherited by
> > all jobs. However, some of our jobs already declare a before_script, and
> > that would shadow the global before_Scrikpt.
> before_Scrikpt/before_scripts

Thanks Thomas said he'd fix that when applying. ;-]

> > There is no technical reason to do our before_script separately from
> > the actual script, so we move the code from the before_scripts to the
> > corresponding scripts.
> Reviewed-by: Romain Naour <romain.naour@smile.fr>

Thanks!

Regards,
Yann E. MORIN.

> Best regards,
> Romain
> 
> 
> > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> > Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> > Cc: Romain Naour <romain.naour@gmail.com>
> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > ---
> >  support/misc/gitlab-ci.yml.in | 12 ++++--------
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/support/misc/gitlab-ci.yml.in b/support/misc/gitlab-ci.yml.in
> > index 0ccf36665e..9c1faf0d5f 100644
> > --- a/support/misc/gitlab-ci.yml.in
> > +++ b/support/misc/gitlab-ci.yml.in
> > @@ -23,9 +23,8 @@
> >          - utils/check-symbols
> >  
> >  .defconfig_check:
> > -    before_script:
> > +    script:
> >          - DEFCONFIG_NAME=$(echo ${CI_JOB_NAME} | sed -e 's,_check$,,g')
> > -    script:
> >          - echo "Configure Buildroot for ${DEFCONFIG_NAME}"
> >          - make ${DEFCONFIG_NAME}
> >          - support/scripts/check-dotconfig.py .config configs/${DEFCONFIG_NAME}
> > @@ -44,10 +43,9 @@
> >        }
> >  
> >  .defconfig_base:
> > -    before_script:
> > +    script:
> >          - DEFCONFIG_NAME=${CI_JOB_NAME}
> >          - OUTPUT_DIR=output
> > -    script:
> >          - echo "Configure Buildroot for ${DEFCONFIG_NAME}"
> >          - make ${DEFCONFIG_NAME}
> >          - ./support/scripts/check-dotconfig.py .config ./configs/${DEFCONFIG_NAME}
> > @@ -72,13 +70,12 @@
> >              - runtime-test.log
> >  
> >  .runtime_test_base:
> > -    before_script:
> > -        - TEST_CASE_NAME=${CI_JOB_NAME}
> >      # Keep build directories so the rootfs can be an artifact of the job. The
> >      # runner will clean up those files for us.
> >      # Multiply every emulator timeout by 10 to avoid sporadic failures in
> >      # elastic runners.
> >      script:
> > +        - TEST_CASE_NAME=${CI_JOB_NAME}
> >          - echo "Starting runtime test ${TEST_CASE_NAME}"
> >          - ./support/testing/run-tests -o test-output/ -d test-dl/ -k --timeout-multiplier 10 ${TEST_CASE_NAME}
> >      artifacts:
> > @@ -91,9 +88,8 @@
> >  
> >  .test_pkg:
> >      stage: build
> > -    before_script:
> > +    script:
> >          - OUTPUT_DIR=${CI_JOB_NAME}
> > -    script:
> >          - echo "Configure Buildroot for ${OUTPUT_DIR}"
> >          - make O=${OUTPUT_DIR} syncconfig
> >          - make O=${OUTPUT_DIR} savedefconfig
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
diff mbox series

Patch

diff --git a/support/misc/gitlab-ci.yml.in b/support/misc/gitlab-ci.yml.in
index 0ccf36665e..9c1faf0d5f 100644
--- a/support/misc/gitlab-ci.yml.in
+++ b/support/misc/gitlab-ci.yml.in
@@ -23,9 +23,8 @@ 
         - utils/check-symbols
 
 .defconfig_check:
-    before_script:
+    script:
         - DEFCONFIG_NAME=$(echo ${CI_JOB_NAME} | sed -e 's,_check$,,g')
-    script:
         - echo "Configure Buildroot for ${DEFCONFIG_NAME}"
         - make ${DEFCONFIG_NAME}
         - support/scripts/check-dotconfig.py .config configs/${DEFCONFIG_NAME}
@@ -44,10 +43,9 @@ 
       }
 
 .defconfig_base:
-    before_script:
+    script:
         - DEFCONFIG_NAME=${CI_JOB_NAME}
         - OUTPUT_DIR=output
-    script:
         - echo "Configure Buildroot for ${DEFCONFIG_NAME}"
         - make ${DEFCONFIG_NAME}
         - ./support/scripts/check-dotconfig.py .config ./configs/${DEFCONFIG_NAME}
@@ -72,13 +70,12 @@ 
             - runtime-test.log
 
 .runtime_test_base:
-    before_script:
-        - TEST_CASE_NAME=${CI_JOB_NAME}
     # Keep build directories so the rootfs can be an artifact of the job. The
     # runner will clean up those files for us.
     # Multiply every emulator timeout by 10 to avoid sporadic failures in
     # elastic runners.
     script:
+        - TEST_CASE_NAME=${CI_JOB_NAME}
         - echo "Starting runtime test ${TEST_CASE_NAME}"
         - ./support/testing/run-tests -o test-output/ -d test-dl/ -k --timeout-multiplier 10 ${TEST_CASE_NAME}
     artifacts:
@@ -91,9 +88,8 @@ 
 
 .test_pkg:
     stage: build
-    before_script:
+    script:
         - OUTPUT_DIR=${CI_JOB_NAME}
-    script:
         - echo "Configure Buildroot for ${OUTPUT_DIR}"
         - make O=${OUTPUT_DIR} syncconfig
         - make O=${OUTPUT_DIR} savedefconfig