Fix build-nodes-json for routed networks

When using routed networks, i.e multiple provision networks
passing in the evironment file to build-nodes-json did'nt work.
It resulted in key errors when looking up the mac address of the
provision_net interface.

This change automates this by creating a provision_net_map, mapping
baremetal_port id's to subnet names.

Also deprecates the --provision-net option as it is no longer needed.
This commit is contained in:
Harald Jensås 2018-09-25 14:51:46 +02:00
parent c1364026fa
commit 43a37ee843
2 changed files with 106 additions and 62 deletions

View File

@ -45,7 +45,7 @@ def _parse_args():
parser.add_argument('--provision_net', parser.add_argument('--provision_net',
dest='provision_net', dest='provision_net',
default='provision', default='provision',
help='Provisioning network name') help='DEPRECATED: This parameter is ignored.')
parser.add_argument('--nodes_json', parser.add_argument('--nodes_json',
dest='nodes_json', dest='nodes_json',
default='nodes.json', default='nodes.json',
@ -82,7 +82,6 @@ def _get_names(args):
if args.env is None: if args.env is None:
bmc_base = args.bmc_prefix bmc_base = args.bmc_prefix
baremetal_base = args.baremetal_prefix baremetal_base = args.baremetal_prefix
provision_net = args.provision_net
# FIXME: This is not necessarily true. # FIXME: This is not necessarily true.
undercloud_name = 'undercloud' undercloud_name = 'undercloud'
else: else:
@ -90,12 +89,11 @@ def _get_names(args):
e = yaml.safe_load(f) e = yaml.safe_load(f)
bmc_base = _get_from_env(e, 'bmc_prefix') bmc_base = _get_from_env(e, 'bmc_prefix')
baremetal_base = _get_from_env(e, 'baremetal_prefix') baremetal_base = _get_from_env(e, 'baremetal_prefix')
provision_net = _get_from_env(e, 'provision_net')
role = e.get('parameter_defaults', {}).get('role') role = e.get('parameter_defaults', {}).get('role')
if role and baremetal_base.endswith('-' + role): if role and baremetal_base.endswith('-' + role):
baremetal_base = baremetal_base[:-len(role) - 1] baremetal_base = baremetal_base[:-len(role) - 1]
undercloud_name = e.get('parameter_defaults', {}).get('undercloud_name') # noqa: E501 undercloud_name = e.get('parameter_defaults', {}).get('undercloud_name') # noqa: E501
return bmc_base, baremetal_base, provision_net, undercloud_name return bmc_base, baremetal_base, undercloud_name
def _get_clients(): def _get_clients():
@ -116,10 +114,17 @@ def _get_ports(neutron, bmc_base, baremetal_base):
raise RuntimeError('Found different numbers of baremetal and ' raise RuntimeError('Found different numbers of baremetal and '
'bmc ports. bmc: %s baremetal: %s' % (bmc_ports, 'bmc ports. bmc: %s baremetal: %s' % (bmc_ports,
bm_ports)) bm_ports))
return bmc_ports, bm_ports provision_net_map = {}
for port in bm_ports:
provision_net_map.update({
port.get('id'):
neutron.list_subnets(
id=port['fixed_ips'][0]['subnet_id'])['subnets'][0].get(
'name')})
return bmc_ports, bm_ports, provision_net_map
def _build_nodes(nova, glance, bmc_ports, bm_ports, provision_net, def _build_nodes(nova, glance, bmc_ports, bm_ports, provision_net_map,
baremetal_base, undercloud_name, driver, physical_network): baremetal_base, undercloud_name, driver, physical_network):
node_template = { node_template = {
'pm_type': driver, 'pm_type': driver,
@ -136,9 +141,6 @@ def _build_nodes(nova, glance, bmc_ports, bm_ports, provision_net,
} }
if physical_network: if physical_network:
node_template.pop('mac') node_template.pop('mac')
node_template.update(
{'ports': [{'address': '', 'physical_network': provision_net}]})
nodes = [] nodes = []
bmc_bm_pairs = [] bmc_bm_pairs = []
cache = {} cache = {}
@ -151,9 +153,11 @@ def _build_nodes(nova, glance, bmc_ports, bm_ports, provision_net,
node = dict(node_template) node = dict(node_template)
node['pm_addr'] = bmc_port['fixed_ips'][0]['ip_address'] node['pm_addr'] = bmc_port['fixed_ips'][0]['ip_address']
bmc_bm_pairs.append((node['pm_addr'], baremetal.name)) bmc_bm_pairs.append((node['pm_addr'], baremetal.name))
provision_net = provision_net_map.get(baremetal_port['id'])
mac = baremetal.addresses[provision_net][0]['OS-EXT-IPS-MAC:mac_addr'] mac = baremetal.addresses[provision_net][0]['OS-EXT-IPS-MAC:mac_addr']
if physical_network: if physical_network:
node['ports'][0]['address'] = mac node.update({'ports': [{'address': mac,
'physical_network': provision_net}]})
else: else:
node['mac'] = [mac] node['mac'] = [mac]
if not cache.get(baremetal.flavor['id']): if not cache.get(baremetal.flavor['id']):
@ -282,14 +286,15 @@ def _write_pairs(bmc_bm_pairs):
def main(): def main():
args = _parse_args() args = _parse_args()
bmc_base, baremetal_base, provision_net, undercloud_name = _get_names(args) bmc_base, baremetal_base, undercloud_name = _get_names(args)
nova, neutron, glance = _get_clients() nova, neutron, glance = _get_clients()
bmc_ports, bm_ports = _get_ports(neutron, bmc_base, baremetal_base) bmc_ports, bm_ports, provision_net_map = _get_ports(neutron, bmc_base,
baremetal_base)
(nodes, (nodes,
bmc_bm_pairs, bmc_bm_pairs,
extra_nodes, extra_nodes,
network_details) = _build_nodes(nova, glance, bmc_ports, bm_ports, network_details) = _build_nodes(nova, glance, bmc_ports, bm_ports,
provision_net, baremetal_base, provision_net_map, baremetal_base,
undercloud_name, args.driver, undercloud_name, args.driver,
args.physical_network) args.physical_network)
_write_nodes(nodes, extra_nodes, network_details, args) _write_nodes(nodes, extra_nodes, network_details, args)

View File

@ -70,13 +70,11 @@ class TestBuildNodesJson(testtools.TestCase):
args.env = None args.env = None
args.bmc_prefix = 'bmc-foo' args.bmc_prefix = 'bmc-foo'
args.baremetal_prefix = 'baremetal-foo' args.baremetal_prefix = 'baremetal-foo'
args.provision_net = 'provision-foo'
args.add_undercloud = False args.add_undercloud = False
bmc_base, baremetal_base, provision_net, undercloud_name = ( bmc_base, baremetal_base, undercloud_name = (
build_nodes_json._get_names(args)) build_nodes_json._get_names(args))
self.assertEqual('bmc-foo', bmc_base) self.assertEqual('bmc-foo', bmc_base)
self.assertEqual('baremetal-foo', baremetal_base) self.assertEqual('baremetal-foo', baremetal_base)
self.assertEqual('provision-foo', provision_net)
self.assertEqual('undercloud', undercloud_name) self.assertEqual('undercloud', undercloud_name)
def test_get_names_no_env_w_undercloud(self): def test_get_names_no_env_w_undercloud(self):
@ -84,13 +82,11 @@ class TestBuildNodesJson(testtools.TestCase):
args.env = None args.env = None
args.bmc_prefix = 'bmc-foo' args.bmc_prefix = 'bmc-foo'
args.baremetal_prefix = 'baremetal-foo' args.baremetal_prefix = 'baremetal-foo'
args.provision_net = 'provision-foo'
args.add_undercloud = True args.add_undercloud = True
bmc_base, baremetal_base, provision_net, undercloud_name = ( bmc_base, baremetal_base, undercloud_name = (
build_nodes_json._get_names(args)) build_nodes_json._get_names(args))
self.assertEqual('bmc-foo', bmc_base) self.assertEqual('bmc-foo', bmc_base)
self.assertEqual('baremetal-foo', baremetal_base) self.assertEqual('baremetal-foo', baremetal_base)
self.assertEqual('provision-foo', provision_net)
self.assertEqual('undercloud', undercloud_name) self.assertEqual('undercloud', undercloud_name)
@mock.patch('openstack_virtual_baremetal.build_nodes_json.open', @mock.patch('openstack_virtual_baremetal.build_nodes_json.open',
@ -104,15 +100,13 @@ class TestBuildNodesJson(testtools.TestCase):
'parameters': { 'parameters': {
'bmc_prefix': 'bmc-foo', 'bmc_prefix': 'bmc-foo',
'baremetal_prefix': 'baremetal-foo', 'baremetal_prefix': 'baremetal-foo',
'provision_net': 'provision-foo'
}, },
} }
mock_load.return_value = mock_env mock_load.return_value = mock_env
bmc_base, baremetal_base, provision_net, undercloud_name = ( bmc_base, baremetal_base, undercloud_name = (
build_nodes_json._get_names(args)) build_nodes_json._get_names(args))
self.assertEqual('bmc-foo', bmc_base) self.assertEqual('bmc-foo', bmc_base)
self.assertEqual('baremetal-foo', baremetal_base) self.assertEqual('baremetal-foo', baremetal_base)
self.assertEqual('provision-foo', provision_net)
self.assertIsNone(undercloud_name) self.assertIsNone(undercloud_name)
@mock.patch('openstack_virtual_baremetal.build_nodes_json.open', @mock.patch('openstack_virtual_baremetal.build_nodes_json.open',
@ -126,15 +120,13 @@ class TestBuildNodesJson(testtools.TestCase):
'parameter_defaults': { 'parameter_defaults': {
'bmc_prefix': 'bmc-foo', 'bmc_prefix': 'bmc-foo',
'baremetal_prefix': 'baremetal-foo', 'baremetal_prefix': 'baremetal-foo',
'provision_net': 'provision-foo'
}, },
} }
mock_load.return_value = mock_env mock_load.return_value = mock_env
bmc_base, baremetal_base, provision_net, undercloud_name = ( bmc_base, baremetal_base, undercloud_name = (
build_nodes_json._get_names(args)) build_nodes_json._get_names(args))
self.assertEqual('bmc-foo', bmc_base) self.assertEqual('bmc-foo', bmc_base)
self.assertEqual('baremetal-foo', baremetal_base) self.assertEqual('baremetal-foo', baremetal_base)
self.assertEqual('provision-foo', provision_net)
self.assertIsNone(undercloud_name) self.assertIsNone(undercloud_name)
@mock.patch('openstack_virtual_baremetal.build_nodes_json.open', @mock.patch('openstack_virtual_baremetal.build_nodes_json.open',
@ -148,16 +140,14 @@ class TestBuildNodesJson(testtools.TestCase):
'parameter_defaults': { 'parameter_defaults': {
'bmc_prefix': 'bmc', 'bmc_prefix': 'bmc',
'baremetal_prefix': 'baremetal', 'baremetal_prefix': 'baremetal',
'provision_net': 'provision',
'role': 'foo', 'role': 'foo',
}, },
} }
mock_load.return_value = mock_env mock_load.return_value = mock_env
bmc_base, baremetal_base, provision_net, undercloud_name = ( bmc_base, baremetal_base, undercloud_name = (
build_nodes_json._get_names(args)) build_nodes_json._get_names(args))
self.assertEqual('bmc', bmc_base) self.assertEqual('bmc', bmc_base)
self.assertEqual('baremetal', baremetal_base) self.assertEqual('baremetal', baremetal_base)
self.assertEqual('provision', provision_net)
self.assertIsNone(undercloud_name) self.assertIsNone(undercloud_name)
@mock.patch('openstack_virtual_baremetal.build_nodes_json.open', @mock.patch('openstack_virtual_baremetal.build_nodes_json.open',
@ -171,16 +161,14 @@ class TestBuildNodesJson(testtools.TestCase):
'parameter_defaults': { 'parameter_defaults': {
'bmc_prefix': 'bmc-foo', 'bmc_prefix': 'bmc-foo',
'baremetal_prefix': 'baremetal-foo-bar', 'baremetal_prefix': 'baremetal-foo-bar',
'provision_net': 'provision-foo',
'role': 'bar', 'role': 'bar',
}, },
} }
mock_load.return_value = mock_env mock_load.return_value = mock_env
bmc_base, baremetal_base, provision_net, undercloud_name = ( bmc_base, baremetal_base, undercloud_name = (
build_nodes_json._get_names(args)) build_nodes_json._get_names(args))
self.assertEqual('bmc-foo', bmc_base) self.assertEqual('bmc-foo', bmc_base)
self.assertEqual('baremetal-foo', baremetal_base) self.assertEqual('baremetal-foo', baremetal_base)
self.assertEqual('provision-foo', provision_net)
self.assertIsNone(undercloud_name) self.assertIsNone(undercloud_name)
@mock.patch('os_client_config.make_client') @mock.patch('os_client_config.make_client')
@ -203,21 +191,42 @@ class TestBuildNodesJson(testtools.TestCase):
def test_get_ports(self): def test_get_ports(self):
neutron = mock.Mock() neutron = mock.Mock()
fake_fixed_ips = [{'subnet_id': 'provision_id'}]
fake_ports = { fake_ports = {
'ports': [ 'ports': [
{'name': 'random'}, {'name': 'random',
{'name': 'bmc_1'}, 'id': 'random_id',
{'name': 'bmc_0'}, 'fixed_ips': fake_fixed_ips},
{'name': 'baremetal_1'}, {'name': 'bmc_1',
{'name': 'baremetal_0'}, 'id': 'bmc_1_id',
'fixed_ips': fake_fixed_ips},
{'name': 'bmc_0',
'id': 'bmc_0_id',
'fixed_ips': fake_fixed_ips},
{'name': 'baremetal_1',
'id': 'baremetal_1_id',
'fixed_ips': fake_fixed_ips},
{'name': 'baremetal_0',
'id': 'baremetal_0_id',
'fixed_ips': fake_fixed_ips},
]
}
fake_subnets = {
'subnets': [
{'name': 'provision',
'id': 'provision_id'}
] ]
} }
neutron.list_ports.return_value = fake_ports neutron.list_ports.return_value = fake_ports
bmc_ports, bm_ports = build_nodes_json._get_ports(neutron, 'bmc', neutron.list_subnets.return_value = fake_subnets
'baremetal') bmc_ports, bm_ports, provision_net_map = build_nodes_json._get_ports(
self.assertEqual([{'name': 'bmc_0'}, {'name': 'bmc_1'}], bmc_ports) neutron, 'bmc', 'baremetal')
self.assertEqual([{'name': 'baremetal_0'}, {'name': 'baremetal_1'}], self.assertEqual([fake_ports['ports'][2], fake_ports['ports'][1]],
bmc_ports)
self.assertEqual([fake_ports['ports'][4], fake_ports['ports'][3]],
bm_ports) bm_ports)
self.assertEqual({'baremetal_0_id': 'provision',
'baremetal_1_id': 'provision'}, provision_net_map)
def test_get_ports_mismatch(self): def test_get_ports_mismatch(self):
neutron = mock.Mock() neutron = mock.Mock()
@ -228,20 +237,38 @@ class TestBuildNodesJson(testtools.TestCase):
def test_get_ports_multiple(self): def test_get_ports_multiple(self):
neutron = mock.Mock() neutron = mock.Mock()
fake_fixed_ips = [{'subnet_id': 'provision_id'}]
fake_ports = { fake_ports = {
'ports': [ 'ports': [
{'name': 'random'}, {'name': 'random',
{'name': 'bmc-foo_0'}, 'id': 'random_id',
{'name': 'bmc-bar_0'}, 'fixed_ips': fake_fixed_ips},
{'name': 'baremetal-foo_0'}, {'name': 'bmc-foo_0',
{'name': 'baremetal-bar_0'}, 'id': 'bmc_foo_0_id',
'fixed_ips': fake_fixed_ips},
{'name': 'bmc-bar_0',
'id': 'bmc_bar_0_id',
'fixed_ips': fake_fixed_ips},
{'name': 'baremetal-foo_0',
'id': 'baremetal_foo_0_id',
'fixed_ips': fake_fixed_ips},
{'name': 'baremetal-bar_0',
'id': 'baremetal_bar_0_id',
'fixed_ips': fake_fixed_ips},
]
}
fake_subnets = {
'subnets': [
{'name': 'provision',
'id': 'provision_id'}
] ]
} }
neutron.list_ports.return_value = fake_ports neutron.list_ports.return_value = fake_ports
bmc_ports, bm_ports = build_nodes_json._get_ports(neutron, 'bmc-foo', neutron.list_subnets.return_value = fake_subnets
'baremetal-foo') bmc_ports, bm_ports, provision_net_map = build_nodes_json._get_ports(
self.assertEqual([{'name': 'bmc-foo_0'}], bmc_ports) neutron, 'bmc-foo', 'baremetal-foo')
self.assertEqual([{'name': 'baremetal-foo_0'}], bm_ports) self.assertEqual([fake_ports['ports'][1]], bmc_ports)
self.assertEqual([fake_ports['ports'][3]], bm_ports)
def _fake_port(self, device_id, ip, mac): def _fake_port(self, device_id, ip, mac):
return {'device_id': device_id, return {'device_id': device_id,
@ -275,7 +302,11 @@ class TestBuildNodesJson(testtools.TestCase):
bmc_ports = [{'fixed_ips': [{'ip_address': '1.1.1.1'}]}, bmc_ports = [{'fixed_ips': [{'ip_address': '1.1.1.1'}]},
{'fixed_ips': [{'ip_address': '1.1.1.2'}]} {'fixed_ips': [{'ip_address': '1.1.1.2'}]}
] ]
bm_ports = [{'device_id': '1'}, {'device_id': '2'}] bm_ports = [{'device_id': '1', 'id': 'port_id_server1'},
{'device_id': '2', 'id': 'port_id_server2'}]
provision_net_map = {'port_id_server1': 'provision',
'port_id_server2': 'provision',
'port_id_server3': 'provision', }
physical_network = False physical_network = False
nova = mock.Mock() nova = mock.Mock()
servers = [mock.Mock(), mock.Mock(), mock.Mock()] servers = [mock.Mock(), mock.Mock(), mock.Mock()]
@ -304,8 +335,8 @@ class TestBuildNodesJson(testtools.TestCase):
bmc_bm_pairs, bmc_bm_pairs,
extra_nodes, extra_nodes,
network_details) = build_nodes_json._build_nodes( network_details) = build_nodes_json._build_nodes(
nova, glance, bmc_ports, bm_ports, 'provision', 'bm', 'undercloud', nova, glance, bmc_ports, bm_ports, provision_net_map, 'bm',
'pxe_ipmitool', physical_network) 'undercloud', 'pxe_ipmitool', physical_network)
expected_nodes = copy.deepcopy(TEST_NODES) expected_nodes = copy.deepcopy(TEST_NODES)
expected_nodes[1]['disk'] = 100 expected_nodes[1]['disk'] = 100
self.assertEqual(expected_nodes, nodes) self.assertEqual(expected_nodes, nodes)
@ -323,7 +354,11 @@ class TestBuildNodesJson(testtools.TestCase):
bmc_ports = [{'fixed_ips': [{'ip_address': '1.1.1.1'}]}, bmc_ports = [{'fixed_ips': [{'ip_address': '1.1.1.1'}]},
{'fixed_ips': [{'ip_address': '1.1.1.2'}]} {'fixed_ips': [{'ip_address': '1.1.1.2'}]}
] ]
bm_ports = [{'device_id': '1'}, {'device_id': '2'}] bm_ports = [{'device_id': '1', 'id': 'port_id_server1'},
{'device_id': '2', 'id': 'port_id_server2'}]
provision_net_map = {'port_id_server1': 'provision',
'port_id_server2': 'provision',
'port_id_server3': 'provision', }
physical_network = False physical_network = False
nova = mock.Mock() nova = mock.Mock()
servers = [mock.Mock(), mock.Mock(), mock.Mock()] servers = [mock.Mock(), mock.Mock(), mock.Mock()]
@ -352,8 +387,8 @@ class TestBuildNodesJson(testtools.TestCase):
bmc_bm_pairs, bmc_bm_pairs,
extra_nodes, extra_nodes,
network_details) = build_nodes_json._build_nodes( network_details) = build_nodes_json._build_nodes(
nova, glance, bmc_ports, bm_ports, 'provision', 'bm', 'undercloud', nova, glance, bmc_ports, bm_ports, provision_net_map, 'bm',
'ipmi', physical_network) 'undercloud', 'ipmi', physical_network)
expected_nodes = copy.deepcopy(TEST_NODES) expected_nodes = copy.deepcopy(TEST_NODES)
expected_nodes[1]['disk'] = 100 expected_nodes[1]['disk'] = 100
for node in expected_nodes: for node in expected_nodes:
@ -372,7 +407,11 @@ class TestBuildNodesJson(testtools.TestCase):
bmc_ports = [{'fixed_ips': [{'ip_address': '1.1.1.1'}]}, bmc_ports = [{'fixed_ips': [{'ip_address': '1.1.1.1'}]},
{'fixed_ips': [{'ip_address': '1.1.1.2'}]} {'fixed_ips': [{'ip_address': '1.1.1.2'}]}
] ]
bm_ports = [{'device_id': '1'}, {'device_id': '2'}] bm_ports = [{'device_id': '1', 'id': 'port_id_server1'},
{'device_id': '2', 'id': 'port_id_server2'}]
provision_net_map = {'port_id_server1': 'provision',
'port_id_server2': 'provision',
'port_id_server3': 'provision', }
physical_network = False physical_network = False
nova = mock.Mock() nova = mock.Mock()
servers = [mock.Mock(), mock.Mock(), mock.Mock()] servers = [mock.Mock(), mock.Mock(), mock.Mock()]
@ -388,8 +427,8 @@ class TestBuildNodesJson(testtools.TestCase):
glance.images.get.return_value = mock_image_get glance.images.get.return_value = mock_image_get
nodes, bmc_bm_pairs, extra_nodes, _ = build_nodes_json._build_nodes( nodes, bmc_bm_pairs, extra_nodes, _ = build_nodes_json._build_nodes(
nova, glance, bmc_ports, bm_ports, 'provision', 'bm-foo', None, nova, glance, bmc_ports, bm_ports, provision_net_map, 'bm-foo',
'pxe_ipmitool', physical_network) None, 'pxe_ipmitool', physical_network)
expected_nodes = copy.deepcopy(TEST_NODES) expected_nodes = copy.deepcopy(TEST_NODES)
expected_nodes[0]['name'] = 'bm-foo-control-0' expected_nodes[0]['name'] = 'bm-foo-control-0'
expected_nodes[0]['capabilities'] = ('boot_option:local,' expected_nodes[0]['capabilities'] = ('boot_option:local,'
@ -495,9 +534,9 @@ class TestBuildNodesJson(testtools.TestCase):
mock_parse_args.return_value = args mock_parse_args.return_value = args
bmc_base = mock.Mock() bmc_base = mock.Mock()
baremetal_base = mock.Mock() baremetal_base = mock.Mock()
provision_net = mock.Mock() provision_net_map = mock.Mock()
undercloud_name = 'undercloud' undercloud_name = 'undercloud'
mock_get_names.return_value = (bmc_base, baremetal_base, provision_net, mock_get_names.return_value = (bmc_base, baremetal_base,
undercloud_name) undercloud_name)
nova = mock.Mock() nova = mock.Mock()
neutron = mock.Mock() neutron = mock.Mock()
@ -505,7 +544,7 @@ class TestBuildNodesJson(testtools.TestCase):
mock_get_clients.return_value = (nova, neutron, glance) mock_get_clients.return_value = (nova, neutron, glance)
bmc_ports = mock.Mock() bmc_ports = mock.Mock()
bm_ports = mock.Mock() bm_ports = mock.Mock()
mock_get_ports.return_value = (bmc_ports, bm_ports) mock_get_ports.return_value = (bmc_ports, bm_ports, provision_net_map)
nodes = mock.Mock() nodes = mock.Mock()
pairs = mock.Mock() pairs = mock.Mock()
extra_nodes = mock.Mock() extra_nodes = mock.Mock()
@ -521,7 +560,7 @@ class TestBuildNodesJson(testtools.TestCase):
mock_get_ports.assert_called_once_with(neutron, bmc_base, mock_get_ports.assert_called_once_with(neutron, bmc_base,
baremetal_base) baremetal_base)
mock_build_nodes.assert_called_once_with(nova, glance, bmc_ports, mock_build_nodes.assert_called_once_with(nova, glance, bmc_ports,
bm_ports, provision_net, bm_ports, provision_net_map,
baremetal_base, baremetal_base,
undercloud_name, undercloud_name,
args.driver, args.driver,