Merge "[ansible] improve cleaning"
This commit is contained in:
commit
8aee66dfaa
|
@ -154,7 +154,8 @@ def _get_configdrive_path(basename):
|
|||
return os.path.join(CONF.tempdir, basename + '.cndrive')
|
||||
|
||||
|
||||
def _get_node_ip(task):
|
||||
def _get_node_ip_dhcp(task):
|
||||
"""Get node IP from DHCP provider."""
|
||||
api = dhcp_factory.DHCPFactory().provider
|
||||
ip_addrs = api.get_ip_addresses(task)
|
||||
if not ip_addrs:
|
||||
|
@ -169,6 +170,18 @@ def _get_node_ip(task):
|
|||
return ip_addrs[0]
|
||||
|
||||
|
||||
def _get_node_ip_heartbeat(task):
|
||||
callback_url = task.node.driver_internal_info.get('agent_url', '')
|
||||
return urlparse.urlparse(callback_url).netloc.split(':')[0]
|
||||
|
||||
|
||||
def _get_node_ip(task):
|
||||
if CONF.ansible.use_ramdisk_callback:
|
||||
return _get_node_ip_heartbeat(task)
|
||||
else:
|
||||
return _get_node_ip_dhcp(task)
|
||||
|
||||
|
||||
# some good code from agent
|
||||
def _reboot_and_finish_deploy(task):
|
||||
wait = CONF.ansible.post_deploy_get_power_state_retry_interval * 1000
|
||||
|
@ -323,7 +336,10 @@ def _prepare_variables(task):
|
|||
def _validate_clean_steps(steps, node_uuid):
|
||||
missing = []
|
||||
for step in steps:
|
||||
name = step.setdefault('name', 'unnamed')
|
||||
name = step.get('name')
|
||||
if not name:
|
||||
missing.append({'name': 'undefined', 'field': 'name'})
|
||||
continue
|
||||
if 'interface' not in step:
|
||||
missing.append({'name': name, 'field': 'interface'})
|
||||
args = step.get('args', {})
|
||||
|
@ -338,12 +354,17 @@ def _validate_clean_steps(steps, node_uuid):
|
|||
LOG.error(msg)
|
||||
raise exception.NodeCleaningFailure(node=node_uuid,
|
||||
reason=msg)
|
||||
if len(set(s['name'] for s in steps)) != len(steps):
|
||||
msg = _("Cleaning steps do not have unique names.")
|
||||
LOG.error(msg)
|
||||
raise exception.NodeCleaningFailure(node=node_uuid,
|
||||
reason=msg)
|
||||
|
||||
|
||||
def _get_clean_steps(task, interface=None, override_priorities=None):
|
||||
def _get_clean_steps(node, interface=None, override_priorities=None):
|
||||
"""Get cleaning steps."""
|
||||
clean_steps_file = task.node.driver_info.get('ansible_clean_steps_config',
|
||||
DEFAULT_CLEAN_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)
|
||||
try:
|
||||
with open(path) as f:
|
||||
|
@ -351,9 +372,9 @@ def _get_clean_steps(task, interface=None, override_priorities=None):
|
|||
except Exception as e:
|
||||
msg = _('Failed to load clean steps from file '
|
||||
'%(file)s: %(exc)s') % {'file': path, 'exc': e}
|
||||
raise exception.NodeCleaningFailure(node=task.node.uuid, reason=msg)
|
||||
raise exception.NodeCleaningFailure(node=node.uuid, reason=msg)
|
||||
|
||||
_validate_clean_steps(internal_steps, task.node.uuid)
|
||||
_validate_clean_steps(internal_steps, node.uuid)
|
||||
|
||||
steps = []
|
||||
override = override_priorities or {}
|
||||
|
@ -454,8 +475,9 @@ class AnsibleDeploy(base.DeployInterface):
|
|||
manager_utils.node_power_action(task, states.REBOOT)
|
||||
if CONF.ansible.use_ramdisk_callback:
|
||||
return states.DEPLOYWAIT
|
||||
|
||||
node = task.node
|
||||
ip_addr = _get_node_ip(task)
|
||||
ip_addr = _get_node_ip_dhcp(task)
|
||||
try:
|
||||
_deploy(task, ip_addr)
|
||||
except Exception as e:
|
||||
|
@ -515,7 +537,7 @@ class AnsibleDeploy(base.DeployInterface):
|
|||
'erase_devices_metadata':
|
||||
CONF.deploy.erase_devices_metadata_priority
|
||||
}
|
||||
return _get_clean_steps(task, interface='deploy',
|
||||
return _get_clean_steps(task.node, interface='deploy',
|
||||
override_priorities=new_priorities)
|
||||
|
||||
def execute_clean_step(self, task, step):
|
||||
|
@ -529,13 +551,14 @@ class AnsibleDeploy(base.DeployInterface):
|
|||
playbook, user, key = _parse_ansible_driver_info(
|
||||
task.node, action='clean')
|
||||
stepname = step['step']
|
||||
try:
|
||||
ip_addr = node.driver_internal_info['ansible_cleaning_ip']
|
||||
except KeyError:
|
||||
raise exception.NodeCleaningFailure(node=node.uuid,
|
||||
reason='undefined node IP '
|
||||
'addresses')
|
||||
node_list = [(node.uuid, ip_addr, user, node.extra)]
|
||||
|
||||
if (not CONF.ansible.use_ramdisk_callback and
|
||||
'ansible_cleaning_ip' in node.driver_internal_info):
|
||||
node_address = node.driver_internal_info['ansible_cleaning_ip']
|
||||
else:
|
||||
node_address = _get_node_ip(task)
|
||||
|
||||
node_list = [(node.uuid, node_address, user, node.extra)]
|
||||
extra_vars = _prepare_extra_vars(node_list)
|
||||
|
||||
LOG.debug('Starting cleaning step %(step)s on node %(node)s',
|
||||
|
@ -576,7 +599,7 @@ class AnsibleDeploy(base.DeployInterface):
|
|||
if use_callback:
|
||||
return states.CLEANWAIT
|
||||
|
||||
ip_addr = _get_node_ip(task)
|
||||
ip_addr = _get_node_ip_dhcp(task)
|
||||
LOG.debug('IP of node %(node)s is %(ip)s',
|
||||
{'node': node.uuid, 'ip': ip_addr})
|
||||
driver_internal_info = node.driver_internal_info
|
||||
|
@ -641,7 +664,7 @@ class AnsibleDeploy(base.DeployInterface):
|
|||
task.upgrade_lock(purpose='clean')
|
||||
node = task.node
|
||||
driver_internal_info = node.driver_internal_info
|
||||
driver_internal_info['ansible_cleaning_ip'] = address
|
||||
driver_internal_info['agent_url'] = callback_url
|
||||
node.driver_internal_info = driver_internal_info
|
||||
node.save()
|
||||
try:
|
||||
|
|
|
@ -28,6 +28,7 @@ from ironic.tests.unit.objects import utils as object_utils
|
|||
from ironic_lib import utils as irlib_utils
|
||||
import mock
|
||||
from oslo_config import cfg
|
||||
import six
|
||||
|
||||
from ironic_staging_drivers.ansible import deploy as ansible_deploy
|
||||
|
||||
|
@ -51,7 +52,7 @@ DRIVER_INFO = {
|
|||
'ansible_deploy_key_file': '/path/key',
|
||||
}
|
||||
DRIVER_INTERNAL_INFO = {
|
||||
'ansible_cleaning_ip': 'http://127.0.0.1/',
|
||||
'ansible_cleaning_ip': '127.0.0.1',
|
||||
'is_whole_disk_image': True,
|
||||
'clean_steps': []
|
||||
}
|
||||
|
@ -81,30 +82,69 @@ class TestAnsibleMethods(db_base.DbTestCase):
|
|||
ansible_deploy._parse_ansible_driver_info,
|
||||
self.node, 'test')
|
||||
|
||||
def test__get_node_ip(self):
|
||||
def test__get_node_ip_dhcp(self):
|
||||
dhcp_provider_mock = mock.Mock()
|
||||
dhcp_factory.DHCPFactory._dhcp_provider = dhcp_provider_mock
|
||||
dhcp_provider_mock.get_ip_addresses.return_value = ['ip']
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
ansible_deploy._get_node_ip(task)
|
||||
ansible_deploy._get_node_ip_dhcp(task)
|
||||
dhcp_provider_mock.get_ip_addresses.assert_called_once_with(
|
||||
task)
|
||||
|
||||
def test__get_node_ip_no_ip(self):
|
||||
def test__get_node_ip_dhcp_no_ip(self):
|
||||
dhcp_provider_mock = mock.Mock()
|
||||
dhcp_factory.DHCPFactory._dhcp_provider = dhcp_provider_mock
|
||||
dhcp_provider_mock.get_ip_addresses.return_value = []
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
self.assertRaises(exception.FailedToGetIPAddressOnPort,
|
||||
ansible_deploy._get_node_ip, task)
|
||||
ansible_deploy._get_node_ip_dhcp, task)
|
||||
|
||||
def test__get_node_ip_multiple_ip(self):
|
||||
def test__get_node_ip_dhcp_multiple_ip(self):
|
||||
# self.config(group='ansible', use_ramdisk_callback=False)
|
||||
# di_info = self.node.driver_internal_info
|
||||
# di_info.pop('ansible_cleaning_ip')
|
||||
# self.node.driver_internal_info = di_info
|
||||
# self.node.save()
|
||||
dhcp_provider_mock = mock.Mock()
|
||||
dhcp_factory.DHCPFactory._dhcp_provider = dhcp_provider_mock
|
||||
dhcp_provider_mock.get_ip_addresses.return_value = ['ip1', 'ip2']
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
self.assertRaises(exception.InstanceDeployFailure,
|
||||
ansible_deploy._get_node_ip, task)
|
||||
ansible_deploy._get_node_ip_dhcp, task)
|
||||
|
||||
def test__get_node_ip_heartbeat(self):
|
||||
di_info = self.node.driver_internal_info
|
||||
di_info['agent_url'] = 'http://1.2.3.4:5678'
|
||||
self.node.driver_internal_info = di_info
|
||||
self.node.save()
|
||||
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
self.assertEqual('1.2.3.4',
|
||||
ansible_deploy._get_node_ip_heartbeat(task))
|
||||
|
||||
@mock.patch.object(ansible_deploy, '_get_node_ip_heartbeat',
|
||||
return_value='127.0.0.1', autospec=True)
|
||||
@mock.patch.object(ansible_deploy, '_get_node_ip_dhcp',
|
||||
return_value='127.0.0.1', autospec=True)
|
||||
def test__get_node_ip_callback(self, ip_dhcp_mock, ip_agent_mock):
|
||||
self.config(group='ansible', use_ramdisk_callback=True)
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
res = ansible_deploy._get_node_ip(task)
|
||||
self.assertEqual(0, ip_dhcp_mock.call_count)
|
||||
ip_agent_mock.assert_called_once_with(task)
|
||||
self.assertEqual('127.0.0.1', res)
|
||||
|
||||
@mock.patch.object(ansible_deploy, '_get_node_ip_heartbeat',
|
||||
return_value='127.0.0.1', autospec=True)
|
||||
@mock.patch.object(ansible_deploy, '_get_node_ip_dhcp',
|
||||
return_value='127.0.0.1', autospec=True)
|
||||
def test__get_node_ip_no_callback(self, ip_dhcp_mock, ip_agent_mock):
|
||||
self.config(group='ansible', use_ramdisk_callback=False)
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
res = ansible_deploy._get_node_ip(task)
|
||||
self.assertEqual(0, ip_agent_mock.call_count)
|
||||
ip_dhcp_mock.assert_called_once_with(task)
|
||||
self.assertEqual('127.0.0.1', res)
|
||||
|
||||
@mock.patch.object(utils, 'node_power_action', autospec=True)
|
||||
@mock.patch.object(fake.FakePower, 'get_power_state',
|
||||
|
@ -326,6 +366,78 @@ class TestAnsibleMethods(db_base.DbTestCase):
|
|||
finish_deploy_mock.assert_called_once_with(task)
|
||||
clean_ramdisk_mock.assert_called_once_with(task)
|
||||
|
||||
def test__validate_clean_steps(self):
|
||||
steps = [{"interface": "deploy",
|
||||
"name": "foo",
|
||||
"args": {"spam": {"required": True, "value": "ham"}}},
|
||||
{"name": "bar",
|
||||
"interface": "deploy"}]
|
||||
self.assertIsNone(ansible_deploy._validate_clean_steps(
|
||||
steps, self.node.uuid))
|
||||
|
||||
def test__validate_clean_steps_missing(self):
|
||||
steps = [{"name": "foo",
|
||||
"interface": "deploy",
|
||||
"args": {"spam": {"value": "ham"},
|
||||
"ham": {"required": True}}},
|
||||
{"name": "bar"},
|
||||
{"interface": "deploy"}]
|
||||
exc = self.assertRaises(exception.NodeCleaningFailure,
|
||||
ansible_deploy._validate_clean_steps,
|
||||
steps, self.node.uuid)
|
||||
self.assertIn("name foo, field ham.value", six.text_type(exc))
|
||||
self.assertIn("name bar, field interface", six.text_type(exc))
|
||||
self.assertIn("name undefined, field name", six.text_type(exc))
|
||||
|
||||
def test__validate_clean_steps_names_not_unique(self):
|
||||
steps = [{"name": "foo",
|
||||
"interface": "deploy"},
|
||||
{"name": "foo",
|
||||
"interface": "deploy"}]
|
||||
exc = self.assertRaises(exception.NodeCleaningFailure,
|
||||
ansible_deploy._validate_clean_steps,
|
||||
steps, self.node.uuid)
|
||||
self.assertIn("unique names", six.text_type(exc))
|
||||
|
||||
@mock.patch.object(ansible_deploy.yaml, 'safe_load', autospec=True)
|
||||
def test__get_clean_steps(self, load_mock):
|
||||
steps = [{"interface": "deploy",
|
||||
"name": "foo",
|
||||
"args": {"spam": {"required": True, "value": "ham"}}},
|
||||
{"name": "bar",
|
||||
"interface": "deploy",
|
||||
"priority": 100}]
|
||||
load_mock.return_value = steps
|
||||
expected = [{"interface": "deploy",
|
||||
"step": "foo",
|
||||
"priority": 10,
|
||||
"abortable": False,
|
||||
"argsinfo": {"spam": {"required": True}},
|
||||
"args": {"spam": "ham"}},
|
||||
{"interface": "deploy",
|
||||
"step": "bar",
|
||||
"priority": 100,
|
||||
"abortable": False,
|
||||
"argsinfo": {},
|
||||
"args": {}}]
|
||||
d_info = self.node.driver_info
|
||||
d_info['ansible_clean_steps_config'] = 'custom_clean'
|
||||
self.node.driver_info = d_info
|
||||
self.node.save()
|
||||
self.config(group='ansible', playbooks_path='/path/to/playbooks')
|
||||
|
||||
with mock.patch.object(ansible_deploy, 'open', mock.mock_open(),
|
||||
create=True) as open_mock:
|
||||
self.assertEqual(
|
||||
expected,
|
||||
ansible_deploy._get_clean_steps(
|
||||
self.node, interface="deploy",
|
||||
override_priorities={"foo": 10}))
|
||||
open_mock.assert_has_calls((
|
||||
mock.call('/path/to/playbooks/custom_clean'),))
|
||||
load_mock.assert_called_once_with(
|
||||
open_mock().__enter__.return_value)
|
||||
|
||||
|
||||
class TestAnsibleDeploy(db_base.DbTestCase):
|
||||
def setUp(self):
|
||||
|
@ -384,7 +496,7 @@ class TestAnsibleDeploy(db_base.DbTestCase):
|
|||
power_mock.assert_called_once_with(task, states.REBOOT)
|
||||
|
||||
@mock.patch.object(ansible_deploy, '_deploy', autospec=True)
|
||||
@mock.patch.object(ansible_deploy, '_get_node_ip',
|
||||
@mock.patch.object(ansible_deploy, '_get_node_ip_dhcp',
|
||||
return_value='127.0.0.1', autospec=True)
|
||||
@mock.patch.object(utils, 'node_power_action', autospec=True)
|
||||
def test_deploy_done(self, power_mock, get_ip_mock, deploy_mock):
|
||||
|
@ -451,7 +563,7 @@ class TestAnsibleDeploy(db_base.DbTestCase):
|
|||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
steps = self.driver.get_clean_steps(task)
|
||||
get_clean_steps_mock.assert_called_once_with(
|
||||
task, interface='deploy',
|
||||
task.node, interface='deploy',
|
||||
override_priorities={
|
||||
'erase_devices': None,
|
||||
'erase_devices_metadata': None})
|
||||
|
@ -471,7 +583,7 @@ class TestAnsibleDeploy(db_base.DbTestCase):
|
|||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
steps = self.driver.get_clean_steps(task)
|
||||
mock_get_clean_steps.assert_called_once_with(
|
||||
task, interface='deploy',
|
||||
task.node, interface='deploy',
|
||||
override_priorities={'erase_devices': 9,
|
||||
'erase_devices_metadata': 98})
|
||||
self.assertEqual(mock_steps, steps)
|
||||
|
@ -491,6 +603,10 @@ class TestAnsibleDeploy(db_base.DbTestCase):
|
|||
DRIVER_INTERNAL_INFO['ansible_cleaning_ip'],
|
||||
'test_u', {})]}
|
||||
prepare_extra_mock.return_value = ironic_nodes
|
||||
di_info = self.node.driver_internal_info
|
||||
di_info['agent_url'] = 'http://127.0.0.1'
|
||||
self.node.driver_internal_info = di_info
|
||||
self.node.save()
|
||||
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
self.driver.execute_clean_step(task, step)
|
||||
|
@ -502,28 +618,6 @@ class TestAnsibleDeploy(db_base.DbTestCase):
|
|||
run_playbook_mock.assert_called_once_with(
|
||||
'test_pl', ironic_nodes, 'test_k', tags=['clean'])
|
||||
|
||||
@mock.patch.object(ansible_deploy, '_run_playbook', autospec=True)
|
||||
@mock.patch.object(ansible_deploy, '_parse_ansible_driver_info',
|
||||
return_value=('test_pl', 'test_u', 'test_k'),
|
||||
autospec=True)
|
||||
def test_execute_clean_step_no_ip(self, parse_driver_info_mock,
|
||||
run_playbook_mock):
|
||||
|
||||
step = {'priority': 10, 'interface': 'deploy',
|
||||
'step': 'erase_devices', 'tags': ['clean']}
|
||||
driver_internal_info = dict(DRIVER_INTERNAL_INFO)
|
||||
del driver_internal_info['ansible_cleaning_ip']
|
||||
self.node.driver_internal_info = driver_internal_info
|
||||
self.node.save()
|
||||
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
self.assertRaises(exception.NodeCleaningFailure,
|
||||
self.driver.execute_clean_step, task, step)
|
||||
|
||||
parse_driver_info_mock.assert_called_once_with(
|
||||
task.node, action='clean')
|
||||
self.assertFalse(run_playbook_mock.called)
|
||||
|
||||
@mock.patch.object(ansible_deploy, '_parse_ansible_driver_info',
|
||||
return_value=('test_pl', 'test_u', 'test_k'),
|
||||
autospec=True)
|
||||
|
@ -536,6 +630,10 @@ class TestAnsibleDeploy(db_base.DbTestCase):
|
|||
run_mock.side_effect = exception.InstanceDeployFailure('Boom')
|
||||
step = {'priority': 10, 'interface': 'deploy',
|
||||
'step': 'erase_devices', 'args': {'tags': ['clean']}}
|
||||
di_info = self.node.driver_internal_info
|
||||
di_info['agent_url'] = 'http://127.0.0.1'
|
||||
self.node.driver_internal_info = di_info
|
||||
self.node.save()
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
self.driver.execute_clean_step(task, step)
|
||||
log_mock.error.assert_called_once_with(
|
||||
|
@ -590,7 +688,7 @@ class TestAnsibleDeploy(db_base.DbTestCase):
|
|||
@mock.patch.object(ansible_deploy, '_parse_ansible_driver_info',
|
||||
return_value=('test_pl', 'test_u', 'test_k'),
|
||||
autospec=True)
|
||||
@mock.patch.object(ansible_deploy, '_get_node_ip',
|
||||
@mock.patch.object(ansible_deploy, '_get_node_ip_dhcp',
|
||||
return_value='127.0.0.1', autospec=True)
|
||||
@mock.patch.object(ansible_deploy, '_run_playbook', autospec=True)
|
||||
@mock.patch.object(utils, 'node_power_action', autospec=True)
|
||||
|
|
Loading…
Reference in New Issue