Merge "[ansible] add defaults to config"

etc/ironic/ironic.conf.sample

@@ -891,6 +891,47 @@
 # value)
 #image_store_keyfile = <None>
 
+# 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 = <None>
+
+# 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]
 

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.")),
 ]
 
 

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. '
+    'ansible_username': _('Deploy ramdisk username for Ansible. '
-                                 'This user must have passwordless sudo '
+                          'This user must have passwordless sudo '
-                                 'permissions. Default is "ansible". '
+                          'permissions. Optional.'),
-                                 'Optional.'),
+    'ansible_key_file': _('Full path to private SSH key file. '
-    'ansible_deploy_key_file': _('Path to private key file. If not specified, '
+                          'If not specified, default keys for user running '
-                                 'default keys for user running '
+                          'ironic-conductor process will be used. '
-                                 'ironic-conductor process will be used. '
+                          'Note that for keys with password, those '
-                                 'Note that for keys with password, those '
+                          'must be pre-loaded into ssh-agent. '
-                                 'must be pre-loaded into ssh-agent. '
+                          'Optional.'),
-                                 'Optional.'),
+    'ansible_playbooks_path': _('Path to folder holding playbooks to use '
-    'ansible_deploy_playbook': _('Name of the Ansible playbook used for '
+                                'for this node. Optional. '
-                                 'deployment. Default is %s. Optional.'
+                                'Default is set in ironic config.'),
-                                 ) % DEFAULT_PLAYBOOKS['deploy'],
+    'ansible_deploy_playbook': _('Name of the Ansible playbook file inside '
-    'ansible_shutdown_playbook': _('Name of the Ansible playbook used to '
+                                 'the "ansible_playbooks_path" folder which '
-                                   'power off the node in-band. '
+                                 'is used for node deployment. Optional.'),
-                                   'Default is %s. Optional.'
+    'ansible_shutdown_playbook': _('Name of the Ansible playbook file inside '
-                                   ) % DEFAULT_PLAYBOOKS['shutdown'],
+                                   'the "ansible_playbooks_path" folder which '
-    'ansible_clean_playbook': _('Name of the Ansible playbook used for '
+                                   'is used for node shutdown. Optional.'),
-                                'cleaning. Default is %s. Optional.'
+    'ansible_clean_playbook': _('Name of the Ansible playbook file inside '
-                                ) % DEFAULT_PLAYBOOKS['clean'],
+                                'the "ansible_playbooks_path" folder which '
-    'ansible_clean_steps_config': _('Name of the file with default cleaning '
+                                'is used for node cleaning. Optional.'),
-                                    'steps configuration. Default is %s. '
+    'ansible_clean_steps_config': _('Name of the file inside the '
-                                    'Optional.'
+                                    '"ansible_playbooks_path" folder with '
-                                    ) % DEFAULT_CLEAN_STEPS
+                                    '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')
+    # TODO(pas-ha) remove in Rocky
-    key = node.driver_info.get('ansible_deploy_key_file')
+    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',
+    clean_steps_file = node.driver_info.get(
-                                            DEFAULT_CLEAN_STEPS)
+        'ansible_clean_steps_config', CONF.ansible.default_clean_steps_config)
-    path = os.path.join(CONF.ansible.playbooks_path, clean_steps_file)
+    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,
+            _run_playbook(node, playbook, extra_vars, key, tags=step_tags)
-                          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 '

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_username': 'test',
-    'ansible_deploy_key_file': '/path/key',
+    '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,
+            run_playbook_mock.assert_called_once_with(
-                                                      'test_k')
+                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',
+            run_playbook_mock.assert_called_once_with(
-                                                 mock.ANY, mock.ANY)
+                task.node, 'shutdown.yaml', mock.ANY, mock.ANY)
 
     @mock.patch.object(ansible_deploy, '_get_node_ip', autospec=True,
                        return_value='1.2.3.4')

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.