| Message ID | 20201019015003.1527746-3-crosa@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | GitLab Custom Runners and Jobs (was: QEMU Gating CI) | expand |
... > 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] Nit pick, is a group for localhost actually needed? Regards, Erik
On Mon, Oct 19, 2020 at 12:27:41PM +0200, Erik Skultety wrote: > ... > > > 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] > > Nit pick, is a group for localhost actually needed? > You're right, it's not needed... I just thought it gave the "localhost" entry some "shelter" and "context". :) And, I think a mostly "ini-like" file without a section triggers an OCD reaction in me. I can remove it if it does something similar to you! :) Thanks! - Cleber. > Regards, > Erik
On Mon, Oct 19, 2020 at 04:25:31PM -0400, Cleber Rosa wrote: > On Mon, Oct 19, 2020 at 12:27:41PM +0200, Erik Skultety wrote: > > ... > > > > > 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] > > > > Nit pick, is a group for localhost actually needed? > > > > You're right, it's not needed... I just thought it gave the > "localhost" entry some "shelter" and "context". :) > > And, I think a mostly "ini-like" file without a section triggers an OCD > reaction in me. I can remove it if it does something similar to you! :) > > Thanks! > - Cleber. I understand, but even though it may be an ini-like config, "sections" have a very distinct meaning in Ansible and that is grouping, so unless there are hosts to group, I'd keep the inventory in a form of a simple list of individual hosts. Regards, Erik
On Sun, 2020-10-18 at 21:50 -0400, Cleber Rosa wrote: > +++ b/scripts/ci/setup/build-environment.yml > @@ -0,0 +1,233 @@ > +--- > +- name: Installation of basic packages to build QEMU > + hosts: all > + tasks: My Ansible-fu is a bit rusty at the moment, so please double-check my claims and apologies in advance for the ones that I will get wrong O:-) > + - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04 > + apt: > + update_cache: yes > + # Originally from tests/docker/dockerfiles/ubuntu1804.docker > + pkg: > + - ccache > + - clang Instead of using the 'apt' module here, and the equivalent module further down, you could just do package: name: - pkg1 - pkg2 ... state: present every single time and let Ansible take care of the differences for you. > + when: "ansible_facts['distribution'] == 'Ubuntu'" Quoting the entire condition is not necessary, you can just have when: ansible_facts['distribution'] == 'Ubuntu' or, my preference, when: - ansible_facts['distribution'] == 'Ubuntu' which results in a nicer diff when you add/remove conditions. > + - 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 Indentation of list items is inconsistent here. > + - name: Install basic packages to build QEMU on FreeBSD 12.x > + pkgng: > + # Originally from packages on .cirrus.yml under the freebsd_12_task > + name: bash,curl,cyrus-sasl,git,glib,gmake,gnutls,gsed,nettle,ninja,perl5,pixman,pkgconf,png,usbredir See above for 'pkgng' vs 'package', but at the very least this should be pkgng: name: - bash - curl ... or each time the list is touched the resulting diff is going to be unmanageable. > + - name: Enable PowerTools repo on CentOS 8 > + ini_file: > + path: /etc/yum.repos.d/CentOS-PowerTools.repo > + section: PowerTools > + option: enabled > + value: "1" > + when: > + - "ansible_facts['distribution'] == 'CentOS'" > + - "ansible_facts['distribution_major_version'] == '8'" Another option would be to use command: 'dnf config-manager --set-enabled Stream-PowerTools -y' args: warn: no but I have to admit the way you're doing it is very clever ;)
On 19/10/2020 03.50, Cleber Rosa wrote: > To run basic jobs on custom runners, the environment needs to be > properly set up. The most common requirement is having the right > packages installed. > > The playbook introduced here covers a number of different Linux > distributions and FreeBSD, and are intended to provide a reproducible > environment. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- [...] > + - name: Enable PowerTools repo on CentOS 8 > + ini_file: > + path: /etc/yum.repos.d/CentOS-PowerTools.repo > + section: PowerTools > + option: enabled > + value: "1" > + when: > + - "ansible_facts['distribution'] == 'CentOS'" > + - "ansible_facts['distribution_major_version'] == '8'" > + > + - name: Install basic packages to build QEMU on CentOS 8 > + dnf: > + # Originally from tests/docker/dockerfiles/centos8.docker > + name: > + - SDL-devel We do not support SDL1 in QEMU anymore, so this should be SDL2-devel now. Yes, we've also got it wrong in the docker files ... I'll send a patch to fix it there. Thomas
On Tue, Oct 20, 2020 at 08:18:54AM +0200, Erik Skultety wrote: > On Mon, Oct 19, 2020 at 04:25:31PM -0400, Cleber Rosa wrote: > > On Mon, Oct 19, 2020 at 12:27:41PM +0200, Erik Skultety wrote: > > > ... > > > > > > > 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] > > > > > > Nit pick, is a group for localhost actually needed? > > > > > > > You're right, it's not needed... I just thought it gave the > > "localhost" entry some "shelter" and "context". :) > > > > And, I think a mostly "ini-like" file without a section triggers an OCD > > reaction in me. I can remove it if it does something similar to you! :) > > > > Thanks! > > - Cleber. > > I understand, but even though it may be an ini-like config, "sections" have a > very distinct meaning in Ansible and that is grouping, so unless there are > hosts to group, I'd keep the inventory in a form of a simple list of individual > hosts. > Fair enough. Changing it for v5. Thanks! - Cleber. > Regards, > Erik
On Tue, Oct 20, 2020 at 07:52:43PM +0200, Andrea Bolognani wrote: > On Sun, 2020-10-18 at 21:50 -0400, Cleber Rosa wrote: > > +++ b/scripts/ci/setup/build-environment.yml > > @@ -0,0 +1,233 @@ > > +--- > > +- name: Installation of basic packages to build QEMU > > + hosts: all > > + tasks: > > My Ansible-fu is a bit rusty at the moment, so please double-check my > claims and apologies in advance for the ones that I will get wrong O:-) > > > + - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04 > > + apt: > > + update_cache: yes > > + # Originally from tests/docker/dockerfiles/ubuntu1804.docker > > + pkg: > > + - ccache > > + - clang > > Instead of using the 'apt' module here, and the equivalent module > further down, you could just do > > package: > name: > - pkg1 > - pkg2 > ... > state: present > > every single time and let Ansible take care of the differences for > you. > I'm almost sure that this was a conscious decision. I remeber it had to do with not being able to set `update_cache`, and failures on recently installed systems and containers that did not update the APT cache. There may be something else, but I'll have to give it another round of testing. FIY, under the hood, package is not really a module, but an action plugin that forwards these very limited options to the set or detected package manager, so it brings uniformity in the playbook, but limits the control too. IMO, it's very low impact to leave it AS IS. > > + when: "ansible_facts['distribution'] == 'Ubuntu'" > > Quoting the entire condition is not necessary, you can just have > > when: ansible_facts['distribution'] == 'Ubuntu' > > or, my preference, > > when: > - ansible_facts['distribution'] == 'Ubuntu' > Yep, I've used the explicit lists when there was more than one condition, but having a standard style is better indeed. > which results in a nicer diff when you add/remove conditions. > > > + - 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 > > Indentation of list items is inconsistent here. > True. Fixed, thanks! > > + - name: Install basic packages to build QEMU on FreeBSD 12.x > > + pkgng: > > + # Originally from packages on .cirrus.yml under the freebsd_12_task > > + name: bash,curl,cyrus-sasl,git,glib,gmake,gnutls,gsed,nettle,ninja,perl5,pixman,pkgconf,png,usbredir > > See above for 'pkgng' vs 'package', but at the very least this should > be > > pkgng: > name: > - bash > - curl > ... > > or each time the list is touched the resulting diff is going to be > unmanageable. > The documentation suggests a comma separated list of package names: https://docs.ansible.com/ansible/2.8/modules/pkgng_module.html#pkgng-module And the reason is that this module is not as smart as others, and will run one separate command for each individual package name value: https://github.com/ansible/ansible/blob/v2.10.3/test/support/integration/plugins/modules/pkgng.py#L214 It's a tradeoff indeed, but I think we're aligned with the docs. > > + - name: Enable PowerTools repo on CentOS 8 > > + ini_file: > > + path: /etc/yum.repos.d/CentOS-PowerTools.repo > > + section: PowerTools > > + option: enabled > > + value: "1" > > + when: > > + - "ansible_facts['distribution'] == 'CentOS'" > > + - "ansible_facts['distribution_major_version'] == '8'" > > Another option would be to use > > command: 'dnf config-manager --set-enabled Stream-PowerTools -y' > args: > warn: no > > but I have to admit the way you're doing it is very clever ;) > Yeah, that would require another package to be installed, and then the command executed... So I think this is cheaper and eaiser indeed. > -- > Andrea Bolognani / Red Hat / Virtualization > Thanks for the review, I'll report on the additional points as soon as I test them. If appropriate, I'll put notes on the v5. - Cleber.
On Wed, Oct 21, 2020 at 09:00:27AM +0200, Thomas Huth wrote: > On 19/10/2020 03.50, Cleber Rosa wrote: > > To run basic jobs on custom runners, the environment needs to be > > properly set up. The most common requirement is having the right > > packages installed. > > > > The playbook introduced here covers a number of different Linux > > distributions and FreeBSD, and are intended to provide a reproducible > > environment. > > > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > > --- > [...] > > + - name: Enable PowerTools repo on CentOS 8 > > + ini_file: > > + path: /etc/yum.repos.d/CentOS-PowerTools.repo > > + section: PowerTools > > + option: enabled > > + value: "1" > > + when: > > + - "ansible_facts['distribution'] == 'CentOS'" > > + - "ansible_facts['distribution_major_version'] == '8'" > > + > > + - name: Install basic packages to build QEMU on CentOS 8 > > + dnf: > > + # Originally from tests/docker/dockerfiles/centos8.docker > > + name: > > + - SDL-devel > > We do not support SDL1 in QEMU anymore, so this should be SDL2-devel now. > Yes, we've also got it wrong in the docker files ... I'll send a patch to > fix it there. > > Thomas Nice catch. I see the dockerfile changes are already merged, so updating them here. - Cleber.
On Mon, 2020-11-09 at 11:37 -0500, Cleber Rosa wrote: > On Tue, Oct 20, 2020 at 07:52:43PM +0200, Andrea Bolognani wrote: > > On Sun, 2020-10-18 at 21:50 -0400, Cleber Rosa wrote: > > > + - name: Install basic packages to build QEMU on FreeBSD 12.x > > > + pkgng: > > > + # Originally from packages on .cirrus.yml under the freebsd_12_task > > > + name: bash,curl,cyrus-sasl,git,glib,gmake,gnutls,gsed,nettle,ninja,perl5,pixman,pkgconf,png,usbredir > > > > See above for 'pkgng' vs 'package', but at the very least this should > > be > > > > pkgng: > > name: > > - bash > > - curl > > ... > > > > or each time the list is touched the resulting diff is going to be > > unmanageable. > > The documentation suggests a comma separated list of package names: > > https://docs.ansible.com/ansible/2.8/modules/pkgng_module.html#pkgng-module > > And the reason is that this module is not as smart as others, and will > run one separate command for each individual package name value: > > https://github.com/ansible/ansible/blob/v2.10.3/test/support/integration/plugins/modules/pkgng.py#L214 > > It's a tradeoff indeed, but I think we're aligned with the docs. I would probably take the performance hit over having to deal with an unmaintainable blob. As a rule of thumb, reviewer time is more valuable than machine time. If the performance hit turns out to be unacceptably big, you could at least do something along the lines of - set_fact: freebsd_packages: - bash - curl ... - pkgng: name: "{ freebsd_packages | join(',') }" to keep things reviewable. -- Andrea Bolognani / Red Hat / Virtualization
diff --git a/docs/devel/ci.rst b/docs/devel/ci.rst index 41a4bbddad..208b5e399b 100644 --- a/docs/devel/ci.rst +++ b/docs/devel/ci.rst @@ -52,3 +52,35 @@ As a general rule, those newly added contributed jobs should run as The precise minimum requirements and exact rules for machine configuration documentation/scripts, and the success rate of jobs are still to be defined. + +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 +set up machines with the environment needed to perform builds and run +QEMU tests. 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 diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml new file mode 100644 index 0000000000..074aaca927 --- /dev/null +++ b/scripts/ci/setup/build-environment.yml @@ -0,0 +1,233 @@ +--- +- name: Installation of basic packages to build QEMU + hosts: all + tasks: + - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04 + apt: + update_cache: yes + # Originally from tests/docker/dockerfiles/ubuntu1804.docker + pkg: + - 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 + - ninja-build + - python3-yaml + - python3-sphinx + - sparse + - 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: + # Originally from packages on .cirrus.yml under the freebsd_12_task + name: bash,curl,cyrus-sasl,git,glib,gmake,gnutls,gsed,nettle,ninja,perl5,pixman,pkgconf,png,usbredir + state: present + when: "ansible_facts['os_family'] == 'FreeBSD'" + + - name: Install basic packages to build QEMU on Fedora 30, 31 and 32 + dnf: + # Originally from tests/docker/dockerfiles/fedora.docker + name: + - SDL2-devel + - bc + - brlapi-devel + - bzip2 + - bzip2-devel + - ccache + - clang + - cyrus-sasl-devel + - dbus-daemon + - device-mapper-multipath-devel + - diffutils + - findutils + - gcc + - gcc-c++ + - genisoimage + - 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 + - libepoxy-devel + - libfdt-devel + - libiscsi-devel + - libjpeg-devel + - libpmem-devel + - libpng-devel + - librbd-devel + - libseccomp-devel + - libssh-devel + - libubsan + - libudev-devel + - libusbx-devel + - libxml2-devel + - libzstd-devel + - llvm + - lzo-devel + - make + - mingw32-SDL2 + - 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 + - mingw64-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 + - ncurses-devel + - nettle-devel + - ninja-build + - nss-devel + - numactl-devel + - perl + - perl-Test-Harness + - pixman-devel + - python3 + - python3-PyYAML + - python3-numpy + - python3-opencv + - python3-pillow + - python3-pip + - python3-sphinx + - python3-virtualenv + - rdma-core-devel + - snappy-devel + - sparse + - spice-server-devel + - systemd-devel + - systemtap-sdt-devel + - tar + - tesseract + - tesseract-langpack-eng + - usbredir-devel + - virglrenderer-devel + - vte291-devel + - which + - xen-devel + - zlib-devel + state: present + when: "ansible_facts['distribution'] == 'Fedora'" + + - name: Enable PowerTools repo on CentOS 8 + ini_file: + path: /etc/yum.repos.d/CentOS-PowerTools.repo + section: PowerTools + option: enabled + value: "1" + when: + - "ansible_facts['distribution'] == 'CentOS'" + - "ansible_facts['distribution_major_version'] == '8'" + + - name: Install basic packages to build QEMU on CentOS 8 + dnf: + # Originally from tests/docker/dockerfiles/centos8.docker + name: + - SDL-devel + - bzip2 + - bzip2-devel + - dbus-daemon + - gcc + - gcc-c++ + - genisoimage + - gettext + - git + - glib2-devel + - libaio-devel + - libepoxy-devel + - libgcrypt-devel + - lzo-devel + - make + - mesa-libEGL-devel + - nettle-devel + - ninja-build + - 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/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
To run basic jobs on custom runners, the environment needs to be properly set up. The most common requirement is having the right packages installed. The playbook introduced here covers a number of different Linux distributions and FreeBSD, and are intended to provide a reproducible environment. Signed-off-by: Cleber Rosa <crosa@redhat.com> --- docs/devel/ci.rst | 32 ++++ scripts/ci/setup/build-environment.yml | 233 +++++++++++++++++++++++++ scripts/ci/setup/inventory | 2 + 3 files changed, 267 insertions(+) create mode 100644 scripts/ci/setup/build-environment.yml create mode 100644 scripts/ci/setup/inventory