Re: [PATCH v4 1/4] Jobs based on custom runners: documentation and configuration placeholder

[Cleber Rosa <crosa@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5100 bytes --]

On Wed, Oct 21, 2020 at 08:45:18AM +0200, Thomas Huth wrote:
> On 19/10/2020 03.50, Cleber Rosa wrote:
> > As described in the included documentation, the "custom runner" jobs
> > extend the GitLab CI jobs already in place.
> > 
> > Those jobs are intended to run on hardware and/or Operating Systems
> > not provided by GitLab's shared runners.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  .gitlab-ci.d/custom-runners.yml | 14 +++++++++
> >  .gitlab-ci.yml                  |  1 +
> >  docs/devel/ci.rst               | 54 +++++++++++++++++++++++++++++++++
> >  docs/devel/index.rst            |  1 +
> >  4 files changed, 70 insertions(+)
> >  create mode 100644 .gitlab-ci.d/custom-runners.yml
> >  create mode 100644 docs/devel/ci.rst
> > 
> > diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
> > new file mode 100644
> > index 0000000000..3004da2bda
> > --- /dev/null
> > +++ b/.gitlab-ci.d/custom-runners.yml
> > @@ -0,0 +1,14 @@
> > +# The CI jobs defined here require GitLab runners installed and
> > +# registered on machines that match their operating system names,
> > +# versions and architectures.  This is in contrast to the other CI
> > +# jobs that are intended to run on GitLab's "shared" runners.
> > +
> > +# Different than the default approach on "shared" runners, based on
> > +# containers, the custom runners have no such *requirement*, as those
> > +# jobs should be capable of running on operating systems with no
> > +# compatible container implementation, or no support from
> > +# gitlab-runner.  To avoid problems that gitlab-runner can cause while
> > +# reusing the GIT repository, let's enable the recursive submodule
> > +# strategy.
> > +variables:
> > +  GIT_SUBMODULE_STRATEGY: recursive
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 8ffd415ca5..b33c433fd7 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -18,6 +18,7 @@ include:
> >    - local: '/.gitlab-ci.d/opensbi.yml'
> >    - local: '/.gitlab-ci.d/containers.yml'
> >    - local: '/.gitlab-ci.d/crossbuilds.yml'
> > +  - local: '/.gitlab-ci.d/custom-runners.yml'
> >  
> >  .native_build_job_template: &native_build_job_definition
> >    stage: build
> > diff --git a/docs/devel/ci.rst b/docs/devel/ci.rst
> > new file mode 100644
> > index 0000000000..41a4bbddad
> > --- /dev/null
> > +++ b/docs/devel/ci.rst
> > @@ -0,0 +1,54 @@
> > +==
> > +CI
> > +==
> > +
> > +QEMU has configurations enabled for a number of different CI services.
> > +The most up to date information about them and their status can be
> > +found at::
> > +
> > +   https://wiki.qemu.org/Testing/CI
> > +
> > +Jobs on Custom Runners
> > +======================
> > +
> > +Besides the jobs run under the various CI systems listed before, there
> > +are a number additional jobs that will run before an actual merge.
> > +These use the same GitLab CI's service/framework already used for all
> > +other GitLab based CI jobs, but rely on additional systems, not the
> > +ones provided by GitLab as "shared runners".
> > +
> > +The architecture of GitLab's CI service allows different machines to
> > +be set up with GitLab's "agent", called gitlab-runner, which will take
> > +care of running jobs created by events such as a push to a branch.
> > +Here, the combination of a machine, properly configured with GitLab's
> > +gitlab-runner, is called a "custom runner" here.
> 
> Nit: Remove one of the two "here" in the above sentence.
> 
> > +The GitLab CI jobs definition for the custom runners are located under::
> > +
> > +  .gitlab-ci.d/custom-runners.yml
> > +
> > +Current Jobs
> > +------------
> > +
> > +The current CI jobs based on custom runners have the primary goal of
> > +catching and preventing regressions on a wider number of host systems
> > +than the ones provided by GitLab's shared runners.
> > +
> > +Also, the mechanics of reliability, capacity and overall maintanance
> 
> s/maintanance/maintenance/
>

Oopsie... thanks.

> > +of the machines provided by the QEMU project itself for those jobs
> > +will be evaluated.
> 
> I'm not sure what this sentence is really good for... of course new stuff
> has to prove its usefulness first, but that's always the case and does not
> need to be mentioned in the documentation, I think? ... maybe that sentence
> is better something for the patch description instead of (hopefully)
> long-lasting documentation here?
>

I think the statement attempts to set the tone here, and answer a
question that I've seen more than once...  It certainly feels a bit
out of place here, but I also think the commit message would be too
transient and almost invisible.  I expect a lot will change in the CI
and in its docs, so I don't think this exact sentence will be too long
lasting.

Let me know if that convinces you it deserves to be in the docs, if not
I'll gladly moved it to the commit message.

Thanks!
- Cleber

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]