cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/config/cc_snap.py b/cloudinit/config/cc_snap.py
> new file mode 100644
> index 0000000..edb21bb
> --- /dev/null
> +++ b/cloudinit/config/cc_snap.py
> @@ -0,0 +1,272 @@
> +# Copyright (C) 2018 Canonical Ltd.
> +#
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +"""Snap: Install, configure and manage snapd and snap packages."""
> +
> +import sys
> +from textwrap import dedent
> +
> +from cloudinit import log as logging
> +from cloudinit.config.schema import (
> +    get_schema_doc, validate_cloudconfig_schema)
> +from cloudinit.settings import PER_INSTANCE
> +from cloudinit import util
> +
> +
> +distros = ['ubuntu']
> +frequency = PER_INSTANCE
> +
> +LOG = logging.getLogger(__name__)
> +
> +schema = {
> +    'id': 'cc_snap',
> +    'name': 'Snap',
> +    'title': 'Install, configure and manage snapd and snap packages',
> +    'description': dedent("""\
> +        This module provides a simple configuration namespace in cloud-init to
> +        both setup snapd and install snaps.
> +
> +        .. note::
> +            Both ``assertions`` and ``commands`` values can be either a
> +            dictionary or a list. If these configs are provided as a
> +            dictionary, the keys of that list are only used to order the
> +            execution of the assertions or commands and the dictionary is
> +            merged with any vendor-data snap configuration provided. If a list
> +            is provided by the user, any vendor-data snap configuration is
> +            ignored.
> +
> +        The ``assertions`` configuration option is a dictionary or list of
> +        properly-signed snap assertions which will run before any snap
> +        ``commands``. They will be added to snapd's assertion database by
> +        invoking ``snap ack <aggregate_assertion_file>``.
> +
> +        Snap ``commands`` is a dictionary or list of individual snap
> +        commands to run on the target system. These commands can be used to
> +        create snap users, install snaps and provide snap configuration.
> +
> +        .. note::
> +            If 'side-loading' private/unpublished snaps on an instance, it is
> +            best to create a snap seed directory and seed.yaml manifest in
> +            **/var/lib/snapd/seed/** which snapd automatically installs on
> +            startup.
> +
> +        **Development only**: The ``squashfuse_in_container`` boolean can be
> +        set true to install squashfuse package when in a container to enable
> +        snap installs. Default is false.
> +        """),
> +    'distros': distros,
> +    'examples': [dedent("""\
> +        snap:
> +            assertions:
> +              00: |
> +              signed_assertion_blob_here
> +              02: |
> +              signed_assertion_blob_here
> +            commands:
> +              00: snap create-user --sudoer --known <snap-user>@mydomain.com
> +              01: snap install canonical-livepatch
> +              02: canonical-livepatch enable <AUTH_TOKEN>
> +    """), dedent("""\
> +        # LXC-based containers require squashfuse before snaps can be installed
> +        snap:
> +            commands:
> +                00: apt-get install squashfuse -y
> +                11: snap install emoj
> +
> +    """), dedent("""\
> +        # Convenience: the snap command can be ommited when specifying commands
> +        # as a list and 'snap' will automatically be prepended.
> +        # The following commands are equivalent:
> +        snap:
> +            commands:
> +                00: ['install', 'vlc']
> +                01: ['snap', 'install', 'vlc']
> +                02: snap install vlc
> +                03: 'snap install vlc'
> +    """)],
> +    'frequency': PER_INSTANCE,
> +    'type': 'object',
> +    'properties': {
> +        'snap': {
> +            'type': 'object',
> +            'properties': {
> +                'assertions': {
> +                    'type': ['object', 'array'],  # Array of strings or dict
> +                    'items': {'type': 'string'},
> +                    'additionalItems': False,  # Reject items non-string
> +                    'minItems': 1,
> +                    'minProperties': 1,
> +                    'uniqueItems': True
> +                },
> +                'commands': {
> +                    'type': ['object', 'array'],  # Array of strings or dict
> +                    'items': {
> +                        'oneOf': [
> +                            {'type': 'array', 'items': {'type': 'string'}},
> +                            {'type': 'string'}]
> +                    },
> +                    'additionalItems': False,  # Reject non-string & non-list
> +                    'minItems': 1,
> +                    'minProperties': 1,
> +                    'uniqueItems': True
> +                },
> +                'squashfuse_in_container': {
> +                    'type': 'boolean'
> +                }
> +            },
> +            'additionalProperties': False,  # Reject keys not in schema
> +            'required': [],
> +            'minProperties': 1
> +        }
> +    }
> +}
> +
> +# TODO schema for 'assertions' and 'commands' are too permissive at the moment.
> +# Once python-jsonschema supports schema draft 6 add support for arbitrary
> +# object keys with 'patternProperties' constraint to validate string values.
> +
> +__doc__ = get_schema_doc(schema)  # Supplement python help()
> +
> +SNAP_CMD = "snap"
> +ASSERTIONS_FILE = "/var/lib/cloud/instance/snapd.assertions"
> +
> +
> +def add_assertions(assertions):
> +    """Import list of assertions.
> +
> +    Import assertions by concatenating each assertion into a
> +    string separated by a '\n'.  Write this string to a instance file and
> +    then invoke `snap ack /path/to/file` and check for errors.
> +    If snap exits 0, then all assertions are imported.
> +    """
> +    if not assertions:
> +        return
> +    LOG.debug('Importing user-provided snap assertions')
> +    if isinstance(assertions, dict):
> +        assertions = assertions.values()
> +    elif not isinstance(assertions, list):
> +        raise TypeError(
> +            'assertion parameter was not a list or dict: {assertions}'.format(
> +                assertions=assertions))
> +
> +    snap_cmd = [SNAP_CMD, 'ack']
> +    combined = "\n".join(assertions)
> +
> +    for asrt in assertions:
> +        LOG.debug('Snap acking: %s', asrt.split('\n')[0:2])
> +
> +    util.write_file(ASSERTIONS_FILE, combined.encode('utf-8'))
> +    util.subp(snap_cmd + [ASSERTIONS_FILE], capture=True)
> +
> +
> +def prepend_snap_commands(commands):
> +    """Ensure user-provided commands start with SNAP_CMD, warn otherwise.
> +
> +    Each command is either a list or string. Perform the following:
> +       - When the command is a list, pop the first element if it is None
> +       - When the command is a list, insert SNAP_CMD as the first element if
> +         not present.
> +       - When the command is a string containing a non-snap command, warn.
> +
> +    Support cut-n-paste snap command sets from public snappy documentation.
> +    Allow flexibility to provide non-snap environment/config setup if neeeded.
> +
> +    @commands: List of commands. Each command element is a list or string.
> +
> +    @return: List of 'fixed up' snap commands.
> +    @raise: TypeError on invalid config item type.
> +    """
> +    warnings = []
> +    errors = []
> +    fixed_commands = []
> +    for command in commands:
> +        if isinstance(command, list):
> +            if command[0] is None:  # Avoid warnings by specifying None
> +                command = command[1:]
> +            elif command[0] != SNAP_CMD:  # Automatically prepend SNAP_CMD
> +                command.insert(0, SNAP_CMD)
> +        elif isinstance(command, str):
> +            if not command.startswith('%s ' % SNAP_CMD):
> +                warnings.append(command)
> +        else:
> +            errors.append(str(command))
> +            continue
> +        fixed_commands.append(command)
> +
> +    if warnings:
> +        LOG.warning(
> +            'Non-snap commands in snap config:\n%s', '\n'.join(warnings))
> +    if errors:
> +        raise TypeError(
> +            'Invalid snap config.'
> +            ' These commands are not a string or list:\n' + '\n'.join(errors))
> +    return fixed_commands
> +
> +
> +def run_commands(commands):
> +    """Run the provided commands provided in snap:commands configuration.
> +
> +     Commands are aggregated,shellified and written to a temporary script file
> +     which is then executed.
> +
> +     @param commands: A list or dict containing commands to run. Keys of a
> +         dict will be used to order the commands provided as dict values.
> +     """
> +    if not commands:
> +        return
> +    LOG.debug('Running user-provided snap commands')
> +    if isinstance(commands, dict):
> +        # Sort commands based on dictionary key
> +        commands = [v for _, v in sorted(commands.items())]
> +    elif not isinstance(commands, list):
> +        raise TypeError(
> +            'commands parameter was not a list or dict: {commands}'.format(
> +                commands=commands))
> +
> +    fixed_snap_commands = prepend_snap_commands(commands)
> +
> +    failed_cmds = []
> +    for command in fixed_snap_commands:
> +        shell = isinstance(command, str)
> +        try:
> +            util.subp(command, shell=shell, status_cb=sys.stderr.write)
> +        except util.ProcessExecutionError:
> +            failed_cmds.append(command)

We will get the failed command output over in cloud-init-output.log, but I don't see the issue with replicating that content 'here' in cloud-init.log too. It can only help to avoid cross-referencing between the two files.

> +    if failed_cmds:
> +        msg = 'Failed to run snap commands:\n{commands}'.format(
> +            commands=failed_cmds)
> +        util.logexc(LOG, msg)
> +        raise RuntimeError(msg)
> +
> +
> +def maybe_install_squashfuse(cloud):
> +    """Install squashfuse if we are in a container."""
> +    if not util.is_container():
> +        return
> +    try:
> +        cloud.distro.update_package_sources()
> +    except Exception as e:
> +        util.logexc(LOG, "Package update failed")
> +        raise
> +    try:
> +        cloud.distro.install_packages(['squashfuse'])
> +    except Exception as e:
> +        util.logexc(LOG, "Failed to install squashfuse")
> +        raise
> +
> +
> +def handle(name, cfg, cloud, log, args):
> +    cfgin = cfg.get('snap', {})
> +    if not cfgin:
> +        LOG.debug(("Skipping module named %s,"
> +                   " no 'snap' key in configuration"), name)
> +        return
> +
> +    validate_cloudconfig_schema(cfg, schema)
> +    if util.is_true(cfgin.get('squashfuse_in_container', False)):
> +        maybe_install_squashfuse(cloud)
> +    add_assertions(cfgin.get('assertions', []))
> +    run_commands(cfgin.get('commands', []))
> +
> +# vi: ts=4 expandtab
> diff --git a/tests/cloud_tests/testcases/modules/snap.yaml b/tests/cloud_tests/testcases/modules/snap.yaml
> new file mode 100644
> index 0000000..e520415
> --- /dev/null
> +++ b/tests/cloud_tests/testcases/modules/snap.yaml
> @@ -0,0 +1,21 @@
> +#
> +# Install snappy
> +#
> +required_features:
> +  - snap
> +cloud_config: |
> +  #cloud-config
> +  package_update: true
> +  snap:
> +    squashfuse_in_container: true
> +    commands:
> +      - snap install hello-world
> +collect_scripts:
> +  snaplist: |
> +    #!/bin/bash
> +    snap list
> +  instance-data.json: |
> +    #!/bin/sh
> +    cat /run/cloud-init/instance-data.json

While the validation of instance-data.json is something that makes sense in base. It also is the snapcore specific integration test request we had to validate that the format and content we expose on various clouds is correct so snapcore can source it. 

I specifically didn't want to add this in base because each test (and each file pulled) costs us an extra ssh round-trip for every single unit test. That cost wasn't worth the additional round trip for something that is a duplicated test on every test case.

Shall I still put it in base?

> +
> +# vi: ts=4 expandtab


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/338366
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/snap-module into cloud-init:master.