[libvirt PATCH v2 2/3] gitlab-ci.yml: Convert only/except to the rules syntax

Erik Skultety posted 3 patches 5 years ago
There is a newer version of this series
[libvirt PATCH v2 2/3] gitlab-ci.yml: Convert only/except to the rules syntax
Posted by Erik Skultety 5 years ago
'rules' syntax replaces the only/except syntax with which it is
mutually exclusive. In some cases the 'rules' syntax is more readable
than the 'only/except' equivalent, in some cases it is not.
The idea behind this conversion is to introduce an explicit env variable
controlling the 'allow_failure' attribute which would be then attached
to a broken build job which would mark this broken build as a soft
failure which is not possible with the 'only/except' syntax.

Yes, the alternative here is to use 'allow_failure' directly on the
broken job, but being explicit is always better for readability than
implicit.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 .gitlab-ci.yml | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 018008ca1c..de6ead01d8 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -43,6 +43,8 @@ stages:
 .native_build_job_template:
   stage: builds
   image: $CI_REGISTRY_IMAGE/ci-$NAME:latest
+  rules:
+    - when: on_success
   cache:
     paths:
       - ccache/
@@ -98,10 +100,8 @@ stages:
       <ci/cirrus/build.yml >ci/cirrus/$NAME.yml
     - cat ci/cirrus/$NAME.yml
     - cirrus-run -v --show-build-log always ci/cirrus/$NAME.yml
-  only:
-    variables:
-      - $CIRRUS_GITHUB_REPO
-      - $CIRRUS_API_TOKEN
+  rules:
+    - if: "$CIRRUS_GITHUB_REPO && $CIRRUS_API_TOKEN"
 
 .cross_build_default_job_template:
   stage: builds
@@ -110,6 +110,8 @@ stages:
     paths:
       - ccache/
     key: "$CI_JOB_NAME"
+  rules:
+    - when: on_success
   before_script:
     - *script_variables
   script:
@@ -552,8 +554,8 @@ potfile:
   image: $CI_REGISTRY_IMAGE/ci-centos-8:latest
   needs:
     - x64-centos-8-container
-  only:
-    - master
+  rules:
+    - if: "$CI_COMMIT_BRANCH == 'master'"
   before_script:
     - *script_variables
   script:
@@ -580,9 +582,8 @@ check-dco:
   image: registry.gitlab.com/libvirt/libvirt-ci/check-dco:master
   script:
     - /check-dco
-  except:
-    variables:
-      - $CI_PROJECT_NAMESPACE == 'libvirt'
+  rules:
+    - if: "$CI_PROJECT_NAMESPACE != 'libvirt'"
   variables:
     GIT_DEPTH: 1000
 
@@ -600,8 +601,5 @@ coverity:
     - cov-analysis-linux64-*/bin/cov-build --dir cov-int ninja -C build
     - tar cfz cov-int.tar.gz cov-int
     - curl https://scan.coverity.com/builds?project=$COVERITY_SCAN_PROJECT_NAME --form token=$COVERITY_SCAN_TOKEN --form email=$GITLAB_USER_EMAIL --form file=@cov-int.tar.gz --form version="$(git describe --tags)" --form description="$(git describe --tags) / $CI_COMMIT_TITLE / $CI_COMMIT_REF_NAME:$CI_PIPELINE_ID"
-  only:
-    refs:
-      - schedules
-    variables:
-      - $COVERITY_SCAN_PROJECT_NAME && $COVERITY_SCAN_TOKEN
+  rules:
+    - if: "$CI_PIPELINE_SOURCE == 'schedule' && $COVERITY_SCAN_PROJECT_NAME && $COVERITY_SCAN_TOKEN"
-- 
2.29.2

Re: [libvirt PATCH v2 2/3] gitlab-ci.yml: Convert only/except to the rules syntax
Posted by Andrea Bolognani 5 years ago
On Thu, 2021-01-14 at 12:03 +0100, Erik Skultety wrote:
> 'rules' syntax replaces the only/except syntax with which it is
> mutually exclusive. In some cases the 'rules' syntax is more readable
> than the 'only/except' equivalent, in some cases it is not.
> The idea behind this conversion is to introduce an explicit env variable
> controlling the 'allow_failure' attribute which would be then attached
> to a broken build job which would mark this broken build as a soft
> failure which is not possible with the 'only/except' syntax.
> 
> Yes, the alternative here is to use 'allow_failure' directly on the
> broken job, but being explicit is always better for readability than
> implicit.

The changes are good, but the motivation you give here is bogus.
Please just drop this paragraph.

> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  .gitlab-ci.yml | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 018008ca1c..de6ead01d8 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -43,6 +43,8 @@ stages:
>  .native_build_job_template:
>    stage: builds
>    image: $CI_REGISTRY_IMAGE/ci-$NAME:latest
> +  rules:
> +    - when: on_success

According to

  https://docs.gitlab.com/ee/ci/yaml/#when

when:on_success is the default, so I don't see the reason to have a
rules: section in this case. More instances of this further down.

Looks good otherwise.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH v2 2/3] gitlab-ci.yml: Convert only/except to the rules syntax
Posted by Erik Skultety 5 years ago
On Thu, Jan 14, 2021 at 12:45:40PM +0100, Andrea Bolognani wrote:
> On Thu, 2021-01-14 at 12:03 +0100, Erik Skultety wrote:
> > 'rules' syntax replaces the only/except syntax with which it is
> > mutually exclusive. In some cases the 'rules' syntax is more readable
> > than the 'only/except' equivalent, in some cases it is not.
> > The idea behind this conversion is to introduce an explicit env variable
> > controlling the 'allow_failure' attribute which would be then attached
> > to a broken build job which would mark this broken build as a soft
> > failure which is not possible with the 'only/except' syntax.
> > 
> > Yes, the alternative here is to use 'allow_failure' directly on the
> > broken job, but being explicit is always better for readability than
> > implicit.
> 
> The changes are good, but the motivation you give here is bogus.
> Please just drop this paragraph.
> 
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> >  .gitlab-ci.yml | 26 ++++++++++++--------------
> >  1 file changed, 12 insertions(+), 14 deletions(-)
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 018008ca1c..de6ead01d8 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -43,6 +43,8 @@ stages:
> >  .native_build_job_template:
> >    stage: builds
> >    image: $CI_REGISTRY_IMAGE/ci-$NAME:latest
> > +  rules:
> > +    - when: on_success
> 
> According to
> 
>   https://docs.gitlab.com/ee/ci/yaml/#when
> 
> when:on_success is the default, so I don't see the reason to have a
> rules: section in this case. More instances of this further down.

I'm adding the rule in the next patch, I didn't want to put it in the next
though to prevent potential confusion. In this patch it's a NOP, in the next
one it will matter though, because if no rule matches the default action is
that the job is not going to be added to the pipeline, so you need a top level
'when' to make sure that if no rule matches the job is still selected for the
pipeline.

Erik

Re: [libvirt PATCH v2 2/3] gitlab-ci.yml: Convert only/except to the rules syntax
Posted by Andrea Bolognani 5 years ago
On Thu, 2021-01-14 at 12:54 +0100, Erik Skultety wrote:
> On Thu, Jan 14, 2021 at 12:45:40PM +0100, Andrea Bolognani wrote:
> > On Thu, 2021-01-14 at 12:03 +0100, Erik Skultety wrote:
> > >  .native_build_job_template:
> > >    stage: builds
> > >    image: $CI_REGISTRY_IMAGE/ci-$NAME:latest
> > > +  rules:
> > > +    - when: on_success
> > 
> > According to
> > 
> >   https://docs.gitlab.com/ee/ci/yaml/#when
> > 
> > when:on_success is the default, so I don't see the reason to have a
> > rules: section in this case. More instances of this further down.
> 
> I'm adding the rule in the next patch, I didn't want to put it in the next
> though to prevent potential confusion. In this patch it's a NOP, in the next
> one it will matter though, because if no rule matches the default action is
> that the job is not going to be added to the pipeline, so you need a top level
> 'when' to make sure that if no rule matches the job is still selected for the
> pipeline.

Oh, I missed this detail! Having when:on_success makes sense then.

-- 
Andrea Bolognani / Red Hat / Virtualization