Thanks for looking at the code! Diff comments:
> diff --git a/cloudinit/config/cc_snap_config.py > b/cloudinit/config/cc_snap_config.py > new file mode 100644 > index 0000000..667b9c6 > --- /dev/null > +++ b/cloudinit/config/cc_snap_config.py > @@ -0,0 +1,177 @@ > +# vi: ts=4 expandtab > +# > +# Copyright (C) 2016 Canonical Ltd. > +# > +# Author: Ryan Harper <ryan.har...@canonical.com> > +# > +# This program is free software: you can redistribute it and/or modify > +# it under the terms of the GNU General Public License version 3, as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +""" > +Snappy > +------ > +**Summary:** snap_config modules allows configuration of snapd. > + > +This module uses the same ``snappy`` namespace for configuration but > +acts only only a subset of the configuration. > + > +If ``assertions`` is set and the user has included a list of assertions > +then cloud-init will collect the assertions into a single assertion file > +and invoke ``snap ack <path to file with assertions>`` which will attempt > +to load the provided assertions into the snapd assertion database. > + > +If ``email`` is set, this value is used to create an authorized user for > +contacting and installing snaps from the Ubuntu Store. This is done by > +calling ``snap create-user`` command. > + > +If ``known`` is set to True, then it is expected the user also included > +an assertion of type ``system-user``. When ``snap create-user`` is called > +cloud-init will append '--known' flag which instructs snapd to look for > +a system-user assertion with the details. If ``known`` is not set, then > +``snap create-user`` will contact the Ubuntu SSO for validating and importing > +a system-user for the instance. > + > +.. note:: > + If the system is already managed, then cloud-init will not attempt to > + create a system-user. > + > +**Internal name:** ``cc_snap_config`` > + > +**Module frequency:** per instance > + > +**Supported distros:** ubuntu > + > +**Config keys**:: > + > + #cloud-config > + snappy: > + assertions: > + - | > + <assertion 1> > + - | > + <assertion 2> > + email: u...@user.org > + known: true > + > +""" > + > +from cloudinit import log as logging > +from cloudinit.settings import PER_INSTANCE > +from cloudinit import util > + > +LOG = logging.getLogger(__name__) AIUI, modules should all change to the above; note we're importing logging from cloudinit.log; which is a nice wrapper for initializing logging specifically for cloud-init and the __name__ allows proper naming of the output. > + > +frequency = PER_INSTANCE > +SNAPPY_CMD = "snap" > +ASSERTIONS_FILE = "/var/lib/cloud/instance/snapd.assertions" > + > +distros = ['ubuntu'] > + > + > +def set_snappy_command(): > + global SNAPPY_CMD > + if util.which("snappy-go"): > + SNAPPY_CMD = "snappy-go" > + elif util.which("snappy"): > + SNAPPY_CMD = "snappy" > + else: > + SNAPPY_CMD = "snap" > + LOG.debug("snappy command is '%s'", SNAPPY_CMD) > + > + > +""" > +snappy: > + assertions: > + - | > + <snap assertion 1> > + - | > + <snap assertion 2> > + email: f...@foo.io > + known: true > +""" > + > + > +def add_assertions(assertions): > + """ Import list of 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: On one hand, I'd like to avoid exploding cloudinit.util with what are proper utility functions, but until another module needs to use the add_assertions I'd leave it here. I think it's reasonable to include the checks here in-case of re-use. I'll defer to Scott or Josh on this one. > + assertions = [] > + > + if not isinstance(assertions, list): > + raise ValueError('assertion parameter was not a list: %s', > assertions) > + > + snap_cmd = [SNAPPY_CMD, 'ack'] > + combined = "\n".join(assertions) > + if len(combined) == 0: > + raise ValueError("Assertion list is empty") > + > + for asrt in assertions: > + LOG.debug('Acking: %s', asrt.split('\n')[0:2]) > + > + util.write_file(ASSERTIONS_FILE, combined.encode('utf-8')) write_file already will bubble up a ProcessExecutionError message; and there's nothing more for us to do AFAIK. Same with snap ack; there's no recovery. > + util.subp(snap_cmd + [ASSERTIONS_FILE], capture=True) > + > + > +def handle(name, cfg, cloud, log, args): > + cfgin = cfg.get('snappy') > + if not cfgin: > + LOG.debug('No snappy config provided, skipping') > + return > + > + if not(util.system_is_snappy()): > + LOG.debug("%s: system not snappy", name) > + return > + > + set_snappy_command() > + > + assertions = cfgin.get('assertions', []) > + if len(assertions) > 0: > + LOG.debug('Importing user-provided snap assertions') > + add_assertions(assertions) > + > + # Create a snap user if requested. > + # Snap systems contact the store with a user's email > + # and extract information needed to create a local user. > + # A user may provide a 'system-user' assertion which includes > + # the required information. Using such an assertion to create > + # a local user requires specifying 'known: true' in the supplied > + # user-data. > + snapuser = cfgin.get('email', None) > + if snapuser: > + usercfg = { > + 'snapuser': snapuser, > + 'known': cfgin.get('known', False), > + } > + > + # query if we're already registered > + out, _ = util.subp([SNAPPY_CMD, 'managed'], capture=True) > + if out.strip() == "true": > + LOG.warning('This device is already managed. ' > + 'Skipping system-user creation') > + return > + > + if usercfg.get('known'): > + # Check that we imported a system-user assertion > + out, _ = util.subp([SNAPPY_CMD, 'known', 'system-user'], > + capture=True) > + if len(out) == 0: > + LOG.error('Missing "system-user" assertion. ' > + 'Check "snappy" user-data assertions.') > + return > + > + cloud.distro.create_user(snapuser, **usercfg) > diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py > index b1192e8..101c7ec 100644 > --- a/cloudinit/distros/__init__.py > +++ b/cloudinit/distros/__init__.py > @@ -445,6 +448,32 @@ class Distro(object): > util.logexc(LOG, "Failed to create user %s", name) > raise e > > + def add_snap_user(self, name, **kwargs): > + """ > + Add a snappy user to the system using snappy tools > + """ > + > + snapuser = kwargs.get('snapuser') > + known = kwargs.get('known', False) > + adduser_cmd = ["snap", "create-user", "--sudoer", "--json"] > + if known: > + adduser_cmd.append("--known") > + adduser_cmd.append(snapuser) > + > + # Run the command > + LOG.debug("Adding snap user %s", name) > + try: > + (out, err) = util.subp(adduser_cmd, logstring=adduser_cmd, IIRC, it's what was here before. > + capture=True) > + LOG.debug("snap create-user returned: %s:%s", out, err) > + jobj = util.load_json(out) > + username = jobj.get('username', None) > + except Exception as e: > + util.logexc(LOG, "Failed to create snap user %s", name) > + raise e > + > + return username > + > def create_user(self, name, **kwargs): > """ > Creates users for the system using the GNU passwd tools. This -- https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/304700 Your team cloud init development team is requested to review the proposed merge of ~raharper/cloud-init:snapuser-create into cloud-init:master. _______________________________________________ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp