Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:snapuser-create into cloud-init:master

2016-10-19 Thread Scott Moser
over all, looks good.
you dont have to clean up the handle, but if you see easy way to do that that'd 
be nice.

is the snappy path now valid on non-snappy system ? (ubuntu server with 'snap' 
support).


lastly, please just review your commit message and make sure its up to date 
with all changes (it may well be, just think you made some changes since you 
wrote it).

-- 
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


Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:snapuser-create into cloud-init:master

2016-10-19 Thread Ryan Harper
On Wed, Oct 19, 2016 at 8:52 AM, Ryan Harper 
wrote:

>
>
> On Wed, Oct 19, 2016 at 8:04 AM, Scott Moser  wrote:
>
>> over all, looks good.
>> you dont have to clean up the handle, but if you see easy way to do that
>> that'd be nice.
>>
>> is the snappy path now valid on non-snappy system ? (ubuntu server with
>> 'snap' support).
>>
>
> It is, and util.is_system_snappy() I think passes where it works; I'll
> confirm.
>


is_system_snappy is looking for 'all-snap' style images;  so snaps on
classic don't
respond.  We'd need to design something for validating that it works on
snaps on classic
setups.

One key difference is that all-snap systems have non-writable /etc ; where
as snapd on classic doesn't.  If we added classic systems to the
is_system_snappy check, then users would get added
to /var/lib/extrausers instead of /etc;  I think that's non-optimal and
introduces changes to existing
behavior.

Ryan

-- 
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


Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:snapuser-create into cloud-init:master

2016-10-19 Thread Ryan Harper
On Wed, Oct 19, 2016 at 8:04 AM, Scott Moser  wrote:

> over all, looks good.
> you dont have to clean up the handle, but if you see easy way to do that
> that'd be nice.
>
> is the snappy path now valid on non-snappy system ? (ubuntu server with
> 'snap' support).
>

It is, and util.is_system_snappy() I think passes where it works; I'll
confirm.


>
>
> lastly, please just review your commit message and make sure its up to
> date with all changes (it may well be, just think you made some changes
> since you wrote it).
>

ACK, it needs updating to account for the snappy: namespace changes.


>
>
> Diff comments:
>
> > diff --git a/cloudinit/config/cc_snap_config.py
> b/cloudinit/config/cc_snap_config.py
> > new file mode 100644
> > index 000..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 
> > +#
> > +#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 `` 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:
> > +- |
>
> should we support assertions as a dictionary ?
> it seemed that an assertion is in fact a dictionary, why not allow the
> user to provide it as one rather than a yaml rendered blob, and then we can
> render it.
>

It's not a dictionary, unfortunately; it's a stream of text separated by
newlines;

The format of an assertion is defined here, if you like:

https://github.com/snapcore/snapd/blob/master/asserts/asserts.go#L304

So, each entry in the list is blob of one or more assertions.  For example
you can do the following:

snap download hello
vi hello_*.snap.assertions

And see there are roughly 5 or so assertions in that single file.



>
> > +
> > +- |
> > +
> > +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__)
>
> logexc is really just for logging an exception. with a traceback to the
> log. agreed its a kind of weird function (taking a logger).
>

not following this feedback.  I want a logger so I can call LOG.NNN


>
> > +
> > +frequency = PER_INSTANCE
> > +SNAPPY_CMD = "snap"
> > +ASSERTIONS_FILE = "/var/lib/cloud/instance/snapd.assertions"
> > +
> > +distros = ['ubuntu']
>
> is this just ubuntu? or any distro with snap?
>

Could be other distros with snap.  I don't have an exhaustive list yet.


>
> > +
> > +
> > +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)

Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:snapuser-create into cloud-init:master

2016-10-19 Thread Scott Moser
over all, looks good.
you dont have to clean up the handle, but if you see easy way to do that that'd 
be nice.

is the snappy path now valid on non-snappy system ? (ubuntu server with 'snap' 
support).


lastly, please just review your commit message and make sure its up to date 
with all changes (it may well be, just think you made some changes since you 
wrote it).


Diff comments:

> diff --git a/cloudinit/config/cc_snap_config.py 
> b/cloudinit/config/cc_snap_config.py
> new file mode 100644
> index 000..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 
> +#
> +#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 .
> +
> +"""
> +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 `` 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:
> +- |

should we support assertions as a dictionary ?
it seemed that an assertion is in fact a dictionary, why not allow the user to 
provide it as one rather than a yaml rendered blob, and then we can render it.

> +
> +- |
> +
> +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__)

logexc is really just for logging an exception. with a traceback to the log. 
agreed its a kind of weird function (taking a logger).

> +
> +frequency = PER_INSTANCE
> +SNAPPY_CMD = "snap"
> +ASSERTIONS_FILE = "/var/lib/cloud/instance/snapd.assertions"
> +
> +distros = ['ubuntu']

is this just ubuntu? or any distro with snap?

> +
> +
> +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:
> +  - |
> +  
> +  - |
> +  
> +  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:

the other thing that this sort of thing does (not necessarily here) is avoid a 
trap on python object initialization.

def foo(mylist=[])
   mylist.append("foo")
   return mylist

calling that multiple times like this:
  mylist()
  ret = mylist()

will actually update the '[]' list.  so thats wy we often do the:
   def foo(mylist=None)
  if foo is None:
  foo = []

> +assertions = []
> +
> +if not isinstance(assertions, list):
> +raise ValueError('assertion parameter was not a list: %s', 
> assertions)

Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:snapuser-create into cloud-init:master

2016-10-14 Thread Jon Grimm
Thanks, it was enlightening and easy to read.  

Diff comments:

> diff --git a/cloudinit/config/cc_snap_config.py 
> b/cloudinit/config/cc_snap_config.py
> new file mode 100644
> index 000..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 
> +#
> +#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 .
> +
> +"""
> +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 `` 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:
> +- |
> +
> +- |
> +
> +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__)

Ah, thanks for the explanation!  Maybe we should doc the cleanup somewhere?  
Possibly a newbie/starter task.

> +
> +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:
> +  - |
> +  
> +  - |
> +  
> +  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:

1) WRT generic utility function so ok to be public (but just resisting moving 
for now).  +1  Seems fine to me too with that explanation. 
2) WRT leaving checks here.   I didn't mind having checks here, its just that 
you have similar checks in multiple places seemed a bit icky -- maybe the check 
at L149 is moved/removed. (again, totally 100% minor nit and certainly a 
judgement call even, just wanted to clarify my earlier comment, I realize I'm a 
bit weird that I like think about consistent error handling policies across a 
codebase).

> +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'))

No worries, there just wasn't any comment so had to check.  Totally understand 
that there may be nothing 

Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:snapuser-create into cloud-init:master

2016-10-13 Thread Ryan Harper
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 000..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 
> +#
> +#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 .
> +
> +"""
> +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 `` 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:
> +- |
> +
> +- |
> +
> +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:
> +  - |
> +  
> +  - |
> +  
> +  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, 

Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:snapuser-create into cloud-init:master

2016-10-13 Thread Jon Grimm
Quick look just because I wanted to know more about the impl. 

Diff comments:

> diff --git a/cloudinit/config/cc_snap_config.py 
> b/cloudinit/config/cc_snap_config.py
> new file mode 100644
> index 000..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 
> +#
> +#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 .
> +
> +"""
> +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 `` 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:
> +- |
> +
> +- |
> +
> +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__)

I guess its not clear to me when this should be used vs util.logexc... config 
modules seem to have almost arbitrary use?  just a question.

> +
> +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:
> +  - |
> +  
> +  - |
> +  
> +  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:

afaict add_assertions() is helper only within module (for that matter maybe 
should be named _add_assertions if so),  correspondingly the error checks here 
on 118 and 121 are sort a bit overprotective of yourself, that is you also have 
a check in handle for len(assertions) > 0, etc in handle().  nit of course.

> +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'))

any useful errors to deal with or log at least around write_file, or the 'snap 
ack'?

> +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