Python 3 Lab Controller support

.github/workflows/review-checks.yml

@@ -136,3 +136,69 @@ jobs:
           # Disable Selenium tests until we have plan for selenium driver + firefox
           rm -rf src/bkr/inttest/server/selenium
           ./run-tests.sh -v ${{ matrix.test-target }}
+
+  unit-tests:
+    runs-on: ubuntu-latest
+    env:
+      # https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/
+      ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node16
+      ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true
+    container:
+      image: ${{ matrix.os-target.name }}
+      options: --init
+    strategy:
+      fail-fast: false
+      matrix:
+        test-target:
+          - Common
+          - Client
+          - LabController
+        os-target:
+          - name: centos:7
+          - name: quay.io/centos/centos:stream9
+            additional_repos: "crb"
+            additional_packages: "epel-release"
+    steps:
+      - name: Fixup CentOS repo files
+        if: matrix.os-target.name == 'centos:7'
+        run: |
+          sed -i s/mirror.centos.org/vault.centos.org/g /etc/yum.repos.d/*.repo
+          sed -i s/^#.*baseurl=http/baseurl=http/g /etc/yum.repos.d/*.repo
+          sed -i s/^mirrorlist=http/#mirrorlist=http/g /etc/yum.repos.d/*.repo
+
+      - name: Set BKR_PY3 environment variable
+        run: echo "BKR_PY3=1" >> $GITHUB_ENV
+        if: matrix.os-target.name != 'centos:7'
+
+      - name: Install DNF and plugins
+        run: yum install -y dnf 'dnf-command(builddep)' 'dnf-command(config-manager)'
+
+      - name: Enable additional repositories
+        run: |
+          IFS=', ' read -r -a repos <<< "${{ matrix.os-target.additional_repos }}"
+          for repo in "${repos[@]}"; do
+            dnf config-manager --set-enabled $repo
+          done
+      - name: Install additional packages
+        run: |
+          IFS=', ' read -r -a packages <<< "${{ matrix.os-target.additional_packages }}"
+          for package in "${packages[@]}"; do
+            dnf install -y $package
+          done
+
+      # Do not upgrade to @v4 as node 20 is incompatible with CentOS 7
+      - name: Checkout
+        uses: actions/checkout@v3
+
+      - name: Add Beaker Server YUM repository
+        if: matrix.os-target.name == 'centos:7'
+        run: |
+          curl -o /etc/yum.repos.d/beaker-server.repo https://beaker-project.org/yum/beaker-server-RedHatEnterpriseLinux.repo
+      # Build dependencies must be sufficient, the same is done during RPM build
+      - name: Install Beaker dependency from specfile
+        run: |
+          dnf builddep -y *.spec
+      - name: Run unit tests
+        run: |
+          pushd ${{ matrix.test-target }}
+          ./run-tests.sh

.packit.yaml

@@ -18,8 +18,6 @@ jobs:
     targets:
       - epel-8-x86_64
       - epel-9-x86_64
-      - centos-stream-8-x86_64
-      - centos-stream-9-x86_64
     identifier: rhel_8_9
 
   - job: copr_build
@@ -35,7 +33,10 @@ jobs:
   - job: copr_build
     trigger: pull_request
     targets:
-      - epel-7-x86_64
+      epel-7-x86_64:
+        additional_repos:
+          - https://beaker-project.org/yum/server/RedHatEnterpriseLinux7/
+          - https://vault.centos.org/centos/7/extras/x86_64/
     identifier: rhel_7
     actions:
       post-upstream-clone:

Common/bkr/common/helpers.py

@@ -12,6 +12,8 @@
 import os
 import fcntl
 import errno
+
+import six
 from six.moves import queue
 
 log = getLogger(__name__)
@@ -100,12 +102,20 @@ def run(self):
             self.finished.clear()
 
 
-class SensitiveUnicode(unicode):
+class SensitiveUnicode(six.text_type):
+    """The intent of this class is to standardize the behavior of Unicode strings across Python 2 and 3,
+    while treating the contents of the string as sensitive.
+
+    In Python 2, it encodes the unicode string to bytes (str), wrapped in SensitiveStr.
+    In Python 3, it preserves the unicode representation (str) by overriding the default 'encode' behavior.
+    """
     def __repr__(self):
         return '<repr blocked>'
 
     def encode(self, *args, **kwargs):
-        return SensitiveStr(super(SensitiveUnicode, self).encode(*args, **kwargs))
+        if six.PY2:
+            return SensitiveStr(super(SensitiveUnicode, self).encode(*args, **kwargs))
+        return self
 
 
 class SensitiveStr(str):
@@ -235,6 +245,10 @@ def siphon(src, dest):
         chunk = src.read(4096)
         if not chunk:
             break
+
+        if six.PY3 and isinstance(chunk, bytes):
+            chunk = chunk.decode('utf-8')
+
         dest.write(chunk)
 
 

LabController/Makefile

@@ -4,17 +4,25 @@
 # the Free Software Foundation; either version 2 of the License, or
 # (at your option) any later version.
 
+BKR_PY3 ?= 0
+COMMAND := python2
+
+ifeq ($(BKR_PY3),1)
+	COMMAND := python3
+endif
+
+
 .PHONY: build
 build:
-	python2 setup.py build
+	$(COMMAND) setup.py build
 
 .PHONY: install
 install: build
-	python2 setup.py install -O1 --skip-build --root $(DESTDIR)
+	$(COMMAND) setup.py install -O1 --skip-build --root $(DESTDIR)
 
 .PHONY: clean
 clean:
-	python2 setup.py clean
+	$(COMMAND) setup.py clean
 	rm -rf build
 
 .PHONY: check

LabController/run-tests.sh

@@ -1,5 +1,16 @@
-#/bin/bash
+#!/bin/bash
 set -x
 
-env PYTHONPATH=src:../Common${PYTHONPATH:+:$PYTHONPATH} \
-    nosetests ${*:--v --traverse-namespace bkr.labcontroller}
+# Use nosetests with python2 interpreter
+if [[ -z ${BKR_PY3} ]] || [[ ${BKR_PY3} != 1 ]]; then
+    command="nosetests ${*:--v --traverse-namespace bkr.labcontroller}";
+else
+    # Check if pytest-3 is available
+    if command -v pytest-3 >/dev/null 2>&1; then
+        command="pytest-3";
+    else
+        command="pytest";
+    fi
+fi
+
+env PYTHONPATH=src:../Common${PYTHONPATH:+:$PYTHONPATH} $command

LabController/setup.py

@@ -1,16 +1,21 @@
 from setuptools import setup, find_packages
-import commands
 from glob import glob
 
+try:
+    from subprocess import getstatusoutput
+except ImportError:
+    from commands import getstatusoutput
+
+
 def systemd_unit_dir():
-    status, output = commands.getstatusoutput('pkg-config --variable systemdsystemunitdir systemd')
+    status, output = getstatusoutput('pkg-config --variable systemdsystemunitdir systemd')
     if status or not output:
         return None # systemd not found
     return output.strip()
 
 def systemd_tmpfiles_dir():
     # There doesn't seem to be a specific pkg-config variable for this
-    status, output = commands.getstatusoutput('pkg-config --variable prefix systemd')
+    status, output = getstatusoutput('pkg-config --variable prefix systemd')
     if status or not output:
         return None # systemd not found
     return output.strip() + '/lib/tmpfiles.d'

LabController/src/bkr/labcontroller/async.py → LabController/src/bkr/labcontroller/concurrency.py

@@ -16,6 +16,7 @@
 import errno
 import logging
 import gevent.event, gevent.socket, gevent.hub
+import six
 
 logger = logging.getLogger(__name__)
 
@@ -29,22 +30,27 @@ def _read_from_pipe(f):
     discarding = False
     while True:
         try:
+            gevent.socket.wait_read(f.fileno())
             chunk = f.read(4096)
             if not chunk:
                 break
             if not discarding:
                 chunks.append(chunk)
                 if len(chunks) >= 1000:
-                    logger.error('Too many chunks read from fd %s, '
-                            'child process is running amok?!', f.fileno())
-                    chunks.append('+++ DISCARDED')
+                    logger.error(
+                        "Too many chunks read from fd %s, "
+                        "child process is running amok?!",
+                        f.fileno(),
+                    )
+                    chunks.append(b"+++ DISCARDED")
                     discarding = True
-        except IOError, e:
+        except IOError as e:
             if e.errno != errno.EAGAIN:
                 raise
-            sys.exc_clear()
-        gevent.socket.wait_read(f.fileno())
-    return ''.join(chunks)
+    if six.PY3:
+        # Keep data in bytes until the end to reduce memory footprint
+        return b"".join(chunks).decode("utf-8")
+    return "".join(chunks)
 
 def _timeout_kill(p, timeout):
     gevent.sleep(timeout)
@@ -54,14 +60,14 @@ def _kill_process_group(pgid):
     # Try SIGTERM first, then SIGKILL just to be safe
     try:
         os.killpg(pgid, signal.SIGTERM)
-    except OSError, e:
+    except OSError as e:
         if e.errno != errno.ESRCH:
             raise
     else:
         gevent.sleep(1)
         try:
             os.killpg(pgid, signal.SIGKILL)
-        except OSError, e:
+        except OSError as e:
             if e.errno != errno.ESRCH:
                 raise