Merge ~raharper/cloud-init:fix/fedora-build into cloud-init:master

[Ryan Harper]

update redhat specfile and tools to support python2 and python3

Note, this needs to rebase after the cloud-init-template.generator fix has landed in master.

[Ryan Harper (raharper)]

We should take an srpm out of this and test-build Fedora on COPR repo before landing.

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

FAILED: Continuous integration, rev:4cbd01cfd7e075ccf4279b931baef60028e42463
https://jenkins.ubuntu.com/server/job/cloud-init-ci/734/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

[Ryan Harper (raharper)]

srpms from this branch now pass all builders on copr

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

bd9d4b4... by Ryan Harper on 2019-06-18

drop fedora specific spec file

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

PASSED: Continuous integration, rev:8aa87376f1338970d21933f862f94f89f9d73566
https://jenkins.ubuntu.com/server/job/cloud-init-ci/738/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

[Dan Watkins (oddbloke)]

Overall this looks good, thanks! Just a few inline comments/questions.

review: Needs Information

[Ryan Harper (raharper)]

Thanks for the review; this needs some testing and I can see if we can keep the install section the same save the interpreter we run.

[Dan Watkins (oddbloke)]

Thanks for the responses, Ryan. The two outstanding issues I believe we have are: ensuring we haven't broken RH builds, and working out if we can/should run setup.py for Python 3 RPM builds.

[Dan Watkins (oddbloke)]

Oh, yep, just saw your summary comment; we are in agreement. :)

cab1663... by Ryan Harper on 2019-06-19

Consolidate %install section by defining python_cmd value and always passing variant

2b5163b... by Ryan Harper on 2019-06-19

setup.py takes --distro for variant

12efb6d... by Ryan Harper on 2019-06-19

setup.py is awkward on arg parsing for --distro

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

FAILED: Continuous integration, rev:12efb6d3e6d07664db030e295d18fd872b867461
https://jenkins.ubuntu.com/server/job/cloud-init-ci/746/
Executed test runs:
    FAILED: Checkout

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

review: Needs Fixing (continuous-integration)

[Chad Smith (chad.smith)]

I think we may have to solve the buildrequires_py* om the specfile and possibly how to copy with httpretty on centos8 for the package name changes?

review: Needs Fixing

[Ryan Harper (raharper)]

Thanks. I've a few questions for clarification.

Unmerged commits

12efb6d... by Ryan Harper on 2019-06-19

setup.py is awkward on arg parsing for --distro

2b5163b... by Ryan Harper on 2019-06-19

setup.py takes --distro for variant

cab1663... by Ryan Harper on 2019-06-19

Consolidate %install section by defining python_cmd value and always passing variant

bd9d4b4... by Ryan Harper on 2019-06-18

drop fedora specific spec file

5336b5f... by Ryan Harper on 2019-06-18

drop devel deps, fetch os_variant and pass to render-cloudcfg

8b97cb8... by Ryan Harper on 2019-06-18

have build env pick whether it uses python3 or not

1045123... by Ryan Harper on 2019-06-18

add --python2 param to brpm call if target build env is using py2

d952e8d... by Ryan Harper on 2019-06-18

fix py3 macros and sitelib paths

25c3eeb... by Ryan Harper on 2019-06-18

add pyver to template

88d85a1... by Ryan Harper on 2019-06-18

work out how to pass variant and skip build for py3

diff --git a/packages/brpm b/packages/brpm
index a154ef2..502cfb4 100755
--- a/packages/brpm
+++ b/packages/brpm
@@ -42,13 +42,14 @@ def run_helper(helper, args=None, strip=True):
     return stdout
 
 
-def read_dependencies(distro, requirements_file='requirements.txt'):
+def read_dependencies(distro, pyver, requirements_file='requirements.txt'):
     """Returns the Python package depedencies from requirements.txt files.
 
     @returns a tuple of (requirements, test_requirements)
     """
     pkg_deps = run_helper(
-        'read-dependencies', args=['--distro', distro]).splitlines()
+        'read-dependencies', args=['--distro', distro,
+                                   '--python-version', pyver]).splitlines()
     test_deps = run_helper(
         'read-dependencies', args=[
             '--requirements-file', 'test-requirements.txt',
@@ -83,9 +84,10 @@ 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
 
-    deps, test_deps = read_dependencies(distro=args.distro)
-    subs['buildrequires'] = deps + test_deps
-    subs['requires'] = deps
+    for pyver in ['2', '3']:
+        deps, test_deps = read_dependencies(distro=args.distro, pyver=pyver)
+        subs['buildrequires_py%s' % pyver] = deps + test_deps
+        subs['requires_py%s' % pyver] = deps
 
     if args.boot == 'sysvinit':
         subs['sysvinit'] = True
@@ -108,7 +110,7 @@ def main():
     parser.add_argument("-d", "--distro", dest="distro",
                         help="select distro (default: %(default)s)",
                         metavar="DISTRO", default='redhat',
-                        choices=('redhat', 'suse'))
+                        choices=('fedora', 'redhat', 'suse'))
     parser.add_argument('--srpm',
                         help='Produce a source rpm',
                         action='store_true')
@@ -155,6 +157,12 @@ def main():
             'make-tarball', ['--long', '--output=' + real_archive_fn])
         print("Archived the code in %r" % (real_archive_fn))
 
+        if args.distro == "fedora":
+            args.distro = "redhat"
+            args.variant = "fedora"
+        else:
+            args.variant = args.distro
+
         # Form the spec file to be used
         tmpl_fn = util.abs_join(find_root(), 'packages',
                                 args.distro, 'cloud-init.spec.in')
@@ -189,7 +197,7 @@ def main():
         for rpm_fn in globs:
             tgt_fn = util.abs_join(os.getcwd(), os.path.basename(rpm_fn))
             shutil.move(rpm_fn, tgt_fn)
-            print("Wrote out %s package %r" % (args.distro, tgt_fn))
+            print("Wrote out %s package %r" % (args.variant, tgt_fn))
     finally:
         if workdir is not None:
             shutil.rmtree(workdir)
diff --git a/packages/pkg-deps.json b/packages/pkg-deps.json
index 72409dd..0470187 100644
--- a/packages/pkg-deps.json
+++ b/packages/pkg-deps.json
@@ -24,32 +24,43 @@
    },
    "redhat" : {
       "build-requires" : [
-         "python-devel",
-         "python-setuptools"
+         "xfsprogs",
+         "rpm-build"
       ],
       "renames" : {
+         "httpretty" : {
+            "3" : "python3-httpretty"
+         },
          "jinja2" : {
-            "3" : "python34-jinja2"
+            "3" : "python3-jinja2"
          },
          "jsonschema" : {
-            "3" : "python34-jsonschema"
+            "3" : "python3-jsonschema"
+         },
+         "prettytable" : {
+            "3": "python3-prettytable"
          },
          "pyflakes" : {
             "2" : "pyflakes",
-            "3" : "python34-pyflakes"
+            "3" : "python3-pyflakes"
          },
          "pyyaml" : {
             "2" : "PyYAML",
-            "3" : "python34-PyYAML"
+            "3" : "python3-PyYAML"
          },
          "pyserial" : {
-            "2" : "pyserial"
+            "2" : "pyserial",
+            "3" : "python3-pyserial"
          },
          "requests" : {
-            "3" : "python34-requests"
+            "3" : "python3-requests"
          },
+         "setuptools" : {
+             "2" : "python-setuptools",
+             "3" : "python3-setuptools"
+        },
          "six" : {
-            "3" : "python34-six"
+            "3" : "python3-six"
          }
       },
       "requires" : [
@@ -59,7 +70,9 @@
          "procps",
          "rsyslog",
          "shadow-utils",
-         "sudo"
+         "sudo",
+         "util-linux",
+         "xfsprogs"
       ]
    },
    "suse" : {
diff --git a/packages/redhat/cloud-init.spec.in b/packages/redhat/cloud-init.spec.in
index 057a578..0746bcd 100644
--- a/packages/redhat/cloud-init.spec.in
+++ b/packages/redhat/cloud-init.spec.in
@@ -2,6 +2,8 @@
 %{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print get_python_lib()")}
 
 %define use_systemd (0%{?fedora} && 0%{?fedora} >= 18) || (0%{?rhel} && 0%{?rhel} >= 7)
+%define use_python3 (0%{?fedora} && 0%{?fedora} >= 29) || (0%{?rhel} && 0%{?rhel} >= 8)
+%define os_variant %(source /etc/os-release; echo $ID ;)
 
 %if %{use_systemd}
 %define init_system systemd
@@ -9,6 +11,14 @@
 %define init_system sysvinit
 %endif
 
+%if %{use_python3}
+%define python_major 3
+%define python_cmd python3
+%define python_minor %(%{python_cmd} -c "import sys; %sys.stdout.write(str(sys.version_info.minor))")
+%else
+%define python_cmd python
+%endif
+
 # See: http://www.zarb.org/~jasonc/macros.php
 # Or: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets
 # Or: http://www.rpm.org/max-rpm/ch-rpm-inside.html
@@ -41,12 +51,29 @@ Requires(post):     chkconfig
 Requires(preun):    chkconfig
 %endif
 
+%if "%{python_major}" == "3"
+BuildRequires: python3-devel
+BuildRequires: python3-setuptools
+%if "%{python_minor}" >= "8"
+BuildRequires: python3-distro
+%endif
+%else
+BuildRequires: python-devel
+BuildRequires: python-setuptools
+%endif
+
 # These are runtime dependencies, but declared as BuildRequires so that
 # - tests can be run here.
 # - parts of cloud-init such (setup.py) use these dependencies.
-{% for r in requires %}
+%if %{use_python3}
+{% for r in requires_py3 %}
 BuildRequires:  {{r}}
 {% endfor %}
+%else
+{% for r in requires_py2 %}
+BuildRequires:  {{r}}
+{% endfor %}
+%endif
 
 # System util packages needed
 %ifarch %{?ix86} x86_64 ia64
@@ -58,11 +85,16 @@ Requires:       dmidecode
 Requires:  python-argparse
 %endif
 
-
 # Install 'dynamic' runtime reqs from *requirements.txt and pkg-deps.json
-{% for r in requires %}
+%if %{use_python3}
+{% for r in requires_py3 %}
+Requires:  {{r}}
+{% endfor %}
+%else
+{% for r in requires_py2 %}
 Requires:       {{r}}
 {% endfor %}
+%endif
 
 # Custom patches
 {% for p in patches %}
@@ -93,14 +125,21 @@ ssh keys and to let the user run various scripts.
 %patch{{loop.index0}} -p1
 {% endfor %}
 
+%if "%{python_major}" == "3"
+# Change shebangs
+sed -i -e 's|#!/usr/bin/env python|#!/usr/bin/env python3|' \
+       -e 's|#!/usr/bin/python|#!/usr/bin/python3|' tools/* cloudinit/ssh_util.py
+%endif
+
+
 %build
-%{__python} setup.py build
+%{python_cmd} setup.py build
 
-%install
 
-%{__python} setup.py install -O1 \
+%install
+%{python_cmd} setup.py install -O1 \
             --skip-build --root $RPM_BUILD_ROOT \
-            --init-system=%{init_system}
+            --init-system=%{init_system} --distro %{os_variant}
 
 # Note that /etc/rsyslog.d didn't exist by default until F15.
 # el6 request: https://bugzilla.redhat.com/show_bug.cgi?id=740420
@@ -109,7 +148,11 @@ cp -p tools/21-cloudinit.conf \
       $RPM_BUILD_ROOT/%{_sysconfdir}/rsyslog.d/21-cloudinit.conf
 
 # Remove the tests
+%if "%{python_major}" == "3"
+rm -rf $RPM_BUILD_ROOT%{python3_sitelib}/tests
+%else
 rm -rf $RPM_BUILD_ROOT%{python_sitelib}/tests
+%endif
 
 # Required dirs...
 mkdir -p $RPM_BUILD_ROOT/%{_sharedstatedir}/cloud
@@ -122,11 +165,12 @@ version_pys=$(cd "$RPM_BUILD_ROOT" && find . -name version.py -type f)
 ( cd "$RPM_BUILD_ROOT" &&
   sed -i "s,@@PACKAGED_VERSION@@,%{version}-%{release}," $version_pys )
 
+
 %clean
 rm -rf $RPM_BUILD_ROOT
 
-%post
 
+%post
 %if "%{init_system}" == "systemd"
 if [ $1 -eq 1 ]
 then
@@ -142,8 +186,8 @@ fi
 /sbin/chkconfig --add %{_initrddir}/cloud-final
 %endif
 
-%preun
 
+%preun
 %if "%{init_system}" == "systemd"
 if [ $1 -eq 0 ]
 then
@@ -166,14 +210,14 @@ then
 fi
 %endif
 
-%postun
 
+%postun
 %if "%{init_system}" == "systemd"
 /bin/systemctl daemon-reload >/dev/null 2>&1 || :
 %endif
 
-%files
 
+%files
 /lib/udev/rules.d/66-azure-ephemeral.rules
 
 %if "%{init_system}" == "systemd"
@@ -188,7 +232,6 @@ fi
 
 %{_sysconfdir}/NetworkManager/dispatcher.d/hook-network-manager
 %{_sysconfdir}/dhcp/dhclient-exit-hooks.d/hook-dhclient
-
 # Program binaries
 %{_bindir}/cloud-init*
 %{_bindir}/cloud-id*
@@ -213,4 +256,8 @@ fi
 %dir %{_sharedstatedir}/cloud
 
 # Python code is here...
+%if "%{python_major}" == "3"
+%{python3_sitelib}/*
+%else
 %{python_sitelib}/*
+%endif
diff --git a/tools/read-dependencies b/tools/read-dependencies
index b4656e6..77c2ec5 100755
--- a/tools/read-dependencies
+++ b/tools/read-dependencies
@@ -23,6 +23,7 @@ DEFAULT_REQUIREMENTS = 'requirements.txt'
 # Map the appropriate package dir needed for each distro choice
 DISTRO_PKG_TYPE_MAP = {
     'centos': 'redhat',
+    'fedora': 'fedora',
     'redhat': 'redhat',
     'debian': 'debian',
     'ubuntu': 'debian',
@@ -51,17 +52,41 @@ MAYBE_RELIABLE_YUM_INSTALL = [
     """,
     'reliable-yum-install']
 
+MAYBE_RELIABLE_DNF_INSTALL = [
+    'sh', '-c',
+    """
+    error() { echo "$@" 1>&2; }
+    n=0; max=10;
+    bcmd="dnf install --downloadonly --assumeyes --setopt=keepcache=1"
+    while n=$(($n+1)); do
+       error ":: running $bcmd $* [$n/$max]"
+       $bcmd "$@"
+       r=$?
+       [ $r -eq 0 ] && break
+       [ $n -ge $max ] && { error "gave up on $bcmd"; exit $r; }
+       nap=$(($n*5))
+       error ":: failed [$r] ($n/$max). sleeping $nap."
+       sleep $nap
+    done
+    error ":: running dnf install --cacheonly --assumeyes $*"
+    dnf install --cacheonly --assumeyes "$@"
+    """,
+    'reliable-dnf-install']
+
+
 ZYPPER_INSTALL = [
     'zypper', '--non-interactive', '--gpg-auto-import-keys', 'install',
     '--auto-agree-with-licenses']
 
 DRY_DISTRO_INSTALL_PKG_CMD = {
     'centos': ['yum', 'install', '--assumeyes'],
+    'fedora': ['dnf', 'install', '--assumeyes'],
     'redhat': ['yum', 'install', '--assumeyes'],
 }
 
 DISTRO_INSTALL_PKG_CMD = {
     'centos': MAYBE_RELIABLE_YUM_INSTALL,
+    'fedora': MAYBE_RELIABLE_DNF_INSTALL,
     'redhat': MAYBE_RELIABLE_YUM_INSTALL,
     'debian': ['apt', 'install', '-y'],
     'ubuntu': ['apt', 'install', '-y'],
@@ -73,6 +98,7 @@ DISTRO_INSTALL_PKG_CMD = {
 # List of base system packages required to enable ci automation
 CI_SYSTEM_BASE_PKGS = {
     'common': ['make', 'sudo', 'tar'],
+    'fedora': ['python3-tox'],
     'redhat': ['python-tox'],
     'centos': ['python-tox'],
     'ubuntu': ['devscripts', 'python3-dev', 'libssl-dev', 'tox', 'sbuild'],
@@ -192,6 +218,13 @@ def main(distro):
     else:
         topd = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 
+    # allow fedora to use redhat, mark it as variant
+    if args.distro in ['fedora']:
+        args.variant = args.distro
+        args.distro = 'redhat'
+    else:
+        args.variant = args.distro
+
     if args.test_distro:
         # Give us all the system deps we need for continuous integration
         if args.req_files:
@@ -234,7 +267,7 @@ def main(distro):
         else:
             all_deps = pip_pkg_names
     if args.install:
-        pkg_install(all_deps, args.distro, args.test_distro, args.dry_run)
+        pkg_install(all_deps, args.variant, args.test_distro, args.dry_run)
     else:
         print('\n'.join(all_deps))
 
diff --git a/tools/run-container b/tools/run-container
index 1d24e15..114803c 100755
--- a/tools/run-container
+++ b/tools/run-container
@@ -218,9 +218,10 @@ get_os_info() {
         { error "Unable to determine OS_NAME/OS_VERSION"; return 1; }
 }
 
-yum_install() {
+_rpm_install() {
+    install_cmd="$1"; shift;
     local n=0 max=10 ret
-    bcmd="yum install --downloadonly --assumeyes --setopt=keepcache=1"
+    bcmd="$install_cmd install --downloadonly --assumeyes --setopt=keepcache=1"
     while n=$((n+1)); do
        error ":: running $bcmd $* [$n/$max]"
        $bcmd "$@"
@@ -231,8 +232,16 @@ yum_install() {
        error ":: failed [$ret] ($n/$max). sleeping $nap."
        sleep $nap
     done
-    error ":: running yum install --cacheonly --assumeyes $*"
-    yum install --cacheonly --assumeyes "$@"
+    error ":: running $install_cmd install --cacheonly --assumeyes $*"
+    $install_cmd install --cacheonly --assumeyes "$@"
+}
+
+yum_install() {
+    _rpm_install "yum" "$@"
+}
+
+dnf_install() {
+    _rpm_install "dnf" "$@"
 }
 
 zypper_install() {
@@ -250,6 +259,7 @@ apt_install() {
 install_packages() {
     get_os_info || return
     case "$OS_NAME" in
+        fedora) dnf_install "$@";;
         centos) yum_install "$@";;
         opensuse) zypper_install "$@";;
         debian|ubuntu) apt_install "$@";;
@@ -492,8 +502,15 @@ main() {
         return
     }
 
+    local distflag=""
+    case "$OS_NAME" in
+        centos) distflag="--distro=redhat";;
+        fedora) distflag="--distro=fedora";;
+        opensuse) distflag="--distro=suse";;
+    esac
+
     inside_as_cd "$name" root "$cdir" \
-        $pyexe ./tools/read-dependencies "--distro=${OS_NAME}" \
+        $pyexe ./tools/read-dependencies "$distflag" \
             --test-distro || {
         errorrc "FAIL: failed to install dependencies with read-dependencies"
         return
@@ -514,18 +531,13 @@ main() {
             }
     fi
 
-    local build_pkg="" build_srcpkg="" pkg_ext="" distflag=""
-    case "$OS_NAME" in
-        centos) distflag="--distro=redhat";;
-        opensuse) distflag="--distro=suse";;
-    esac
-
+    local build_pkg="" build_srcpkg="" pkg_ext="" pyver=""
     case "$OS_NAME" in
         debian|ubuntu)
-            build_pkg="./packages/bddeb -d" 
+            build_pkg="./packages/bddeb -d"
             build_srcpkg="./packages/bddeb -S -d"
             pkg_ext=".deb";;
-        centos|opensuse)
+        centos|fedora|opensuse|redhat)
             build_pkg="./packages/brpm $distflag"
             build_srcpkg="./packages/brpm $distflag --srpm"
             pkg_ext=".rpm";;