cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Wesley,

Thanks.

Please add a 'doc' in 
 doc/examples/cloud-config-lxd.txt

like the other files there.

see inline comments

Diff comments:

> 
> === added file 'cloudinit/config/cc_lxd.py'
> --- cloudinit/config/cc_lxd.py	1970-01-01 00:00:00 +0000
> +++ cloudinit/config/cc_lxd.py	2016-02-04 07:34:57 +0000
> @@ -0,0 +1,50 @@
> +# vi: ts=4 expandtab
> +#
> +#    Copyright (C) 2016 Canonical Ltd.
> +#
> +#    Author: Wesley Wiedenmeier <wesley.wiedenmeier@xxxxxxxxxxxxx>
> +#
> +#    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/>.
> +
> +"""
> +This module initializes lxd using 'lxd init'
> +
> +Example config:
> +  #cloud-config
> +  lxd:
> +    init:
> +      network_address: <ip addr>
> +      network_port: <port>
> +      storage_backend: <zfs/dir>
> +      storage_create_device: <dev>
> +      storage_create_loop: <size>
> +      storage_pool: <name>
> +      trust_password: <password>
> +"""
> +
> +from cloudinit import util
> +
> +
> +def handle(name, cfg, cloud, log, args):
> +    if not cfg.get('lxd') and cfg['lxd'].get('init'):
> +        log.debug("Skipping module named %s, not present or disabled by cfg")
> +        return

please check to make sure cfg['lxd'] is a dict and warn if not.
the code above would fail oddly with:
 lxd: "foobar"
or
 lxd: ['foobar']

> +    lxd_conf = cfg['lxd']['init']
> +    keys = ('network_address', 'network_port', 'storage_backend',
> +            'storage_create_device', 'storage_create_loop', 'storage_pool',
> +            'trust_password')
> +    cmd = ['lxd', 'init', '--auto']
> +    for k in keys:
> +        if lxd_conf.get(k):
> +            cmd.extend(["--%s" % k.replace('_', '-'), lxd_conf[k]])
> +    util.subp(cmd)

At very least do a check for util.which('lxd') and warn if not present.
it would be ncie to install the package if necessary (although in almost all realities in ubuntu lxd will be present anywhere where you'd try to use it -- wily+)

> 
> === added file 'tests/unittests/test_handler/test_handler_lxd.py'
> --- tests/unittests/test_handler/test_handler_lxd.py	1970-01-01 00:00:00 +0000
> +++ tests/unittests/test_handler/test_handler_lxd.py	2016-02-04 07:34:57 +0000
> @@ -0,0 +1,62 @@
> +from cloudinit.config import cc_lxd
> +from cloudinit import (util, distros, helpers, cloud)
> +from cloudinit.sources import DataSourceNoCloud
> +from .. import helpers as t_help
> +
> +import logging
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +class TestLxd(t_help.TestCase):
> +    def setUp(self):
> +        super(TestLxd, self).setUp()
> +        self.unapply = []
> +        apply_patches([(util, 'subp', self._mock_subp)])
> +        self.subp_called = []
> +
> +    def tearDown(self):
> +        apply_patches([i for i in reversed(self.unapply)])

i think you dont ever unapply (where would self.unapply get added?)
feel free to use mock if you're more comfortable with that.

> +
> +    def _mock_subp(self, *args, **kwargs):
> +        if 'args' not in kwargs:
> +            kwargs['args'] = args[0]
> +        self.subp_called.append(kwargs)
> +        return
> +
> +    def _get_cloud(self, distro):
> +        cls = distros.fetch(distro)
> +        paths = helpers.Paths({})
> +        d = cls(distro, {}, paths)
> +        ds = DataSourceNoCloud.DataSourceNoCloud({}, d, paths)
> +        cc = cloud.Cloud(ds, paths, {}, d, None)
> +        return cc
> +
> +    def test_lxd_init(self):
> +        cfg = {
> +            'lxd': {
> +                'init': {
> +                    'network_address': '0.0.0.0',
> +                    'storage_backend': 'zfs',
> +                    'storage_pool': 'poolname',
> +                }
> +            }
> +        }
> +        cc = self._get_cloud('ubuntu')
> +        cc_lxd.handle('cc_lxd', cfg, cc, LOG, [])
> +
> +        self.assertEqual(
> +                self.subp_called[0].get('args'),
> +                ['lxd', 'init', '--auto', '--network-address', '0.0.0.0',
> +                 '--storage-backend', 'zfs', '--storage-pool', 'poolname'])
> +
> +
> +def apply_patches(patches):
> +    ret = []
> +    for (ref, name, replace) in patches:
> +        if replace is None:
> +            continue
> +        orig = getattr(ref, name)
> +        setattr(ref, name, replace)
> +        ret.append((ref, name, orig))
> +    return ret


-- 
https://code.launchpad.net/~wesley-wiedenmeier/cloud-init/lxd-init/+merge/285018
Your team cloud init development team is requested to review the proposed merge of lp:~wesley-wiedenmeier/cloud-init/lxd-init into lp:cloud-init.