Merge ~chad.smith/cloud-init:pyver-fix into cloud-init:master

[Chad Smith]

makefile: fix python 2/3 detection in the Makefile

Fix detection of python2 in a non-python3 environment.
The previous version ahead ended up using python 3.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:a4f5cf8dfc587b59662af3754146d85798263292
https://jenkins.ubuntu.com/server/job/cloud-init-ci/478/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/478/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/478/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/478/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/478/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/478/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/478/rebuild

review: Needs Fixing (continuous-integration)

[Scott Moser (smoser)]

I'm going to grab this.
your request included a bunch of unrelated stuff, so I'll just cherry pick the Makefile commit.
thanks.

review: Approve

diff --git a/Makefile b/Makefile
index 09cd147..1cec9ae 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@
 CWD=$(shell pwd)
 PYVER ?= $(shell for p in python3 python2; do \
-	out=$(which $$p 2>&1) && echo $$p && exit; done; \
+	out=$$(command -v $$p 2>&1) && echo $$p && exit; done; \
 	exit 1)
 noseopts ?= -v
 
@@ -53,6 +53,12 @@ unittest: clean_pyc
 unittest3: clean_pyc
 	nosetests3 $(noseopts) tests/unittests
 
+ci-deps-ubuntu:
+	@$(PYVER) $(CWD)/tools/read-dependencies --distro ubuntu --install
+
+ci-deps-centos:
+	@$(PYVER) $(CWD)/tools/read-dependencies --distro centos --install
+
 pip-requirements:
 	@echo "Installing cloud-init dependencies..."
 	$(PIP_INSTALL) -r "$@.txt" -q
@@ -75,6 +81,7 @@ clean_pyc:
 clean: clean_pyc
 	rm -rf /var/log/cloud-init.log /var/lib/cloud/
 
+
 yaml:
 	@$(PYVER) $(CWD)/tools/validate-yaml.py $(YAML_FILES)
 
diff --git a/packages/bddeb b/packages/bddeb
index f415209..e45af6e 100755
--- a/packages/bddeb
+++ b/packages/bddeb
@@ -24,19 +24,6 @@ if "avoid-pep8-E402-import-not-top-of-file":
     from cloudinit import templater
     from cloudinit import util
 
-# Package names that will showup in requires which have unique package names.
-# Format is '<pypi-name>': {'<python_major_version>': <pkg_name_or_none>, ...}.
-NONSTD_NAMED_PACKAGES = {
-    'argparse': {'2': 'python-argparse', '3': None},
-    'contextlib2': {'2': 'python-contextlib2', '3': None},
-    'cheetah': {'2': 'python-cheetah', '3': None},
-    'pyserial': {'2': 'python-serial', '3': 'python3-serial'},
-    'pyyaml': {'2': 'python-yaml', '3': 'python3-yaml'},
-    'six': {'2': 'python-six', '3': 'python3-six'},
-    'pep8': {'2': 'pep8', '3': 'python3-pep8'},
-    'pyflakes': {'2': 'pyflakes', '3': 'pyflakes'},
-}
-
 DEBUILD_ARGS = ["-S", "-d"]
 
 
@@ -59,7 +46,6 @@ def write_debian_folder(root, templ_data, is_python2, cloud_util_deps):
     else:
         pyver = "3"
         python = "python3"
-    pkgfmt = "{}-{}"
 
     deb_dir = util.abs_join(root, 'debian')
 
@@ -74,30 +60,23 @@ def write_debian_folder(root, templ_data, is_python2, cloud_util_deps):
                              params=templ_data)
 
     # Write out the control file template
-    reqs = run_helper('read-dependencies').splitlines()
+    reqs_output = run_helper(
+        'read-dependencies',
+        args=['--distro', 'debian', '--python-version', pyver])
+    reqs = reqs_output.splitlines()
     test_reqs = run_helper(
-        'read-dependencies', ['test-requirements.txt']).splitlines()
-
-    pypi_pkgs = [p.lower().strip() for p in reqs]
-    pypi_test_pkgs = [p.lower().strip() for p in test_reqs]
+        'read-dependencies',
+        ['--requirements-file', 'test-requirements.txt',
+         '--system-pkg-names', '--python-version', pyver]).splitlines()
 
-    # Map to known packages
     requires = ['cloud-utils | cloud-guest-utils'] if cloud_util_deps else []
-    test_requires = []
-    lists = ((pypi_pkgs, requires), (pypi_test_pkgs, test_requires))
-    for pypilist, target in lists:
-        for p in pypilist:
-            if p in NONSTD_NAMED_PACKAGES:
-                if NONSTD_NAMED_PACKAGES[p][pyver]:
-                    target.append(NONSTD_NAMED_PACKAGES[p][pyver])
-            else:  # Then standard package prefix
-                target.append(pkgfmt.format(python, p))
-
+    # We consolidate all deps as Build-Depends as our package build runs all
+    # tests so we need all runtime dependencies anyway.
+    requires.extend(reqs + test_reqs + [python])
     templater.render_to_file(util.abs_join(find_root(),
                                            'packages', 'debian', 'control.in'),
                              util.abs_join(deb_dir, 'control'),
-                             params={'requires': ','.join(requires),
-                                     'test_requires': ','.join(test_requires),
+                             params={'build_depends': ','.join(requires),
                                      'python': python})
 
     templater.render_to_file(util.abs_join(find_root(),
diff --git a/packages/brpm b/packages/brpm
index 89696ab..b8e7c72 100755
--- a/packages/brpm
+++ b/packages/brpm
@@ -27,17 +27,6 @@ if "avoid-pep8-E402-import-not-top-of-file":
     from cloudinit import templater
     from cloudinit import util
 
-# Map python requirements to package names.  If a match isn't found
-# here, we assume 'python-<pypi_name>'.
-PACKAGE_MAP = {
-    'redhat': {
-        'pyserial': 'pyserial',
-        'pyyaml': 'PyYAML',
-    },
-    'suse': {
-        'pyyaml': 'python-yaml',
-    }
-}
 
 # Subdirectories of the ~/rpmbuild dir
 RPM_BUILD_SUBDIRS = ['BUILD', 'RPMS', 'SOURCES', 'SPECS', 'SRPMS']
@@ -57,19 +46,15 @@ def read_dependencies():
     '''Returns the Python depedencies from requirements.txt.  This explicitly
     removes 'argparse' from the list of requirements for python >= 2.7,
     because with 2.7 argparse became part of the standard library.'''
-    stdout = run_helper('read-dependencies')
-    return [p.lower().strip() for p in stdout.splitlines()
-            if p != 'argparse' or (p == 'argparse' and
-                                   sys.version_info[0:2] < (2, 7))]
-
-
-def translate_dependencies(deps, distro):
-    '''Maps python requirements into package names.  We assume
-    python-<pypi_name> for packages not listed explicitly in
-    PACKAGE_MAP.'''
-    return [PACKAGE_MAP[distro][req]
-            if req in PACKAGE_MAP[distro] else 'python-%s' % req
-            for req in deps]
+    pkg_deps = run_helper(
+        'read-dependencies', args=['--distro', 'redhat']).splitlines()
+    test_deps = run_helper(
+        'read-dependencies', args=[
+            '--requirements-file', 'test-requirements.txt',
+            '--system-pkg-names']).splitlines()
+    return [pkg_dep for pkg_dep in pkg_deps + test_deps
+            if p != 'python-argparse' or (p == 'python-argparse' and
+                                          sys.version_info[0:2] < (2, 7))]
 
 
 def read_version():
@@ -99,10 +84,9 @@ def generate_spec_contents(args, version_data, tmpl_fn, top_dir, arc_fn):
         rpm_upstream_version = version_data['version']
     subs['rpm_upstream_version'] = rpm_upstream_version
 
-    # Map to known packages
-    python_deps = read_dependencies()
-    package_deps = translate_dependencies(python_deps, args.distro)
-    subs['requires'] = package_deps
+    deps = read_dependencies()
+    subs['buildrequires'] = deps
+    subs['requires'] = deps
 
     if args.boot == 'sysvinit':
         subs['sysvinit'] = True
diff --git a/packages/debian/control.in b/packages/debian/control.in
index 6c39d53..265b261 100644
--- a/packages/debian/control.in
+++ b/packages/debian/control.in
@@ -3,20 +3,13 @@ Source: cloud-init
 Section: admin
 Priority: optional
 Maintainer: Scott Moser <[e-mail]>
-Build-Depends: debhelper (>= 9),
-               dh-python,
-               dh-systemd,
-               ${python},
-               ${test_requires},
-               ${requires}
+Build-Depends: ${build_depends}
 XS-Python-Version: all
 Standards-Version: 3.9.6
 
 Package: cloud-init
 Architecture: all
-Depends: procps,
-         ${python},
-         ${misc:Depends},
+Depends: ${misc:Depends},
          ${${python}:Depends}
 Recommends: eatmydata, sudo, software-properties-common, gdisk
 XB-Python-Version: ${python:Versions}
diff --git a/packages/pkg-deps.json b/packages/pkg-deps.json
new file mode 100644
index 0000000..042cde7
--- /dev/null
+++ b/packages/pkg-deps.json
@@ -0,0 +1 @@
+{"debian": {"build-requires": ["debhelper", "dh-python", "dh-systemd"], "requires": ["procps"], "renames": {"argparse": {"2": "python-argparse", "3": null},"contextlib2": {"2": "python-contextlib2", "3": null}, "cheetah": {"2": "python-cheetah", "3": null}, "pyserial": {"2": "python-serial", "3": "python3-serial"}, "pyyaml": {"2": "python-yaml", "3": "python3-yaml"}, "six": {"2": "python-six", "3": "python3-six"}, "pep8": {"2": "pep8", "3": "python3-pep8"}, "pyflakes": {"2": "pyflakes", "3": "pyflakes"}}}, "redhat": {"requires": ["shadow-utils", "rsyslog", "iproute", "e2fsprogs", "net-tools", "procps", "shadow-utils", "sudo >= 1.7.2p2-3"], "build-requires": ["python-devel", "python-setuptools", "python-cheetah"], "renames": {"pyserial": {"2": "pyserial", "3": null}, "pyyaml": {"2": "PyYAML", "3": null}}}, "suse": {"build-requires": ["fdupes", "filesystem", "python-devel", "python-setuptools", "python-cheetah"], "requires": ["iproute2", "e2fsprogs", "net-tools", "procps", "sudo"], "renames": {"pyyaml": {"2": "python-yaml", "3": null}}}}
diff --git a/packages/redhat/cloud-init.spec.in b/packages/redhat/cloud-init.spec.in
index fd3cf93..a62d860 100644
--- a/packages/redhat/cloud-init.spec.in
+++ b/packages/redhat/cloud-init.spec.in
@@ -18,21 +18,12 @@ Source0:        ${archive_name}
 BuildArch:      noarch
 BuildRoot:      %{_tmppath}
 
-BuildRequires:  python-devel
-BuildRequires:  python-setuptools
-BuildRequires:  python-cheetah
-
-# System util packages needed
-Requires:       shadow-utils
-Requires:       rsyslog
-Requires:       iproute
-Requires:       e2fsprogs
-Requires:       net-tools
-Requires:       procps
-Requires:       shadow-utils
-Requires:       sudo >= 1.7.2p2-3
-
-# Install pypi 'dynamic' requirements
+# Install 'dynamic' build reqs from *requirements.txt and pkg-deps.json
+#for $r in $buildrequires
+BuildRequires:  ${r}
+#end for
+
+# Install 'dynamic' runtime reqs from *requirements.txt and pkg-deps.json
 #for $r in $requires
 Requires:       ${r}
 #end for
diff --git a/packages/suse/cloud-init.spec.in b/packages/suse/cloud-init.spec.in
index 6ce0be8..444401c 100644
--- a/packages/suse/cloud-init.spec.in
+++ b/packages/suse/cloud-init.spec.in
@@ -22,11 +22,9 @@ BuildRoot:      %{_tmppath}/%{name}-%{version}-build
 BuildArch:      noarch
 %endif
 
-BuildRequires:        fdupes
-BuildRequires:        filesystem
-BuildRequires:        python-devel
-BuildRequires:        python-setuptools
-BuildRequires:        python-cheetah
+#for $r in $buildrequires
+BuildRequires:        ${r}
+#end for
 
 %if 0%{?suse_version} && 0%{?suse_version} <= 1210
   %define initsys sysvinit
@@ -34,13 +32,6 @@ BuildRequires:        python-cheetah
   %define initsys systemd
 %endif
 
-# System util packages needed
-Requires:       iproute2
-Requires:       e2fsprogs
-Requires:       net-tools
-Requires:       procps
-Requires:       sudo
-
 # Install pypi 'dynamic' requirements
 #for $r in $requires
 Requires:       ${r}
diff --git a/tools/read-dependencies b/tools/read-dependencies
index f434905..e01e718 100755
--- a/tools/read-dependencies
+++ b/tools/read-dependencies
@@ -1,43 +1,183 @@
 #!/usr/bin/env python
+"""List pip dependencies or system package dependencies for cloud-init."""
 
 # You might be tempted to rewrite this as a shell script, but you
 # would be surprised to discover that things like 'egrep' or 'sed' may
 # differ between Linux and *BSD.
 
+try:
+    from argparse import ArgumentParser
+except ImportError:
+    raise RuntimeError(
+        'Could not import python-argparse. Please install python-argparse '
+        'package to continue')
+
+import json
 import os
 import re
-import sys
 import subprocess
+import sys
 
-if 'CLOUD_INIT_TOP_D' in os.environ:
-    topd = os.path.realpath(os.environ.get('CLOUD_INIT_TOP_D'))
-else:
-    topd = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 
-for fname in ("setup.py", "requirements.txt"):
-    if not os.path.isfile(os.path.join(topd, fname)):
-        sys.stderr.write("Unable to locate '%s' file that should "
-                         "exist in cloud-init root directory." % fname)
-        sys.exit(1)
-
-if len(sys.argv) > 1:
-    reqfile = sys.argv[1]
-else:
-    reqfile = "requirements.txt"
-
-with open(os.path.join(topd, reqfile), "r") as fp:
-    for line in fp:
-        line = line.strip()
-        if not line or line.startswith("#"):
+# Map the appropriate package dir needed for each distro choice
+DISTRO_PKG_TYPE_MAP = {
+    'centos': 'redhat',
+    'redhat': 'redhat',
+    'debian': 'debian',
+    'ubuntu': 'debian',
+    'opensuse': 'suse',
+    'suse': 'suse'
+}
+
+DISTRO_INSTALL_PKG_CMD = {
+    'cent': ['yum', 'install'],
+    'redhat': ['yum', 'install'],
+    'debian': ['apt', 'install'],
+    'ubuntu': ['apt', 'install'],
+    'opensuse': ['zypper', 'install'],
+    'suse': ['zypper', 'install']
+}
+
+PY_26_ONLY = ['argparse']
+
+
+# JSON definition of distro-specific package dependencies
+DISTRO_PKG_DEPS_PATH = "packages/pkg-deps.json"
+
+
+def get_parser():
+    """Return an argument parser for this command."""
+    parser = ArgumentParser(description=__doc__)
+    parser.add_argument(
+        '-r', '--requirements-file', type=str, dest='req_file',
+        default='requirements.txt', help='The pip-style requirements file')
+    parser.add_argument(
+        '-d', '--distro', type=str, choices=DISTRO_PKG_TYPE_MAP.keys(),
+        help='The name of the distro to generate package deps for.')
+    parser.add_argument(
+        '-s', '--system-pkg-names', action='store_true', default=False,
+        dest='system_pkg_names',
+        help='The name of the distro to generate package deps for.')
+    parser.add_argument(
+        '-i', '--install', action='store_true', default=False,
+        dest='install',
+        help='When specified, install the required system packages.')
+    parser.add_argument(
+        '-v', '--python-version', type=str, dest='python_version', default="2",
+        choices=["2", "3"],
+        help='The version of python we want to generate system package '
+             'dependencies for.')
+    return parser
+
+
+def get_package_deps_from_json(topdir, distro):
+    """Get a dict of build and runtime package requirements for a distro.
+
+    @param topdir: The root directory in which to search for the
+        DISTRO_PKG_DEPS_PATH json blob of package requirements information.
+    @param distro: The specific distribution shortname to pull dependencies
+        for.
+    @return: Dict containing "requires", "build-requires" and "rename" lists
+        for a given distribution.
+    """
+    with open(os.path.join(topdir, DISTRO_PKG_DEPS_PATH), 'r') as stream:
+        deps = json.loads(stream.read())
+    if distro is None:
+        return {}
+    return deps[DISTRO_PKG_TYPE_MAP[distro]]
+
+
+def parse_pip_requirements(requirements_path):
+    """Return the pip requirement names from pip-style requirements_path."""
+    dep_names = []
+    with open(requirements_path, "r") as fp:
+        for line in fp:
+            line = line.strip()
+            if not line or line.startswith("#"):
+                continue
+
+            # remove pip-style markers
+            dep = line.split(';')[0]
+
+            # remove version requirements
+            if re.search('[>=.<]+', dep):
+                dep_names.append(re.split("[>=.<]*", dep)[0].strip())
+            else:
+                dep_names.append(dep)
+    return dep_names
+
+
+def translate_pip_to_system_pkg(pip_requires, renames, python_ver="2"):
+    """Translate pip package names to distro-specific package names.
+
+    @param pip_requires: List of versionless pip package names to translate.
+    @param renames: Dict containg special case renames from pip name to system
+        package name for the distro.
+    """
+    if python_ver == "2":
+        prefix = "python-"
+    else:
+        prefix = "python3-"
+    standard_pkg_name = "{0}{1}"
+    translated_names = []
+    for pip_name in pip_requires:
+        pip_name = pip_name.lower()
+        if pip_name in PY_26_ONLY and sys.version_info[0:2] > (2, 6):
+            # Skip PY26 pkg deps unless we are actually on python 2.6
             continue
+        # Find a rename if present for the distro package and python version
+        rename = renames.get(pip_name, {}).get(python_ver, None)
+        if rename:
+            translated_names.append(rename)
+        else:
+            translated_names.append(
+                standard_pkg_name.format(prefix, pip_name))
+    return translated_names
+
+
+def main(distro):
+    parser = get_parser()
+    args = parser.parse_args()
+    if 'CLOUD_INIT_TOP_D' in os.environ:
+        topd = os.path.realpath(os.environ.get('CLOUD_INIT_TOP_D'))
+    else:
+        topd = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
+    req_path = os.path.join(topd, args.req_file)
+    if not os.path.isfile(req_path):
+        sys.stderr.write("Unable to locate '%s' file that should "
+                         "exist in cloud-init root directory." % req_path)
+        return 1
+    pip_pkg_names = parse_pip_requirements(req_path)
+    deps_from_json = get_package_deps_from_json(topd, args.distro)
+    renames = deps_from_json.get('renames', {})
+    translated_pip_names = translate_pip_to_system_pkg(
+        pip_pkg_names, renames, args.python_version)
+    all_deps = []
+    if args.distro:
+        all_deps.extend(
+            translated_pip_names + deps_from_json['requires'] +
+            deps_from_json['build-requires'])
+    else:
+        if args.system_pkg_names:
+            all_deps = translated_pip_names
+        else:
+            all_deps = pip_pkg_names
+    if args.install:
+        pkg_install(all_deps, args.distro)
+    else:
+        print('\n'.join(all_deps))
+
 
-        # remove pip-style markers
-        dep = line.split(';')[0]
+def pkg_install(pkg_list, distro):
+    """Install a list of packages using the DISTRO_INSTALL_PKG_CMD"""
+    print("Installing deps:", ' '.join(pkg_list))
+    cmd = ['sudo'] + DISTRO_INSTALL_PKG_CMD[distro] + pkg_list
+    subprocess.check_call(cmd)
 
-        # remove version requirements
-        dep = re.split("[>=.<]*", dep)[0].strip()
-        print(dep)
 
-sys.exit(0)
+if __name__ == "__main__":
+    parser = get_parser()
+    args = parser.parse_args()
+    sys.exit(main(args.distro))
 
 # vi: ts=4 expandtab