Merge ~smoser/cloud-init:bug/fix-ci-redhat into cloud-init:master

[Scott Moser]

tools/run-centos: cleanups and move to using read-dependencies

These changes are all in an effort to get tools/run-centos using
read-dependencies rather than the 'setup-centos' script with a separate
set of dependencies listed.

 - tools/read-dependencies: support taking multiple --requirements options.
   This allows run-centos to get both test and build dependencies.
   Ultimately, I think it might be nicer for read-dependencies to take a
   list of "goals" (build, test, run or test-tox) rather than having the
   caller need to know to provide multiple --requirements.

 - packages/pkg-deps.json: drop the version on the sudo package.
   centos 6 has newer (1.8.6p3) version than listed, so its not a problem.

 - test_handler_disk_setup.py: a test case here was using assertLogs
   which is not present in the version of unittest2 that is available in
   centos 6 epel. We just adjust it to use with_logs = True.

 - tools/run-cents:
   - improve usage with example
   - add 'inside_as_cd' to provide the dir you want to cd first to.
   - avoid the intermediate tarball on disk in the container.
   - add 'prep' subcommand and use it to install pre-dependencies.
   - use read-dependencies.

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

PASSED: Continuous integration, rev:2d2ba08ad2beef90c5f2690ed835baa9638ac141
https://jenkins.ubuntu.com/server/job/cloud-init-ci/524/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/524
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/524
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/524
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/524
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/524

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

With this changeset applied i can:

for ver in 6 7; do
   ./tools/run-centos -vv $ver --unittest --rpm --srpm --artifact 2>&1 |
       tee out${ver}.log; done

[Joshua Powers (powersj)]

Only reviewed run-centos: clean up looks good and runs as expected locally. Thanks for taking the time to convert over to using read-dependencies.

review: Approve

[Chad Smith (chad.smith)]

Approve as is, but it'd be nice to check all requirements-files specified to read-dependencies.

review: Approve

[Scott Moser (smoser)]

Approving per Chad and Josh.

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

PASSED: Continuous integration, rev:c0e1a31e3c7abd5e193145e3b56b05dcd488d469
https://jenkins.ubuntu.com/server/job/cloud-init-ci/528/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/528
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/528
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/528
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/528
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/528

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

review: Approve (continuous-integration)

diff --git a/packages/pkg-deps.json b/packages/pkg-deps.json
index 8b8f3c3..822d29d 100644
--- a/packages/pkg-deps.json
+++ b/packages/pkg-deps.json
@@ -62,7 +62,7 @@
          "procps",
          "rsyslog",
          "shadow-utils",
-         "sudo >= 1.7.2p2-3"
+         "sudo"
       ]
    },
    "suse" : {
diff --git a/tests/unittests/test_handler/test_handler_disk_setup.py b/tests/unittests/test_handler/test_handler_disk_setup.py
index 916a0d7..8a6d49e 100644
--- a/tests/unittests/test_handler/test_handler_disk_setup.py
+++ b/tests/unittests/test_handler/test_handler_disk_setup.py
@@ -3,7 +3,7 @@
 import random
 
 from cloudinit.config import cc_disk_setup
-from ..helpers import ExitStack, mock, TestCase
+from ..helpers import CiTestCase, ExitStack, mock, TestCase
 
 
 class TestIsDiskUsed(TestCase):
@@ -174,32 +174,32 @@ class TestUpdateFsSetupDevices(TestCase):
             return_value=('/dev/xdb1', False))
 @mock.patch('cloudinit.config.cc_disk_setup.device_type', return_value=None)
 @mock.patch('cloudinit.config.cc_disk_setup.util.subp', return_value=('', ''))
-class TestMkfsCommandHandling(TestCase):
+class TestMkfsCommandHandling(CiTestCase):
+
+    with_logs = True
 
     def test_with_cmd(self, subp, *args):
         """mkfs honors cmd and logs warnings when extra_opts or overwrite are
         provided."""
-        with self.assertLogs(
-                'cloudinit.config.cc_disk_setup') as logs:
-            cc_disk_setup.mkfs({
-                'cmd': 'mkfs -t %(filesystem)s -L %(label)s %(device)s',
-                'filesystem': 'ext4',
-                'device': '/dev/xdb1',
-                'label': 'with_cmd',
-                'extra_opts': ['should', 'generate', 'warning'],
-                'overwrite': 'should generate warning too'
-            })
+        cc_disk_setup.mkfs({
+            'cmd': 'mkfs -t %(filesystem)s -L %(label)s %(device)s',
+            'filesystem': 'ext4',
+            'device': '/dev/xdb1',
+            'label': 'with_cmd',
+            'extra_opts': ['should', 'generate', 'warning'],
+            'overwrite': 'should generate warning too'
+        })
 
         self.assertIn(
-            'WARNING:cloudinit.config.cc_disk_setup:fs_setup:extra_opts ' +
+            'extra_opts ' +
             'ignored because cmd was specified: mkfs -t ext4 -L with_cmd ' +
             '/dev/xdb1',
-            logs.output)
+            self.logs.getvalue())
         self.assertIn(
-            'WARNING:cloudinit.config.cc_disk_setup:fs_setup:overwrite ' +
+            'overwrite ' +
             'ignored because cmd was specified: mkfs -t ext4 -L with_cmd ' +
             '/dev/xdb1',
-            logs.output)
+            self.logs.getvalue())
 
         subp.assert_called_once_with(
             'mkfs -t ext4 -L with_cmd /dev/xdb1', shell=True)
diff --git a/tools/read-dependencies b/tools/read-dependencies
index 4ba2c1b..8a58534 100755
--- a/tools/read-dependencies
+++ b/tools/read-dependencies
@@ -18,6 +18,7 @@ import re
 import subprocess
 import sys
 
+DEFAULT_REQUIREMENTS = 'requirements.txt'
 
 # Map the appropriate package dir needed for each distro choice
 DISTRO_PKG_TYPE_MAP = {
@@ -51,8 +52,9 @@ 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')
+        '-r', '--requirements-file', type=str, dest='req_files',
+        action='append', default=None,
+        help='pip-style requirements file [default=%s]' % DEFAULT_REQUIREMENTS)
     parser.add_argument(
         '-d', '--distro', type=str, choices=DISTRO_PKG_TYPE_MAP.keys(),
         help='The name of the distro to generate package deps for.')
@@ -144,12 +146,24 @@ def main(distro):
         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)
+
+    if args.req_files is None:
+        args.req_files = [os.path.join(topd, DEFAULT_REQUIREMENTS)]
+        if not os.path.isfile(args.req_files[0]):
+            sys.stderr.write("Unable to locate '%s' file that should "
+                             "exist in cloud-init root directory." %
+                             args.req_files[0])
+            sys.exit(1)
+
+    bad_files = [r for r in args.req_files if not os.path.isfile(r)]
+    if bad_files:
+        sys.stderr.write(
+            "Unable to find requirements files: %s\n" % ','.join(bad_files))
+        sys.exit(1)
+
+    pip_pkg_names = set()
+    for req_path in args.req_files:
+        pip_pkg_names.update(set(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(
@@ -174,7 +188,7 @@ def pkg_install(pkg_list, distro, dry_run=False):
     """Install a list of packages using the DISTRO_INSTALL_PKG_CMD."""
     print('Installing deps: {0}{1}'.format(
           '(dryrun)' if dry_run else '', ' '.join(pkg_list)))
-    pkg_list.extend(EXTRA_SYSTEM_BASE_PKGS)
+    pkg_list = list(pkg_list) + EXTRA_SYSTEM_BASE_PKGS
     install_cmd = []
     if dry_run:
        install_cmd.append('echo')
diff --git a/tools/run-centos b/tools/run-centos
index de21d75..99ba6be 100755
--- a/tools/run-centos
+++ b/tools/run-centos
@@ -14,17 +14,22 @@ errorrc() { local r=$?; error "$@" "ret=$r"; return $r; }
 
 Usage() {
     cat <<EOF
-Usage: ${0##*/} [ options ] CentOS version
+Usage: ${0##*/} [ options ] version
 
     This utility can makes it easier to run tests, build rpm and source rpm
         generation inside a LXC of the specified version of CentOS.
 
+    version is major release number (6 or 7)
+
     options:
       -a | --artifact keep .rpm artifacts
       -k | --keep     keep container after tests
       -r | --rpm      build .rpm
       -s | --srpm     build .src.rpm
       -u | --unittest run unit tests
+
+    Example:
+      * ${0##*/} --rpm --srpm --unittest 6
 EOF
 }
 
@@ -48,6 +53,10 @@ inside_as() {
     # executes cmd with args inside container as user in users home dir.
     local name="$1" user="$2"
     shift 2
+    if [ "$user" = "root" ]; then
+        inside "$name" "$@"
+        return
+    fi
     local stuffed="" b64=""
     stuffed=$(getopt --shell sh --options "" -- -- "$@")
     stuffed=${stuffed# -- }
@@ -56,6 +65,12 @@ inside_as() {
         'cd; eval set -- "$(echo '$b64' | base64 --decode)" && exec "$@"'
 }
 
+inside_as_cd() {
+    local name="$1" user="$2" dir="$3"
+    shift 3
+    inside_as "$name" "$user" sh -c 'cd "$0" && exec "$@"' "$dir" "$@"
+}
+
 inside() {
     local name="$1"
     shift
@@ -63,26 +78,52 @@ inside() {
 }
 
 inject_cloud_init(){
-    local name="$1"
-    tarball_name='cloud-init.tar.gz'
-    top_d=$(git rev-parse --show-toplevel) ||
-        fail "failed to get top level"
-    cd "$top_d" ||
-        fail "failed to cd to git top dir"
-    tar_folder=${PWD##*/}
-    cd ..
-    tar -czf "$TEMP_D/$tarball_name" "$tar_folder" ||
-        fail "failed: creating tarball_name"
-    cd "$tar_folder" ||
-        fail "failed: changing directory"
-
-    user='centos'
-    tarball="/home/$user/$tarball_name"
-    inside "$name" useradd "$user"
-    lxc file push "$TEMP_D/$tarball_name" "$name/home/$user"/
-    inside "$name" chown "$user:$user" "$tarball"
-    inside_as "$name" "$user" tar -C "/home/$user" -xzf "$tarball" ||
-        fail "failed: extracting tarball"
+    # take current cloud-init git dir and put it inside $name at
+    # ~$user/cloud-init.
+    local name="$1" user="$2" top_d="" dname="" pstat=""
+    top_d=$(git rev-parse --show-toplevel) || {
+        errorrc "Failed to get git top level in $PWD";
+        return
+    }
+    dname=$(basename "${top_d}") || return
+    debug 1 "collecting ${top_d} ($dname) into user $user in $name."
+    tar -C "${top_d}/.." -cpf - "$dname" |
+        inside_as "$name" "$user" sh -ec '
+            dname=$1
+            rm -Rf "$dname"
+            tar -xpf -
+            [ "$dname" = "cloud-init" ] || mv "$dname" cloud-init' \
+            extract "$dname"
+    [ "${PIPESTATUS[*]}" = "0 0" ] || {
+        error "Failed to push tarball of '$top_d' into $name" \
+            " for user $user (dname=$dname)"
+        return 1
+    }
+    return 0
+}
+
+prep() {
+    # we need some very basic things not present in the container.
+    #  - git
+    #  - tar (CentOS 6 lxc container does not have it)
+    #  - python-argparse (or python3)
+    local needed="" pair="" pkg="" cmd="" needed=""
+    for pair in tar:tar git:git; do
+        pkg=${pair#*:}
+        cmd=${pair%%:*}
+        command -v $cmd >/dev/null 2>&1 || needed="${needed} $pkg"
+    done
+    if ! command -v python3; then
+        python -c "import argparse" >/dev/null 2>&1 ||
+            needed="${needed} python-argparse"
+    fi
+    needed=${needed# }
+    if [ -z "$needed" ]; then
+        error "No prep packages needed"
+        return 0
+    fi
+    error "Installing prep packages: ${needed}"
+    yum install --assumeyes ${needed}
 }
 
 start_container() {
@@ -121,8 +162,8 @@ delete_container() {
 }
 
 main() {
-    local short_opts="ahkrsuv:"
-    local long_opts="artifact,help,keep,rpm,srpm,unittest,verbose:"
+    local short_opts="ahkrsuv"
+    local long_opts="artifact,help,keep,rpm,srpm,unittest,verbose"
     local getopt_out=""
     getopt_out=$(getopt --name "${0##*/}" \
         --options "${short_opts}" --long "${long_opts}" -- "$@") &&
@@ -149,60 +190,70 @@ main() {
 
     [ $# -eq 1 ] || { bad_Usage "ERROR: Must provide version!"; return; }
     version="$1"
+    case "$version" in
+        6|7) :;;
+        *) error "Expected version of 6 or 7, not '$version'"; return;;
+    esac
 
     TEMP_D=$(mktemp -d "${TMPDIR:-/tmp}/${0##*/}.XXXXXX") ||
         fail "failed to make tempdir"
     trap cleanup EXIT
 
     # program starts here
-    local uuid="" name=""
+    local uuid="" name="" user="ci-test" cdir=""
+    cdir="/home/$user/cloud-init"
     uuid=$(uuidgen -t) || { error "no uuidgen"; return 1; }
     name="cloud-init-centos-${uuid%%-*}"
 
     start_container "images:centos/$version" "$name"
-    # CentOS 6 does not come with tar
-    if [ "$version" = "6" ]; then
-        inside "$name" yum install --assumeyes tar || {
-            errorrc "FAIL: yum install tar failed";
-        }
-    fi
+
+    # prep the container (install very basic dependencies)
+    inside "$name" bash -s prep <"$0" ||
+        { errorrc "Failed to prep container $name"; return; }
+
+    # add the user
+    inside "$name" useradd "$user"
 
     debug 1 "inserting cloud-init"
-    inject_cloud_init "$name" || {
+    inject_cloud_init "$name" "$user" || {
         errorrc "FAIL: injecting cloud-init into $name failed."
         return
     }
 
-    # install dependencies
-    debug 1 "installing dependencies"
-    inside "$name" /bin/sh <tools/setup-centos ||
-        fail "failed: setting up container $name"
+    inside_as_cd "$name" root "$cdir" \
+        ./tools/read-dependencies \
+            --requirements-file=requirements.txt \
+            --requirements-file=test-requirements.txt \
+            --distro=centos --install || {
+        errorrc "FAIL: failed to install dependencies with read-dependencies"
+        return
+    }
 
-    local errors=0 do_cd="cd $tar_folder"
-    inside_as "$name" "$user" sh -ec "$do_cd; git checkout .; git status" ||
+    local errors=0
+    inside_as_cd "$name" "$user" "$cdir" \
+        sh -ec "git checkout .; git status" ||
             { errorrc "git checkout failed."; errors=$(($errors+1)); }
 
     if [ -n "$unittest" ]; then
         debug 1 "running unit tests."
-        inside_as "$name" "$user" sh -ec "$do_cd; nosetests tests/unittests" ||
+        inside_as_cd "$name" "$user" "$cdir" nosetests tests/unittests ||
             { errorrc "nosetests failed."; errors=$(($errors+1)); }
     fi
 
     if [ -n "$srpm" ]; then
         debug 1 "building srpm."
-        inside_as "$name" "$user" sh -ec "$do_cd; ./packages/brpm --srpm" ||
+        inside_as_cd "$name" "$user" "$cdir" ./packages/brpm --srpm ||
             { errorrc "brpm --srpm."; errors=$(($errors+1)); }
     fi
 
     if [ -n "$rpm" ]; then
         debug 1 "building rpm."
-        inside_as "$name" "$user" sh -ec "$do_cd; ./packages/brpm" ||
+        inside_as_cd "$name" "$user" "$cdir" ./packages/brpm ||
             { errorrc "brpm failed."; errors=$(($errors+1)); }
     fi
 
     if [ -n "$artifact" ]; then
-        cmd="ls /home/$user/$tar_folder/*.rpm"
-        for built_rpm in $(lxc exec "$name" -- sh -c "$cmd"); do
+        for built_rpm in $(inside "$name" sh -c "echo $cdir/*.rpm"); do
             lxc file pull "$name/$built_rpm" .
         done
     fi
@@ -214,5 +265,10 @@ main() {
     return 0
 }
 
-main "$@"
+if [ "$1" = "prep" ]; then
+    shift
+    prep "$@"
+else
+    main "$@"
+fi
 # vi: ts=4 expandtab
diff --git a/tools/setup-centos b/tools/setup-centos
deleted file mode 100755
index bc5da8a..0000000
--- a/tools/setup-centos
+++ /dev/null
@@ -1,49 +0,0 @@
-#!/bin/sh
-# This file is part of cloud-init. See LICENSE file for license information.
-set -fux
-export LANG=C
-
-packages="
-    file
-    git
-    pyserial
-    python-argparse
-    python-cheetah
-    python-configobj
-    python-devel
-    python-jinja2
-    python-jsonpatch
-    python-oauthlib
-    python-pip
-    python-prettytable
-    python-requests
-    python-six
-    PyYAML
-    rpm-build
-"
-
-pips="
-    contextlib2
-    httpretty
-    mock
-    nose
-    pep8
-    unittest2
-"
-
-error() { echo "$@" 1>&2; }
-fail() { [ $# -eq 0 ] || error "$@"; exit 1; }
-info() { echo "$@"; }
-
-pips=$(for p in $pips; do echo "$p"; done | sort -u)
-packages=$(for p in $packages; do echo "$p"; done | sort -u)
-
-if ! rpm -q epel-release >/dev/null; then
-    yum install --assumeyes epel-release ||
-        fail "failed: yum install epel-release"
-fi
-yum install --assumeyes $packages ||
-        fail "failed: yum install" "$packages"
-
-pip install --upgrade $pips ||
-    fail "failed: pip install $pips"