cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/tests/cloud_tests/args.py b/tests/cloud_tests/args.py
> index 371b044..ca52b61 100644
> --- a/tests/cloud_tests/args.py
> +++ b/tests/cloud_tests/args.py
> @@ -185,6 +246,17 @@ def normalize_output_args(args):
>      return args
>  
>  
> +def normalize_output_deb_args(args):
> +    """

Docstrings should be one intro line followed by a blank line and then additional details per PEP 257 https://www.python.org/dev/peps/pep-0257/

> +    normalize OUTPUT_DEB arguments
> +    args: parsed args
> +    return_value: updated args, or None if erros occurred
> +    """
> +    # make sure to use abspath for deb
> +    args.deb = os.path.abspath(args.deb)
> +    return args
> +
> +
>  def normalize_setup_args(args):
>      """
>      normalize SETUP arguments
> diff --git a/tests/cloud_tests/bddeb.py b/tests/cloud_tests/bddeb.py
> new file mode 100644
> index 0000000..b36331b
> --- /dev/null
> +++ b/tests/cloud_tests/bddeb.py
> @@ -0,0 +1,124 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +from tests.cloud_tests import (config, LOG)
> +from tests.cloud_tests import (platforms, images, snapshots, instances)
> +from tests.cloud_tests.stage import (PlatformComponent, run_stage, run_single)
> +
> +from cloudinit import util as c_util
> +
> +from functools import partial
> +import os
> +
> +build_deps = ['devscripts', 'equivs', 'git', 'tar']
> +
> +
> +def _out(cmd_res):
> +    """

This can all fit on one line
"""Get clean output from cmd result."""

> +    get clean output from cmd result
> +    """
> +    return cmd_res[0].strip()
> +
> +
> +def build_deb(args, instance):
> +    """

docstring one line format followed by a blank line and additional details below.
"""Build deb on a remote container, returning it locally to args.deb.

@param args: Command line arguments.
@return_value: tuple of results and failure count.
"""

> +    build deb on system and copy out to location at args.deb
> +    args: cmdline arguments
> +    return_value: tuple of results and fail count
> +    """
> +    # update remote system package list and install build deps
> +    LOG.debug('installing build deps')
> +    pkgs = ' '.join(build_deps)
> +    cmd = 'apt-get update && apt-get install --yes {}'.format(pkgs)
> +    instance.execute(['/bin/sh', '-c', cmd])
> +    instance.execute(['mk-build-deps', '--install', '-t',

If I understand this correctly, this call will break if we add new build dependencies to cloud-init because we would be installing downstream (published) cloud-init and not upstream(master) right? I feel like what we are looking for is a make ci-deps target in cloud-init which we can call after we push the source_tar over to the remote system.  If you agree, can you put an comment like  "# XXX Remove this call once we have a ci-deps Makefile target?

> +                      'apt-get --no-install-recommends --yes', 'cloud-init'])
> +
> +    # local tmpfile that must be deleted
> +    local_tarball = _out(c_util.subp(['mktemp'], capture=True))
> +
> +    try:

Instead of calling out to mktemp, we could import tempfile and with tempfile.NamedTemporaryFile() as local_tarball:


Then you can drop the finally: section and the call to mktemp as that tempfile is autoremoved when it leaves scope.

> +        # paths to use in remote system
> +        remote_tarball = _out(instance.execute(['mktemp']))
> +        extract_dir = _out(instance.execute(['mktemp', '--directory']))
> +        bddeb_path = os.path.join(extract_dir, 'packages', 'bddeb')
> +        git_env = {'GIT_DIR': os.path.join(extract_dir, '.git'),
> +                   'GIT_WORK_TREE': extract_dir}
> +
> +        # create a tarball of cloud init tree and copy to remote system

probably don't need the comment as the log messages explain this well.

> +        LOG.debug('creating tarball of cloud-init at: %s', local_tarball)
> +        c_util.subp(['tar', 'cf', local_tarball, '--owner', 'root',
> +                     '--group', 'root', '-C', args.cloud_init, '.'])
> +        LOG.debug('copying to remote system at: %s', remote_tarball)
> +        instance.push_file(local_tarball, remote_tarball)
> +
> +        # extract tarball in remote system and commit anything uncommitted
> +        LOG.debug('extracting tarball in remote system at: %s', extract_dir)
> +        instance.execute(['tar', 'xf', remote_tarball, '-C', extract_dir])
> +        instance.execute(['git', 'commit', '-a', '-m', 'tmp', '--allow-empty'],

Not sure we really have to commit anything here, as we can also specify SKIP_UNCOMITTED_CHANGES_CHECK=1 on the builddeb call. Which also means we can drop the git_env above I think.

> +                         env=git_env)
> +
> +        # build the deb, ignoring missing deps (flake8)
> +        output_link = '/root/cloud-init_all.deb'
> +        LOG.debug('building deb in remote system at: %s', output_link)
> +        bddeb_args = args.bddeb_args.split() if args.bddeb_args else []
> +        instance.execute([bddeb_path, '-d'] + bddeb_args, env=git_env)
> +
> +        # copy the deb back to the host system
> +        LOG.debug('copying built deb to host at: %s', args.deb)
> +        instance.pull_file(output_link, args.deb)
> +
> +    finally:
> +        os.remove(local_tarball)
> +
> +
> +def setup_build(args):
> +    """

docstring nit. sorry :/

> +    set build system up then run build
> +    args: cmdline arguments
> +    return_value: tuple of results and fail count
> +    """
> +    res = ({}, 1)
> +
> +    # set up platform
> +    LOG.info('setting up platform: %s', args.build_platform)
> +    platform_config = config.load_platform_config(args.build_platform)
> +    platform_call = partial(platforms.get_platform, args.build_platform,
> +                            platform_config)
> +    with PlatformComponent(platform_call) as platform:
> +
> +        # set up image
> +        LOG.info('acquiring image for os: %s', args.build_os)
> +        img_conf = config.load_os_config(platform.platform_name, args.build_os)
> +        image_call = partial(images.get_image, platform, img_conf)
> +        with PlatformComponent(image_call) as image:
> +
> +            # set up snapshot
> +            snapshot_call = partial(snapshots.get_snapshot, image)
> +            with PlatformComponent(snapshot_call) as snapshot:
> +
> +                # create instance with cloud-config to set it up
> +                LOG.info('creating instance to build deb in')
> +                empty_cloud_config = "#cloud-config\n{}"
> +                instance_call = partial(
> +                    instances.get_instance, snapshot, empty_cloud_config,
> +                    use_desc='build cloud-init deb')
> +                with PlatformComponent(instance_call) as instance:
> +
> +                    # build the deb
> +                    res = run_single('build deb on system',
> +                                     partial(build_deb, args, instance))
> +
> +    return res
> +
> +
> +def bddeb(args):
> +    """
> +    entry point for build deb
> +    args: cmdline arguments
> +    return_value: fail count
> +    """
> +    LOG.info('preparing to build cloud-init deb')
> +    (res, failed) = run_stage('build deb', [partial(setup_build, args)])
> +    return failed
> +
> +# vi: ts=4 expandtab


-- 
https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/323588
Your team cloud init development team is requested to review the proposed merge of ~powersj/cloud-init:integration-test-revamp into cloud-init:master.