Merge "Add missing allocations for active nodes"
This commit is contained in:
commit
c0021b8a74
|
@ -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))
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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')
|
||||
|
||||
|
|
|
@ -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()
|
||||
|
|
Loading…
Reference in New Issue