Merge "Add missing allocations for active nodes"

This commit is contained in:
Zuul 2019-09-20 12:23:03 +00:00 committed by Gerrit Code Review
commit c0021b8a74
4 changed files with 69 additions and 14 deletions

View File

@ -113,9 +113,10 @@ _ROLES_INPUT_SCHEMA = {
class CheckExistingInstancesAction(base.TripleOAction):
"""Detect which requested instances have already been provisioned."""
def __init__(self, instances):
def __init__(self, instances, default_resource_class='baremetal'):
super(CheckExistingInstancesAction, self).__init__()
self.instances = instances
self.default_resource_class = default_resource_class
def run(self, context):
try:
@ -129,16 +130,16 @@ class CheckExistingInstancesAction(base.TripleOAction):
not_found = []
found = []
for request in self.instances:
if 'hostname' not in request:
continue
ident = request.get('name', request['hostname'])
try:
instance = provisioner.show_instance(request['hostname'])
instance = provisioner.show_instance(ident)
# TODO(dtantsur): replace Error with a specific exception
except (sdk_exc.ResourceNotFound, metalsmith.exceptions.Error):
not_found.append(request)
except Exception as exc:
message = ('Failed to request instance information for '
'hostname %s' % request['hostname'])
message = ('Failed to request instance information for %s'
% ident)
LOG.exception(message)
return actions.Result(
error="%s. %s: %s" % (message, type(exc).__name__, exc)
@ -155,13 +156,24 @@ class CheckExistingInstancesAction(base.TripleOAction):
"hostname") % (request['hostname'], instance.uuid)
return actions.Result(error=error)
if (not instance.allocation
and instance.state == metalsmith.InstanceState.ACTIVE
and 'name' in request):
# Existing node is missing an allocation record,
# so create one without triggering allocation
LOG.debug('Reserving existing %s' % request['name'])
self.get_baremetal_client(context).allocation.create(
resource_class=request.get('resource_class') or
self.default_resource_class,
name=request['hostname'],
node=request['name']
)
found.append(_instance_to_dict(provisioner.connection,
instance))
if found:
LOG.info('Found existing instances: %s',
', '.join('%s (on node %s)' % (i['hostname'], i['uuid'])
for i in found))
', '.join(r['hostname'] for r in found))
if not_found:
LOG.info('Instance(s) %s do not exist',
', '.join(r['hostname'] for r in not_found))

View File

@ -96,8 +96,8 @@ class TripleOAction(actions.Action):
ironic_endpoint.url,
token=security_ctx.auth_token,
region_name=ironic_endpoint.region,
# 1.46 for conductor_group
os_ironic_api_version='1.46',
# 1.58 for allocations backfill
os_ironic_api_version='1.58',
# FIXME(lucasagomes):Paramtetize max_retries and
# max_interval. At the moment since we are dealing with
# a critical bug (#1612622) let's just hardcode the times

View File

@ -465,7 +465,11 @@ class TestCheckExistingInstances(base.TestCase):
'capabilities': {'answer': '42'},
'image': {'href': 'overcloud-full'}}
]
existing = mock.MagicMock(hostname='host2')
existing = mock.MagicMock(hostname='host2', allocation=None)
existing.to_dict.return_value = {
'hostname': 'host2',
'allocation': None
}
pr.show_instance.side_effect = [
sdk_exc.ResourceNotFound(""),
metalsmith.exceptions.Error(""),
@ -475,7 +479,11 @@ class TestCheckExistingInstances(base.TestCase):
result = action.run(mock.Mock())
self.assertEqual({
'instances': [existing.to_dict.return_value],
'instances': [{
'hostname': 'host2',
'allocation': None,
'port_map': {}
}],
'not_found': [{
'hostname': 'host1',
'image': {'href': 'overcloud-full'},
@ -488,6 +496,41 @@ class TestCheckExistingInstances(base.TestCase):
mock.call(host) for host in ['host1', 'host3', 'host2']
])
@mock.patch('tripleo_common.actions.baremetal_deploy.'
'CheckExistingInstancesAction.get_baremetal_client')
def test_existing_no_allocation(self, mock_gbc, mock_pr):
pr = mock_pr.return_value
instances = [
{'name': 'server2', 'resource_class': 'compute',
'hostname': 'host2',
'capabilities': {'answer': '42'},
'image': {'href': 'overcloud-full'}}
]
existing = mock.MagicMock(
hostname='host2', allocation=None,
state=metalsmith.InstanceState.ACTIVE)
existing.to_dict.return_value = {
'hostname': 'host2',
}
pr.show_instance.return_value = existing
mock_create = mock_gbc.return_value.allocation.create
action = baremetal_deploy.CheckExistingInstancesAction(instances)
ctx = mock.Mock()
result = action.run(ctx)
mock_gbc.assert_called_once_with(ctx)
mock_create.assert_called_once_with(
name='host2', node='server2', resource_class='compute')
self.assertEqual({
'instances': [{
'hostname': 'host2',
'port_map': {}
}],
'not_found': []
}, result)
pr.show_instance.assert_called_once_with('server2')
def test_hostname_mismatch(self, mock_pr):
instances = [
{'hostname': 'host1',
@ -509,7 +552,7 @@ class TestCheckExistingInstances(base.TestCase):
action = baremetal_deploy.CheckExistingInstancesAction(instances)
result = action.run(mock.Mock())
self.assertIn("hostname host0", result.error)
self.assertIn("for host0", result.error)
self.assertIn("RuntimeError: boom", result.error)
mock_pr.return_value.show_instance.assert_called_once_with('host0')

View File

@ -39,7 +39,7 @@ class TestActionsBase(tests_base.TestCase):
url='http://ironic/v1', region='ironic-region')
self.action.get_baremetal_client(mock_cxt)
mock_client.assert_called_once_with(
'http://ironic/v1', max_retries=12, os_ironic_api_version='1.46',
'http://ironic/v1', max_retries=12, os_ironic_api_version='1.58',
region_name='ironic-region', retry_interval=5, token=mock.ANY)
mock_endpoint.assert_called_once_with(mock_cxt.security, 'ironic')
mock_cxt.assert_not_called()