Merge pull request #311 from AdamWill/fix-tests-py3

Fix oz with recent Python, fix a major bug with `useuefi` arg, fix up some issues in the tests, add support for running tests in containers, add a Github Action workflow for CI

Author: nullr0ute

.github/workflows/ci.yml

@@ -0,0 +1,41 @@
+---
+name: Run CI checks
+
+on: [pull_request]
+
+jobs:
+  unittests-fedora:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repo
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - name: Install make and docker
+        run: apt-get install make docker docker.io
+      - name: Run the tests
+        run: make container-unittests-fedora
+  unittests-el7:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repo
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - name: Install make and docker
+        run: apt-get install make docker
+      - name: Run the tests
+        run: make container-unittests-el7
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout the repo
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - name: Install make, pylint and flake8
+        run: apt-get install make pylint flake8
+      - name: Run pylint
+        run: make pylint
+      - name: Run flake8
+        run: make flake8

.travis.yml

@@ -0,0 +1,16 @@
+# this container definition is intended *only* for running the oz test suite, it is not
+# a general-purpose oz container definition!
+
+FROM quay.io/centos/centos:7
+RUN set -exo pipefail \
+    && yum install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm \
+    && yum install -y python-requests m2crypto libvirt-python python-lxml python-libguestfs pytest python-monotonic libvirt \
+    && yum clean all \
+    && rm -rf /var/cache/* /var/log/yum*
+
+COPY ./ /oz
+# the XML generation tests are inherently unreliable before Python 3.8,
+# as there was no consistent ordering of XML element attributes. See
+# https://docs.python.org/3/library/xml.etree.elementtree.html#xml.etree.ElementTree.tostring
+RUN printf "#!/bin/sh\n/usr/sbin/libvirtd -d\ncd /oz\npy.test -vv -k 'not test_xml_generation and not modify_libvirt_xml_for_serial' tests/" > /usr/local/bin/runtests.sh && chmod ugo+x /usr/local/bin/runtests.sh
+CMD /usr/local/bin/runtests.sh

Containerfile.tests.el7

@@ -0,0 +1,14 @@
+# this container definition is intended *only* for running the oz test suite, it is not
+# a general-purpose oz container definition!
+
+FROM quay.io/fedora/fedora:latest
+RUN set -exo pipefail \
+    && dnf install -y --setopt install_weak_deps=false --nodocs \
+    python3-requests python3-m2crypto python3-setuptools python3-libvirt python3-lxml python3-libguestfs python3-pytest python3-monotonic \
+    libvirt-daemon libvirt-daemon-kvm libvirt-daemon-qemu libvirt-daemon-config-network systemd \
+    && dnf clean all \
+    && rm -rf /var/cache/* /var/log/dnf*
+
+COPY ./ /oz
+RUN printf "#!/bin/sh\n/usr/sbin/libvirtd -d\ncd /oz\npy.test -vv tests/" > /usr/local/bin/runtests.sh && chmod ugo+x /usr/local/bin/runtests.sh
+CMD /usr/local/bin/runtests.sh

Containerfile.tests.fedora

@@ -46,18 +46,35 @@ unittests:
 
 tests: unittests
 
+container-unittests-fedora:
+	docker rm -f oz-tests-fedora
+	docker build -f Containerfile.tests.fedora -t oz-tests-fedora-image .
+	docker run --name oz-tests-fedora oz-tests-fedora-image
+
+container-unittests-el7:
+	docker rm -f oz-tests-el7
+	docker build -f Containerfile.tests.el7 -t oz-tests-el7-image .
+	docker run --name oz-tests-el7 oz-tests-el7-image
+
+container-unittests: container-unittests-fedora container-unittests-el7
+
 test-coverage:
 	python-coverage run --source oz /usr/bin/py.test --verbose tests
 	python-coverage html
 	xdg-open htmlcov/index.html
 
 pylint:
-	pylint-3 --rcfile=pylint.conf oz oz-install oz-customize oz-cleanup-cache oz-generate-icicle
+	pylint --rcfile=pylint.conf oz oz-install oz-customize oz-cleanup-cache oz-generate-icicle
 
 flake8:
-	flake8-3 --ignore=E501 oz
+	flake8 --ignore=E501 oz
+
+container-clean:
+	docker rm -f oz-tests-fedora
+	docker rm -f oz-tests-el7
+	docker image rm -f -i oz-tests-fedora-image oz-tests-el7-image
 
 clean:
 	rm -rf MANIFEST build dist usr *~ oz.spec *.pyc oz/*~ oz/*.pyc examples/*~ oz/auto/*~ man/*~ docs/*~ man/*.html $(VENV_DIR) tests/tdl/*~ tests/factory/*~ tests/results.xml htmlcov
 
-.PHONY: sdist oz.spec signed-tarball signed-rpm rpm srpm deb release man2html virtualenv unittests tests test-coverage pylint clean
+.PHONY: sdist oz.spec signed-tarball signed-rpm rpm srpm deb release man2html virtualenv unittests container-unittests-fedora container-unittests-el7 container-unittests tests test-coverage pylint clean container-clean

Makefile

@@ -4,7 +4,40 @@ images containing an operating systems and, optionally, programs.
 
 The best way to install it is to make an RPM out of it by running "make rpm"
 or "make srpm", and then installing the resulting RPM on the target machine.
+This will probably only work on Fedora or on RHEL 8 or later.
 
 Once you have the RPM installed, you will need to build up TDL files to feed
 as input into the building process.  Please see the examples directory to get
 an idea of what a TDL file looks like.
+
+You can run the oz unit tests in containers by installing make and docker
+(or podman-docker), then running:
+
+make container-unittests
+
+You can clean up the containers and images by running:
+
+make container-clean
+
+Otherwise, you can try running the tests directly on your host, but oz uses the
+guestfs Python module, which is not available from pypi, and needs a running
+libvirtd for most of the tests to run. To install all the test requirements on
+Fedora:
+
+dnf install python3-requests python3-m2crypto python3-libvirt python3-lxml python3-libguestfs python3-pytest python3-monotonic
+
+If you wish to test on EL 7, make that:
+
+yum install python-requests m2crypto libvirt-python python-lxml python-libguestfs pytest python-monotonic
+
+then run the tests:
+
+py.test tests/
+
+You can try `make virtualenv` then `make unittests` to run the tests in a
+virtualenv if you like, but this still requires at least the libguestfs library
+installed on the host, and a running libvirtd. You may also want to install
+m2crypto and libvirt libraries on the host, as otherwise pip will have to
+compile them, and you'll need their build dependencies.
+
+You can use `make pylint` and `make flake8` to run lint checks.

README

@@ -1,45 +1,41 @@
-Summary: Library and utilities for automated guest OS installs
-Name: oz
+Name:    oz
 Version: @VERSION@
 Release: @RELEASE@%{?dist}
+Summary: Library and utilities for automated guest OS installs
 License: LGPLv2
-Group: Development/Libraries
-URL: http://github.com/clalancette/oz
-Source0: http://github.com/clalancette/%{name}/archive/%{name}-%{version}.tar.gz
+URL:     http://github.com/clalancette/oz
+
+Source0: https://github.com/clalancette/%{name}/archive/v%{version}/%{name}-%{version}.tar.gz
+
 BuildArch: noarch
+
+BuildRequires: python3
+BuildRequires: python3-devel
+BuildRequires: python3-setuptools
 Requires: python3
-Requires: python3-libguestfs >= 1.18
 Requires: python3-lxml
+Requires: python3-libguestfs >= 1.18
 Requires: python3-libvirt
+Requires: python3-m2crypto
+Requires: python3-monotonic
+Requires: python3-requests
 # in theory, oz doesn't really require libvirtd to be local to operate
 # properly.  However, because of the libguestfs manipulations, in practice
 # it really does.  Make it depend on libvirt (so we get libvirtd) for now,
 # unless/until we are able to make it really be remote.
-%if 0%{?fedora} >= 17
 Requires: libvirt-daemon-kvm
 Requires: libvirt-daemon-qemu
 Requires: libvirt-daemon-config-network
-%else
-Requires: libvirt >= 0.9.7
-%endif
-Requires: python3-requests
 Requires: genisoimage
 Requires: mtools
-%if 0%{?fedora} < 26 or 0%{?rhel} < 8
-Requires: python-uuid
-%endif
 Requires: openssh-clients
-Requires: python3-m2crypto
-Requires: python3-monotonic
-
-BuildRequires: python3
 
 %description
 Oz is a set of libraries and utilities for doing automated guest OS
 installations, with minimal input from the user.
 
 %prep
-%setup -q
+%autosetup -p1
 
 %build
 %py3_build
@@ -66,7 +62,8 @@ if [ ! -f %{_sysconfdir}/oz/id_rsa-icicle-gen ]; then
 fi
 
 %files
-%doc README COPYING examples
+%license COPYING
+%doc README examples
 %dir %attr(0755, root, root) %{_sysconfdir}/oz/
 %config(noreplace) %{_sysconfdir}/oz/oz.cfg
 %dir %attr(0755, root, root) %{_localstatedir}/lib/oz/
@@ -78,13 +75,13 @@ fi
 %dir %attr(0755, root, root) %{_localstatedir}/lib/oz/jeos/
 %dir %attr(0755, root, root) %{_localstatedir}/lib/oz/kernels/
 %dir %attr(0755, root, root) %{_localstatedir}/lib/oz/screenshots/
-%{python3_sitelib}/oz
 %{_bindir}/oz-install
 %{_bindir}/oz-generate-icicle
 %{_bindir}/oz-customize
 %{_bindir}/oz-cleanup-cache
-%{python3_sitelib}/%{name}-%{version}-py%{python3_version}.egg-info
 %{_mandir}/man1/*
+%{python3_sitelib}/oz
+%{python3_sitelib}/%{name}*.egg-info
 
 %changelog
 * Sat Feb  5 2022 Peter Robinson <[email protected]> - 0.18.0-1

oz.spec.in

@@ -248,7 +248,7 @@ class FedoraGuest(oz.RedHat.RedHatLinuxCDYumGuest):
     # ignored now; we leave it in place for backwards API compatibility.
     def __init__(self, tdl, config, auto, nicmodel, haverepo, diskbus,  # pylint: disable=unused-argument
                  brokenisomethod, output_disk=None, macaddress=None,    # pylint: disable=unused-argument
-                 assumed_update=None, useuefi=False):
+                 assumed_update=None):
         if int(tdl.update) < 31:
             self.config = version_to_config[tdl.update]
         else:
@@ -263,8 +263,7 @@ def __init__(self, tdl, config, auto, nicmodel, haverepo, diskbus,  # pylint: di
         oz.RedHat.RedHatLinuxCDYumGuest.__init__(self, tdl, config, auto,
                                                  output_disk, nicmodel, diskbus,
                                                  True, True, self.config.directkernel,
-                                                 macaddress, self.config.use_yum,
-                                                 self.config.useuefi)
+                                                 macaddress, self.config.use_yum)
 
         if self.assumed_update is not None:
             self.log.warning("==== WARN: TDL contains Fedora update %s, which is newer than Oz knows about; pretending this is Fedora %s, but this may fail ====", tdl.update, assumed_update)

oz/Fedora.py

@@ -129,7 +129,7 @@ def _libvirt_error_handler(ctxt, err):
         self._discover_libvirt_type()
 
     def __init__(self, tdl, config, auto, output_disk, nicmodel, clockoffset,
-                 mousetype, diskbus, iso_allowed, url_allowed, macaddress, useuefi):
+                 mousetype, diskbus, iso_allowed, url_allowed, macaddress):
         self.tdl = tdl
 
         # for backwards compatibility
@@ -502,7 +502,7 @@ def _generate_xml(self, bootdev, installdev, kernel=None, initrd=None,
             oz.ozutil.lxml_subelement(osNode, "loader", loader, {'readonly': 'yes', 'type': 'pflash'})
             oz.ozutil.lxml_subelement(osNode, "nvram", None, {'template': nvram})
         # x86_64 has legacy requirements so we check for defaults as well as for edk2
-        if self.tdl.arch in ["x86_64"] and self.config.useuefi == True:
+        if self.tdl.arch == "x86_64" and hasattr(self.config, "useuefi") and self.config.useuefi is True:
             loader, nvram = oz.ozutil.find_uefi_firmware(self.tdl.arch)
             oz.ozutil.lxml_subelement(osNode, "loader", loader, {'readonly': 'yes', 'type': 'pflash'})
             oz.ozutil.lxml_subelement(osNode, "nvram", None, {'template': nvram})
@@ -1333,10 +1333,10 @@ def __init__(self, version, sysid, volid, space_size, set_size, seqnum):
             self.seqnum = seqnum
 
     def __init__(self, tdl, config, auto, output_disk, nicmodel, clockoffset,
-                 mousetype, diskbus, iso_allowed, url_allowed, macaddress, useuefi):
+                 mousetype, diskbus, iso_allowed, url_allowed, macaddress):
         Guest.__init__(self, tdl, config, auto, output_disk, nicmodel,
                        clockoffset, mousetype, diskbus, iso_allowed,
-                       url_allowed, macaddress, useuefi)
+                       url_allowed, macaddress)
 
         self.orig_iso = os.path.join(self.data_dir, "isos",
                                      self.tdl.distro + self.tdl.update + self.tdl.arch + "-" + self.tdl.installtype + ".iso")
@@ -1801,9 +1801,9 @@ class FDGuest(Guest):
     Class for guest installation via floppy disk.
     """
     def __init__(self, tdl, config, auto, output_disk, nicmodel, clockoffset,
-                 mousetype, diskbus, macaddress, useuefi):
+                 mousetype, diskbus, macaddress):
         Guest.__init__(self, tdl, config, auto, output_disk, nicmodel,
-                       clockoffset, mousetype, diskbus, False, True, macaddress, useuefi)
+                       clockoffset, mousetype, diskbus, False, True, macaddress)
         self.orig_floppy = os.path.join(self.data_dir, "floppies",
                                         self.tdl.distro + self.tdl.update + self.tdl.arch + ".img")
         self.modified_floppy_cache = os.path.join(self.data_dir, "floppies",

oz/Guest.py

@@ -33,10 +33,10 @@ class LinuxCDGuest(oz.Guest.CDGuest):
     Class for Linux installation.
     """
     def __init__(self, tdl, config, auto, output_disk, nicmodel, diskbus,
-                 iso_allowed, url_allowed, macaddress, useuefi):
+                 iso_allowed, url_allowed, macaddress):
         oz.Guest.CDGuest.__init__(self, tdl, config, auto, output_disk,
                                   nicmodel, None, None, diskbus, iso_allowed,
-                                  url_allowed, macaddress, useuefi)
+                                  url_allowed, macaddress)
 
     def _test_ssh_connection(self, guestaddr):
         """

oz/Linux.py

@@ -31,12 +31,12 @@ class RHEL8Guest(oz.RedHat.RedHatLinuxCDYumGuest):
     Class for RHEL-8 installation
     """
     def __init__(self, tdl, config, auto, output_disk=None, netdev=None,
-                 diskbus=None, macaddress=None, useuefi=True):
+                 diskbus=None, macaddress=None):
         # dnf distro
         oz.RedHat.RedHatLinuxCDYumGuest.__init__(self, tdl, config, auto,
                                                  output_disk, netdev, diskbus,
                                                  True, True, "cpio", macaddress,
-                                                 False, useuefi)
+                                                 False)
         self.virtio_channel_name = 'org.fedoraproject.anaconda.log.0'
 
     def _modify_iso(self):

oz/RHEL_8.py

@@ -31,11 +31,11 @@ class RHEL9Guest(oz.RHEL_8.RHEL8Guest):
     Class for RHEL-9 installation
     """
     def __init__(self, tdl, config, auto, output_disk=None, netdev=None,
-                 diskbus=None, macaddress=None, useuefi=True):
+                 diskbus=None, macaddress=None):
         # dnf distro
         oz.RHEL_8.RHEL8Guest.__init__(self, tdl, config, auto,
                                       output_disk, netdev, diskbus,
-                                       macaddress, useuefi)
+                                       macaddress)
 
         # method and ks options were dropped
         self.cmdline = "inst.repo=" + self.url + " inst.ks=file:/ks.cfg"