diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 0d54a145c3..e839b9396e 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -891,6 +891,47 @@ # value) #image_store_keyfile = +# Name of the user to use for Ansible when connecting to the +# ramdisk over SSH. It may be overriden by per-node +# 'ansible_username' option in node's 'driver_info' field. +# (string value) +#default_username = ansible + +# Absolute path to the private SSH key file to use by Ansible +# by default when connecting to the ramdisk over SSH. Default +# is to use default SSH keys configured for the user running +# the ironic-conductor service. Private keys with password +# must be pre-loaded into 'ssh-agent'. It may be overriden by +# per-node 'ansible_key_file' option in node's 'driver_info' +# field. (string value) +#default_key_file = + +# Path (relative to $playbooks_path or absolute) to the +# default playbook used for deployment. It may be overriden by +# per-node 'ansible_deploy_playbook' option in node's +# 'driver_info' field. (string value) +#default_deploy_playbook = deploy.yaml + +# Path (relative to $playbooks_path or absolute) to the +# default playbook used for graceful in-band shutdown of the +# node. It may be overriden by per-node +# 'ansible_shutdown_playbook' option in node's 'driver_info' +# field. (string value) +#default_shutdown_playbook = shutdown.yaml + +# Path (relative to $playbooks_path or absolute) to the +# default playbook used for node cleaning. It may be overriden +# by per-node 'ansible_clean_playbook' option in node's +# 'driver_info' field. (string value) +#default_clean_playbook = clean.yaml + +# Path (relative to $playbooks_path or absolute) to the +# default auxiliary cleaning steps file used during the node +# cleaning. It may be overriden by per-node +# 'ansible_clean_steps_config' option in node's 'driver_info' +# field. (string value) +#default_clean_steps_config = clean_steps.yaml + [api] diff --git a/ironic/conf/ansible.py b/ironic/conf/ansible.py index cfd5162f0c..b72af6bb0a 100644 --- a/ironic/conf/ansible.py +++ b/ironic/conf/ansible.py @@ -89,6 +89,51 @@ opts = [ 'to image store. ' 'Is not used by default playbooks included with ' 'the driver.')), + cfg.StrOpt('default_username', + default='ansible', + help=_("Name of the user to use for Ansible when connecting " + "to the ramdisk over SSH. It may be overriden " + "by per-node 'ansible_username' option " + "in node's 'driver_info' field.")), + cfg.StrOpt('default_key_file', + help=_("Absolute path to the private SSH key file to use " + "by Ansible by default when connecting to the ramdisk " + "over SSH. Default is to use default SSH keys " + "configured for the user running the ironic-conductor " + "service. Private keys with password must be pre-loaded " + "into 'ssh-agent'. It may be overriden by per-node " + "'ansible_key_file' option in node's " + "'driver_info' field.")), + cfg.StrOpt('default_deploy_playbook', + default='deploy.yaml', + help=_("Path (relative to $playbooks_path or absolute) " + "to the default playbook used for deployment. " + "It may be overriden by per-node " + "'ansible_deploy_playbook' option in node's " + "'driver_info' field.")), + cfg.StrOpt('default_shutdown_playbook', + default='shutdown.yaml', + help=_("Path (relative to $playbooks_path or absolute) " + "to the default playbook used for graceful in-band " + "shutdown of the node. " + "It may be overriden by per-node " + "'ansible_shutdown_playbook' option in node's " + "'driver_info' field.")), + cfg.StrOpt('default_clean_playbook', + default='clean.yaml', + help=_("Path (relative to $playbooks_path or absolute) " + "to the default playbook used for node cleaning. " + "It may be overriden by per-node " + "'ansible_clean_playbook' option in node's " + "'driver_info' field.")), + cfg.StrOpt('default_clean_steps_config', + default='clean_steps.yaml', + help=_("Path (relative to $playbooks_path or absolute) " + "to the default auxiliary cleaning steps file used " + "during the node cleaning. " + "It may be overriden by per-node " + "'ansible_clean_steps_config' option in node's " + "'driver_info' field.")), ] diff --git a/ironic/drivers/modules/ansible/deploy.py b/ironic/drivers/modules/ansible/deploy.py index af30290e29..8a4adadbaf 100644 --- a/ironic/drivers/modules/ansible/deploy.py +++ b/ironic/drivers/modules/ansible/deploy.py @@ -48,56 +48,78 @@ LOG = log.getLogger(__name__) METRICS = metrics_utils.get_metrics_logger(__name__) -DEFAULT_PLAYBOOKS = { - 'deploy': 'deploy.yaml', - 'shutdown': 'shutdown.yaml', - 'clean': 'clean.yaml' -} -DEFAULT_CLEAN_STEPS = 'clean_steps.yaml' - OPTIONAL_PROPERTIES = { - 'ansible_deploy_username': _('Deploy ramdisk username for Ansible. ' - 'This user must have passwordless sudo ' - 'permissions. Default is "ansible". ' - 'Optional.'), - 'ansible_deploy_key_file': _('Path to private key file. If not specified, ' - 'default keys for user running ' - 'ironic-conductor process will be used. ' - 'Note that for keys with password, those ' - 'must be pre-loaded into ssh-agent. ' - 'Optional.'), - 'ansible_deploy_playbook': _('Name of the Ansible playbook used for ' - 'deployment. Default is %s. Optional.' - ) % DEFAULT_PLAYBOOKS['deploy'], - 'ansible_shutdown_playbook': _('Name of the Ansible playbook used to ' - 'power off the node in-band. ' - 'Default is %s. Optional.' - ) % DEFAULT_PLAYBOOKS['shutdown'], - 'ansible_clean_playbook': _('Name of the Ansible playbook used for ' - 'cleaning. Default is %s. Optional.' - ) % DEFAULT_PLAYBOOKS['clean'], - 'ansible_clean_steps_config': _('Name of the file with default cleaning ' - 'steps configuration. Default is %s. ' - 'Optional.' - ) % DEFAULT_CLEAN_STEPS + 'ansible_username': _('Deploy ramdisk username for Ansible. ' + 'This user must have passwordless sudo ' + 'permissions. Optional.'), + 'ansible_key_file': _('Full path to private SSH key file. ' + 'If not specified, default keys for user running ' + 'ironic-conductor process will be used. ' + 'Note that for keys with password, those ' + 'must be pre-loaded into ssh-agent. ' + 'Optional.'), + 'ansible_playbooks_path': _('Path to folder holding playbooks to use ' + 'for this node. Optional. ' + 'Default is set in ironic config.'), + 'ansible_deploy_playbook': _('Name of the Ansible playbook file inside ' + 'the "ansible_playbooks_path" folder which ' + 'is used for node deployment. Optional.'), + 'ansible_shutdown_playbook': _('Name of the Ansible playbook file inside ' + 'the "ansible_playbooks_path" folder which ' + 'is used for node shutdown. Optional.'), + 'ansible_clean_playbook': _('Name of the Ansible playbook file inside ' + 'the "ansible_playbooks_path" folder which ' + 'is used for node cleaning. Optional.'), + 'ansible_clean_steps_config': _('Name of the file inside the ' + '"ansible_playbooks_path" folder with ' + 'cleaning steps configuration. Optional.'), } -COMMON_PROPERTIES = OPTIONAL_PROPERTIES -INVENTORY_FILE = os.path.join(CONF.ansible.playbooks_path, 'inventory') +COMMON_PROPERTIES = OPTIONAL_PROPERTIES +# TODO(pas-ha) remove in Rocky +DEPRECATED_PROPERTIES = { + 'ansible_deploy_username': { + 'name': 'ansible_username', + 'warned': False}, + 'ansible_deploy_key_file': { + 'name': 'ansible_key_file', + 'warned': False}} class PlaybookNotFound(exception.IronicException): _msg_fmt = _('Failed to set ansible playbook for action %(action)s') +def _get_playbooks_path(node): + return node.driver_info.get('ansible_playbooks_path', + CONF.ansible.playbooks_path) + + def _parse_ansible_driver_info(node, action='deploy'): - user = node.driver_info.get('ansible_deploy_username', 'ansible') - key = node.driver_info.get('ansible_deploy_key_file') + # TODO(pas-ha) remove in Rocky + for old, new in DEPRECATED_PROPERTIES.items(): + if old in node.driver_info: + if not new['warned']: + LOG.warning("Driver property '%(old)s' is deprecated, " + "and will be ignored in Rocky release. " + "Use '%(new)s' instead.", old=old, new=new['name']) + new['warned'] = True + # TODO(pas-ha) simplify in Rocky + user = node.driver_info.get( + 'ansible_username', + node.driver_info.get('ansible_deploy_username', + CONF.ansible.default_username)) + key = node.driver_info.get( + 'ansible_key_file', + node.driver_info.get('ansible_deploy_key_file', + CONF.ansible.default_key_file)) playbook = node.driver_info.get('ansible_%s_playbook' % action, - DEFAULT_PLAYBOOKS.get(action)) + getattr(CONF.ansible, + 'default_%s_playbook' % action, + None)) if not playbook: raise PlaybookNotFound(action=action) - return playbook, user, key + return os.path.basename(playbook), user, key def _get_configdrive_path(basename): @@ -119,12 +141,14 @@ def _prepare_extra_vars(host_list, variables=None): return extra_vars -def _run_playbook(name, extra_vars, key, tags=None, notags=None): +def _run_playbook(node, name, extra_vars, key, tags=None, notags=None): """Execute ansible-playbook.""" - playbook = os.path.join(CONF.ansible.playbooks_path, name) + root = _get_playbooks_path(node) + playbook = os.path.join(root, name) + inventory = os.path.join(root, 'inventory') ironic_vars = {'ironic': extra_vars} args = [CONF.ansible.ansible_playbook_script, playbook, - '-i', INVENTORY_FILE, + '-i', inventory, '-e', json.dumps(ironic_vars), ] @@ -326,9 +350,11 @@ def _validate_clean_steps(steps, node_uuid): def _get_clean_steps(node, interface=None, override_priorities=None): """Get cleaning steps.""" - clean_steps_file = node.driver_info.get('ansible_clean_steps_config', - DEFAULT_CLEAN_STEPS) - path = os.path.join(CONF.ansible.playbooks_path, clean_steps_file) + clean_steps_file = node.driver_info.get( + 'ansible_clean_steps_config', CONF.ansible.default_clean_steps_config) + path = os.path.join(node.driver_info.get('ansible_playbooks_path', + CONF.ansible.playbooks_path), + os.path.basename(clean_steps_file)) try: with open(path) as f: internal_steps = yaml.safe_load(f) @@ -403,6 +429,8 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): deploy_utils.check_for_missing_params(params, error_msg) # validate root device hints, proper exceptions are raised from there _parse_root_device_hints(node) + # TODO(pas-ha) validate that all playbooks and ssh key (if set) + # are pointing to actual files def _ansible_deploy(self, task, node_address): """Internal function for deployment to a node.""" @@ -418,7 +446,7 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): LOG.debug('Starting deploy on node %s', node.uuid) # any caller should manage exceptions raised from here - _run_playbook(playbook, extra_vars, key) + _run_playbook(node, playbook, extra_vars, key) @METRICS.timer('AnsibleDeploy.deploy') @task_manager.require_exclusive_lock @@ -501,8 +529,7 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): {'node': node.uuid, 'step': stepname}) step_tags = step['args'].get('tags', []) try: - _run_playbook(playbook, extra_vars, key, - tags=step_tags) + _run_playbook(node, playbook, extra_vars, key, tags=step_tags) except exception.InstanceDeployFailure as e: LOG.error("Ansible failed cleaning step %(step)s " "on node %(node)s.", @@ -591,7 +618,7 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): node, action='shutdown') node_list = [(node.uuid, node_address, user, node.extra)] extra_vars = _prepare_extra_vars(node_list) - _run_playbook(playbook, extra_vars, key) + _run_playbook(node, playbook, extra_vars, key) _wait_until_powered_off(task) except Exception as e: LOG.warning('Failed to soft power off node %(node_uuid)s ' diff --git a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py index f70c765f12..fe94b339bd 100644 --- a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py +++ b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py @@ -41,8 +41,8 @@ INSTANCE_INFO = { DRIVER_INFO = { 'deploy_kernel': 'glance://deploy_kernel_uuid', 'deploy_ramdisk': 'glance://deploy_ramdisk_uuid', - 'ansible_deploy_username': 'test', - 'ansible_deploy_key_file': '/path/key', + 'ansible_username': 'test', + 'ansible_key_file': '/path/key', 'ipmi_address': '127.0.0.1', } DRIVER_INTERNAL_INFO = { @@ -71,12 +71,47 @@ class AnsibleDeployTestCaseBase(db_base.DbTestCase): class TestAnsibleMethods(AnsibleDeployTestCaseBase): def test__parse_ansible_driver_info(self): + self.node.driver_info['ansible_deploy_playbook'] = 'spam.yaml' playbook, user, key = ansible_deploy._parse_ansible_driver_info( self.node, 'deploy') - self.assertEqual(ansible_deploy.DEFAULT_PLAYBOOKS['deploy'], playbook) + self.assertEqual('spam.yaml', playbook) self.assertEqual('test', user) self.assertEqual('/path/key', key) + def test__parse_ansible_driver_info_defaults(self): + self.node.driver_info.pop('ansible_username') + self.node.driver_info.pop('ansible_key_file') + self.config(group='ansible', + default_username='spam', + default_key_file='/ham/eggs', + default_deploy_playbook='parrot.yaml') + playbook, user, key = ansible_deploy._parse_ansible_driver_info( + self.node, 'deploy') + # testing absolute path to the playbook + self.assertEqual('parrot.yaml', playbook) + self.assertEqual('spam', user) + self.assertEqual('/ham/eggs', key) + + @mock.patch.object(ansible_deploy.LOG, 'warning', autospec=True) + def test__parse_ansible_driver_info_deprecated_opts(self, warn_mock): + self.node.driver_info[ + 'ansible_deploy_username'] = self.node.driver_info.pop( + 'ansible_username') + self.node.driver_info[ + 'ansible_deploy_key_file'] = self.node.driver_info.pop( + 'ansible_key_file') + playbook, user, key = ansible_deploy._parse_ansible_driver_info( + self.node, 'deploy') + self.assertEqual(ansible_deploy.CONF.ansible.default_deploy_playbook, + playbook) + self.assertEqual('test', user) + self.assertEqual('/path/key', key) + self.assertEqual(2, warn_mock.call_count) + # check that we remeber about warnings havig been displayed + playbook, user, key = ansible_deploy._parse_ansible_driver_info( + self.node, 'deploy') + self.assertEqual(2, warn_mock.call_count) + def test__parse_ansible_driver_info_no_playbook(self): self.assertRaises(exception.IronicException, ansible_deploy._parse_ansible_driver_info, @@ -101,13 +136,14 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase): self.config(group='ansible', ansible_extra_args='--timeout=100') extra_vars = {'foo': 'bar'} - ansible_deploy._run_playbook('deploy', extra_vars, '/path/to/key', + ansible_deploy._run_playbook(self.node, 'deploy', + extra_vars, '/path/to/key', tags=['spam'], notags=['ham']) execute_mock.assert_called_once_with( 'env', 'ANSIBLE_CONFIG=/path/to/config', 'ansible-playbook', '/path/to/playbooks/deploy', '-i', - ansible_deploy.INVENTORY_FILE, '-e', '{"ironic": {"foo": "bar"}}', + '/path/to/playbooks/inventory', '-e', '{"ironic": {"foo": "bar"}}', '--tags=spam', '--skip-tags=ham', '--private-key=/path/to/key', '-vvv', '--timeout=100') @@ -119,12 +155,13 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase): self.config(debug=False) extra_vars = {'foo': 'bar'} - ansible_deploy._run_playbook('deploy', extra_vars, '/path/to/key') + ansible_deploy._run_playbook(self.node, 'deploy', extra_vars, + '/path/to/key') execute_mock.assert_called_once_with( 'env', 'ANSIBLE_CONFIG=/path/to/config', 'ansible-playbook', '/path/to/playbooks/deploy', '-i', - ansible_deploy.INVENTORY_FILE, '-e', '{"ironic": {"foo": "bar"}}', + '/path/to/playbooks/inventory', '-e', '{"ironic": {"foo": "bar"}}', '--private-key=/path/to/key') @mock.patch.object(com_utils, 'execute', return_value=('out', 'err'), @@ -135,12 +172,13 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase): self.config(debug=True) extra_vars = {'foo': 'bar'} - ansible_deploy._run_playbook('deploy', extra_vars, '/path/to/key') + ansible_deploy._run_playbook(self.node, 'deploy', extra_vars, + '/path/to/key') execute_mock.assert_called_once_with( 'env', 'ANSIBLE_CONFIG=/path/to/config', 'ansible-playbook', '/path/to/playbooks/deploy', '-i', - ansible_deploy.INVENTORY_FILE, '-e', '{"ironic": {"foo": "bar"}}', + '/path/to/playbooks/inventory', '-e', '{"ironic": {"foo": "bar"}}', '--private-key=/path/to/key', '-vvvv') @mock.patch.object(com_utils, 'execute', @@ -155,12 +193,13 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase): exc = self.assertRaises(exception.InstanceDeployFailure, ansible_deploy._run_playbook, - 'deploy', extra_vars, '/path/to/key') + self.node, 'deploy', extra_vars, + '/path/to/key') self.assertIn('VIKINGS!', six.text_type(exc)) execute_mock.assert_called_once_with( 'env', 'ANSIBLE_CONFIG=/path/to/config', 'ansible-playbook', '/path/to/playbooks/deploy', '-i', - ansible_deploy.INVENTORY_FILE, '-e', '{"ironic": {"foo": "bar"}}', + '/path/to/playbooks/inventory', '-e', '{"ironic": {"foo": "bar"}}', '--private-key=/path/to/key') def test__parse_partitioning_info_root_msdos(self): @@ -619,7 +658,7 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): prepare_extra_mock.assert_called_once_with( ironic_nodes['ironic_nodes']) run_playbook_mock.assert_called_once_with( - 'test_pl', ironic_nodes, 'test_k', tags=['clean']) + task.node, 'test_pl', ironic_nodes, 'test_k', tags=['clean']) @mock.patch.object(ansible_deploy, '_parse_ansible_driver_info', return_value=('test_pl', 'test_u', 'test_k'), @@ -735,7 +774,7 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): [(self.node['uuid'], '127.0.0.1', 'test_u', {'ham': 'spam'})], variables=_vars) run_playbook_mock.assert_called_once_with( - 'test_pl', ironic_nodes, 'test_k') + task.node, 'test_pl', ironic_nodes, 'test_k') @mock.patch.object(ansible_deploy, '_run_playbook', autospec=True) @mock.patch.object(ansible_deploy, '_prepare_extra_vars', autospec=True) @@ -770,8 +809,8 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): prepare_extra_mock.assert_called_once_with( [(self.node['uuid'], '127.0.0.1', 'test_u', {'ham': 'spam'})], variables=_vars) - run_playbook_mock.assert_called_once_with('test_pl', ironic_nodes, - 'test_k') + run_playbook_mock.assert_called_once_with( + task.node, 'test_pl', ironic_nodes, 'test_k') @mock.patch.object(fake.FakePower, 'get_power_state', return_value=states.POWER_OFF) @@ -804,7 +843,7 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): @mock.patch.object(utils, 'node_power_action', autospec=True) def test_reboot_and_finish_deploy_soft_poweroff_retry(self, power_action_mock, - ansible_mock): + run_playbook_mock): self.config(group='ansible', post_deploy_get_power_state_retry_interval=0) self.config(group='ansible', @@ -834,8 +873,8 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): ((task, states.POWER_ON),)] self.assertEqual(expected_power_calls, power_action_mock.call_args_list) - ansible_mock.assert_called_once_with('shutdown.yaml', - mock.ANY, mock.ANY) + run_playbook_mock.assert_called_once_with( + task.node, 'shutdown.yaml', mock.ANY, mock.ANY) @mock.patch.object(ansible_deploy, '_get_node_ip', autospec=True, return_value='1.2.3.4') diff --git a/releasenotes/notes/ansible-deploy-15da234580ca0c30.yaml b/releasenotes/notes/ansible-deploy-15da234580ca0c30.yaml index c49b3e700b..526966e530 100644 --- a/releasenotes/notes/ansible-deploy-15da234580ca0c30.yaml +++ b/releasenotes/notes/ansible-deploy-15da234580ca0c30.yaml @@ -9,3 +9,21 @@ features: its subclasses, but must be explicitly enabled in the ``[DEFAULT]enabled_deploy_interfaces`` configuration file option to actually allow setting nodes to use it. + + For migration from the ``staging-ansible`` interface from the + ``ironic-staging-drivers`` project to this ``ansible`` interface, + operators have to consider the following differences: + + - callback-less operation is not supported + - driver_info fields 'ansible_deploy_username' and + 'ansible_deploy_key_file' are deprecated and will be removed + in the Rocky release, use 'ansible_username' and 'ansible_key_file' + respectively + - base path for playbooks can be defined in driver_info as well + (as 'ansible_playbooks_path' field, defaults to the value of + ``[ansible]/playbooks_path`` from ironic configuration file + - default playbooks for actions and cleaning steps file can be set in + ironic configuration file as various ``[ansible]/default_*`` options. + + Please read the ``ansible`` deploy interface documentation for more + information.