Adapt the package building scripts to use Python 3

[paride]

Changes:

• Do not template debian/rules as python3 is the only supported version
• Drop six from requirements.txt
• Makefile: drop everything related to Python 2
• run-container: install the CI deps only on ubuntu|debian
• read-version: update the shebang to use Python 3
• brpm: read_dependencies(): drop unused argument
• read-dependencies: switch to Py3 and drop the --python-version option
• pkg-deps.json: drop the Python version field and update the redhat deps
• Update RPM the spec file to use Python 3 when building the RPM
• bddeb: drop support for Python 2

[smoser]

is there a reason for this? Don't you want to be able to test ?

[paride]

The immediate reason why I'm passing --test-distro only on ubuntu|debian is that python3-tox is not available on CentOS.

Anyway --test-distro only does two thing on top on what is done without it:

1. Adds the packages defined in CI_SYSTEM_BASE_PKGS to the dependencies
2. Actually installs the packages on the system

and in my understanding is meant to enable testing of the CI automation, something we are doing only on Ubuntu anyway. The deps in test-requirements.txt are still installed as brpm and bddeb explicitly make read-dependencies read it, so the unit testing can still be done even without --test-distro provided that all the deps are available (not the case on CentOS, but different story).

[smoser]

I guess if you're trying to make this work despite lack of available deps on centos (7?) then the change/knowledge to ignore should go into tools/read-dependencies. Because if someone goes to fix this, they'll now have to remember to add the '--test-distro' flag back here, and fix the tools/read-dependencies to do have the right packages.

but make this is fine. thanks for explaining.

we really do need to have the ability to run test on linux other than ubuntu "really easily". Hopefully @OddBloke's pytest will land and make that possible?

[paride]

@smoser thanks for your feedback here.

Deps are missing in CentOS 8 too (at least: python3-tox, python3-contextlib2, python3-httpretty, and python3-unittest2 is only available in the PowerTools repo).

I'm not sure I agree the "install the test deps on ubuntu/debian only" logic should go in read-dependencies. If I leave run-container as it is it will always call read-dependencies --test-distro, which if the distro is (say) CentOS 8 can do two things:

1. Try to setup the test environment and fail, because dependencies are missing. This is consistent with everything, but not good for us, as we need to run the thing.
2. Skip setting up the test environment if distro != ubuntu|debian (or similar conditions). This is lying to the user, isn't it? A test environment has been asked for, but it hasn't been set up.

I think that not asking for a test environment (that is, not passing --test-distro) when we know it can't be set up is the correct thing to do.

[raharper]

@paride Now that pytest change has landed #211 does that mean we can run tests on Centos8? or are there still deps missing?

I'm not sure I agree the "install the test deps on ubuntu/debian only" logic should go in read-dependencies. If I leave run-container as it is it will always call read-dependencies --test-distro, which if the distro is (say) CentOS 8 can do two things:

I'd be OK not passing --test-distro to Centos8 as long as we logged a warning that we skip it due to the fact that it's currently broken. We should open a bug for centos8 test-enviroment as an upstream bug and mention that in the warning.

[paride]

There are still deps missing: contextlib2, httpretty, unittest2. Of these unittest2 is available in the PowerTools repository, the other two are not (according to pkgs.org). I'm +1 with opening a bug and mentioning it in a warning message.

[paride]

@raharper I had a look at https://www.rdoproject.org/what/repos/ but it seems that it only has python2 versions of the libraries we need, and no httpretty. I enabled both the openstack-train and openstack-train-testing repos, which should have the most up to date packages.

[root@centos8 ~]# yum repolist
Last metadata expiration check: 0:04:41 ago on Thu Mar 12 18:36:28 2020.
repo id                                                              repo name                                                                                            status
AppStream                                                            CentOS-8 - AppStream                                                                                 5103
BaseOS                                                               CentOS-8 - Base                                                                                      2107
*epel                                                                Extra Packages for Enterprise Linux 8 - x86_64                                                       4979
*epel-modular                                                        Extra Packages for Enterprise Linux Modular 8 - x86_64                                                  0
extras                                                               CentOS-8 - Extras                                                                                       3
openstack-train                                                      OpenStack Train Repository                                                                           2212
openstack-train-testing                                              OpenStack Train Testing                                                                              2236
rdo-qemu-ev                                                          RDO CentOS-7 - QEMU EV                                                                                 33

[root@centos8 ~]# yum --disablerepo="*" --enablerepo="openstack-train" list available | grep -E '(contextlib2|unittest2|httpretty)'
python2-contextlib2.noarch                            0.5.5-8.el7                   openstack-train
python2-unittest2.noarch                              1.1.0-15.el7                  openstack-train

[root@centos8 ~]# yum --disablerepo="*" --enablerepo="openstack-train-testing" list available | grep -E '(contextlib2|unittest2|httpretty)'
python2-contextlib2.noarch                            0.5.5-8.el7                   openstack-train-testing
python2-unittest2.noarch                              1.1.0-15.el7                  openstack-train-testing

[powersj]

@paride please address the conflicts

[paride]

Conflicts resolved

[github-actions]

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging powersj, and he will ensure that someone takes a look soon.

(If the pull request is closed, please do feel free to reopen it if you wish to continue working on it.)

[powersj]

Updating the branch and removing the stale tag.

@blackboxsw this is on you to review

[blackboxsw]

Generally speaking, looks great @paride I have a couple of suggestions as I need to support the a copr build release process manually at the moment during upstream releases

./tools/run-container --source-package --unittest --artifacts=./srpm/ centos/8

Which comes from our upstream release docs @ https://github.com/canonical/uss-tableflip/blob/master/doc/upstream_release_process.md

Since we only know of a couple of missing deps for CentOS, I think it is worth trying to install every test dep described in test-requirements.txt that we can instead of ignoring all test deps. Let's also still leave your warning message about LP: #1867151

Below is a diff suggestion (see what you think) which captures my comments but generally has the following points:

• install as many deps as we can on centos, leave it to centos folks to sort the missing deps if they want unit tests to work
• ensure on centOS our run-container --unittest no-ops by setting unittest=""
• Add a centos distro reference for renames since fedora/redhat are slightly different and have those missing deps
• Add an OMIT: in rename to pkg-deps.json for the missing deps and allow read-dependencies to skip specific deps

Thanks for your patience on my lack of prioritization on this review.

diff --git a/packages/pkg-deps.json b/packages/pkg-deps.json
index dbb5fbcdd..114e921e4 100644
--- a/packages/pkg-deps.json
+++ b/packages/pkg-deps.json
@@ -13,14 +13,13 @@
          "procps"
       ]
    },
-   "redhat" : {
+   "centos" : {
       "build-requires" : [
          "python3-devel"
       ],
       "renames" : {
-         "httpretty" : "",
-         "unittest2" : "",
-         "contextlib2" : ""
+         "unittest2" : "OMIT: LP: #1867151 Missing CentOS pkg deps",
+         "contextlib2" : "OMIT: LP: #1867151 Missing CentOS pkg deps"
       },
       "requires" : [
          "e2fsprogs",
@@ -32,6 +31,20 @@
          "sudo"
       ]
    },
+   "redhat" : {
+      "build-requires" : [
+         "python3-devel"
+      ],
+      "requires" : [
+         "e2fsprogs",
+         "iproute",
+         "net-tools",
+         "procps",
+         "rsyslog",
+         "shadow-utils",
+         "sudo"
+      ]
+   },
    "suse" : {
       "renames" : {
       },
diff --git a/tools/read-dependencies b/tools/read-dependencies
index 693d21130..23d00163f 100755
--- a/tools/read-dependencies
+++ b/tools/read-dependencies
@@ -126,6 +126,9 @@ def get_package_deps_from_json(topdir, distro):
         deps = json.loads(stream.read())
     if distro is None:
         return {}
+    if deps.get(distro):  # If we have a specific distro defined, use it.
+        return deps[distro]
+    # Use generic distro dependency map via DISTRO_PKG_TYPE_MAP
     return deps[DISTRO_PKG_TYPE_MAP[distro]]
 
 
@@ -162,7 +165,9 @@ def translate_pip_to_system_pkg(pip_requires, renames):
     for pip_name in pip_requires:
         pip_name = pip_name.lower()
         # Find a rename if present for the distro package and python version
-        rename = renames.get(pip_name, {})
+        rename = renames.get(pip_name, "")
+        if rename.startswith("OMIT:"):
+            continue
         if rename:
             translated_names.append(rename)
         else:
diff --git a/tools/run-container b/tools/run-container
index c25ebfc68..31a39b069 100755
--- a/tools/run-container
+++ b/tools/run-container
@@ -463,11 +463,10 @@ main() {
         return
     }
 
-    local rdcmd=(python3 tools/read-dependencies "--distro=${OS_NAME}" --install)
+    local rdcmd=(python3 tools/read-dependencies "--distro=${OS_NAME}" --install --test-distro)
     if [[ ${OS_NAME} == centos ]]; then
-        error "WARNING: Can't setup CentOS for testing (missing deps, LP: #1867151)."
-    else
-        rdcmd+=('--test-distro')
+        error "WARNING: Unable to install all CentOS for test dependencies (missing deps, LP: #1867151)."
+        unittest=""
     fi
     inside_as_cd "$name" root "$cdir" "${rdcmd[@]}" || {
         errorrc "FAIL: failed to install dependencies with read-dependencies"

[paride]

Thanks for the review @blackboxsw, I agree on all the line with your suggestions. I applied your patch (split in 2 commits), rebased the branch resolving conflicts, and tested the actual copr build: it works! See:

https://copr.fedorainfracloud.org/coprs/g/cloud-init/cloud-init-dev/build/1356341/

[blackboxsw]

I think we can re-enable unittests now that tip has dropped unittest2 and contextlib2 deps.

[paride]

@blackboxsw thanks for reviewing again. I dropped unittext2 and contextlib2, dropped the OMIT logic, re-enabled the unittests in run-container, and rebased.

[blackboxsw]

Excellent @paride thanks for all the work on this!