openstack/metalsmith
Code Issues Proposed changes

Split InstanceConfig into GenericConfig and CloudInitConfig

Browse Source 
As a side effect allows providing custom user data, not only users.

Change-Id: Ia4c305ea996b3bb58c5898777cc7a3c8b93a413d
This commit is contained in:
Dmitry Tantsur 2019-05-24 15:07:51 +02:00
parent 4920cc26a3
commit 349e5074e3
8 changed files with 171 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
metalsmith/__init__.py
View File

@ -13,9 +13,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from metalsmith._config import InstanceConfig
from metalsmith._instance import Instance
from metalsmith._instance import InstanceState
from metalsmith._provisioner import Provisioner
from metalsmith.instance_config import InstanceConfig
__all__ = ['Instance', 'InstanceConfig', 'InstanceState', 'Provisioner']

6
metalsmith/_cmd.py
View File

@ -19,10 +19,10 @@ import sys
from openstack import config as os_config
from metalsmith import _config
from metalsmith import _format
from metalsmith import _provisioner
from metalsmith import _utils
from metalsmith import instance_config
from metalsmith import sources
@ -63,9 +63,11 @@ def _do_deploy(api, args, formatter):
ramdisk=args.image_ramdisk,
checksum=args.image_checksum)
config = _config.InstanceConfig(ssh_keys=ssh_keys)
if args.user_name:
config = instance_config.CloudInitConfig(ssh_keys=ssh_keys)
config.add_user(args.user_name, sudo=args.passwordless_sudo)
else:
config = instance_config.GenericConfig(ssh_keys=ssh_keys)
node = api.reserve_node(resource_class=args.resource_class,
conductor_group=args.conductor_group,

8
metalsmith/_provisioner.py
View File

@ -21,12 +21,12 @@ from openstack import connection
from openstack import exceptions as os_exc
import six
from metalsmith import _config
from metalsmith import _instance
from metalsmith import _nics
from metalsmith import _scheduler
from metalsmith import _utils
from metalsmith import exceptions
from metalsmith import instance_config
from metalsmith import sources
@ -304,8 +304,8 @@ class Provisioner(_utils.GetNodeMixin):
the value of the local_gb property is used.
:param swap_size_mb: The size of the swap partition. It's an error
to specify it for a whole disk image.
:param config: :py:class:`metalsmith.InstanceConfig` object with
the configuration to pass to the instance.
:param config: configuration to pass to the instance, one of
objects from :py:mod:`metalsmith.instance_config`.
:param hostname: Hostname to assign to the instance. If provided,
overrides the ``hostname`` passed to ``reserve_node``.
:param netboot: Whether to use networking boot for final instances.
@ -328,7 +328,7 @@ class Provisioner(_utils.GetNodeMixin):
:raises: :py:class:`metalsmith.exceptions.Error`
"""
if config is None:
config = _config.InstanceConfig()
config = instance_config.GenericConfig()
if isinstance(image, six.string_types):
image = sources.GlanceImage(image)

124
metalsmith/_config.py → metalsmith/instance_config.py
View File

@ -13,6 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import copy
import json
import logging
import warnings
@ -25,41 +26,23 @@ from metalsmith import _utils
LOG = logging.getLogger(__name__)
class InstanceConfig(object):
class GenericConfig(object):
"""Configuration of the target instance.
The information attached to this object will be passed via a configdrive
to the instance's first boot script (e.g. cloud-init).
This class represents generic configuration compatible with most first-boot
implementations. Use :py:class:`CloudInitConfig` for features specific to
`cloud-init <https://cloudinit.readthedocs.io/>`_.
:ivar ssh_keys: List of SSH public keys.
:ivar users: Users to add on first boot.
:ivar user_data: User data as a string.
"""
def __init__(self, ssh_keys=None):
def __init__(self, ssh_keys=None, user_data=None):
self.ssh_keys = ssh_keys or []
self.users = []
def add_user(self, name, admin=True, password_hash=None, sudo=False,
**kwargs):
"""Add a user to be created on first boot.
:param name: user name.
:param admin: whether to add the user to the admin group (wheel).
:param password_hash: user password hash, if password authentication
is expected.
:param sudo: whether to allow the user sudo without password.
:param kwargs: other arguments to pass.
"""
kwargs['name'] = name
if admin:
kwargs.setdefault('groups', []).append('wheel')
if password_hash:
kwargs['passwd'] = password_hash
if sudo:
kwargs['sudo'] = 'ALL=(ALL) NOPASSWD:ALL'
if self.ssh_keys:
kwargs.setdefault('ssh_authorized_keys', self.ssh_keys)
self.users.append(kwargs)
self.user_data = user_data
def generate(self, node):
"""Generate the config drive information.
@ -88,17 +71,19 @@ class InstanceConfig(object):
'availability_zone': '',
'files': [],
'meta': {}}
user_data = {}
user_data_str = None
if self.users:
user_data['users'] = self.users
if user_data:
user_data_str = "#cloud-config\n" + json.dumps(user_data)
user_data = self.populate_user_data()
return {'meta_data': metadata,
'user_data': user_data_str}
'user_data': user_data}
def populate_user_data(self):
"""Get user data for this configuration.
Can be overridden to provide additional features.
:return: user data as a string.
"""
return self.user_data
def build_configdrive(self, node):
"""Make the config drive ISO.
@ -122,3 +107,72 @@ class InstanceConfig(object):
'metadata %(meta)s', {'node': _utils.log_res(node),
'meta': metadata})
return configdrive.build(metadata, user_data=user_data, **cd)
class CloudInitConfig(GenericConfig):
"""Configuration of the target instance using cloud-init.
Compared to :class:`GenericConfig`, this adds support for managing users.
:ivar ssh_keys: List of SSH public keys.
:ivar user_data: Cloud-init script as a dictionary.
:ivar users: Users to add on first boot.
"""
def __init__(self, ssh_keys=None, user_data=None):
if user_data is not None and not isinstance(user_data, dict):
raise TypeError('Custom user data must be a dictionary for '
'CloudInitConfig, got %r' % user_data)
super(CloudInitConfig, self).__init__(ssh_keys, user_data or {})
self.users = []
def add_user(self, name, admin=True, password_hash=None, sudo=False,
**kwargs):
"""Add a user to be created on first boot.
:param name: user name.
:param admin: whether to add the user to the admin group (wheel).
:param password_hash: user password hash, if password authentication
is expected.
:param sudo: whether to allow the user sudo without password.
:param kwargs: other arguments to pass.
"""
kwargs['name'] = name
if admin:
kwargs.setdefault('groups', []).append('wheel')
if password_hash:
kwargs['passwd'] = password_hash
if sudo:
kwargs['sudo'] = 'ALL=(ALL) NOPASSWD:ALL'
if self.ssh_keys:
kwargs.setdefault('ssh_authorized_keys', self.ssh_keys)
self.users.append(kwargs)
def populate_user_data(self):
"""Get user data for this configuration.
Takes the custom user data and appends requested users to it.
:return: user data as a string.
"""
if not isinstance(self.user_data, dict):
raise TypeError('Custom user data must be a dictionary for '
'CloudInitConfig, got %r' % self.user_data)
if self.users:
user_data = copy.deepcopy(self.user_data)
user_data.setdefault('users', []).extend(self.users)
else:
user_data = self.user_data
if user_data:
return "#cloud-config\n" + json.dumps(user_data)
class InstanceConfig(CloudInitConfig):
"""DEPRECATED, use :class:`.GenericConfig` or :class:`.CloudInitConfig`."""
def __init__(self, *args, **kwargs):
warnings.warn('InstanceConfig is deprecated, use GenericConfig or '
'CloudInitConfig instead', DeprecationWarning)
super(InstanceConfig, self).__init__(*args, **kwargs)

8
metalsmith/test/test_cmd.py
View File

@ -22,9 +22,9 @@ import six
import testtools
from metalsmith import _cmd
from metalsmith import _config
from metalsmith import _instance
from metalsmith import _provisioner
from metalsmith import instance_config
from metalsmith import sources
@ -301,7 +301,8 @@ class TestDeploy(testtools.TestCase):
config = mock_pr.return_value.provision_node.call_args[1]['config']
self.assertEqual(['foo'], config.ssh_keys)
@mock.patch.object(_config.InstanceConfig, 'add_user', autospec=True)
@mock.patch.object(instance_config.CloudInitConfig, 'add_user',
autospec=True)
def test_args_user_name(self, mock_add_user, mock_pr):
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
'--user-name', 'banana', '--resource-class', 'compute']
@ -311,7 +312,8 @@ class TestDeploy(testtools.TestCase):
self.assertEqual([], config.ssh_keys)
mock_add_user.assert_called_once_with(config, 'banana', sudo=False)
@mock.patch.object(_config.InstanceConfig, 'add_user', autospec=True)
@mock.patch.object(instance_config.CloudInitConfig, 'add_user',
autospec=True)
def test_args_user_name_with_sudo(self, mock_add_user, mock_pr):
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
'--user-name', 'banana', '--resource-class', 'compute',

65
metalsmith/test/test_config.py → metalsmith/test/test_instance_config.py
View File

@ -19,17 +19,21 @@ import mock
from openstack.baremetal import configdrive
import testtools
from metalsmith import _config
import metalsmith
from metalsmith import _utils
from metalsmith import instance_config
class TestInstanceConfig(testtools.TestCase):
class TestGenericConfig(testtools.TestCase):
CLASS = instance_config.GenericConfig
def setUp(self):
super(TestInstanceConfig, self).setUp()
super(TestGenericConfig, self).setUp()
self.node = mock.Mock(id='1234')
self.node.name = 'node name'
def _check(self, config, expected_metadata, expected_userdata=None):
def _check(self, config, expected_metadata, expected_userdata=None,
cloud_init=True):
expected_m = {'public_keys': {},
'uuid': '1234',
'name': 'node name',
@ -51,38 +55,47 @@ class TestInstanceConfig(testtools.TestCase):
if expected_userdata:
self.assertIsNotNone(user_data)
user_data = user_data.decode('utf-8')
header, user_data = user_data.split('\n', 1)
self.assertEqual('#cloud-config', header)
if cloud_init:
header, user_data = user_data.split('\n', 1)
self.assertEqual('#cloud-config', header)
user_data = json.loads(user_data)
self.assertEqual(expected_userdata, user_data)
def test_default(self):
config = _config.InstanceConfig()
config = self.CLASS()
self._check(config, {})
def test_ssh_keys(self):
config = _config.InstanceConfig(ssh_keys=['abc', 'def'])
config = self.CLASS(ssh_keys=['abc', 'def'])
self._check(config, {'public_keys': {'0': 'abc', '1': 'def'}})
def test_ssh_keys_as_dict(self):
config = _config.InstanceConfig(ssh_keys={'default': 'abc'})
config = self.CLASS(ssh_keys={'default': 'abc'})
self._check(config, {'public_keys': {'default': 'abc'}})
def test_custom_user_data(self):
config = self.CLASS(user_data='{"answer": 42}')
self._check(config, {}, {"answer": 42}, cloud_init=False)
class TestCloudInitConfig(TestGenericConfig):
CLASS = instance_config.CloudInitConfig
def test_add_user(self):
config = _config.InstanceConfig()
config = self.CLASS()
config.add_user('admin')
self._check(config, {},
{'users': [{'name': 'admin',
'groups': ['wheel']}]})
def test_add_user_admin(self):
config = _config.InstanceConfig()
config = self.CLASS()
config.add_user('admin', admin=False)
self._check(config, {},
{'users': [{'name': 'admin'}]})
def test_add_user_sudo(self):
config = _config.InstanceConfig()
config = self.CLASS()
config.add_user('admin', sudo=True)
self._check(config, {},
{'users': [{'name': 'admin',
@ -90,7 +103,7 @@ class TestInstanceConfig(testtools.TestCase):
'sudo': 'ALL=(ALL) NOPASSWD:ALL'}]})
def test_add_user_passwd(self):
config = _config.InstanceConfig()
config = self.CLASS()
config.add_user('admin', password_hash='123')
self._check(config, {},
{'users': [{'name': 'admin',
@ -98,9 +111,33 @@ class TestInstanceConfig(testtools.TestCase):
'passwd': '123'}]})
def test_add_user_with_keys(self):
config = _config.InstanceConfig(ssh_keys=['abc', 'def'])
config = self.CLASS(ssh_keys=['abc', 'def'])
config.add_user('admin')
self._check(config, {'public_keys': {'0': 'abc', '1': 'def'}},
{'users': [{'name': 'admin',
'groups': ['wheel'],
'ssh_authorized_keys': ['abc', 'def']}]})
# Overriding tests since CloudInitConfig does not support plain strings
# for user_data, only dictionaries.
def test_custom_user_data(self):
config = self.CLASS(user_data={'answer': 42})
self._check(config, {}, {'answer': 42})
def test_custom_user_data_with_users(self):
config = self.CLASS(user_data={'answer': 42})
config.add_user('admin')
self._check(config, {},
{'users': [{'name': 'admin',
'groups': ['wheel']}],
'answer': 42})
def test_user_data_not_dict(self):
self.assertRaises(TypeError, self.CLASS, user_data="string")
config = self.CLASS()
config.user_data = "string"
self.assertRaises(TypeError, config.populate_user_data)
class TestDeprecatedInstanceConfig(TestCloudInitConfig):
CLASS = metalsmith.InstanceConfig

6
metalsmith/test/test_provisioner.py
View File

@ -19,11 +19,11 @@ from openstack import exceptions as os_exc
import requests
import testtools
from metalsmith import _config
from metalsmith import _instance
from metalsmith import _provisioner
from metalsmith import _utils
from metalsmith import exceptions
from metalsmith import instance_config
from metalsmith import sources
@ -395,7 +395,7 @@ class TestProvisionNode(Base):
],
}
self.configdrive_mock = self.useFixture(
fixtures.MockPatchObject(_config.InstanceConfig,
fixtures.MockPatchObject(instance_config.GenericConfig,
'generate', autospec=True)
).mock
@ -459,7 +459,7 @@ class TestProvisionNode(Base):
self.assertFalse(self.api.network.delete_port.called)
def test_with_config(self):
config = mock.Mock(spec=_config.InstanceConfig)
config = mock.Mock(spec=instance_config.GenericConfig)
inst = self.pr.provision_node(self.node, 'image',
[{'network': 'network'}],
config=config)

14
releasenotes/notes/instance-config-c4ea6a169f782138.yaml Normal file
View File

@ -0,0 +1,14 @@
---
features:
- |
It is now possible to provide custom ``user_data`` into instance
configuration.
upgrade:
- |
The ``InstanceConfig`` class has been split into ``GenericConfig`` and
``CloudInitConfig`` for clarity on which features come from what.
deprecations:
- |
The ``metalsmith.InstanceConfig`` class is deprecated, use
``GenericConfig`` or ``CloudInitConfig`` from the new module
``metalsmith.instance_config``.