openstack
/
tripleo-common
Code Issues Proposed changes

Merge "Make instances name/hostname optional"

This commit is contained in:
Zuul 2019-07-24 01:29:30 +00:00 committed by Gerrit Code Review
parent b03d092daa cc4a344783
commit 8c703cede1
2 changed files with 37 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
tripleo_common/actions/baremetal_deploy.py
View File

@ -110,6 +110,8 @@ class CheckExistingInstancesAction(base.TripleOAction):
not_found = []
found = []
for request in self.instances:
if 'hostname' not in request:
continue
try:
instance = provisioner.show_instance(request['hostname'])
# TODO(dtantsur): replace Error with a specific exception
@ -333,7 +335,8 @@ class ExpandRolesAction(base.TripleOAction):
return actions.Result(error=six.text_type(exc))
instances = []
parameter_defaults = {'HostnameMap': {}}
hostname_map = {}
parameter_defaults = {'HostnameMap': hostname_map}
for role in self.roles:
name = role['name']
hostname_format = role.get('hostname_format')
@ -351,12 +354,11 @@ class ExpandRolesAction(base.TripleOAction):
# TODO(dtantsur): ordering-dependent logic here, can we do
# better?
for index, instance in enumerate(role['instances']):
# _validate_instances ensures that either of these is
# not empty
hostname = instance.get('hostname') or instance.get('name')
gen_name = (hostname_format.replace('%index%', str(index))
.replace('%stackname%', self.stackname))
parameter_defaults['HostnameMap'][gen_name] = hostname
if 'hostname' not in instance:
instance['hostname'] = gen_name
hostname_map[gen_name] = instance['hostname']
instances.append(instance)
else:
count = role.get('count', 1)
@ -371,7 +373,7 @@ class ExpandRolesAction(base.TripleOAction):
if 'profile' in role:
inst['profile'] = role['profile']
instances.append(inst)
parameter_defaults['HostnameMap'][hostname] = hostname
hostname_map[hostname] = hostname
return {'instances': instances,
'environment': {'parameter_defaults': parameter_defaults}}
@ -438,10 +440,6 @@ def _validate_instances(instances, default_image='overcloud-full'):
if inst.get('name') and not inst.get('hostname'):
inst['hostname'] = inst['name']
if not inst.get('hostname'):
raise ValueError('Either hostname or name is required in %s'
% inst)
# Set the default image so that the source validation can succeed.
inst.setdefault('image', default_image)
@ -453,10 +451,11 @@ def _validate_instances(instances, default_image='overcloud-full'):
# NOTE(dtantsur): validate image parameters
_get_source(inst)
if inst['hostname'] in hostnames:
raise ValueError('Hostname %s is used more than once' %
inst['hostname'])
hostnames.add(inst['hostname'])
if inst.get('hostname'):
if inst['hostname'] in hostnames:
raise ValueError('Hostname %s is used more than once' %
inst['hostname'])
hostnames.add(inst['hostname'])
if inst.get('name'):
if inst['name'] in names:

39
tripleo_common/tests/actions/test_baremetal_deploy.py
View File

@ -56,8 +56,17 @@ class TestReserveNodes(base.TestCase):
action = baremetal_deploy.ReserveNodesAction(instances)
result = action.run(mock.Mock())
self.assertIn("hostname or name is required", result.error)
self.assertFalse(mock_pr.return_value.reserve_node.called)
self.assertEqual(
[{'node': mock_pr.return_value.reserve_node.return_value.id,
'instance': req} for req in instances],
result['reservations'])
mock_pr.return_value.reserve_node.assert_has_calls([
mock.call(resource_class='baremetal', traits=None,
capabilities=None, candidates=None),
mock.call(resource_class='compute', traits=None,
capabilities={'answer': '42'}, candidates=None),
])
self.assertFalse(mock_pr.return_value.unprovision_node.called)
@mock.patch.object(baremetal_deploy.LOG, 'exception', autospec=True)
@ -413,17 +422,6 @@ class TestCheckExistingInstances(base.TestCase):
mock.call(host) for host in ['host1', 'host3', 'host2']
])
def test_missing_hostname(self, mock_pr):
instances = [
{'hostname': 'host1'},
{'resource_class': 'compute', 'capabilities': {'answer': '42'}}
]
action = baremetal_deploy.CheckExistingInstancesAction(instances)
result = action.run(mock.Mock())
self.assertIn("hostname or name is required", result.error)
self.assertFalse(mock_pr.return_value.show_instance.called)
def test_hostname_mismatch(self, mock_pr):
instances = [
{'hostname': 'host1'},
@ -624,6 +622,7 @@ class TestExpandRoles(base.TestCase):
'profile': 'control-X'},
# Name provides the default for hostname later on.
{'name': 'node-0', 'profile': 'control',
'hostname': 'overcloud-controller-1',
'traits': ['CUSTOM_FOO'], 'nics': [{'subnet': 'leaf-2'}]},
],
result['instances'])
@ -639,7 +638,7 @@ class TestExpandRoles(base.TestCase):
'compute-0.example.com': 'compute-0.example.com',
'compute-1.example.com': 'compute-1.example.com',
'overcloud-controller-0': 'controller-X.example.com',
'overcloud-controller-1': 'node-0',
'overcloud-controller-1': 'overcloud-controller-1',
}
},
result['environment']['parameter_defaults'])
@ -678,7 +677,17 @@ class TestExpandRoles(base.TestCase):
]
action = baremetal_deploy.ExpandRolesAction(roles)
result = action.run(mock.Mock())
self.assertIn("Either hostname or name is required", result.error)
self.assertEqual(
[
{'hostname': 'compute-0.example.com', 'profile': 'compute'},
{'hostname': 'compute-1.example.com', 'profile': 'compute'},
{'hostname': 'overcloud-controller-0', 'profile': 'control-X'},
# Name provides the default for hostname later on.
{'name': 'node-0', 'profile': 'control',
'hostname': 'overcloud-controller-1',
'traits': ['CUSTOM_FOO'], 'nics': [{'subnet': 'leaf-2'}]},
],
result['instances'])
class TestPopulateEnvironment(base.TestCase):