[PATCH v2 0/2] QEMU Gating CI

[PATCH v2 0/2] QEMU Gating CI

[Cleber Rosa]

The idea about a public facing Gating CI for QEMU was summarized in an
RFC[1].  Since then, it was decided that a simpler version should be
attempted first.

At this point, there are two specific runners (an aarch64 and an s390)
registered with GitLab, at https://gitlab.com/qemu-project, currently
setup to the "qemu" repository.

Changes from v1:

- Added jobs that require specific GitLab runners already available
  (Ubuntu 20.04 on aarch64, and Ubuntu 18.04 on s390x)
- Removed jobs that require specific GitLab runners not yet available
  (Fedora 30, FreeBSD 12.1)
- Updated documentation
- Added copyright and license to new scripts
- Moved script to from "contrib" to "scripts/ci/"
- Moved setup playbooks form "contrib" to "scripts/ci/setup"
- Moved "gating.yml" to ".gitlab-ci.d" directory
- Removed "staging" only branch restriction on jobs defined in
  ".gitlab-ci.yml", assumes that the additional jobs on the staging
  branch running on the freely available gitlab shared runner are
  positive
- Dropped patches 1-3 (already merged)
- Simplified amount of version specifity on Ubuntu, from 18.04.3 to
  simply 18.04 (assumes no diverse minor levels will be used or
  specific runners)

Changes from the RFC patches[2] accompanying the RFC document:

- Moved gating job definitions to .gitlab-ci-gating.yml
- Added info on "--disable-libssh" build option requirement
  (https://bugs.launchpad.net/qemu/+bug/1838763) to Ubuntu 18.04 jobs
- Added info on "--disable-glusterfs" build option requirement
  (there's no static version of those libs in distro supplied
  packages) to one
- Dropped ubuntu-18.04.3-x86_64-notools job definition, because it
  doesn't fall into the general scope of gating job described by PMM
  (and it did not run any test)
- Added w32 and w64 cross builds based on Fedora 30
- Added a FreeBSD based job that builds all targets and runs `make
  check`
- Added "-j`nproc`" and "-j`sysctl -n hw.ncpu`" options to make as a
  simple but effective way of speeding up the builds and tests by
  using a number of make jobs matching the number of CPUs
- Because the Ansible playbooks reference the content on Dockerfiles,
  some fixes to some Dockerfiles caught in the process were included
- New patch with script to check or wait on a pipeline execution

[1] - https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg00231.html
[2] - https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg00154.html

Cleber Rosa (2):
  GitLab Gating CI: introduce pipeline-status contrib script
  GitLab Gating CI: initial set of jobs, documentation and scripts

 .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++
 .gitlab-ci.yml                         |   1 +
 docs/devel/testing.rst                 | 147 +++++++++++++++++
 scripts/ci/gitlab-pipeline-status      | 156 ++++++++++++++++++
 scripts/ci/setup/build-environment.yml | 217 +++++++++++++++++++++++++
 scripts/ci/setup/gitlab-runner.yml     |  72 ++++++++
 scripts/ci/setup/inventory             |   2 +
 scripts/ci/setup/vars.yml              |  13 ++
 8 files changed, 754 insertions(+)
 create mode 100644 .gitlab-ci.d/gating.yml
 create mode 100755 scripts/ci/gitlab-pipeline-status
 create mode 100644 scripts/ci/setup/build-environment.yml
 create mode 100644 scripts/ci/setup/gitlab-runner.yml
 create mode 100644 scripts/ci/setup/inventory
 create mode 100644 scripts/ci/setup/vars.yml

-- 
2.25.4

[PATCH v2 1/2] GitLab Gating CI: introduce pipeline-status contrib script

[Cleber Rosa]

This script is intended to be used right after a push to a branch.

By default, it will look for the pipeline associated with the commit
that is the HEAD of the *local* staging branch.  It can be used as a
one time check, or with the `--wait` option to wait until the pipeline
completes.

If the pipeline is successful, then a merge of the staging branch into
the master branch should be the next step.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 scripts/ci/gitlab-pipeline-status | 156 ++++++++++++++++++++++++++++++
 1 file changed, 156 insertions(+)
 create mode 100755 scripts/ci/gitlab-pipeline-status

diff --git a/scripts/ci/gitlab-pipeline-status b/scripts/ci/gitlab-pipeline-status
new file mode 100755
index 0000000000..4a9de39872
--- /dev/null
+++ b/scripts/ci/gitlab-pipeline-status
@@ -0,0 +1,156 @@
+#!/usr/bin/env python3
+#
+# Copyright (c) 2019-2020 Red Hat, Inc.
+#
+# Author:
+#  Cleber Rosa <crosa@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+"""
+Checks the GitLab pipeline status for a given commit commit
+"""
+
+# pylint: disable=C0103
+
+import argparse
+import http.client
+import json
+import os
+import subprocess
+import time
+import sys
+
+
+def get_local_staging_branch_commit():
+    """
+    Returns the commit sha1 for the *local* branch named "staging"
+    """
+    result = subprocess.run(['git', 'rev-parse', 'staging'],
+                            stdin=subprocess.DEVNULL,
+                            stdout=subprocess.PIPE,
+                            stderr=subprocess.DEVNULL,
+                            cwd=os.path.dirname(__file__),
+                            universal_newlines=True).stdout.strip()
+    if result == 'staging':
+        raise ValueError("There's no local staging branch")
+    if len(result) != 40:
+        raise ValueError("Branch staging HEAD doesn't look like a sha1")
+    return result
+
+
+def get_pipeline_status(project_id, commit_sha1):
+    """
+    Returns the JSON content of the pipeline status API response
+    """
+    url = '/api/v4/projects/{}/pipelines?sha={}'.format(project_id,
+                                                        commit_sha1)
+    connection = http.client.HTTPSConnection('gitlab.com')
+    connection.request('GET', url=url)
+    response = connection.getresponse()
+    if response.code != http.HTTPStatus.OK:
+        raise ValueError("Failed to receive a successful response")
+    json_response = json.loads(response.read())
+    # afaict, there should one one pipeline for the same project + commit
+    # if this assumption is false, we can add further filters to the
+    # url, such as username, and order_by.
+    if not json_response:
+        raise ValueError("No pipeline found")
+    return json_response[0]
+
+
+def wait_on_pipeline_success(timeout, interval,
+                             project_id, commit_sha):
+    """
+    Waits for the pipeline to end up to the timeout given
+    """
+    start = time.time()
+    while True:
+        if time.time() >= (start + timeout):
+            print("Waiting on the pipeline success timed out")
+            return False
+
+        status = get_pipeline_status(project_id, commit_sha)
+        if status['status'] == 'running':
+            time.sleep(interval)
+            print('running...')
+            continue
+
+        if status['status'] == 'success':
+            return True
+
+        msg = "Pipeline ended unsuccessfully, check: %s" % status['web_url']
+        print(msg)
+        return False
+
+
+def main():
+    """
+    Script entry point
+    """
+    parser = argparse.ArgumentParser(
+        prog='pipeline-status',
+        description='check or wait on a pipeline status')
+
+    parser.add_argument('-t', '--timeout', type=int, default=7200,
+                        help=('Amount of time (in seconds) to wait for the '
+                              'pipeline to complete.  Defaults to '
+                              '%(default)s'))
+    parser.add_argument('-i', '--interval', type=int, default=60,
+                        help=('Amount of time (in seconds) to wait between '
+                              'checks of the pipeline status.  Defaults '
+                              'to %(default)s'))
+    parser.add_argument('-w', '--wait', action='store_true', default=False,
+                        help=('Wether to wait, instead of checking only once '
+                              'the status of a pipeline'))
+    parser.add_argument('-p', '--project-id', type=int, default=11167699,
+                        help=('The GitLab project ID. Defaults to the project '
+                              'for https://gitlab.com/qemu-project/qemu, that '
+                              'is, "%(default)s"'))
+    try:
+        default_commit = get_local_staging_branch_commit()
+        commit_required = False
+    except ValueError:
+        default_commit = ''
+        commit_required = True
+    parser.add_argument('-c', '--commit', required=commit_required,
+                        default=default_commit,
+                        help=('Look for a pipeline associated with the given '
+                              'commit.  If one is not explicitly given, the '
+                              'commit associated with the local branch named '
+                              '"staging" is used.  Default: %(default)s'))
+    parser.add_argument('--verbose', action='store_true', default=False,
+                        help=('A minimal verbosity level that prints the '
+                              'overall result of the check/wait'))
+
+    args = parser.parse_args()
+
+    try:
+        if args.wait:
+            success = wait_on_pipeline_success(
+                args.timeout,
+                args.interval,
+                args.project_id,
+                args.commit)
+        else:
+            status = get_pipeline_status(args.project_id,
+                                         args.commit)
+            success = status['status'] == 'success'
+    except Exception as error:      # pylint: disable=W0703
+        success = False
+        if args.verbose:
+            print("ERROR: %s" % error.args[0])
+
+    if success:
+        if args.verbose:
+            print('success')
+        sys.exit(0)
+    else:
+        if args.verbose:
+            print('failure')
+        sys.exit(1)
+
+
+if __name__ == '__main__':
+    main()
-- 
2.25.4

[PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Cleber Rosa]

This is a mapping of Peter's "remake-merge-builds" and
"pull-buildtest" scripts, gone through some updates, adding some build
option and removing others.

The jobs currently cover the machines that the QEMU project owns, and that
are setup and ready to run jobs:

 - Ubuntu 18.04 on S390x
 - Ubuntu 20.04 on aarch64

During the development of this set of jobs, the GitLab CI was tested
with many other architectures, including ppc64, s390x and aarch64,
along with the other OSs (not included here):

 - Fedora 30
 - FreeBSD 12.1

More information can be found in the documentation itself.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++
 .gitlab-ci.yml                         |   1 +
 docs/devel/testing.rst                 | 147 +++++++++++++++++
 scripts/ci/setup/build-environment.yml | 217 +++++++++++++++++++++++++
 scripts/ci/setup/gitlab-runner.yml     |  72 ++++++++
 scripts/ci/setup/inventory             |   2 +
 scripts/ci/setup/vars.yml              |  13 ++
 7 files changed, 598 insertions(+)
 create mode 100644 .gitlab-ci.d/gating.yml
 create mode 100644 scripts/ci/setup/build-environment.yml
 create mode 100644 scripts/ci/setup/gitlab-runner.yml
 create mode 100644 scripts/ci/setup/inventory
 create mode 100644 scripts/ci/setup/vars.yml

diff --git a/.gitlab-ci.d/gating.yml b/.gitlab-ci.d/gating.yml
new file mode 100644
index 0000000000..5562df5708
--- /dev/null
+++ b/.gitlab-ci.d/gating.yml
@@ -0,0 +1,146 @@
+variables:
+  GIT_SUBMODULE_STRATEGY: recursive
+
+# All ubuntu-18.04 jobs should run successfully in an environment
+# setup by the scripts/ci/setup/build-environment.yml task
+# "Install basic packages to build QEMU on Ubuntu 18.04/20.04"
+ubuntu-18.04-s390x-all-linux-static:
+ tags:
+ - ubuntu_18.04
+ - s390x
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ # --disable-libssh is needed because of https://bugs.launchpad.net/qemu/+bug/1838763
+ # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages
+ - ./configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+ - make --output-sync -j`nproc` check-tcg V=1
+
+ubuntu-18.04-s390x-all:
+ tags:
+ - ubuntu_18.04
+ - s390x
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --disable-libssh
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+
+ubuntu-18.04-s390x-alldbg:
+ tags:
+ - ubuntu_18.04
+ - s390x
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --enable-debug --disable-libssh
+ - make clean
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+
+ubuntu-18.04-s390x-clang:
+ tags:
+ - ubuntu_18.04
+ - s390x
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --disable-libssh --cc=clang --cxx=clang++ --enable-sanitizers
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+
+ubuntu-18.04-s390x-tci:
+ tags:
+ - ubuntu_18.04
+ - s390x
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --disable-libssh --enable-tcg-interpreter
+ - make --output-sync -j`nproc`
+
+ubuntu-18.04-s390x-notcg:
+ tags:
+ - ubuntu_18.04
+ - s390x
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --disable-libssh --disable-tcg
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+
+# All ubuntu-20.04 jobs should run successfully in an environment
+# setup by the scripts/ci/setup/qemu/build-environment.yml task
+# "Install basic packages to build QEMU on Ubuntu 18.04/20.04"
+ubuntu-20.04-aarch64-all-linux-static:
+ tags:
+ - ubuntu_20.04
+ - aarch64
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ # --disable-libssh is needed because of https://bugs.launchpad.net/qemu/+bug/1838763
+ # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages
+ - ./configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+ - make --output-sync -j`nproc` check-tcg V=1
+
+ubuntu-20.04-aarch64-all:
+ tags:
+ - ubuntu_20.04
+ - aarch64
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --disable-libssh
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+
+ubuntu-20.04-aarch64-alldbg:
+ tags:
+ - ubuntu_20.04
+ - aarch64
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --enable-debug --disable-libssh
+ - make clean
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+
+ubuntu-20.04-aarch64-clang:
+ tags:
+ - ubuntu_20.04
+ - aarch64
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --disable-libssh --cc=clang --cxx=clang++ --enable-sanitizers
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
+
+ubuntu-20.04-aarch64-tci:
+ tags:
+ - ubuntu_20.04
+ - aarch64
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --disable-libssh --enable-tcg-interpreter
+ - make --output-sync -j`nproc`
+
+ubuntu-20.04-aarch64-notcg:
+ tags:
+ - ubuntu_20.04
+ - aarch64
+ rules:
+ - if: '$CI_COMMIT_REF_NAME == "staging"'
+ script:
+ - ./configure --disable-libssh --disable-tcg
+ - make --output-sync -j`nproc`
+ - make --output-sync -j`nproc` check V=1
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 349c77aa58..49efe38529 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,6 +1,7 @@
 include:
   - local: '/.gitlab-ci.d/edk2.yml'
   - local: '/.gitlab-ci.d/opensbi.yml'
+  - local: '/.gitlab-ci.d/gating.yml'
 
 .update_apt_template: &before_script_apt
  before_script:
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index c1ff24370b..f8dab788ea 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -1003,3 +1003,150 @@ exercise as many corner cases as possible. It is a useful test suite
 to run to exercise QEMU's linux-user code::
 
   https://linux-test-project.github.io/
+
+CI
+==
+
+QEMU has configurations enabled for a number of different CI services.
+The most update information about them and their status can be found
+at::
+
+   https://wiki.qemu.org/Testing/CI
+
+Gating CI
+----------
+
+A Pull Requests will only to be merged if they successfully go through
+a different set of CI jobs.  GitLab's CI is the service/framework used
+for executing the gating jobs.
+
+The architecture of GitLab's CI service allows different machines to be
+setup with GitLab's "agent", called gitlab-runner, which will take care
+of running jobs created by events such as a push to a branch.
+
+Even though gitlab-runner can execute jobs on environments such as
+containers, this initial implementation assumes the shell executor is
+used, effectively running jobs on the same machine (be them physical
+or virtual) the gitlab-runner agent is running.  This means those
+machines must be setup in advance, with the requirements matching the
+jobs expected to be executed there.
+
+Machine configuration for gating jobs
+-------------------------------------
+
+The GitLab's CI architecture allows different parties to provide
+different machines that will run different jobs.  At this point, QEMU
+will deploy a limited set of machines and jobs.  Documentation and/or
+scripts to setup those machines is located under::
+
+  scripts/ci/setup
+
+Ansible playbooks have been provided to perform two different tasks
+related to setting gitlab-runner and the build environment.
+
+Other organizations involved in QEMU development may, in the near
+future, contribute their own setup documentation/scripts under
+``scripts/ci/orgs/$ORG_NAME`` directory.
+
+The GitLab CI jobs definition for the gating jobs are located under::
+
+  .gitlab-ci.d/gating.yml
+
+Once the gating CI has proved mature with this set of initial jobs,
+other members in the community may provide their own machine
+configuration documentation/scripts, and accompanying job definitions.
+Those contributed jobs should run as non-gating, until their
+reliability is verified.
+
+Machine Setup Howto
+-------------------
+
+For all Linux based systems, the setup can be mostly automated by the
+execution of two Ansible playbooks.  Start by adding your machines to
+the ``inventory`` file under ``scripts/ci/setup``, such as this::
+
+  [local]
+  fully.qualified.domain
+  other.machine.hostname
+
+You may need to set some variables in the inventory file itself.  One
+very common need is to tell Ansible to use a Python 3 interpreter on
+those hosts.  This would look like::
+
+  [local]
+  fully.qualified.domain ansible_python_interpreter=/usr/bin/python3
+  other.machine.hostname ansible_python_interpreter=/usr/bin/python3
+
+Build environment
+~~~~~~~~~~~~~~~~~
+
+The ``scripts/ci/setup/build-environment.yml`` Ansible playbook
+will setup machines with the environment needed to perform builds of
+QEMU and runs of the tests defined in the current gating jobs.  It
+covers a number of different Linux distributions and FreeBSD.
+
+To run the playbook, execute::
+
+  cd scripts/ci/setup
+  ansible-playbook -i inventory build-environment.yml
+
+gitlab-runner setup and registration
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The gitlab-runner agent needs to be installed on each machine that
+will run jobs.  The association between a machine and a GitLab project
+happens with a registration token.  To find the registration token for
+your repository/project, navigate on GitLab's web UI to:
+
+ * Settings (the gears like icon), then
+ * CI/CD, then
+ * Runners, and click on the "Expand" button, then
+ * Under "Set up a specific Runner manually", look for the value under
+   "Use the following registration token during setup"
+
+Edit the ``scripts/ci/setup/vars.yml`` file, setting the
+``gitlab_runner_registration_token`` variable to the value obtained
+earlier.
+
+.. note:: gitlab-runner is not available from the standard location
+          for all OS and architectures combinations.  For some systems,
+          a custom build may be necessary.  Some builds are avaiable
+          at https://cleber.fedorapeople.org/gitlab-runner/ and this
+          URI may be used as a value on ``vars.yml``
+
+To run the playbook, execute::
+
+  cd scripts/ci/setup
+  ansible-playbook -i inventory gitlab-runner.yml
+
+.. note:: there are currently limitations to gitlab-runner itself when
+          setting up a service under FreeBSD systems.  You will need to
+          perform steps 4 to 10 manually, as described at
+          https://docs.gitlab.com/runner/install/freebsd.html
+
+Following the registration, it's necessary to configure the runner tags,
+and optionally other configurations on the GitLab UI.  Navigate to:
+
+ * Settings (the gears like icon), then
+ * CI/CD, then
+ * Runners, and click on the "Expand" button, then
+ * "Runners activated for this project", then
+ * Click on the "Edit" icon (next to the "Lock" Icon)
+
+Under tags, add values matching the jobs a runner should run.  For a
+FreeBSD 12.1 x86_64 system, the tags should be set as::
+
+  freebsd12.1,x86_64
+
+Because the job definition at ``.gitlab-ci.d/gating.yml`` would contain::
+
+  freebsd-12.1-x86_64-all:
+   tags:
+   - freebsd_12.1
+   - x86_64
+
+It's also recommended to:
+
+ * increase the "Maximum job timeout" to something like ``2h``
+ * uncheck the "Run untagged jobs" check box
+ * give it a better Description
diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
new file mode 100644
index 0000000000..89b35386c7
--- /dev/null
+++ b/scripts/ci/setup/build-environment.yml
@@ -0,0 +1,217 @@
+---
+- name: Installation of basic packages to build QEMU
+  hosts: all
+  vars_files:
+    - vars.yml
+  tasks:
+    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
+      apt:
+        update_cache: yes
+        # This matches the packages on tests/docker/Dockerfiles/ubuntu1804.docker
+        pkg:
+          - flex
+          - bison
+          - ccache
+          - clang
+          - gcc
+          - gettext
+          - git
+          - glusterfs-common
+          - libaio-dev
+          - libattr1-dev
+          - libbrlapi-dev
+          - libbz2-dev
+          - libcacard-dev
+          - libcap-ng-dev
+          - libcurl4-gnutls-dev
+          - libdrm-dev
+          - libepoxy-dev
+          - libfdt-dev
+          - libgbm-dev
+          - libgtk-3-dev
+          - libibverbs-dev
+          - libiscsi-dev
+          - libjemalloc-dev
+          - libjpeg-turbo8-dev
+          - liblzo2-dev
+          - libncurses5-dev
+          - libncursesw5-dev
+          - libnfs-dev
+          - libnss3-dev
+          - libnuma-dev
+          - libpixman-1-dev
+          - librados-dev
+          - librbd-dev
+          - librdmacm-dev
+          - libsasl2-dev
+          - libsdl2-dev
+          - libseccomp-dev
+          - libsnappy-dev
+          - libspice-protocol-dev
+          - libssh-dev
+          - libusb-1.0-0-dev
+          - libusbredirhost-dev
+          - libvdeplug-dev
+          - libvte-2.91-dev
+          - libzstd-dev
+          - make
+          - python3-yaml
+          - python3-sphinx
+          - sparse
+          - texinfo
+          - xfslibs-dev
+        state: present
+      when: "ansible_facts['distribution'] == 'Ubuntu'"
+
+    - name: Install packages to build QEMU on Ubuntu 18.04/20.04 on non-s390x
+      apt:
+        update_cache: yes
+        pkg:
+         - libspice-server-dev
+         - libxen-dev
+        state: present
+      when:
+        - "ansible_facts['distribution'] == 'Ubuntu'"
+        - "ansible_facts['architecture'] != 's390x'"
+
+    - name: Install basic packages to build QEMU on FreeBSD 12.x
+      pkgng:
+        # This matches the packages on .cirrus.yml under the freebsd_12_task
+        name: bash,bison,curl,cyrus-sasl,git,glib,gmake,gnutls,gsed,nettle,perl5,pixman,pkgconf,png,usbredir
+        state: present
+      when: "ansible_facts['os_family'] == 'FreeBSD'"
+
+    - name: Install basic packages to build QEMU on Fedora 30
+      dnf:
+        # This matches the packages on tests/docker/Dockerfiles/fedora.docker
+        name:
+          - bc
+          - bison
+          - brlapi-devel
+          - bzip2
+          - bzip2-devel
+          - ccache
+          - clang
+          - cyrus-sasl-devel
+          - dbus-daemon
+          - device-mapper-multipath-devel
+          - findutils
+          - flex
+          - gcc
+          - gcc-c++
+          - gettext
+          - git
+          - glib2-devel
+          - glusterfs-api-devel
+          - gnutls-devel
+          - gtk3-devel
+          - hostname
+          - libaio-devel
+          - libasan
+          - libattr-devel
+          - libblockdev-mpath-devel
+          - libcap-ng-devel
+          - libcurl-devel
+          - libfdt-devel
+          - libiscsi-devel
+          - libjpeg-devel
+          - libpmem-devel
+          - libpng-devel
+          - librbd-devel
+          - libseccomp-devel
+          - libssh-devel
+          - libubsan
+          - libusbx-devel
+          - libxml2-devel
+          - llvm
+          - lzo-devel
+          - make
+          - mingw32-bzip2
+          - mingw32-curl
+          - mingw32-glib2
+          - mingw32-gmp
+          - mingw32-gnutls
+          - mingw32-gtk3
+          - mingw32-libjpeg-turbo
+          - mingw32-libpng
+          - mingw32-libtasn1
+          - mingw32-nettle
+          - mingw32-nsis
+          - mingw32-pixman
+          - mingw32-pkg-config
+          - mingw32-SDL2
+          - mingw64-bzip2
+          - mingw64-curl
+          - mingw64-glib2
+          - mingw64-gmp
+          - mingw64-gnutls
+          - mingw64-gtk3
+          - mingw64-libjpeg-turbo
+          - mingw64-libpng
+          - mingw64-libtasn1
+          - mingw64-nettle
+          - mingw64-pixman
+          - mingw64-pkg-config
+          - mingw64-SDL2
+          - ncurses-devel
+          - nettle-devel
+          - nss-devel
+          - numactl-devel
+          - perl
+          - perl-Test-Harness
+          - pixman-devel
+          - python3
+          - python3-sphinx
+          - PyYAML
+          - rdma-core-devel
+          - SDL2-devel
+          - snappy-devel
+          - sparse
+          - spice-server-devel
+          - systemd-devel
+          - systemtap-sdt-devel
+          - tar
+          - texinfo
+          - usbredir-devel
+          - virglrenderer-devel
+          - vte291-devel
+          - which
+          - xen-devel
+          - zlib-devel
+          - libzstd-devel
+        state: present
+      when: "ansible_facts['distribution'] == 'Fedora'"
+
+    - name: Install basic packages to build QEMU on CentOS 8
+      dnf:
+        # This matches the packages on tests/docker/Dockerfiles/centos8.docker
+        name:
+          - SDL-devel
+          - bison
+          - bzip2
+          - bzip2-devel
+          - dbus-daemon
+          - flex
+          - gcc
+          - gcc-c++
+          - gettext
+          - git
+          - glib2-devel
+          - libaio-devel
+          - libepoxy-devel
+          - lzo-devel
+          - make
+          - mesa-libEGL-devel
+          - nettle-devel
+          - perl-Test-Harness
+          - pixman-devel
+          - python36
+          - rdma-core-devel
+          - spice-glib-devel
+          - spice-server
+          - tar
+          - zlib-devel
+        state: present
+      when:
+        - "ansible_facts['distribution'] == 'CentOS'"
+        - "ansible_facts['distribution_major_version'] == '8'"
diff --git a/scripts/ci/setup/gitlab-runner.yml b/scripts/ci/setup/gitlab-runner.yml
new file mode 100644
index 0000000000..c2f52dad10
--- /dev/null
+++ b/scripts/ci/setup/gitlab-runner.yml
@@ -0,0 +1,72 @@
+---
+- name: Installation of gitlab-runner
+  hosts: all
+  vars_files:
+    - vars.yml
+  tasks:
+    - debug:
+        msg: 'Checking for a valid GitLab registration token'
+      failed_when: "gitlab_runner_registration_token == 'PLEASE_PROVIDE_A_VALID_TOKEN'"
+
+    - name: Checks the availability of official gitlab-runner builds in the archive
+      uri:
+        url: https://s3.amazonaws.com/gitlab-runner-downloads/v{{ gitlab_runner_version  }}/binaries/gitlab-runner-linux-386
+        method: HEAD
+        status_code:
+          - 200
+          - 403
+      register: gitlab_runner_available_archive
+
+    - name: Update base url
+      set_fact:
+        gitlab_runner_base_url: https://s3.amazonaws.com/gitlab-runner-downloads/v{{ gitlab_runner_version  }}/binaries/gitlab-runner-
+      when: gitlab_runner_available_archive.status == 200
+    - debug:
+        msg: Base gitlab-runner url is {{ gitlab_runner_base_url  }}
+
+    - name: Set OS name (FreeBSD)
+      set_fact:
+        gitlab_runner_os: freebsd
+      when: "ansible_facts['system'] == 'FreeBSD'"
+
+    - name: Create a group for the gitlab-runner service
+      group:
+        name: gitlab-runner
+
+    - name: Create a user for the gitlab-runner service
+      user:
+        user: gitlab-runner
+        group: gitlab-runner
+        comment: GitLab Runner
+        home: /home/gitlab-runner
+        shell: /bin/bash
+
+    - name: Remove the .bash_logout file when on Ubuntu systems
+      file:
+        path: /home/gitlab-runner/.bash_logout
+        state: absent
+      when: "ansible_facts['distribution'] == 'Ubuntu'"
+
+    - name: Downloads the matching gitlab-runner
+      get_url:
+        dest: /usr/local/bin/gitlab-runner
+        url: "{{ gitlab_runner_base_url }}{{ gitlab_runner_os }}-{{ gitlab_runner_arch }}"
+        owner: gitlab-runner
+        group: gitlab-runner
+        mode: u=rwx,g=rwx,o=rx
+
+    - name: Register the gitlab-runner
+      command: "/usr/local/bin/gitlab-runner register --non-interactive --url {{ gitlab_runner_server_url }} --registration-token {{ gitlab_runner_registration_token }} --executor shell  --description '{{ ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] }} {{ ansible_facts[\"architecture\"] }} ({{ ansible_facts[\"os_family\"] }})'"
+
+    - name: Install the gitlab-runner service using its own functionality
+      command: /usr/local/bin/gitlab-runner install --user gitlab-runner --working-directory /home/gitlab-runner
+      register: gitlab_runner_install_service_result
+      failed_when: "gitlab_runner_install_service_result.rc != 0 and \"already exists\" not in gitlab_runner_install_service_result.stderr"
+      when: "ansible_facts['os_family'] != 'FreeBSD'"
+
+    - name: Enable the gitlab-runner service
+      service:
+        name: gitlab-runner
+        state: started
+        enabled: yes
+      when: "ansible_facts['os_family'] != 'FreeBSD'"
diff --git a/scripts/ci/setup/inventory b/scripts/ci/setup/inventory
new file mode 100644
index 0000000000..8bb7ba6b33
--- /dev/null
+++ b/scripts/ci/setup/inventory
@@ -0,0 +1,2 @@
+[local]
+localhost
diff --git a/scripts/ci/setup/vars.yml b/scripts/ci/setup/vars.yml
new file mode 100644
index 0000000000..621435d030
--- /dev/null
+++ b/scripts/ci/setup/vars.yml
@@ -0,0 +1,13 @@
+# The version of the gitlab-runner to use
+gitlab_runner_version: 13.1.1
+# The base location of gitlab-runner binaries, this will be suffixed by $OS-$ARCH
+gitlab_runner_base_url: https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-
+# The URL of the gitlab server to use, usually https://gitlab.com unless you're
+# using a private GitLab instance
+gitlab_runner_server_url: https://gitlab.com
+# Defaults to linux, checks can be used to change this
+gitlab_runner_os: linux
+# Defaults to amd64 (x86_64), checks can be used to change this
+gitlab_runner_arch: amd64
+# A unique token made available by GitLab to your project for registering runners
+gitlab_runner_registration_token: PLEASE_PROVIDE_A_VALID_TOKEN
-- 
2.25.4

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Erik Skultety]

On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> This is a mapping of Peter's "remake-merge-builds" and
> "pull-buildtest" scripts, gone through some updates, adding some build
> option and removing others.
>
> The jobs currently cover the machines that the QEMU project owns, and that
> are setup and ready to run jobs:
>
>  - Ubuntu 18.04 on S390x
>  - Ubuntu 20.04 on aarch64
>
> During the development of this set of jobs, the GitLab CI was tested
> with many other architectures, including ppc64, s390x and aarch64,
> along with the other OSs (not included here):
>
>  - Fedora 30
>  - FreeBSD 12.1
>
> More information can be found in the documentation itself.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++
>  .gitlab-ci.yml                         |   1 +
>  docs/devel/testing.rst                 | 147 +++++++++++++++++
>  scripts/ci/setup/build-environment.yml | 217 +++++++++++++++++++++++++
>  scripts/ci/setup/gitlab-runner.yml     |  72 ++++++++
>  scripts/ci/setup/inventory             |   2 +
>  scripts/ci/setup/vars.yml              |  13 ++
>  7 files changed, 598 insertions(+)
>  create mode 100644 .gitlab-ci.d/gating.yml
>  create mode 100644 scripts/ci/setup/build-environment.yml
>  create mode 100644 scripts/ci/setup/gitlab-runner.yml
>  create mode 100644 scripts/ci/setup/inventory
>  create mode 100644 scripts/ci/setup/vars.yml
>
> diff --git a/.gitlab-ci.d/gating.yml b/.gitlab-ci.d/gating.yml
> new file mode 100644
> index 0000000000..5562df5708
> --- /dev/null
> +++ b/.gitlab-ci.d/gating.yml
> @@ -0,0 +1,146 @@
> +variables:
> +  GIT_SUBMODULE_STRATEGY: recursive
> +
> +# All ubuntu-18.04 jobs should run successfully in an environment
> +# setup by the scripts/ci/setup/build-environment.yml task
> +# "Install basic packages to build QEMU on Ubuntu 18.04/20.04"
> +ubuntu-18.04-s390x-all-linux-static:
> + tags:
> + - ubuntu_18.04
> + - s390x
> + rules:
> + - if: '$CI_COMMIT_REF_NAME == "staging"'
> + script:
> + # --disable-libssh is needed because of https://bugs.launchpad.net/qemu/+bug/1838763
> + # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages
> + - ./configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh
> + - make --output-sync -j`nproc`
> + - make --output-sync -j`nproc` check V=1
> + - make --output-sync -j`nproc` check-tcg V=1

I know this patch doesn't introduce a FreeBSD job, but later in the patch it's
clear you'd want to introduce them at some point, so:
'nproc' doesn't exist on FreeBSD, but `getconf _NPROCESSORS_ONLN` does, so you
may want to use it right from the start.

...

> +
> +CI
> +==
> +
> +QEMU has configurations enabled for a number of different CI services.
> +The most update information about them and their status can be found

s/update/up to date/

> +at::
> +
> +   https://wiki.qemu.org/Testing/CI
> +
> +Gating CI
> +----------
> +
> +A Pull Requests will only to be merged if they successfully go through

s/A /
s/to be/be/

> +a different set of CI jobs.  GitLab's CI is the service/framework used

s/a different set/different sets/       (I may be wrong with this one)

...

> +
> +gitlab-runner setup and registration
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The gitlab-runner agent needs to be installed on each machine that
> +will run jobs.  The association between a machine and a GitLab project
> +happens with a registration token.  To find the registration token for
> +your repository/project, navigate on GitLab's web UI to:
> +
> + * Settings (the gears like icon), then
> + * CI/CD, then
> + * Runners, and click on the "Expand" button, then
> + * Under "Set up a specific Runner manually", look for the value under
> +   "Use the following registration token during setup"
> +
> +Edit the ``scripts/ci/setup/vars.yml`` file, setting the
> +``gitlab_runner_registration_token`` variable to the value obtained
> +earlier.
> +
> +.. note:: gitlab-runner is not available from the standard location
> +          for all OS and architectures combinations.  For some systems,
> +          a custom build may be necessary.  Some builds are avaiable
> +          at https://cleber.fedorapeople.org/gitlab-runner/ and this
> +          URI may be used as a value on ``vars.yml``
> +
> +To run the playbook, execute::
> +
> +  cd scripts/ci/setup
> +  ansible-playbook -i inventory gitlab-runner.yml
> +
> +.. note:: there are currently limitations to gitlab-runner itself when
> +          setting up a service under FreeBSD systems.  You will need to
> +          perform steps 4 to 10 manually, as described at
> +          https://docs.gitlab.com/runner/install/freebsd.html

What kinds of limitations? Is it architecture constrained maybe? I'm asking
because we have all of the steps covered by an Ansible playbook, so I kinda got
triggered by the word "manually". Also, the document only mentions 9 steps
overall.

Regards,
Erik

Re: [PATCH v2 1/2] GitLab Gating CI: introduce pipeline-status contrib script

[Erik Skultety]

On Wed, Jul 08, 2020 at 10:46:56PM -0400, Cleber Rosa wrote:
> This script is intended to be used right after a push to a branch.
>
> By default, it will look for the pipeline associated with the commit
> that is the HEAD of the *local* staging branch.  It can be used as a
> one time check, or with the `--wait` option to wait until the pipeline
> completes.
>
> If the pipeline is successful, then a merge of the staging branch into
> the master branch should be the next step.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  scripts/ci/gitlab-pipeline-status | 156 ++++++++++++++++++++++++++++++
>  1 file changed, 156 insertions(+)
>  create mode 100755 scripts/ci/gitlab-pipeline-status
>
> diff --git a/scripts/ci/gitlab-pipeline-status b/scripts/ci/gitlab-pipeline-status
> new file mode 100755
> index 0000000000..4a9de39872
> --- /dev/null
> +++ b/scripts/ci/gitlab-pipeline-status
> @@ -0,0 +1,156 @@
> +#!/usr/bin/env python3
> +#
> +# Copyright (c) 2019-2020 Red Hat, Inc.
> +#
> +# Author:
> +#  Cleber Rosa <crosa@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +
> +"""
> +Checks the GitLab pipeline status for a given commit commit

s/commit$/(hash|sha|ID|)

> +"""
> +
> +# pylint: disable=C0103
> +
> +import argparse
> +import http.client
> +import json
> +import os
> +import subprocess
> +import time
> +import sys
> +
> +
> +def get_local_staging_branch_commit():
> +    """
> +    Returns the commit sha1 for the *local* branch named "staging"
> +    """
> +    result = subprocess.run(['git', 'rev-parse', 'staging'],

If one day Peter decides that "staging" is not a cool name anymore and use a
different name for the branch :) we should account for that and make it a
variable, possibly even parametrize this function with it.

> +                            stdin=subprocess.DEVNULL,
> +                            stdout=subprocess.PIPE,
> +                            stderr=subprocess.DEVNULL,
> +                            cwd=os.path.dirname(__file__),
> +                            universal_newlines=True).stdout.strip()
> +    if result == 'staging':
> +        raise ValueError("There's no local staging branch")

"There's no local branch named 'staging'" would IMO be more descriptive, so as
not to confuse it with staging in git.

> +    if len(result) != 40:
> +        raise ValueError("Branch staging HEAD doesn't look like a sha1")
> +    return result
> +
> +
> +def get_pipeline_status(project_id, commit_sha1):
> +    """
> +    Returns the JSON content of the pipeline status API response
> +    """
> +    url = '/api/v4/projects/{}/pipelines?sha={}'.format(project_id,
> +                                                        commit_sha1)
> +    connection = http.client.HTTPSConnection('gitlab.com')
> +    connection.request('GET', url=url)
> +    response = connection.getresponse()
> +    if response.code != http.HTTPStatus.OK:
> +        raise ValueError("Failed to receive a successful response")
> +    json_response = json.loads(response.read())

a blank line separating the commentary block would slightly help readability

> +    # afaict, there should one one pipeline for the same project + commit

s/one one/be only one/

> +    # if this assumption is false, we can add further filters to the
> +    # url, such as username, and order_by.
> +    if not json_response:
> +        raise ValueError("No pipeline found")
> +    return json_response[0]
> +
> +
> +def wait_on_pipeline_success(timeout, interval,
> +                             project_id, commit_sha):
> +    """
> +    Waits for the pipeline to end up to the timeout given

"Waits for the pipeline to finish within the given timeout"

> +    """
> +    start = time.time()
> +    while True:
> +        if time.time() >= (start + timeout):
> +            print("Waiting on the pipeline success timed out")

s/success//
(the pipeline doesn't always have to finish with success)

> +            return False
> +
> +        status = get_pipeline_status(project_id, commit_sha)
> +        if status['status'] == 'running':
> +            time.sleep(interval)
> +            print('running...')
> +            continue
> +
> +        if status['status'] == 'success':
> +            return True
> +
> +        msg = "Pipeline ended unsuccessfully, check: %s" % status['web_url']

I think more common expression is "Pipeline failed"

> +        print(msg)
> +        return False
> +
...

Code-wise looks OK to me, but since I don't know what Peter's requirements
on/expectations of this script are, I can't do a more thorough review.

Regards,
Erik

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Philippe Mathieu-Daudé]

On 7/9/20 4:46 AM, Cleber Rosa wrote:
> This is a mapping of Peter's "remake-merge-builds" and
> "pull-buildtest" scripts, gone through some updates, adding some build
> option and removing others.
> 
> The jobs currently cover the machines that the QEMU project owns, and that
> are setup and ready to run jobs:
> 
>  - Ubuntu 18.04 on S390x
>  - Ubuntu 20.04 on aarch64
> 
> During the development of this set of jobs, the GitLab CI was tested
> with many other architectures, including ppc64, s390x and aarch64,
> along with the other OSs (not included here):
> 
>  - Fedora 30
>  - FreeBSD 12.1
> 
> More information can be found in the documentation itself.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++
>  .gitlab-ci.yml                         |   1 +
>  docs/devel/testing.rst                 | 147 +++++++++++++++++

Time to consider moving the CI doc in a separate file...

>  scripts/ci/setup/build-environment.yml | 217 +++++++++++++++++++++++++
>  scripts/ci/setup/gitlab-runner.yml     |  72 ++++++++
>  scripts/ci/setup/inventory             |   2 +
>  scripts/ci/setup/vars.yml              |  13 ++

Should we name these last two as inventory.template
and vars.yml.template?

Maybe you can add them with gitlab-runner.yml and a part of
the documentation in a first patch,

Then build-environment.yml and another part of the doc
in another patch,

Finally gating.yml and the related missing doc as the
last patch?

>  7 files changed, 598 insertions(+)
>  create mode 100644 .gitlab-ci.d/gating.yml
>  create mode 100644 scripts/ci/setup/build-environment.yml
>  create mode 100644 scripts/ci/setup/gitlab-runner.yml
>  create mode 100644 scripts/ci/setup/inventory
>  create mode 100644 scripts/ci/setup/vars.yml
[...]

Re: [PATCH v2 1/2] GitLab Gating CI: introduce pipeline-status contrib script

[Philippe Mathieu-Daudé]

On 7/9/20 10:55 AM, Erik Skultety wrote:
> On Wed, Jul 08, 2020 at 10:46:56PM -0400, Cleber Rosa wrote:
>> This script is intended to be used right after a push to a branch.
>>
>> By default, it will look for the pipeline associated with the commit
>> that is the HEAD of the *local* staging branch.  It can be used as a
>> one time check, or with the `--wait` option to wait until the pipeline
>> completes.
>>
>> If the pipeline is successful, then a merge of the staging branch into
>> the master branch should be the next step.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  scripts/ci/gitlab-pipeline-status | 156 ++++++++++++++++++++++++++++++
>>  1 file changed, 156 insertions(+)
>>  create mode 100755 scripts/ci/gitlab-pipeline-status
>>
>> diff --git a/scripts/ci/gitlab-pipeline-status b/scripts/ci/gitlab-pipeline-status
>> new file mode 100755
>> index 0000000000..4a9de39872
>> --- /dev/null
>> +++ b/scripts/ci/gitlab-pipeline-status
>> @@ -0,0 +1,156 @@
>> +#!/usr/bin/env python3
>> +#
>> +# Copyright (c) 2019-2020 Red Hat, Inc.
>> +#
>> +# Author:
>> +#  Cleber Rosa <crosa@redhat.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>> +# later.  See the COPYING file in the top-level directory.
>> +
>> +"""
>> +Checks the GitLab pipeline status for a given commit commit
> 
> s/commit$/(hash|sha|ID|)
> 
>> +"""
>> +
>> +# pylint: disable=C0103
>> +
>> +import argparse
>> +import http.client
>> +import json
>> +import os
>> +import subprocess
>> +import time
>> +import sys
>> +
>> +
>> +def get_local_staging_branch_commit():
>> +    """
>> +    Returns the commit sha1 for the *local* branch named "staging"
>> +    """
>> +    result = subprocess.run(['git', 'rev-parse', 'staging'],
> 
> If one day Peter decides that "staging" is not a cool name anymore and use a
> different name for the branch :) we should account for that and make it a
> variable, possibly even parametrize this function with it.

This script can be used by any fork, not only Peter.
So having a parameter (default to 'staging') is a requisite IMO.

>> +                            stdin=subprocess.DEVNULL,
>> +                            stdout=subprocess.PIPE,
>> +                            stderr=subprocess.DEVNULL,
>> +                            cwd=os.path.dirname(__file__),
>> +                            universal_newlines=True).stdout.strip()
>> +    if result == 'staging':
>> +        raise ValueError("There's no local staging branch")
> 
> "There's no local branch named 'staging'" would IMO be more descriptive, so as
> not to confuse it with staging in git.
> 
>> +    if len(result) != 40:
>> +        raise ValueError("Branch staging HEAD doesn't look like a sha1")
>> +    return result
>> +
>> +
>> +def get_pipeline_status(project_id, commit_sha1):
>> +    """
>> +    Returns the JSON content of the pipeline status API response
>> +    """
>> +    url = '/api/v4/projects/{}/pipelines?sha={}'.format(project_id,
>> +                                                        commit_sha1)
>> +    connection = http.client.HTTPSConnection('gitlab.com')
>> +    connection.request('GET', url=url)
>> +    response = connection.getresponse()
>> +    if response.code != http.HTTPStatus.OK:
>> +        raise ValueError("Failed to receive a successful response")
>> +    json_response = json.loads(response.read())
> 
> a blank line separating the commentary block would slightly help readability
> 
>> +    # afaict, there should one one pipeline for the same project + commit
> 
> s/one one/be only one/

'afaict' is not a word.

> 
>> +    # if this assumption is false, we can add further filters to the
>> +    # url, such as username, and order_by.
>> +    if not json_response:
>> +        raise ValueError("No pipeline found")
>> +    return json_response[0]
>> +
>> +
>> +def wait_on_pipeline_success(timeout, interval,
>> +                             project_id, commit_sha):
>> +    """
>> +    Waits for the pipeline to end up to the timeout given
> 
> "Waits for the pipeline to finish within the given timeout"
> 
>> +    """
>> +    start = time.time()
>> +    while True:
>> +        if time.time() >= (start + timeout):
>> +            print("Waiting on the pipeline success timed out")
> 
> s/success//
> (the pipeline doesn't always have to finish with success)
> 
>> +            return False
>> +
>> +        status = get_pipeline_status(project_id, commit_sha)
>> +        if status['status'] == 'running':
>> +            time.sleep(interval)
>> +            print('running...')

If we want to automate the use of this script by a daemon, it would
be better to use the logging class. Then maybe 'running...' is for
the DEBUG level, Other print() calls can be updated to WARN/INFO
levels.

>> +            continue
>> +
>> +        if status['status'] == 'success':
>> +            return True
>> +
>> +        msg = "Pipeline ended unsuccessfully, check: %s" % status['web_url']
> 
> I think more common expression is "Pipeline failed"
> 
>> +        print(msg)
>> +        return False
>> +
> ...
> 
> Code-wise looks OK to me, but since I don't know what Peter's requirements
> on/expectations of this script are, I can't do a more thorough review.
> 
> Regards,
> Erik
> 

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Daniel P. Berrangé]

On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> This is a mapping of Peter's "remake-merge-builds" and
> "pull-buildtest" scripts, gone through some updates, adding some build
> option and removing others.
> 
> The jobs currently cover the machines that the QEMU project owns, and that
> are setup and ready to run jobs:
> 
>  - Ubuntu 18.04 on S390x
>  - Ubuntu 20.04 on aarch64
> 
> During the development of this set of jobs, the GitLab CI was tested
> with many other architectures, including ppc64, s390x and aarch64,
> along with the other OSs (not included here):
> 
>  - Fedora 30
>  - FreeBSD 12.1
> 
> More information can be found in the documentation itself.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++

AFAIK, the jobs in this file just augment what is already defined
in the main .gitlab-ci.yml. Also since we're providing setup info
for other people to configure custom runners, these jobs are usable
for non-gating CI scenarios too.

IOW, the jobs in this file happen to be usable for gating, but they
are not the only gating jobs, and can be used for non-gating reasons.

This is a complicated way of saying that gating.yml is not a desirable
filename, so I'd suggest splitting it in two and having these files
named based on what their contents is, rather than their use case:

   .gitlab-ci.d/runners-s390x.yml
   .gitlab-ci.d/runners-aarch64.yml

The existing jobs in .gitlab-ci.yml could possibly be moved into
a .gitlab-ci.d/runners-shared.yml file for consistency.

>  .gitlab-ci.yml                         |   1 +
>  docs/devel/testing.rst                 | 147 +++++++++++++++++
>  scripts/ci/setup/build-environment.yml | 217 +++++++++++++++++++++++++
>  scripts/ci/setup/gitlab-runner.yml     |  72 ++++++++
>  scripts/ci/setup/inventory             |   2 +
>  scripts/ci/setup/vars.yml              |  13 ++
>  7 files changed, 598 insertions(+)
>  create mode 100644 .gitlab-ci.d/gating.yml
>  create mode 100644 scripts/ci/setup/build-environment.yml
>  create mode 100644 scripts/ci/setup/gitlab-runner.yml
>  create mode 100644 scripts/ci/setup/inventory
>  create mode 100644 scripts/ci/setup/vars.yml
> 
> diff --git a/.gitlab-ci.d/gating.yml b/.gitlab-ci.d/gating.yml
> new file mode 100644
> index 0000000000..5562df5708
> --- /dev/null
> +++ b/.gitlab-ci.d/gating.yml
> @@ -0,0 +1,146 @@
> +variables:
> +  GIT_SUBMODULE_STRATEGY: recursive
> +
> +# All ubuntu-18.04 jobs should run successfully in an environment
> +# setup by the scripts/ci/setup/build-environment.yml task
> +# "Install basic packages to build QEMU on Ubuntu 18.04/20.04"
> +ubuntu-18.04-s390x-all-linux-static:
> + tags:
> + - ubuntu_18.04
> + - s390x
> + rules:
> + - if: '$CI_COMMIT_REF_NAME == "staging"'

I think I'd make it more flexible in particular to allow multiple
branches. For example I have multiple subsystems and have separate
branches for each.

This could be as simple as allowing a regex prefix

  - if: '$CI_COMMIT_REF_NAME =~ /^staging/'




> diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
> new file mode 100644
> index 0000000000..89b35386c7
> --- /dev/null
> +++ b/scripts/ci/setup/build-environment.yml
> @@ -0,0 +1,217 @@
> +---
> +- name: Installation of basic packages to build QEMU
> +  hosts: all
> +  vars_files:
> +    - vars.yml
> +  tasks:
> +    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
> +      apt:
> +        update_cache: yes
> +        # This matches the packages on tests/docker/Dockerfiles/ubuntu1804.docker

I'd be inclined to actually use docker on the custom runners.

eg. instead of having separate physical machines or VMs for each
(distro, arch) pair, have a single host distro for the arch. Then
use docker to provide the build environment against each distro.

IOW, a RHEL-8 aarch64 host, running docker for ubuntu18.04, fedora30
etc.

That way we don't end up duplicating all these packages, and instead
can use  tests/docker/Dockerfiles/ubuntu1804.docker.  This ensures
that if a user needs to reproduce a build failure on their own local
aarch64 machine, they can run docker and get the exact same build
architecture.

It also has the benefit that we don't need to worry about how to
setup gitlab runners for every distro we care about. We only need to
do gitlab runner for the standard host distro, which spawns a pristine
throwaway docker env.

I appreciate this is a big change from what you've done in this patch
though, so don't consider this comment a blocker for initial merge.
I think we should do this as the long term strategy though. Essentially
for Linux builds, everything should always be container based.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Andrea Bolognani]

On Thu, 2020-07-09 at 11:30 +0100, Daniel P. Berrangé wrote:
> On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > +- name: Installation of basic packages to build QEMU
> > +  hosts: all
> > +  vars_files:
> > +    - vars.yml
> > +  tasks:
> > +    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
> > +      apt:
> > +        update_cache: yes
> > +        # This matches the packages on tests/docker/Dockerfiles/ubuntu1804.docker
> 
> I'd be inclined to actually use docker on the custom runners.
> 
> eg. instead of having separate physical machines or VMs for each
> (distro, arch) pair, have a single host distro for the arch. Then
> use docker to provide the build environment against each distro.
> 
> IOW, a RHEL-8 aarch64 host, running docker for ubuntu18.04, fedora30
> etc.
> 
> That way we don't end up duplicating all these packages, and instead
> can use  tests/docker/Dockerfiles/ubuntu1804.docker.  This ensures
> that if a user needs to reproduce a build failure on their own local
> aarch64 machine, they can run docker and get the exact same build
> architecture.
> 
> It also has the benefit that we don't need to worry about how to
> setup gitlab runners for every distro we care about. We only need to
> do gitlab runner for the standard host distro, which spawns a pristine
> throwaway docker env.
> 
> I appreciate this is a big change from what you've done in this patch
> though, so don't consider this comment a blocker for initial merge.
> I think we should do this as the long term strategy though. Essentially
> for Linux builds, everything should always be container based.

Agreed. You should be able to set up a fairly minimal environment,
which consists of Docker, gitlab-runner and not much else, using a
long-term supported distro such as CentOS and then just schedule
whatever container build on it. No need to provision a new machine
every time a new Fedora release comes out, just create a container
image for it and add it to the mix.

Additionally, the gitlab-runner Docker executor provides more
isolation than the shell executor, so running untrusted builds
becomes a more reasonable proposition - this is how the shared
runners on gitlab.com work - and you don't have to worry about your
jobs cleaning up properly after themselves nearly as much.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v2 1/2] GitLab Gating CI: introduce pipeline-status contrib script

[Thomas Huth]

On 09/07/2020 04.46, Cleber Rosa wrote:
> This script is intended to be used right after a push to a branch.
> 
> By default, it will look for the pipeline associated with the commit
> that is the HEAD of the *local* staging branch.  It can be used as a
> one time check, or with the `--wait` option to wait until the pipeline
> completes.
> 
> If the pipeline is successful, then a merge of the staging branch into
> the master branch should be the next step.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  scripts/ci/gitlab-pipeline-status | 156 ++++++++++++++++++++++++++++++
>  1 file changed, 156 insertions(+)
>  create mode 100755 scripts/ci/gitlab-pipeline-status

FWIW, I gave it a try and it works for me:

running...
running...
running...
running...
Pipeline ended unsuccessfully, check:
https://gitlab.com/huth/qemu/-/pipelines/164735644

(yes, that problem with the test that takes too long still persists, but
that's another topic)

Tested-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 1/2] GitLab Gating CI: introduce pipeline-status contrib script

[Thomas Huth]

On 09/07/2020 12.13, Philippe Mathieu-Daudé wrote:
> On 7/9/20 10:55 AM, Erik Skultety wrote:
>> On Wed, Jul 08, 2020 at 10:46:56PM -0400, Cleber Rosa wrote:
>>> This script is intended to be used right after a push to a branch.
>>>
>>> By default, it will look for the pipeline associated with the commit
>>> that is the HEAD of the *local* staging branch.  It can be used as a
>>> one time check, or with the `--wait` option to wait until the pipeline
>>> completes.
>>>
>>> If the pipeline is successful, then a merge of the staging branch into
>>> the master branch should be the next step.
>>>
>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>> ---
>>>  scripts/ci/gitlab-pipeline-status | 156 ++++++++++++++++++++++++++++++
>>>  1 file changed, 156 insertions(+)
>>>  create mode 100755 scripts/ci/gitlab-pipeline-status
>>>
>>> diff --git a/scripts/ci/gitlab-pipeline-status b/scripts/ci/gitlab-pipeline-status
>>> new file mode 100755
>>> index 0000000000..4a9de39872
>>> --- /dev/null
>>> +++ b/scripts/ci/gitlab-pipeline-status
>>> @@ -0,0 +1,156 @@
>>> +#!/usr/bin/env python3
>>> +#
>>> +# Copyright (c) 2019-2020 Red Hat, Inc.
>>> +#
>>> +# Author:
>>> +#  Cleber Rosa <crosa@redhat.com>
>>> +#
>>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>>> +# later.  See the COPYING file in the top-level directory.
>>> +
>>> +"""
>>> +Checks the GitLab pipeline status for a given commit commit
>>
>> s/commit$/(hash|sha|ID|)
>>
>>> +"""
>>> +
>>> +# pylint: disable=C0103
>>> +
>>> +import argparse
>>> +import http.client
>>> +import json
>>> +import os
>>> +import subprocess
>>> +import time
>>> +import sys
>>> +
>>> +
>>> +def get_local_staging_branch_commit():
>>> +    """
>>> +    Returns the commit sha1 for the *local* branch named "staging"
>>> +    """
>>> +    result = subprocess.run(['git', 'rev-parse', 'staging'],
>>
>> If one day Peter decides that "staging" is not a cool name anymore and use a
>> different name for the branch :) we should account for that and make it a
>> variable, possibly even parametrize this function with it.
> 
> This script can be used by any fork, not only Peter.
> So having a parameter (default to 'staging') is a requisite IMO.
> 
>>> +                            stdin=subprocess.DEVNULL,
>>> +                            stdout=subprocess.PIPE,
>>> +                            stderr=subprocess.DEVNULL,
>>> +                            cwd=os.path.dirname(__file__),
>>> +                            universal_newlines=True).stdout.strip()
>>> +    if result == 'staging':
>>> +        raise ValueError("There's no local staging branch")
>>
>> "There's no local branch named 'staging'" would IMO be more descriptive, so as
>> not to confuse it with staging in git.
>>
>>> +    if len(result) != 40:
>>> +        raise ValueError("Branch staging HEAD doesn't look like a sha1")
>>> +    return result
>>> +
>>> +
>>> +def get_pipeline_status(project_id, commit_sha1):
>>> +    """
>>> +    Returns the JSON content of the pipeline status API response
>>> +    """
>>> +    url = '/api/v4/projects/{}/pipelines?sha={}'.format(project_id,
>>> +                                                        commit_sha1)
>>> +    connection = http.client.HTTPSConnection('gitlab.com')
>>> +    connection.request('GET', url=url)
>>> +    response = connection.getresponse()
>>> +    if response.code != http.HTTPStatus.OK:
>>> +        raise ValueError("Failed to receive a successful response")
>>> +    json_response = json.loads(response.read())
>>
>> a blank line separating the commentary block would slightly help readability
>>
>>> +    # afaict, there should one one pipeline for the same project + commit
>>
>> s/one one/be only one/
> 
> 'afaict' is not a word.
> 
>>
>>> +    # if this assumption is false, we can add further filters to the
>>> +    # url, such as username, and order_by.
>>> +    if not json_response:
>>> +        raise ValueError("No pipeline found")
>>> +    return json_response[0]
>>> +
>>> +
>>> +def wait_on_pipeline_success(timeout, interval,
>>> +                             project_id, commit_sha):
>>> +    """
>>> +    Waits for the pipeline to end up to the timeout given
>>
>> "Waits for the pipeline to finish within the given timeout"
>>
>>> +    """
>>> +    start = time.time()
>>> +    while True:
>>> +        if time.time() >= (start + timeout):
>>> +            print("Waiting on the pipeline success timed out")
>>
>> s/success//
>> (the pipeline doesn't always have to finish with success)
>>
>>> +            return False
>>> +
>>> +        status = get_pipeline_status(project_id, commit_sha)
>>> +        if status['status'] == 'running':
>>> +            time.sleep(interval)
>>> +            print('running...')
> 
> If we want to automate the use of this script by a daemon, it would
> be better to use the logging class. Then maybe 'running...' is for
> the DEBUG level, Other print() calls can be updated to WARN/INFO
> levels.
> 
>>> +            continue
>>> +
>>> +        if status['status'] == 'success':
>>> +            return True
>>> +
>>> +        msg = "Pipeline ended unsuccessfully, check: %s" % status['web_url']
>>
>> I think more common expression is "Pipeline failed"
>>
>>> +        print(msg)
>>> +        return False
>>> +
>> ...
>>
>> Code-wise looks OK to me, but since I don't know what Peter's requirements
>> on/expectations of this script are, I can't do a more thorough review.

Ok, I'll add some of the trivial suggestions and take this patch through
my gitlab tree, so Peter can start integrating the gitlab CI in his
merge workflow. We can then later add additional features like the
parameter for the branch name or the logging class.
(I'll skip the second patch for now since there were more questions
raised there which should likely be answered / discussed first)

 Thomas

Re: [PATCH v2 0/2] QEMU Gating CI

[Peter Maydell]

On Thu, 9 Jul 2020 at 03:47, Cleber Rosa <crosa@redhat.com> wrote:
>
> The idea about a public facing Gating CI for QEMU was summarized in an
> RFC[1].  Since then, it was decided that a simpler version should be
> attempted first.
>
> At this point, there are two specific runners (an aarch64 and an s390)
> registered with GitLab, at https://gitlab.com/qemu-project, currently
> setup to the "qemu" repository.

Could I have a simple "this is the command you need to run to
test your staging branch with the project gitlab" set of
getting-started instructions, please?

thanks
-- PMM

Re: [PATCH v2 0/2] QEMU Gating CI

[Cleber Rosa]

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

On Mon, Jul 20, 2020 at 05:18:54PM +0100, Peter Maydell wrote:
> On Thu, 9 Jul 2020 at 03:47, Cleber Rosa <crosa@redhat.com> wrote:
> >
> > The idea about a public facing Gating CI for QEMU was summarized in an
> > RFC[1].  Since then, it was decided that a simpler version should be
> > attempted first.
> >
> > At this point, there are two specific runners (an aarch64 and an s390)
> > registered with GitLab, at https://gitlab.com/qemu-project, currently
> > setup to the "qemu" repository.
> 
> Could I have a simple "this is the command you need to run to
> test your staging branch with the project gitlab" set of
> getting-started instructions, please?
> 
> thanks
> -- PMM
> 

Hi Peter,

Sure.  It's important that PATCH 2/2 in this series is included in a
branch that you need to push to the "staging" branch on the
https://gitlab.com/qemu-project/qemu repo (it could be just that one
patch).  Then, you can run:

  ./scripts/ci/gitlab-pipeline-status --verbose -w

And that should be it.  You can drop '--verbose' if you just want the
final outcome as the result.

A longer explanation: that command defaults to checking for the
pipeline associated with the commit for the *local* staging branch.
You can check the pipeline results on other commits, such as the
latest master (by the time I write this):

  $ ./scripts/ci/gitlab-pipeline-status --verbose -c 1e6c50ad8559c18b21041ef69d8fff781a8db0bb
  success

Please let me if anything goes wrong, as I wasn't able to submit jobs
to those two machines (did so on emulated ones) and the devil lies
in the details.

Thanks,
- Cleber.

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

Re: [PATCH v2 0/2] QEMU Gating CI

[Peter Maydell]

On Mon, 20 Jul 2020 at 18:22, Cleber Rosa <crosa@redhat.com> wrote:
> Sure.  It's important that PATCH 2/2 in this series is included in a
> branch that you need to push to the "staging" branch on the
> https://gitlab.com/qemu-project/qemu repo (it could be just that one
> patch).  Then, you can run:
>
>   ./scripts/ci/gitlab-pipeline-status --verbose -w
>
> And that should be it.  You can drop '--verbose' if you just want the
> final outcome as the result.

I tried this (local branch named "staging", pushed to gitlab
remote "staging" branch), but it said:

e104462:bionic:qemu$ ./scripts/ci/gitlab-pipeline-status --verbose -w
ERROR: No pipeline found
failure

It does seem to have kicked off the pipeline on gitlab though:
https://gitlab.com/qemu-project/qemu/-/pipelines/171671136/builds
OTOH I can't see anything on that web page that suggests that
it's submitting jobs to the s390 or aarch64 boxes -- is it
intended to?

thanks
-- PMM

Re: [PATCH v2 0/2] QEMU Gating CI

[Daniel P. Berrangé]

On Tue, Jul 28, 2020 at 03:48:38PM +0100, Peter Maydell wrote:
> On Mon, 20 Jul 2020 at 18:22, Cleber Rosa <crosa@redhat.com> wrote:
> > Sure.  It's important that PATCH 2/2 in this series is included in a
> > branch that you need to push to the "staging" branch on the
> > https://gitlab.com/qemu-project/qemu repo (it could be just that one
> > patch).  Then, you can run:
> >
> >   ./scripts/ci/gitlab-pipeline-status --verbose -w
> >
> > And that should be it.  You can drop '--verbose' if you just want the
> > final outcome as the result.
> 
> I tried this (local branch named "staging", pushed to gitlab
> remote "staging" branch), but it said:
> 
> e104462:bionic:qemu$ ./scripts/ci/gitlab-pipeline-status --verbose -w
> ERROR: No pipeline found
> failure
> 
> It does seem to have kicked off the pipeline on gitlab though:
> https://gitlab.com/qemu-project/qemu/-/pipelines/171671136/builds
> OTOH I can't see anything on that web page that suggests that
> it's submitting jobs to the s390 or aarch64 boxes -- is it
> intended to?

It looks like those jobs are all in the "test" stage of the pipeline, so
it is waiting for the earlier stages to complete before running the jobs.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 0/2] QEMU Gating CI

[Cleber Rosa]

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

On Tue, Jul 28, 2020 at 03:48:38PM +0100, Peter Maydell wrote:
> On Mon, 20 Jul 2020 at 18:22, Cleber Rosa <crosa@redhat.com> wrote:
> > Sure.  It's important that PATCH 2/2 in this series is included in a
> > branch that you need to push to the "staging" branch on the
> > https://gitlab.com/qemu-project/qemu repo (it could be just that one
> > patch).  Then, you can run:
> >
> >   ./scripts/ci/gitlab-pipeline-status --verbose -w
> >
> > And that should be it.  You can drop '--verbose' if you just want the
> > final outcome as the result.
> 
> I tried this (local branch named "staging", pushed to gitlab
> remote "staging" branch), but it said:
> 
> e104462:bionic:qemu$ ./scripts/ci/gitlab-pipeline-status --verbose -w
> ERROR: No pipeline found
> failure
>

Hi Peter,

I think this may just have been a timing issue.  GitLab usually does
take a few seconds after it receives a branch push to create a
pipeline.  Let me know if you'd like to see this within the script, or
if you'd rather put a sleep between your push and the
"gitlab-pipeline-status" execution.

> It does seem to have kicked off the pipeline on gitlab though:
> https://gitlab.com/qemu-project/qemu/-/pipelines/171671136/builds

There's already new content on the staging branch, but supposing my local
staging branch contained commit 6e7c2dcb50907aa6be0cbc37f81801d2fa67f7b4
(https://gitlab.com/qemu-project/qemu/-/commit/6e7c2dcb50907aa6be0cbc37f81801d2fa67f7b4),
the command you ran:

  ./scripts/ci/gitlab-pipeline-status --verbose -w

Should have behaved as this (output from my machine):

  /scripts/ci/gitlab-pipeline-status --verbose -w -c 6e7c2dcb50907aa6be0cbc37f81801d2fa67f7b4
  running...

> OTOH I can't see anything on that web page that suggests that
> it's submitting jobs to the s390 or aarch64 boxes -- is it
> intended to?
>

All the jobs for that pipeline have been created as expected, for
instance:

   https://gitlab.com/qemu-project/qemu/-/jobs/659874849

But given the recent changes to the GitLab YAML adding other phases,
it's waiting for the previous phases.

The jobs introduced with this patch are in the test phase because they
do builds and tests.  But IIRC there's a way to tell jobs to avoid
being blocked by previous phases.  I'll look into that.

Until then, I hope to see the job results soon from the s390 and aarch64
boxes.

Thanks,
- Cleber.

> thanks
> -- PMM
> 

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

Re: [PATCH v2 0/2] QEMU Gating CI

[Peter Maydell]

On Tue, 28 Jul 2020 at 16:51, Cleber Rosa <crosa@redhat.com> wrote:
>
> On Tue, Jul 28, 2020 at 03:48:38PM +0100, Peter Maydell wrote:
> > On Mon, 20 Jul 2020 at 18:22, Cleber Rosa <crosa@redhat.com> wrote:
> > > Sure.  It's important that PATCH 2/2 in this series is included in a
> > > branch that you need to push to the "staging" branch on the
> > > https://gitlab.com/qemu-project/qemu repo (it could be just that one
> > > patch).  Then, you can run:
> > >
> > >   ./scripts/ci/gitlab-pipeline-status --verbose -w
> > >
> > > And that should be it.  You can drop '--verbose' if you just want the
> > > final outcome as the result.
> >
> > I tried this (local branch named "staging", pushed to gitlab
> > remote "staging" branch), but it said:
> >
> > e104462:bionic:qemu$ ./scripts/ci/gitlab-pipeline-status --verbose -w
> > ERROR: No pipeline found
> > failure
> >
>
> Hi Peter,
>
> I think this may just have been a timing issue.  GitLab usually does
> take a few seconds after it receives a branch push to create a
> pipeline.  Let me know if you'd like to see this within the script, or
> if you'd rather put a sleep between your push and the
> "gitlab-pipeline-status" execution.

Ah, right. I ran the command again and it does (eventually)
print "running...". I think the ideal behaviour would be for
the script to have some kind of "waiting for pipeline to start..."
phase where it sits and polls for the pipeline to appear,
with a pretty long timeout (minutes?).

> > It does seem to have kicked off the pipeline on gitlab though:
> > https://gitlab.com/qemu-project/qemu/-/pipelines/171671136/builds
>
> There's already new content on the staging branch, but supposing my local
> staging branch contained commit 6e7c2dcb50907aa6be0cbc37f81801d2fa67f7b4
> (https://gitlab.com/qemu-project/qemu/-/commit/6e7c2dcb50907aa6be0cbc37f81801d2fa67f7b4),
> the command you ran:
>
>   ./scripts/ci/gitlab-pipeline-status --verbose -w
>
> Should have behaved as this (output from my machine):
>
>   /scripts/ci/gitlab-pipeline-status --verbose -w -c 6e7c2dcb50907aa6be0cbc37f81801d2fa67f7b4
>   running...
>
> > OTOH I can't see anything on that web page that suggests that
> > it's submitting jobs to the s390 or aarch64 boxes -- is it
> > intended to?
> >
>
> All the jobs for that pipeline have been created as expected, for
> instance:
>
>    https://gitlab.com/qemu-project/qemu/-/jobs/659874849
>
> But given the recent changes to the GitLab YAML adding other phases,
> it's waiting for the previous phases.

The page now says "This job has been skipped"...

thanks
-- PMM

Re: [PATCH v2 0/2] QEMU Gating CI

[Cleber Rosa]

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

On Tue, Jul 28, 2020 at 03:51:34PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 28, 2020 at 03:48:38PM +0100, Peter Maydell wrote:
> > On Mon, 20 Jul 2020 at 18:22, Cleber Rosa <crosa@redhat.com> wrote:
> > > Sure.  It's important that PATCH 2/2 in this series is included in a
> > > branch that you need to push to the "staging" branch on the
> > > https://gitlab.com/qemu-project/qemu repo (it could be just that one
> > > patch).  Then, you can run:
> > >
> > >   ./scripts/ci/gitlab-pipeline-status --verbose -w
> > >
> > > And that should be it.  You can drop '--verbose' if you just want the
> > > final outcome as the result.
> > 
> > I tried this (local branch named "staging", pushed to gitlab
> > remote "staging" branch), but it said:
> > 
> > e104462:bionic:qemu$ ./scripts/ci/gitlab-pipeline-status --verbose -w
> > ERROR: No pipeline found
> > failure
> > 
> > It does seem to have kicked off the pipeline on gitlab though:
> > https://gitlab.com/qemu-project/qemu/-/pipelines/171671136/builds
> > OTOH I can't see anything on that web page that suggests that
> > it's submitting jobs to the s390 or aarch64 boxes -- is it
> > intended to?
> 
> It looks like those jobs are all in the "test" stage of the pipeline, so
> it is waiting for the earlier stages to complete before running the jobs.
>

Hi Daniel,

Right.  IIUC the criteria for putting jobs in the test stage at the
moment is "if they include running tests (in addition to builds) they
should be in test".  Looking at that from this perspective, they're in
the right place.

But, these jobs don't depend on anything else, including container
builds, so there's no reason to have them wait for so long to run.
The solution may be to rename the first stage (containers) to something
more generic (and unfortunately less descriptive) so that all of them
will run concurrently and earlier.

Thoughts?
- Cleber.

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [PATCH v2 0/2] QEMU Gating CI

[Daniel P. Berrangé]

On Tue, Jul 28, 2020 at 12:13:06PM -0400, Cleber Rosa wrote:
> On Tue, Jul 28, 2020 at 03:51:34PM +0100, Daniel P. Berrangé wrote:
> > On Tue, Jul 28, 2020 at 03:48:38PM +0100, Peter Maydell wrote:
> > > On Mon, 20 Jul 2020 at 18:22, Cleber Rosa <crosa@redhat.com> wrote:
> > > > Sure.  It's important that PATCH 2/2 in this series is included in a
> > > > branch that you need to push to the "staging" branch on the
> > > > https://gitlab.com/qemu-project/qemu repo (it could be just that one
> > > > patch).  Then, you can run:
> > > >
> > > >   ./scripts/ci/gitlab-pipeline-status --verbose -w
> > > >
> > > > And that should be it.  You can drop '--verbose' if you just want the
> > > > final outcome as the result.
> > > 
> > > I tried this (local branch named "staging", pushed to gitlab
> > > remote "staging" branch), but it said:
> > > 
> > > e104462:bionic:qemu$ ./scripts/ci/gitlab-pipeline-status --verbose -w
> > > ERROR: No pipeline found
> > > failure
> > > 
> > > It does seem to have kicked off the pipeline on gitlab though:
> > > https://gitlab.com/qemu-project/qemu/-/pipelines/171671136/builds
> > > OTOH I can't see anything on that web page that suggests that
> > > it's submitting jobs to the s390 or aarch64 boxes -- is it
> > > intended to?
> > 
> > It looks like those jobs are all in the "test" stage of the pipeline, so
> > it is waiting for the earlier stages to complete before running the jobs.
> >
> 
> Hi Daniel,
> 
> Right.  IIUC the criteria for putting jobs in the test stage at the
> moment is "if they include running tests (in addition to builds) they
> should be in test".  Looking at that from this perspective, they're in
> the right place.
> 
> But, these jobs don't depend on anything else, including container
> builds, so there's no reason to have them wait for so long to run.
> The solution may be to rename the first stage (containers) to something
> more generic (and unfortunately less descriptive) so that all of them
> will run concurrently and earlier.

Just add 'needs: []'  to any jobs that don't depend on earlier jobs.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 0/2] QEMU Gating CI

[Cleber Rosa]

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

On Tue, Jul 28, 2020 at 05:15:17PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 28, 2020 at 12:13:06PM -0400, Cleber Rosa wrote:
> > On Tue, Jul 28, 2020 at 03:51:34PM +0100, Daniel P. Berrangé wrote:
> > > On Tue, Jul 28, 2020 at 03:48:38PM +0100, Peter Maydell wrote:
> > > > On Mon, 20 Jul 2020 at 18:22, Cleber Rosa <crosa@redhat.com> wrote:
> > > > > Sure.  It's important that PATCH 2/2 in this series is included in a
> > > > > branch that you need to push to the "staging" branch on the
> > > > > https://gitlab.com/qemu-project/qemu repo (it could be just that one
> > > > > patch).  Then, you can run:
> > > > >
> > > > >   ./scripts/ci/gitlab-pipeline-status --verbose -w
> > > > >
> > > > > And that should be it.  You can drop '--verbose' if you just want the
> > > > > final outcome as the result.
> > > > 
> > > > I tried this (local branch named "staging", pushed to gitlab
> > > > remote "staging" branch), but it said:
> > > > 
> > > > e104462:bionic:qemu$ ./scripts/ci/gitlab-pipeline-status --verbose -w
> > > > ERROR: No pipeline found
> > > > failure
> > > > 
> > > > It does seem to have kicked off the pipeline on gitlab though:
> > > > https://gitlab.com/qemu-project/qemu/-/pipelines/171671136/builds
> > > > OTOH I can't see anything on that web page that suggests that
> > > > it's submitting jobs to the s390 or aarch64 boxes -- is it
> > > > intended to?
> > > 
> > > It looks like those jobs are all in the "test" stage of the pipeline, so
> > > it is waiting for the earlier stages to complete before running the jobs.
> > >
> > 
> > Hi Daniel,
> > 
> > Right.  IIUC the criteria for putting jobs in the test stage at the
> > moment is "if they include running tests (in addition to builds) they
> > should be in test".  Looking at that from this perspective, they're in
> > the right place.
> > 
> > But, these jobs don't depend on anything else, including container
> > builds, so there's no reason to have them wait for so long to run.
> > The solution may be to rename the first stage (containers) to something
> > more generic (and unfortunately less descriptive) so that all of them
> > will run concurrently and earlier.
> 
> Just add 'needs: []'  to any jobs that don't depend on earlier jobs.
>

Great, thanks!

Long version: I remember something like this was possible (which I
mentioned in the other reply), but the documentation about stages
(https://docs.gitlab.com/ee/ci/yaml/#stages) made me loose hope when
writing this reply.  Thanks again!

- Cleber.

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [PATCH v2 0/2] QEMU Gating CI

[Cleber Rosa]

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

On Tue, Jul 28, 2020 at 05:08:50PM +0100, Peter Maydell wrote:
> On Tue, 28 Jul 2020 at 16:51, Cleber Rosa <crosa@redhat.com> wrote:
> >
> > On Tue, Jul 28, 2020 at 03:48:38PM +0100, Peter Maydell wrote:
> > > On Mon, 20 Jul 2020 at 18:22, Cleber Rosa <crosa@redhat.com> wrote:
> > > > Sure.  It's important that PATCH 2/2 in this series is included in a
> > > > branch that you need to push to the "staging" branch on the
> > > > https://gitlab.com/qemu-project/qemu repo (it could be just that one
> > > > patch).  Then, you can run:
> > > >
> > > >   ./scripts/ci/gitlab-pipeline-status --verbose -w
> > > >
> > > > And that should be it.  You can drop '--verbose' if you just want the
> > > > final outcome as the result.
> > >
> > > I tried this (local branch named "staging", pushed to gitlab
> > > remote "staging" branch), but it said:
> > >
> > > e104462:bionic:qemu$ ./scripts/ci/gitlab-pipeline-status --verbose -w
> > > ERROR: No pipeline found
> > > failure
> > >
> >
> > Hi Peter,
> >
> > I think this may just have been a timing issue.  GitLab usually does
> > take a few seconds after it receives a branch push to create a
> > pipeline.  Let me know if you'd like to see this within the script, or
> > if you'd rather put a sleep between your push and the
> > "gitlab-pipeline-status" execution.
> 
> Ah, right. I ran the command again and it does (eventually)
> print "running...". I think the ideal behaviour would be for
> the script to have some kind of "waiting for pipeline to start..."
> phase where it sits and polls for the pipeline to appear,
> with a pretty long timeout (minutes?).
>

Fair enough.  I'll send a patch to change the script behavior.

> > > It does seem to have kicked off the pipeline on gitlab though:
> > > https://gitlab.com/qemu-project/qemu/-/pipelines/171671136/builds
> >
> > There's already new content on the staging branch, but supposing my local
> > staging branch contained commit 6e7c2dcb50907aa6be0cbc37f81801d2fa67f7b4
> > (https://gitlab.com/qemu-project/qemu/-/commit/6e7c2dcb50907aa6be0cbc37f81801d2fa67f7b4),
> > the command you ran:
> >
> >   ./scripts/ci/gitlab-pipeline-status --verbose -w
> >
> > Should have behaved as this (output from my machine):
> >
> >   /scripts/ci/gitlab-pipeline-status --verbose -w -c 6e7c2dcb50907aa6be0cbc37f81801d2fa67f7b4
> >   running...
> >
> > > OTOH I can't see anything on that web page that suggests that
> > > it's submitting jobs to the s390 or aarch64 boxes -- is it
> > > intended to?
> > >
> >
> > All the jobs for that pipeline have been created as expected, for
> > instance:
> >
> >    https://gitlab.com/qemu-project/qemu/-/jobs/659874849
> >
> > But given the recent changes to the GitLab YAML adding other phases,
> > it's waiting for the previous phases.
> 
> The page now says "This job has been skipped"...
>

I saw that, and I was very disappointed... I double checked the
machines, the runners status, tag names and they all seem to be OK.

So, I think the reason for the skip (there's an open issue on GitLab
itself about not communicating to users the reason) is that GitLab
does a late evaluation of the job condition.  For those jobs the
condition is:

   rules:
   - if: '$CI_COMMIT_REF_NAME == "staging"'

Which by the time the job was evaluated it was no longer true (there
was new content on the staging branch).  There are multiple ways to
solve the problem, including (and in my order of preference):

 1. using '$CI_COMMIT_BRANCH' instead of '$CI_COMMIT_REF_NAME', given
    that the pushed branch name should be kept stable even if the content
    (thus reference name) changes

 2. not changing anything if you believe that under normal
    circunstances one pipeline for the staging will be running at a
    time.

I'll prepare a new version with #1, unless you have a strong feeling
against it.

- Cleber.

> thanks
> -- PMM
> 

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

Re: [PATCH v2 0/2] QEMU Gating CI

[Philippe Mathieu-Daudé]

On 7/28/20 6:33 PM, Cleber Rosa wrote:
> On Tue, Jul 28, 2020 at 05:08:50PM +0100, Peter Maydell wrote:
>> On Tue, 28 Jul 2020 at 16:51, Cleber Rosa <crosa@redhat.com> wrote:
>>>
...
>>>> OTOH I can't see anything on that web page that suggests that
>>>> it's submitting jobs to the s390 or aarch64 boxes -- is it
>>>> intended to?
>>>>
>>>
>>> All the jobs for that pipeline have been created as expected, for
>>> instance:
>>>
>>>    https://gitlab.com/qemu-project/qemu/-/jobs/659874849
>>>
>>> But given the recent changes to the GitLab YAML adding other phases,
>>> it's waiting for the previous phases.
>>
>> The page now says "This job has been skipped"...
>>
> 
> I saw that, and I was very disappointed... I double checked the
> machines, the runners status, tag names and they all seem to be OK.
> 
> So, I think the reason for the skip (there's an open issue on GitLab
> itself about not communicating to users the reason) is that GitLab
> does a late evaluation of the job condition.  For those jobs the
> condition is:
> 
>    rules:
>    - if: '$CI_COMMIT_REF_NAME == "staging"'
> 
> Which by the time the job was evaluated it was no longer true (there
> was new content on the staging branch).  There are multiple ways to
> solve the problem, including (and in my order of preference):
> 
>  1. using '$CI_COMMIT_BRANCH' instead of '$CI_COMMIT_REF_NAME', given
>     that the pushed branch name should be kept stable even if the content
>     (thus reference name) changes
> 
>  2. not changing anything if you believe that under normal
>     circunstances one pipeline for the staging will be running at a
>     time.

For mainstream, usually one at a time is enough, because if you tests
various and one is merged, then you need to rerun on top of the merged
one, so it is not very useful.

For other users, it is useful. I'd certainly finish a patch, run the
script, switch branch in another console, do some other work, run
another instance of the script concurrently with the 1st one, etc...

> 
> I'll prepare a new version with #1, unless you have a strong feeling
> against it.
> 
> - Cleber.
> 
>> thanks
>> -- PMM
>>

Re: [PATCH v2 0/2] QEMU Gating CI

[Peter Maydell]

On Tue, 28 Jul 2020 at 17:34, Cleber Rosa <crosa@redhat.com> wrote:
> So, I think the reason for the skip (there's an open issue on GitLab
> itself about not communicating to users the reason) is that GitLab
> does a late evaluation of the job condition.  For those jobs the
> condition is:
>
>    rules:
>    - if: '$CI_COMMIT_REF_NAME == "staging"'
>
> Which by the time the job was evaluated it was no longer true (there
> was new content on the staging branch).

What caused the new content on the staging branch? I only
pushed the once (to go to commit 6e7c2dcb50907aa) -- is some
other mechanism updating it? Looking at what it now seems to
point to it looks like it's being mirrored from the git.qemu.org
repo... Either we should turn that off, or tests need to be
pushed to git.qemu.org:staging (which might introduce a delay
if the mirror isn't immediate)...

thanks
-- PMM

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Stefan Hajnoczi]

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

On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:

Awesome, thanks for creating this stuff! Minor suggestions:

> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index c1ff24370b..f8dab788ea 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -1003,3 +1003,150 @@ exercise as many corner cases as possible. It is a useful test suite
>  to run to exercise QEMU's linux-user code::
>  
>    https://linux-test-project.github.io/
> +
> +CI
> +==
> +
> +QEMU has configurations enabled for a number of different CI services.
> +The most update information about them and their status can be found
> +at::
> +
> +   https://wiki.qemu.org/Testing/CI
> +
> +Gating CI
> +----------
> +
> +A Pull Requests will only to be merged if they successfully go through
> +a different set of CI jobs.  GitLab's CI is the service/framework used

s/A Pull Requests/Pull Requests/
s/will only to be merged/will only be merged/

I suggest simplifying the first sentence:

  Code is only merged after passing the "gating" set of CI jobs.

Whether they are called Pull Requests or Merge Requests shouldn't matter
:).

> +for executing the gating jobs.
> +
> +The architecture of GitLab's CI service allows different machines to be
> +setup with GitLab's "agent", called gitlab-runner, which will take care

s/setup/set up/ throughout this document
https://grammarist.com/spelling/set-up-vs-setup/

> +of running jobs created by events such as a push to a branch.
> +
> +Even though gitlab-runner can execute jobs on environments such as
> +containers, this initial implementation assumes the shell executor is
> +used, effectively running jobs on the same machine (be them physical

s/them/they/

> +or virtual) the gitlab-runner agent is running.  This means those

s/the/where the/

> +machines must be setup in advance, with the requirements matching the
> +jobs expected to be executed there.
> +
> +Machine configuration for gating jobs
> +-------------------------------------
> +
> +The GitLab's CI architecture allows different parties to provide
> +different machines that will run different jobs.  At this point, QEMU
> +will deploy a limited set of machines and jobs.  Documentation and/or
> +scripts to setup those machines is located under::
> +
> +  scripts/ci/setup
> +
> +Ansible playbooks have been provided to perform two different tasks
> +related to setting gitlab-runner and the build environment.

s/setting/setting up/

> +
> +Other organizations involved in QEMU development may, in the near
> +future, contribute their own setup documentation/scripts under

Comments about relative time lack context in a long-lived document like
this one:
s/in the near future//

> diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
> new file mode 100644
> index 0000000000..89b35386c7
> --- /dev/null
> +++ b/scripts/ci/setup/build-environment.yml
> @@ -0,0 +1,217 @@
> +---
> +- name: Installation of basic packages to build QEMU
> +  hosts: all
> +  vars_files:
> +    - vars.yml
> +  tasks:
> +    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
> +      apt:
> +        update_cache: yes
> +        # This matches the packages on tests/docker/Dockerfiles/ubuntu1804.docker

These comments will not age well :). If you really want to leave a note
then I suggest "Originally from
tests/docker/Dockerfiles/ubuntu1804.docker".

> diff --git a/scripts/ci/setup/inventory b/scripts/ci/setup/inventory
> new file mode 100644
> index 0000000000..8bb7ba6b33
> --- /dev/null
> +++ b/scripts/ci/setup/inventory
> @@ -0,0 +1,2 @@
> +[local]
> +localhost
> diff --git a/scripts/ci/setup/vars.yml b/scripts/ci/setup/vars.yml

Perhaps this file can be called vars.yml.template and an entry for
vars.yml can be added to .gitignore. A file that needs local editing
should not be commited to git in-place. Otherwise it's easy to
accidentally commit the local changes to git (and expose the private
GitLab token!).

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

Re: [PATCH v2 1/2] GitLab Gating CI: introduce pipeline-status contrib script

[Cleber Rosa]

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

On Thu, Jul 09, 2020 at 10:55:19AM +0200, Erik Skultety wrote:
> On Wed, Jul 08, 2020 at 10:46:56PM -0400, Cleber Rosa wrote:
> > This script is intended to be used right after a push to a branch.
> >
> > By default, it will look for the pipeline associated with the commit
> > that is the HEAD of the *local* staging branch.  It can be used as a
> > one time check, or with the `--wait` option to wait until the pipeline
> > completes.
> >
> > If the pipeline is successful, then a merge of the staging branch into
> > the master branch should be the next step.
> >
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  scripts/ci/gitlab-pipeline-status | 156 ++++++++++++++++++++++++++++++
> >  1 file changed, 156 insertions(+)
> >  create mode 100755 scripts/ci/gitlab-pipeline-status
> >
> > diff --git a/scripts/ci/gitlab-pipeline-status b/scripts/ci/gitlab-pipeline-status
> > new file mode 100755
> > index 0000000000..4a9de39872
> > --- /dev/null
> > +++ b/scripts/ci/gitlab-pipeline-status
> > @@ -0,0 +1,156 @@
> > +#!/usr/bin/env python3
> > +#
> > +# Copyright (c) 2019-2020 Red Hat, Inc.
> > +#
> > +# Author:
> > +#  Cleber Rosa <crosa@redhat.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > +# later.  See the COPYING file in the top-level directory.
> > +
> > +"""
> > +Checks the GitLab pipeline status for a given commit commit
> 
> s/commit$/(hash|sha|ID|)
> 

Just for the record, this was picked up by Thomas (thanks Thomas!).

> > +"""
> > +
> > +# pylint: disable=C0103
> > +
> > +import argparse
> > +import http.client
> > +import json
> > +import os
> > +import subprocess
> > +import time
> > +import sys
> > +
> > +
> > +def get_local_staging_branch_commit():
> > +    """
> > +    Returns the commit sha1 for the *local* branch named "staging"
> > +    """
> > +    result = subprocess.run(['git', 'rev-parse', 'staging'],
> 
> If one day Peter decides that "staging" is not a cool name anymore and use a
> different name for the branch :) we should account for that and make it a
> variable, possibly even parametrize this function with it.
>

This function is currently only called to set a default for the
-c/--commit command line option, so users can always set it to the
commit ID of any branch.  But, your point still holds with regards to
future extensibility.  I'll send a patch with that change.

> > +                            stdin=subprocess.DEVNULL,
> > +                            stdout=subprocess.PIPE,
> > +                            stderr=subprocess.DEVNULL,
> > +                            cwd=os.path.dirname(__file__),
> > +                            universal_newlines=True).stdout.strip()
> > +    if result == 'staging':
> > +        raise ValueError("There's no local staging branch")
> 
> "There's no local branch named 'staging'" would IMO be more descriptive, so as
> not to confuse it with staging in git.
>

Ack, also picked up by Thomas.

> > +    if len(result) != 40:
> > +        raise ValueError("Branch staging HEAD doesn't look like a sha1")
> > +    return result
> > +
> > +
> > +def get_pipeline_status(project_id, commit_sha1):
> > +    """
> > +    Returns the JSON content of the pipeline status API response
> > +    """
> > +    url = '/api/v4/projects/{}/pipelines?sha={}'.format(project_id,
> > +                                                        commit_sha1)
> > +    connection = http.client.HTTPSConnection('gitlab.com')
> > +    connection.request('GET', url=url)
> > +    response = connection.getresponse()
> > +    if response.code != http.HTTPStatus.OK:
> > +        raise ValueError("Failed to receive a successful response")
> > +    json_response = json.loads(response.read())
> 
> a blank line separating the commentary block would slightly help readability
>

It would also be a good idea to follow PEP 257, but since there's currently
no check/enforcement, I'll defer to one it's introduced (hopefully soon).

> > +    # afaict, there should one one pipeline for the same project + commit
> 
> s/one one/be only one/
>

Ack, also picked up by Thomas (thanks again!).

> > +    # if this assumption is false, we can add further filters to the
> > +    # url, such as username, and order_by.
> > +    if not json_response:
> > +        raise ValueError("No pipeline found")
> > +    return json_response[0]
> > +
> > +
> > +def wait_on_pipeline_success(timeout, interval,
> > +                             project_id, commit_sha):
> > +    """
> > +    Waits for the pipeline to end up to the timeout given
> 
> "Waits for the pipeline to finish within the given timeout"
> 

Absolutely better, and also picked up by Thomas.

> > +    """
> > +    start = time.time()
> > +    while True:
> > +        if time.time() >= (start + timeout):
> > +            print("Waiting on the pipeline success timed out")
> 
> s/success//
> (the pipeline doesn't always have to finish with success)
>

You're right, your suggestion improves the message, indeed.

But, I think the wording is still confusing as it took me some time to
understand that this timeout was about how long this script will wait.
(my fault here).  So I'm going to propose that this changes to:

"Timeout (-t/--timeout) of %i seconds reached, won't wait any longer
for the pipeline to complete".

> > +            return False
> > +
> > +        status = get_pipeline_status(project_id, commit_sha)
> > +        if status['status'] == 'running':
> > +            time.sleep(interval)
> > +            print('running...')
> > +            continue
> > +
> > +        if status['status'] == 'success':
> > +            return True
> > +
> > +        msg = "Pipeline ended unsuccessfully, check: %s" % status['web_url']
> 
> I think more common expression is "Pipeline failed"
>

Agreed, and already addressed by Thomas (will I run out of thanks?).

> > +        print(msg)
> > +        return False
> > +
> ...
> 
> Code-wise looks OK to me, but since I don't know what Peter's requirements
> on/expectations of this script are, I can't do a more thorough review.
> 
> Regards,
> Erik

Thanks Erik!

- Cleber.

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

Re: [PATCH v2 1/2] GitLab Gating CI: introduce pipeline-status contrib script

[Cleber Rosa]

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

On Thu, Jul 09, 2020 at 12:13:11PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/9/20 10:55 AM, Erik Skultety wrote:
> > On Wed, Jul 08, 2020 at 10:46:56PM -0400, Cleber Rosa wrote:
> >> This script is intended to be used right after a push to a branch.
> >>
> >> By default, it will look for the pipeline associated with the commit
> >> that is the HEAD of the *local* staging branch.  It can be used as a
> >> one time check, or with the `--wait` option to wait until the pipeline
> >> completes.
> >>
> >> If the pipeline is successful, then a merge of the staging branch into
> >> the master branch should be the next step.
> >>
> >> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> >> ---
> >>  scripts/ci/gitlab-pipeline-status | 156 ++++++++++++++++++++++++++++++
> >>  1 file changed, 156 insertions(+)
> >>  create mode 100755 scripts/ci/gitlab-pipeline-status
> >>
> >> diff --git a/scripts/ci/gitlab-pipeline-status b/scripts/ci/gitlab-pipeline-status
> >> new file mode 100755
> >> index 0000000000..4a9de39872
> >> --- /dev/null
> >> +++ b/scripts/ci/gitlab-pipeline-status
> >> @@ -0,0 +1,156 @@
> >> +#!/usr/bin/env python3
> >> +#
> >> +# Copyright (c) 2019-2020 Red Hat, Inc.
> >> +#
> >> +# Author:
> >> +#  Cleber Rosa <crosa@redhat.com>
> >> +#
> >> +# This work is licensed under the terms of the GNU GPL, version 2 or
> >> +# later.  See the COPYING file in the top-level directory.
> >> +
> >> +"""
> >> +Checks the GitLab pipeline status for a given commit commit
> > 
> > s/commit$/(hash|sha|ID|)
> > 
> >> +"""
> >> +
> >> +# pylint: disable=C0103
> >> +
> >> +import argparse
> >> +import http.client
> >> +import json
> >> +import os
> >> +import subprocess
> >> +import time
> >> +import sys
> >> +
> >> +
> >> +def get_local_staging_branch_commit():
> >> +    """
> >> +    Returns the commit sha1 for the *local* branch named "staging"
> >> +    """
> >> +    result = subprocess.run(['git', 'rev-parse', 'staging'],
> > 
> > If one day Peter decides that "staging" is not a cool name anymore and use a
> > different name for the branch :) we should account for that and make it a
> > variable, possibly even parametrize this function with it.
> 
> This script can be used by any fork, not only Peter.
> So having a parameter (default to 'staging') is a requisite IMO.
>

Right, as explained in the reply to Erik, this is just used for
finding the commit ID for the staging branch.  Still, I'm making it
configurable in a new patch, and if people want, we can change the
current behavior to accept any kind of revision (but this would
probably mean changing the options or names, given that -c/--commit is
quite descriptive).

> >> +                            stdin=subprocess.DEVNULL,
> >> +                            stdout=subprocess.PIPE,
> >> +                            stderr=subprocess.DEVNULL,
> >> +                            cwd=os.path.dirname(__file__),
> >> +                            universal_newlines=True).stdout.strip()
> >> +    if result == 'staging':
> >> +        raise ValueError("There's no local staging branch")
> > 
> > "There's no local branch named 'staging'" would IMO be more descriptive, so as
> > not to confuse it with staging in git.
> > 
> >> +    if len(result) != 40:
> >> +        raise ValueError("Branch staging HEAD doesn't look like a sha1")
> >> +    return result
> >> +
> >> +
> >> +def get_pipeline_status(project_id, commit_sha1):
> >> +    """
> >> +    Returns the JSON content of the pipeline status API response
> >> +    """
> >> +    url = '/api/v4/projects/{}/pipelines?sha={}'.format(project_id,
> >> +                                                        commit_sha1)
> >> +    connection = http.client.HTTPSConnection('gitlab.com')
> >> +    connection.request('GET', url=url)
> >> +    response = connection.getresponse()
> >> +    if response.code != http.HTTPStatus.OK:
> >> +        raise ValueError("Failed to receive a successful response")
> >> +    json_response = json.loads(response.read())
> > 
> > a blank line separating the commentary block would slightly help readability
> > 
> >> +    # afaict, there should one one pipeline for the same project + commit
> > 
> > s/one one/be only one/
> 
> 'afaict' is not a word.
>

Yes, good point.  Thomas has addressed this.

> > 
> >> +    # if this assumption is false, we can add further filters to the
> >> +    # url, such as username, and order_by.
> >> +    if not json_response:
> >> +        raise ValueError("No pipeline found")
> >> +    return json_response[0]
> >> +
> >> +
> >> +def wait_on_pipeline_success(timeout, interval,
> >> +                             project_id, commit_sha):
> >> +    """
> >> +    Waits for the pipeline to end up to the timeout given
> > 
> > "Waits for the pipeline to finish within the given timeout"
> > 
> >> +    """
> >> +    start = time.time()
> >> +    while True:
> >> +        if time.time() >= (start + timeout):
> >> +            print("Waiting on the pipeline success timed out")
> > 
> > s/success//
> > (the pipeline doesn't always have to finish with success)
> > 
> >> +            return False
> >> +
> >> +        status = get_pipeline_status(project_id, commit_sha)
> >> +        if status['status'] == 'running':
> >> +            time.sleep(interval)
> >> +            print('running...')
> 
> If we want to automate the use of this script by a daemon, it would
> be better to use the logging class. Then maybe 'running...' is for
> the DEBUG level, Other print() calls can be updated to WARN/INFO
> levels.
>

Makes sense.  I'll look into using proper logging in a future
improvement series.

Thanks,
- Cleber.

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

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Cleber Rosa]

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

On Thu, Jul 09, 2020 at 10:55:07AM +0200, Erik Skultety wrote:
> On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > This is a mapping of Peter's "remake-merge-builds" and
> > "pull-buildtest" scripts, gone through some updates, adding some build
> > option and removing others.
> >
> > The jobs currently cover the machines that the QEMU project owns, and that
> > are setup and ready to run jobs:
> >
> >  - Ubuntu 18.04 on S390x
> >  - Ubuntu 20.04 on aarch64
> >
> > During the development of this set of jobs, the GitLab CI was tested
> > with many other architectures, including ppc64, s390x and aarch64,
> > along with the other OSs (not included here):
> >
> >  - Fedora 30
> >  - FreeBSD 12.1
> >
> > More information can be found in the documentation itself.
> >
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++
> >  .gitlab-ci.yml                         |   1 +
> >  docs/devel/testing.rst                 | 147 +++++++++++++++++
> >  scripts/ci/setup/build-environment.yml | 217 +++++++++++++++++++++++++
> >  scripts/ci/setup/gitlab-runner.yml     |  72 ++++++++
> >  scripts/ci/setup/inventory             |   2 +
> >  scripts/ci/setup/vars.yml              |  13 ++
> >  7 files changed, 598 insertions(+)
> >  create mode 100644 .gitlab-ci.d/gating.yml
> >  create mode 100644 scripts/ci/setup/build-environment.yml
> >  create mode 100644 scripts/ci/setup/gitlab-runner.yml
> >  create mode 100644 scripts/ci/setup/inventory
> >  create mode 100644 scripts/ci/setup/vars.yml
> >
> > diff --git a/.gitlab-ci.d/gating.yml b/.gitlab-ci.d/gating.yml
> > new file mode 100644
> > index 0000000000..5562df5708
> > --- /dev/null
> > +++ b/.gitlab-ci.d/gating.yml
> > @@ -0,0 +1,146 @@
> > +variables:
> > +  GIT_SUBMODULE_STRATEGY: recursive
> > +
> > +# All ubuntu-18.04 jobs should run successfully in an environment
> > +# setup by the scripts/ci/setup/build-environment.yml task
> > +# "Install basic packages to build QEMU on Ubuntu 18.04/20.04"
> > +ubuntu-18.04-s390x-all-linux-static:
> > + tags:
> > + - ubuntu_18.04
> > + - s390x
> > + rules:
> > + - if: '$CI_COMMIT_REF_NAME == "staging"'
> > + script:
> > + # --disable-libssh is needed because of https://bugs.launchpad.net/qemu/+bug/1838763
> > + # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages
> > + - ./configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh
> > + - make --output-sync -j`nproc`
> > + - make --output-sync -j`nproc` check V=1
> > + - make --output-sync -j`nproc` check-tcg V=1
> 
> I know this patch doesn't introduce a FreeBSD job, but later in the patch it's
> clear you'd want to introduce them at some point, so:
> 'nproc' doesn't exist on FreeBSD, but `getconf _NPROCESSORS_ONLN` does, so you
> may want to use it right from the start.
> 
> ...
>

Sure, thanks for the info.

> > +
> > +CI
> > +==
> > +
> > +QEMU has configurations enabled for a number of different CI services.
> > +The most update information about them and their status can be found
> 
> s/update/up to date/
>

Good catch!

> > +at::
> > +
> > +   https://wiki.qemu.org/Testing/CI
> > +
> > +Gating CI
> > +----------
> > +
> > +A Pull Requests will only to be merged if they successfully go through
> 
> s/A /
> s/to be/be/
> 
> > +a different set of CI jobs.  GitLab's CI is the service/framework used
> 
> s/a different set/different sets/       (I may be wrong with this one)
> 
> ...

I think you're right.  But, to keep it simpler, I'm using:

"Pull Requests will only be merged if they successfully go through
different CI jobs."

> 
> > +
> > +gitlab-runner setup and registration
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +The gitlab-runner agent needs to be installed on each machine that
> > +will run jobs.  The association between a machine and a GitLab project
> > +happens with a registration token.  To find the registration token for
> > +your repository/project, navigate on GitLab's web UI to:
> > +
> > + * Settings (the gears like icon), then
> > + * CI/CD, then
> > + * Runners, and click on the "Expand" button, then
> > + * Under "Set up a specific Runner manually", look for the value under
> > +   "Use the following registration token during setup"
> > +
> > +Edit the ``scripts/ci/setup/vars.yml`` file, setting the
> > +``gitlab_runner_registration_token`` variable to the value obtained
> > +earlier.
> > +
> > +.. note:: gitlab-runner is not available from the standard location
> > +          for all OS and architectures combinations.  For some systems,
> > +          a custom build may be necessary.  Some builds are avaiable
> > +          at https://cleber.fedorapeople.org/gitlab-runner/ and this
> > +          URI may be used as a value on ``vars.yml``
> > +
> > +To run the playbook, execute::
> > +
> > +  cd scripts/ci/setup
> > +  ansible-playbook -i inventory gitlab-runner.yml
> > +
> > +.. note:: there are currently limitations to gitlab-runner itself when
> > +          setting up a service under FreeBSD systems.  You will need to
> > +          perform steps 4 to 10 manually, as described at
> > +          https://docs.gitlab.com/runner/install/freebsd.html
> 
> What kinds of limitations? Is it architecture constrained maybe? I'm asking
> because we have all of the steps covered by an Ansible playbook, so I kinda got
> triggered by the word "manually". Also, the document only mentions 9 steps
> overall.
>

FreeBSD's "service management" (systemd/sys-v like) is not covered by
the GO library[1] used on gitlab-runner.  It's not ideal, and the
second best solution would be to script the equivalent handling within
the playbook, but I remember trying and finding some inconsistencies.
Then, I had to give it up and defer to whenever a FreeBSD job is
actually added.

> Regards,
> Erik

[1] - https://github.com/ayufan/golang-kardianos-service

Thanks for the review, I'll be sending a v3 shortly.
- Cleber.

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

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Cleber Rosa]

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

On Thu, Jul 09, 2020 at 12:07:32PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/9/20 4:46 AM, Cleber Rosa wrote:
> > This is a mapping of Peter's "remake-merge-builds" and
> > "pull-buildtest" scripts, gone through some updates, adding some build
> > option and removing others.
> > 
> > The jobs currently cover the machines that the QEMU project owns, and that
> > are setup and ready to run jobs:
> > 
> >  - Ubuntu 18.04 on S390x
> >  - Ubuntu 20.04 on aarch64
> > 
> > During the development of this set of jobs, the GitLab CI was tested
> > with many other architectures, including ppc64, s390x and aarch64,
> > along with the other OSs (not included here):
> > 
> >  - Fedora 30
> >  - FreeBSD 12.1
> > 
> > More information can be found in the documentation itself.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++
> >  .gitlab-ci.yml                         |   1 +
> >  docs/devel/testing.rst                 | 147 +++++++++++++++++
> 
> Time to consider moving the CI doc in a separate file...
> 
> >  scripts/ci/setup/build-environment.yml | 217 +++++++++++++++++++++++++
> >  scripts/ci/setup/gitlab-runner.yml     |  72 ++++++++
> >  scripts/ci/setup/inventory             |   2 +
> >  scripts/ci/setup/vars.yml              |  13 ++
> 
> Should we name these last two as inventory.template
> and vars.yml.template?
>

I get your point, and actually like your suggestion but I've not seen
this done on any Ansible based projects I've come across.  The
inventory file, for instance, is tends to be this "localhost" version
pretty much everywhere.  The same goes for vars.yml, so I'm inclined
to leave them as is...  but if you have strong feelings against it,
I won't fuss nor fight.

> Maybe you can add them with gitlab-runner.yml and a part of
> the documentation in a first patch,
> 
> Then build-environment.yml and another part of the doc
> in another patch,
> 
> Finally gating.yml and the related missing doc as the
> last patch?
>

Sounds good.  This is indeed a very large patch as it is.

Thanks!
- Cleber.

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

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Cleber Rosa]

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

On Thu, Jul 09, 2020 at 11:30:29AM +0100, Daniel P. Berrangé wrote:
> On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > This is a mapping of Peter's "remake-merge-builds" and
> > "pull-buildtest" scripts, gone through some updates, adding some build
> > option and removing others.
> > 
> > The jobs currently cover the machines that the QEMU project owns, and that
> > are setup and ready to run jobs:
> > 
> >  - Ubuntu 18.04 on S390x
> >  - Ubuntu 20.04 on aarch64
> > 
> > During the development of this set of jobs, the GitLab CI was tested
> > with many other architectures, including ppc64, s390x and aarch64,
> > along with the other OSs (not included here):
> > 
> >  - Fedora 30
> >  - FreeBSD 12.1
> > 
> > More information can be found in the documentation itself.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++
> 
> AFAIK, the jobs in this file just augment what is already defined
> in the main .gitlab-ci.yml. Also since we're providing setup info
> for other people to configure custom runners, these jobs are usable
> for non-gating CI scenarios too.
>

If you mean that they introduced new jobs, you're right.

> IOW, the jobs in this file happen to be usable for gating, but they
> are not the only gating jobs, and can be used for non-gating reasons.
>

Right, I do not doubt these jobs may be useful to other people and on
scenarios other than "before merging a patch series".

> This is a complicated way of saying that gating.yml is not a desirable
> filename, so I'd suggest splitting it in two and having these files
> named based on what their contents is, rather than their use case:
> 
>    .gitlab-ci.d/runners-s390x.yml
>    .gitlab-ci.d/runners-aarch64.yml
> 
> The existing jobs in .gitlab-ci.yml could possibly be moved into
> a .gitlab-ci.d/runners-shared.yml file for consistency.
>

Do you imply that every gitlab CI job should be a gating job?  And
that the same jobs should be used when other people with their own
forks?  I find this problematic because:

* It would trigger pipelines with jobs that, unless every user has the
  same runners configured, would have unfulfilled jobs that don't have
  a matching hardware.

* It dilutes the idea that those jobs are inherently different with
  regards to the management of their infrastructure.

* It destroys the notion of layered testing, for whatever people find
  that worth it, where a faster turnaround could/would be possible
  with fewer jobs for every push, and many more jobs before a merge.

Finally, I find the split by runner architecture you suggested
problematic because different organizations may have jobs for the same
architecture.  I believe that files for different organizations may be
a better organization instead.  Entries in the MAINTAINERS are one
example where the grouping by architecture may not be optimal.

> >  .gitlab-ci.yml                         |   1 +
> >  docs/devel/testing.rst                 | 147 +++++++++++++++++
> >  scripts/ci/setup/build-environment.yml | 217 +++++++++++++++++++++++++
> >  scripts/ci/setup/gitlab-runner.yml     |  72 ++++++++
> >  scripts/ci/setup/inventory             |   2 +
> >  scripts/ci/setup/vars.yml              |  13 ++
> >  7 files changed, 598 insertions(+)
> >  create mode 100644 .gitlab-ci.d/gating.yml
> >  create mode 100644 scripts/ci/setup/build-environment.yml
> >  create mode 100644 scripts/ci/setup/gitlab-runner.yml
> >  create mode 100644 scripts/ci/setup/inventory
> >  create mode 100644 scripts/ci/setup/vars.yml
> > 
> > diff --git a/.gitlab-ci.d/gating.yml b/.gitlab-ci.d/gating.yml
> > new file mode 100644
> > index 0000000000..5562df5708
> > --- /dev/null
> > +++ b/.gitlab-ci.d/gating.yml
> > @@ -0,0 +1,146 @@
> > +variables:
> > +  GIT_SUBMODULE_STRATEGY: recursive
> > +
> > +# All ubuntu-18.04 jobs should run successfully in an environment
> > +# setup by the scripts/ci/setup/build-environment.yml task
> > +# "Install basic packages to build QEMU on Ubuntu 18.04/20.04"
> > +ubuntu-18.04-s390x-all-linux-static:
> > + tags:
> > + - ubuntu_18.04
> > + - s390x
> > + rules:
> > + - if: '$CI_COMMIT_REF_NAME == "staging"'
> 
> I think I'd make it more flexible in particular to allow multiple
> branches. For example I have multiple subsystems and have separate
> branches for each.
> 
> This could be as simple as allowing a regex prefix
> 
>   - if: '$CI_COMMIT_REF_NAME =~ /^staging/'
> 
> 
> 
> 
> > diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
> > new file mode 100644
> > index 0000000000..89b35386c7
> > --- /dev/null
> > +++ b/scripts/ci/setup/build-environment.yml
> > @@ -0,0 +1,217 @@
> > +---
> > +- name: Installation of basic packages to build QEMU
> > +  hosts: all
> > +  vars_files:
> > +    - vars.yml
> > +  tasks:
> > +    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
> > +      apt:
> > +        update_cache: yes
> > +        # This matches the packages on tests/docker/Dockerfiles/ubuntu1804.docker
> 
> I'd be inclined to actually use docker on the custom runners.
> 
> eg. instead of having separate physical machines or VMs for each
> (distro, arch) pair, have a single host distro for the arch. Then
> use docker to provide the build environment against each distro.
> 
> IOW, a RHEL-8 aarch64 host, running docker for ubuntu18.04, fedora30
> etc.
>

I've come across so many caveats and corner cases that having the
lowest common denominator proved to be the smart and sane thing to do.
For instance, building on the example you gave, running a RHEL 8
aarch64 host is a NO GO Today because RHEL 8 doesn't ship with docker
and the gitlab runner needs to be taught to talk to, say, Podman.

gitlab-runner-helper images for different architectures, used to
prepare the docker images before running jobs, also proved to be a
challenge.

Finally, it's going to be very important for some organizations to
run tests outside of container environments.  For instance, Red Hat
needs to run QEMU+KVM tests on bare metal and on VMs, in addition
to containers.

> That way we don't end up duplicating all these packages, and instead
> can use  tests/docker/Dockerfiles/ubuntu1804.docker.  This ensures
> that if a user needs to reproduce a build failure on their own local
> aarch64 machine, they can run docker and get the exact same build
> architecture.
>

Like I explained before, containers-only won't cut it.  So, we need
tooling that is environment agnostic.  So far, ansible playbooks seem
to be a reasonable solution.  But duplicating information bothers me
as much as it seems to bother you, so we need to engage in common
tooling that is capable of generating those container environments,
but not be limited to it.  One example of a tool that seems to be
a good candidate is "Libvirt's" lcitool.

> It also has the benefit that we don't need to worry about how to
> setup gitlab runners for every distro we care about. We only need to
> do gitlab runner for the standard host distro, which spawns a pristine
> throwaway docker env.
> 
> I appreciate this is a big change from what you've done in this patch
> though, so don't consider this comment a blocker for initial merge.
> I think we should do this as the long term strategy though. Essentially
> for Linux builds, everything should always be container based.
>

Thanks for taking the time to give this feedback, and for making me
check again the premises that led to the proposal of this design.

Hopefully, this will be an improvement over the current state of
"pre-merge" testing, and at the same time can be the first step
towards a scenario that has more of the characteristics you pointed
out.

Best!
- Cleber.

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Cleber Rosa]

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

On Thu, Jul 09, 2020 at 01:28:27PM +0200, Andrea Bolognani wrote:
> On Thu, 2020-07-09 at 11:30 +0100, Daniel P. Berrangé wrote:
> > On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > > +- name: Installation of basic packages to build QEMU
> > > +  hosts: all
> > > +  vars_files:
> > > +    - vars.yml
> > > +  tasks:
> > > +    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
> > > +      apt:
> > > +        update_cache: yes
> > > +        # This matches the packages on tests/docker/Dockerfiles/ubuntu1804.docker
> > 
> > I'd be inclined to actually use docker on the custom runners.
> > 
> > eg. instead of having separate physical machines or VMs for each
> > (distro, arch) pair, have a single host distro for the arch. Then
> > use docker to provide the build environment against each distro.
> > 
> > IOW, a RHEL-8 aarch64 host, running docker for ubuntu18.04, fedora30
> > etc.
> > 
> > That way we don't end up duplicating all these packages, and instead
> > can use  tests/docker/Dockerfiles/ubuntu1804.docker.  This ensures
> > that if a user needs to reproduce a build failure on their own local
> > aarch64 machine, they can run docker and get the exact same build
> > architecture.
> > 
> > It also has the benefit that we don't need to worry about how to
> > setup gitlab runners for every distro we care about. We only need to
> > do gitlab runner for the standard host distro, which spawns a pristine
> > throwaway docker env.
> > 
> > I appreciate this is a big change from what you've done in this patch
> > though, so don't consider this comment a blocker for initial merge.
> > I think we should do this as the long term strategy though. Essentially
> > for Linux builds, everything should always be container based.
> 
> Agreed. You should be able to set up a fairly minimal environment,
> which consists of Docker, gitlab-runner and not much else, using a
> long-term supported distro such as CentOS and then just schedule
> whatever container build on it. No need to provision a new machine
> every time a new Fedora release comes out, just create a container
> image for it and add it to the mix.
>

Hi Andrea,

There's nothing preventing this from happening, but limiting the
runners to this configuration, prevents a lot more from happening.

> Additionally, the gitlab-runner Docker executor provides more
> isolation than the shell executor, so running untrusted builds
> becomes a more reasonable proposition - this is how the shared
> runners on gitlab.com work - and you don't have to worry about your
> jobs cleaning up properly after themselves nearly as much.
>

I understand and agree to the the benefits of using the gitlab-runner
Docker executor... until you want to run tests on non-Docker
environments :).

Hopefully the explanation on my previous reply to Daniel will also
serve for the points you raised here.  I would have loved to have
worked on a more abstract, container only environments, but that
proved to be unrealistic.

Cheers,
- Cleber.

> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 

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

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Cleber Rosa]

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

On Wed, Jul 29, 2020 at 11:16:29AM +0100, Stefan Hajnoczi wrote:
> On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> 
> Awesome, thanks for creating this stuff! Minor suggestions:
> 
> > diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> > index c1ff24370b..f8dab788ea 100644
> > --- a/docs/devel/testing.rst
> > +++ b/docs/devel/testing.rst
> > @@ -1003,3 +1003,150 @@ exercise as many corner cases as possible. It is a useful test suite
> >  to run to exercise QEMU's linux-user code::
> >  
> >    https://linux-test-project.github.io/
> > +
> > +CI
> > +==
> > +
> > +QEMU has configurations enabled for a number of different CI services.
> > +The most update information about them and their status can be found
> > +at::
> > +
> > +   https://wiki.qemu.org/Testing/CI
> > +
> > +Gating CI
> > +----------
> > +
> > +A Pull Requests will only to be merged if they successfully go through
> > +a different set of CI jobs.  GitLab's CI is the service/framework used
> 
> s/A Pull Requests/Pull Requests/
> s/will only to be merged/will only be merged/
> 
> I suggest simplifying the first sentence:
> 
>   Code is only merged after passing the "gating" set of CI jobs.
> 
> Whether they are called Pull Requests or Merge Requests shouldn't matter
> :).
>

Yep, makes sense.

> > +for executing the gating jobs.
> > +
> > +The architecture of GitLab's CI service allows different machines to be
> > +setup with GitLab's "agent", called gitlab-runner, which will take care
> 
> s/setup/set up/ throughout this document
> https://grammarist.com/spelling/set-up-vs-setup/
>

Nice catch, thanks.

> > +of running jobs created by events such as a push to a branch.
> > +
> > +Even though gitlab-runner can execute jobs on environments such as
> > +containers, this initial implementation assumes the shell executor is
> > +used, effectively running jobs on the same machine (be them physical
> 
> s/them/they/
> 
> > +or virtual) the gitlab-runner agent is running.  This means those
> 
> s/the/where the/
>

Right, thanks.

> > +machines must be setup in advance, with the requirements matching the
> > +jobs expected to be executed there.
> > +
> > +Machine configuration for gating jobs
> > +-------------------------------------
> > +
> > +The GitLab's CI architecture allows different parties to provide
> > +different machines that will run different jobs.  At this point, QEMU
> > +will deploy a limited set of machines and jobs.  Documentation and/or
> > +scripts to setup those machines is located under::
> > +
> > +  scripts/ci/setup
> > +
> > +Ansible playbooks have been provided to perform two different tasks
> > +related to setting gitlab-runner and the build environment.
> 
> s/setting/setting up/
> 
> > +
> > +Other organizations involved in QEMU development may, in the near
> > +future, contribute their own setup documentation/scripts under
> 
> Comments about relative time lack context in a long-lived document like
> this one:
> s/in the near future//
>

You're right.  That was already bothering but I couldn't tell why.

> > diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml
> > new file mode 100644
> > index 0000000000..89b35386c7
> > --- /dev/null
> > +++ b/scripts/ci/setup/build-environment.yml
> > @@ -0,0 +1,217 @@
> > +---
> > +- name: Installation of basic packages to build QEMU
> > +  hosts: all
> > +  vars_files:
> > +    - vars.yml
> > +  tasks:
> > +    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
> > +      apt:
> > +        update_cache: yes
> > +        # This matches the packages on tests/docker/Dockerfiles/ubuntu1804.docker
> 
> These comments will not age well :). If you really want to leave a note
> then I suggest "Originally from
> tests/docker/Dockerfiles/ubuntu1804.docker".
> 
> > diff --git a/scripts/ci/setup/inventory b/scripts/ci/setup/inventory
> > new file mode 100644
> > index 0000000000..8bb7ba6b33
> > --- /dev/null
> > +++ b/scripts/ci/setup/inventory
> > @@ -0,0 +1,2 @@
> > +[local]
> > +localhost
> > diff --git a/scripts/ci/setup/vars.yml b/scripts/ci/setup/vars.yml
> 
> Perhaps this file can be called vars.yml.template and an entry for
> vars.yml can be added to .gitignore. A file that needs local editing
> should not be commited to git in-place. Otherwise it's easy to
> accidentally commit the local changes to git (and expose the private
> GitLab token!).

Right... Philippe has already suggested this, and you've definitely
increased its significance with the data leak example.  So yes, let's
do this rename.

Thanks for the feedback and suggestions!
- Cleber.

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

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Daniel P. Berrangé]

On Thu, Sep 03, 2020 at 08:11:39PM -0400, Cleber Rosa wrote:
> On Thu, Jul 09, 2020 at 11:30:29AM +0100, Daniel P. Berrangé wrote:
> > On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > > This is a mapping of Peter's "remake-merge-builds" and
> > > "pull-buildtest" scripts, gone through some updates, adding some build
> > > option and removing others.
> > > 
> > > The jobs currently cover the machines that the QEMU project owns, and that
> > > are setup and ready to run jobs:
> > > 
> > >  - Ubuntu 18.04 on S390x
> > >  - Ubuntu 20.04 on aarch64
> > > 
> > > During the development of this set of jobs, the GitLab CI was tested
> > > with many other architectures, including ppc64, s390x and aarch64,
> > > along with the other OSs (not included here):
> > > 
> > >  - Fedora 30
> > >  - FreeBSD 12.1
> > > 
> > > More information can be found in the documentation itself.
> > > 
> > > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > > ---
> > >  .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++
> > 
> > AFAIK, the jobs in this file just augment what is already defined
> > in the main .gitlab-ci.yml. Also since we're providing setup info
> > for other people to configure custom runners, these jobs are usable
> > for non-gating CI scenarios too.
> >
> 
> If you mean that they introduced new jobs, you're right.
> 
> > IOW, the jobs in this file happen to be usable for gating, but they
> > are not the only gating jobs, and can be used for non-gating reasons.
> >
> 
> Right, I do not doubt these jobs may be useful to other people and on
> scenarios other than "before merging a patch series".
> 
> > This is a complicated way of saying that gating.yml is not a desirable
> > filename, so I'd suggest splitting it in two and having these files
> > named based on what their contents is, rather than their use case:
> > 
> >    .gitlab-ci.d/runners-s390x.yml
> >    .gitlab-ci.d/runners-aarch64.yml
> > 
> > The existing jobs in .gitlab-ci.yml could possibly be moved into
> > a .gitlab-ci.d/runners-shared.yml file for consistency.
> >
> 
> Do you imply that every gitlab CI job should be a gating job?  And
> that the same jobs should be used when other people with their own
> forks?  I find this problematic because:
> 
> * It would trigger pipelines with jobs that, unless every user has the
>   same runners configured, would have unfulfilled jobs that don't have
>   a matching hardware.

Jobs that require a custom runner should not be set to run by default,
but individual contributors must absolutely be able to opt-in to running
those jobs simply by registering a runner on their account.

> * It dilutes the idea that those jobs are inherently different with
>   regards to the management of their infrastructure.

I don't really know what yiu mean here, but "Inherantly different"
does not sound like a desirable property.

> * It destroys the notion of layered testing, for whatever people find
>   that worth it, where a faster turnaround could/would be possible
>   with fewer jobs for every push, and many more jobs before a merge.

The key goal of CI is to reduce the burden on maintainers. The biggest
cost is if we merge code and failure is noticed after merge. IT is
still a large cost, however, if Peter only finds a CI failure when he
attempts the pre-merge test. He has to throw out the pull request
putting more work on the subsystem maintainer. The subsystem maintainer
may have to throw it back to the original author.

The ideal scenario that we need to strive towards is that the original
author has tested their code with 100% coverage of all the CI jobs QMEU
has defined.

Any time there is a job that is not run by authors, but only by the
maintainers, we are putting increased burden on the maintainers, so
must be minimize that.

IOW, layered testing is not desirable as goal. Rather layered testing
is just a default setup, but we'd encourage contributors to run the
full set of CI jobs, especially if they are frequent contributors.
The more they run themselves, the less burden on subsystem maintainers
and Peter, and thus the better we all scale.

> Finally, I find the split by runner architecture you suggested
> problematic because different organizations may have jobs for the same
> architecture.  I believe that files for different organizations may be
> a better organization instead.  Entries in the MAINTAINERS are one
> example where the grouping by architecture may not be optimal.

I don't think we should be structuring jobs around organizations. We
should be defining a set of desired jobs we wish to be able to run.
Any organization can bring a runner that is capable of running the
jobs and donate it to the QEMU project for our formal CI runner
The organization is not defining the job though - QEMU is  defining
the jobs we expect to have used for testing.

This is key because any contributor needs to be able to spin up an
identical envrionment to replicate any build failures. We don't want
runners for merge testing that are built as a blackbox by someone.
That is the single biggest painpoint with Peter's current merge
jobs - we can't easily replicate Peter's merge env even if we had
the matching hardware available.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Daniel P. Berrangé]

On Thu, Sep 03, 2020 at 08:18:17PM -0400, Cleber Rosa wrote:
> On Thu, Jul 09, 2020 at 01:28:27PM +0200, Andrea Bolognani wrote:
> > On Thu, 2020-07-09 at 11:30 +0100, Daniel P. Berrangé wrote:
> > > On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > > > +- name: Installation of basic packages to build QEMU
> > > > +  hosts: all
> > > > +  vars_files:
> > > > +    - vars.yml
> > > > +  tasks:
> > > > +    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
> > > > +      apt:
> > > > +        update_cache: yes
> > > > +        # This matches the packages on tests/docker/Dockerfiles/ubuntu1804.docker
> > > 
> > > I'd be inclined to actually use docker on the custom runners.
> > > 
> > > eg. instead of having separate physical machines or VMs for each
> > > (distro, arch) pair, have a single host distro for the arch. Then
> > > use docker to provide the build environment against each distro.
> > > 
> > > IOW, a RHEL-8 aarch64 host, running docker for ubuntu18.04, fedora30
> > > etc.
> > > 
> > > That way we don't end up duplicating all these packages, and instead
> > > can use  tests/docker/Dockerfiles/ubuntu1804.docker.  This ensures
> > > that if a user needs to reproduce a build failure on their own local
> > > aarch64 machine, they can run docker and get the exact same build
> > > architecture.
> > > 
> > > It also has the benefit that we don't need to worry about how to
> > > setup gitlab runners for every distro we care about. We only need to
> > > do gitlab runner for the standard host distro, which spawns a pristine
> > > throwaway docker env.
> > > 
> > > I appreciate this is a big change from what you've done in this patch
> > > though, so don't consider this comment a blocker for initial merge.
> > > I think we should do this as the long term strategy though. Essentially
> > > for Linux builds, everything should always be container based.
> > 
> > Agreed. You should be able to set up a fairly minimal environment,
> > which consists of Docker, gitlab-runner and not much else, using a
> > long-term supported distro such as CentOS and then just schedule
> > whatever container build on it. No need to provision a new machine
> > every time a new Fedora release comes out, just create a container
> > image for it and add it to the mix.
> >
> 
> Hi Andrea,
> 
> There's nothing preventing this from happening, but limiting the
> runners to this configuration, prevents a lot more from happening.
> 
> > Additionally, the gitlab-runner Docker executor provides more
> > isolation than the shell executor, so running untrusted builds
> > becomes a more reasonable proposition - this is how the shared
> > runners on gitlab.com work - and you don't have to worry about your
> > jobs cleaning up properly after themselves nearly as much.
> >
> 
> I understand and agree to the the benefits of using the gitlab-runner
> Docker executor... until you want to run tests on non-Docker
> environments :).
> 
> Hopefully the explanation on my previous reply to Daniel will also
> serve for the points you raised here.  I would have loved to have
> worked on a more abstract, container only environments, but that
> proved to be unrealistic.

For Linux targets, it should be possible to have exclusively container
based testing environments. At worst you can run a privileged container
and expose arbitrary host resources to it, so you can do anything in
the container that you would otherwise do in bare metal. For non-Linux,
we should be able to satisfy our needs with VMs, and indeed VMs can
be used for Linux too if we want to emulate some specific hardware for
testing that we don't have accessible to containers on bare metal.
IOW, the testing environment can be entirely defined by the recipes
we have in tests/docker and tests/vm. Bare metal hosts are simply a
way to host the containers or vms.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Andrea Bolognani]

On Thu, 2020-09-03 at 17:12 -0400, Cleber Rosa wrote:
> On Thu, Jul 09, 2020 at 10:55:07AM +0200, Erik Skultety wrote:
> > On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > > +.. note:: there are currently limitations to gitlab-runner itself when
> > > +          setting up a service under FreeBSD systems.  You will need to
> > > +          perform steps 4 to 10 manually, as described at
> > > +          https://docs.gitlab.com/runner/install/freebsd.html
> > 
> > What kinds of limitations? Is it architecture constrained maybe? I'm asking
> > because we have all of the steps covered by an Ansible playbook, so I kinda got
> > triggered by the word "manually". Also, the document only mentions 9 steps
> > overall.
> 
> FreeBSD's "service management" (systemd/sys-v like) is not covered by
> the GO library[1] used on gitlab-runner.  It's not ideal, and the
> second best solution would be to script the equivalent handling within
> the playbook, but I remember trying and finding some inconsistencies.
> Then, I had to give it up and defer to whenever a FreeBSD job is
> actually added.
> 
> [1] - https://github.com/ayufan/golang-kardianos-service

Note that this is a fork of

  https://github.com/kardianos/service

where FreeBSD support was added recently with

  https://github.com/kardianos/service/commit/14b2cc59a290407a6f1cb3daba59069429d9665b

I'm not sure why gitlab-runner would use a fork rather than the
primary repository, but perhaps they can be convinced to switch and
gain better FreeBSD support in the process.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Philippe Mathieu-Daudé]

On 7/9/20 4:46 AM, Cleber Rosa wrote:
> This is a mapping of Peter's "remake-merge-builds" and
> "pull-buildtest" scripts, gone through some updates, adding some build
> option and removing others.
> 
> The jobs currently cover the machines that the QEMU project owns, and that
> are setup and ready to run jobs:
> 
>  - Ubuntu 18.04 on S390x
>  - Ubuntu 20.04 on aarch64
> 
> During the development of this set of jobs, the GitLab CI was tested
> with many other architectures, including ppc64, s390x and aarch64,
> along with the other OSs (not included here):
> 
>  - Fedora 30
>  - FreeBSD 12.1
> 
> More information can be found in the documentation itself.
[...]
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index c1ff24370b..f8dab788ea 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -1003,3 +1003,150 @@ exercise as many corner cases as possible. It is a useful test suite
>  to run to exercise QEMU's linux-user code::
>  
>    https://linux-test-project.github.io/
> +
> +CI
> +==
> +
> +QEMU has configurations enabled for a number of different CI services.
> +The most update information about them and their status can be found
> +at::
> +
> +   https://wiki.qemu.org/Testing/CI
> +
> +Gating CI
> +----------
> +
> +A Pull Requests will only to be merged if they successfully go through
> +a different set of CI jobs.  GitLab's CI is the service/framework used
> +for executing the gating jobs.
> +
> +The architecture of GitLab's CI service allows different machines to be
> +setup with GitLab's "agent", called gitlab-runner, which will take care
> +of running jobs created by events such as a push to a branch.

Now that GitLab announced the free tier time limit, it may be worth
mentioning (if set up) the CI pipeline can be skipped using:

  $ git push --push-option=ci.skip [...]

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Gerd Hoffmann]

  Hi,

> > IOW, a RHEL-8 aarch64 host, running docker for ubuntu18.04, fedora30
> > etc.
> 
> I've come across so many caveats and corner cases that having the
> lowest common denominator proved to be the smart and sane thing to do.
> For instance, building on the example you gave, running a RHEL 8
> aarch64 host is a NO GO Today because RHEL 8 doesn't ship with docker
> and the gitlab runner needs to be taught to talk to, say, Podman.

podman got support for the docker daemon api (v2.0 I think), so 
this should be possible soon, without changing gitlab runner.

Also there is a kubernetes runner so you can run the thing in pretty
much any cluster.  k3s.io seems to be the easiest way to boot up a small
kubernetes cluster on pretty much any linux system.  Works on aarch64
too.  Didn't try (yet) to run the gitlab runner there.

> Finally, it's going to be very important for some organizations to
> run tests outside of container environments.  For instance, Red Hat
> needs to run QEMU+KVM tests on bare metal and on VMs, in addition
> to containers.

Also for *BSD testing.
For setting up VM images we should use tests/vm.

> Like I explained before, containers-only won't cut it.  So, we need
> tooling that is environment agnostic.  So far, ansible playbooks seem
> to be a reasonable solution.  But duplicating information bothers me
> as much as it seems to bother you, so we need to engage in common
> tooling that is capable of generating those container environments,
> but not be limited to it.  One example of a tool that seems to be
> a good candidate is "Libvirt's" lcitool.

What is wrong with the docker files we have?

It is a good idea anyway to use the same images for local test builds
and CI.  Makes it easier to reproduce CI failures locally.

take care,
  Gerd

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Cleber Rosa]

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

On Fri, Sep 04, 2020 at 11:11:25AM +0200, Andrea Bolognani wrote:
> On Thu, 2020-09-03 at 17:12 -0400, Cleber Rosa wrote:
> > On Thu, Jul 09, 2020 at 10:55:07AM +0200, Erik Skultety wrote:
> > > On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > > > +.. note:: there are currently limitations to gitlab-runner itself when
> > > > +          setting up a service under FreeBSD systems.  You will need to
> > > > +          perform steps 4 to 10 manually, as described at
> > > > +          https://docs.gitlab.com/runner/install/freebsd.html
> > > 
> > > What kinds of limitations? Is it architecture constrained maybe? I'm asking
> > > because we have all of the steps covered by an Ansible playbook, so I kinda got
> > > triggered by the word "manually". Also, the document only mentions 9 steps
> > > overall.
> > 
> > FreeBSD's "service management" (systemd/sys-v like) is not covered by
> > the GO library[1] used on gitlab-runner.  It's not ideal, and the
> > second best solution would be to script the equivalent handling within
> > the playbook, but I remember trying and finding some inconsistencies.
> > Then, I had to give it up and defer to whenever a FreeBSD job is
> > actually added.
> > 
> > [1] - https://github.com/ayufan/golang-kardianos-service
> 
> Note that this is a fork of
> 
>   https://github.com/kardianos/service
> 
> where FreeBSD support was added recently with
> 
>   https://github.com/kardianos/service/commit/14b2cc59a290407a6f1cb3daba59069429d9665b
>

That's good news!

> I'm not sure why gitlab-runner would use a fork rather than the
> primary repository, but perhaps they can be convinced to switch and
> gain better FreeBSD support in the process.
>

I can only imagine they were using the fork because the primary
repository did not have the bits they needed there... and were not
willing or were not successful and getting them there.

We can ask/hope/wait for gitlab to switch, and then this will no
longer be an issue indeed.

Thanks,
- Cleber.

> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 

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

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Cleber Rosa]

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

On Fri, Sep 04, 2020 at 09:23:47AM +0100, Daniel P. Berrangé wrote:
> On Thu, Sep 03, 2020 at 08:18:17PM -0400, Cleber Rosa wrote:
> > On Thu, Jul 09, 2020 at 01:28:27PM +0200, Andrea Bolognani wrote:
> > > On Thu, 2020-07-09 at 11:30 +0100, Daniel P. Berrangé wrote:
> > > > On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > > > > +- name: Installation of basic packages to build QEMU
> > > > > +  hosts: all
> > > > > +  vars_files:
> > > > > +    - vars.yml
> > > > > +  tasks:
> > > > > +    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
> > > > > +      apt:
> > > > > +        update_cache: yes
> > > > > +        # This matches the packages on tests/docker/Dockerfiles/ubuntu1804.docker
> > > > 
> > > > I'd be inclined to actually use docker on the custom runners.
> > > > 
> > > > eg. instead of having separate physical machines or VMs for each
> > > > (distro, arch) pair, have a single host distro for the arch. Then
> > > > use docker to provide the build environment against each distro.
> > > > 
> > > > IOW, a RHEL-8 aarch64 host, running docker for ubuntu18.04, fedora30
> > > > etc.
> > > > 
> > > > That way we don't end up duplicating all these packages, and instead
> > > > can use  tests/docker/Dockerfiles/ubuntu1804.docker.  This ensures
> > > > that if a user needs to reproduce a build failure on their own local
> > > > aarch64 machine, they can run docker and get the exact same build
> > > > architecture.
> > > > 
> > > > It also has the benefit that we don't need to worry about how to
> > > > setup gitlab runners for every distro we care about. We only need to
> > > > do gitlab runner for the standard host distro, which spawns a pristine
> > > > throwaway docker env.
> > > > 
> > > > I appreciate this is a big change from what you've done in this patch
> > > > though, so don't consider this comment a blocker for initial merge.
> > > > I think we should do this as the long term strategy though. Essentially
> > > > for Linux builds, everything should always be container based.
> > > 
> > > Agreed. You should be able to set up a fairly minimal environment,
> > > which consists of Docker, gitlab-runner and not much else, using a
> > > long-term supported distro such as CentOS and then just schedule
> > > whatever container build on it. No need to provision a new machine
> > > every time a new Fedora release comes out, just create a container
> > > image for it and add it to the mix.
> > >
> > 
> > Hi Andrea,
> > 
> > There's nothing preventing this from happening, but limiting the
> > runners to this configuration, prevents a lot more from happening.
> > 
> > > Additionally, the gitlab-runner Docker executor provides more
> > > isolation than the shell executor, so running untrusted builds
> > > becomes a more reasonable proposition - this is how the shared
> > > runners on gitlab.com work - and you don't have to worry about your
> > > jobs cleaning up properly after themselves nearly as much.
> > >
> > 
> > I understand and agree to the the benefits of using the gitlab-runner
> > Docker executor... until you want to run tests on non-Docker
> > environments :).
> > 
> > Hopefully the explanation on my previous reply to Daniel will also
> > serve for the points you raised here.  I would have loved to have
> > worked on a more abstract, container only environments, but that
> > proved to be unrealistic.
> 
> For Linux targets, it should be possible to have exclusively container
> based testing environments. At worst you can run a privileged container
> and expose arbitrary host resources to it, so you can do anything in
> the container that you would otherwise do in bare metal. For non-Linux,
> we should be able to satisfy our needs with VMs, and indeed VMs can
> be used for Linux too if we want to emulate some specific hardware for
> testing that we don't have accessible to containers on bare metal.
> IOW, the testing environment can be entirely defined by the recipes
> we have in tests/docker and tests/vm. Bare metal hosts are simply a
> way to host the containers or vms.
>

I don't think you're following my point.  It's not about what's
possible to be done, but what's the baseline of the test environment
we want to have.

It's unwise to attempt to compare the results of a test that run under
a container with "arbitrary host resources" exposed to it, to the
results of a test running on the host without the container layer.
Think, for instance, if QE would be willing to do the former only, and
sign off on it, when customers are using the later.

I hope this makes the point clearer.
- Cleber.

> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [PATCH v2 2/2] GitLab Gating CI: initial set of jobs, documentation and scripts

[Cleber Rosa]

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

On Fri, Sep 04, 2020 at 09:18:16AM +0100, Daniel P. Berrangé wrote:
> On Thu, Sep 03, 2020 at 08:11:39PM -0400, Cleber Rosa wrote:
> > On Thu, Jul 09, 2020 at 11:30:29AM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Jul 08, 2020 at 10:46:57PM -0400, Cleber Rosa wrote:
> > > > This is a mapping of Peter's "remake-merge-builds" and
> > > > "pull-buildtest" scripts, gone through some updates, adding some build
> > > > option and removing others.
> > > > 
> > > > The jobs currently cover the machines that the QEMU project owns, and that
> > > > are setup and ready to run jobs:
> > > > 
> > > >  - Ubuntu 18.04 on S390x
> > > >  - Ubuntu 20.04 on aarch64
> > > > 
> > > > During the development of this set of jobs, the GitLab CI was tested
> > > > with many other architectures, including ppc64, s390x and aarch64,
> > > > along with the other OSs (not included here):
> > > > 
> > > >  - Fedora 30
> > > >  - FreeBSD 12.1
> > > > 
> > > > More information can be found in the documentation itself.
> > > > 
> > > > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > > > ---
> > > >  .gitlab-ci.d/gating.yml                | 146 +++++++++++++++++
> > > 
> > > AFAIK, the jobs in this file just augment what is already defined
> > > in the main .gitlab-ci.yml. Also since we're providing setup info
> > > for other people to configure custom runners, these jobs are usable
> > > for non-gating CI scenarios too.
> > >
> > 
> > If you mean that they introduced new jobs, you're right.
> > 
> > > IOW, the jobs in this file happen to be usable for gating, but they
> > > are not the only gating jobs, and can be used for non-gating reasons.
> > >
> > 
> > Right, I do not doubt these jobs may be useful to other people and on
> > scenarios other than "before merging a patch series".
> > 
> > > This is a complicated way of saying that gating.yml is not a desirable
> > > filename, so I'd suggest splitting it in two and having these files
> > > named based on what their contents is, rather than their use case:
> > > 
> > >    .gitlab-ci.d/runners-s390x.yml
> > >    .gitlab-ci.d/runners-aarch64.yml
> > > 
> > > The existing jobs in .gitlab-ci.yml could possibly be moved into
> > > a .gitlab-ci.d/runners-shared.yml file for consistency.
> > >
> > 
> > Do you imply that every gitlab CI job should be a gating job?  And
> > that the same jobs should be used when other people with their own
> > forks?  I find this problematic because:
> > 
> > * It would trigger pipelines with jobs that, unless every user has the
> >   same runners configured, would have unfulfilled jobs that don't have
> >   a matching hardware.
> 
> Jobs that require a custom runner should not be set to run by default,
> but individual contributors must absolutely be able to opt-in to running
> those jobs simply by registering a runner on their account.
>

Agreed, and that's why they have been put into this diffent "gating"
class here.

> > * It dilutes the idea that those jobs are inherently different with
> >   regards to the management of their infrastructure.
> 
> I don't really know what yiu mean here, but "Inherantly different"
> does not sound like a desirable property.
>

Organizations and individuals will have responsibility over the
infrastructure they choose to add, which is "inherently different"
from the gitlab shared machines.  Not sure there's a way around it.

> > * It destroys the notion of layered testing, for whatever people find
> >   that worth it, where a faster turnaround could/would be possible
> >   with fewer jobs for every push, and many more jobs before a merge.
> 
> The key goal of CI is to reduce the burden on maintainers. The biggest
> cost is if we merge code and failure is noticed after merge. IT is
> still a large cost, however, if Peter only finds a CI failure when he
> attempts the pre-merge test. He has to throw out the pull request
> putting more work on the subsystem maintainer. The subsystem maintainer
> may have to throw it back to the original author.
> 
> The ideal scenario that we need to strive towards is that the original
> author has tested their code with 100% coverage of all the CI jobs QMEU
> has defined.
>

I agree... but it's also unrealistic at this point, right?  For
instance, do we have s390x boxes to run all of those?  Avocado has
been using Travis CI for s390x/ppc64/aarch64, and those are quite
unreliable even with a load many orders of magnitude smaller then the
QEMU project.  So, resources are needed to have this flat, 100%
coverage, "ideal scenario" you describe.

> Any time there is a job that is not run by authors, but only by the
> maintainers, we are putting increased burden on the maintainers, so
> must be minimize that.
>

I agree.  But if resources are limited, then should the testing scope
be decresead so that it's equalized?

> IOW, layered testing is not desirable as goal. Rather layered testing
> is just a default setup, but we'd encourage contributors to run the
> full set of CI jobs, especially if they are frequent contributors.
> The more they run themselves, the less burden on subsystem maintainers
> and Peter, and thus the better we all scale.
>

We agree on goals, we don't agree on the strategy though.

> > Finally, I find the split by runner architecture you suggested
> > problematic because different organizations may have jobs for the same
> > architecture.  I believe that files for different organizations may be
> > a better organization instead.  Entries in the MAINTAINERS are one
> > example where the grouping by architecture may not be optimal.
> 
> I don't think we should be structuring jobs around organizations. We
> should be defining a set of desired jobs we wish to be able to run.
> Any organization can bring a runner that is capable of running the
> jobs and donate it to the QEMU project for our formal CI runner
> The organization is not defining the job though - QEMU is  defining
> the jobs we expect to have used for testing.
>

This was disscussed previously[1].

> This is key because any contributor needs to be able to spin up an
> identical envrionment to replicate any build failures. We don't want
> runners for merge testing that are built as a blackbox by someone.
> That is the single biggest painpoint with Peter's current merge
> jobs - we can't easily replicate Peter's merge env even if we had
> the matching hardware available.
>

With the right automation, such as the playbooks introduced here, any
person with the same hardware should have an environment to replicate
a job and debug and issue.

[1] - https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg00231.html

Best regards,
- Cleber.

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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