From faecdb45578bb3024823741a0f3e6420a4e43fc1 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Wed, 8 Mar 2017 09:42:13 +0000 Subject: [PATCH] Ironic inventory source duplicates MACs When using the 'ironic' Bifrost inventory source to obtain an Ansible inventory from ironic, if any nodes have multiple ports then the resulting inventory contains a 'nics' list variable where each item has the same MAC address. This change fixes this issue by reinitialising each new_nic variable such that all items in the nics list do not reference the same dict. We also introduce a couple of unit tests for the ironic inventory source - one to test the simple case of a single node with a single NIC, another to test the broken case of a single node with multiple NICS. Change-Id: I173fbbdbebafc05536a153dcf59e4b2f37aebdbc Closes-Bug: #1671014 --- bifrost/inventory.py | 2 +- bifrost/tests/unit/test_inventory.py | 115 +++++++++++++++++++++++++++ test-requirements.txt | 1 + 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/bifrost/inventory.py b/bifrost/inventory.py index 2d239f661..b70b73f8f 100755 --- a/bifrost/inventory.py +++ b/bifrost/inventory.py @@ -336,8 +336,8 @@ def _process_shade(groups, hostvars): # return the network information to the inventory. nics = cloud.list_nics_for_machine(machine['uuid']) new_nics = [] - new_nic = {} for nic in nics: + new_nic = {} if 'address' in nic: new_nic['mac'] = nic['address'] new_nics.append(new_nic) diff --git a/bifrost/tests/unit/test_inventory.py b/bifrost/tests/unit/test_inventory.py index a895afab9..9c6503a7c 100644 --- a/bifrost/tests/unit/test_inventory.py +++ b/bifrost/tests/unit/test_inventory.py @@ -19,6 +19,8 @@ test_inventory Tests for `inventory` module. """ +import mock + from bifrost import inventory from bifrost.tests import base @@ -40,3 +42,116 @@ class TestBifrostInventoryUnit(base.TestCase): self.assertIsNone(inventory._val_or_none(array, 1)) self.assertEqual('yes', inventory._val_or_none(array, 2)) self.assertIsNone(inventory._val_or_none(array, 4)) + + def test__process_shade(self): + inventory.shade = mock_shade = mock.Mock() + inventory.SHADE_LOADED = True + (groups, hostvars) = inventory._prepare_inventory() + mock_cloud = mock_shade.operator_cloud.return_value + mock_cloud.list_machines.return_value = [ + { + 'driver_info': { + 'ipmi_address': '1.2.3.4', + }, + 'links': [], + 'name': 'node1', + 'ports': [], + 'properties': { + 'cpus': 42, + }, + 'uuid': 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45', + }, + ] + mock_cloud.list_nics_for_machine.return_value = [ + { + 'address': '00:11:22:33:44:55', + 'uuid': 'e2be93b5-a8f6-46a2-bec7-571b8ecf2938', + }, + ] + (groups, hostvars) = inventory._process_shade(groups, hostvars) + mock_shade.operator_cloud.assert_called_once_with( + auth_type='None', auth={'endpoint': 'http://localhost:6385/'}) + mock_cloud.list_machines.assert_called_once_with() + mock_cloud.list_nics_for_machine.assert_called_once_with( + 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45') + self.assertIn('baremetal', groups) + self.assertIn('hosts', groups['baremetal']) + self.assertEqual(groups['baremetal'], {'hosts': ['node1']}) + self.assertIn('node1', hostvars) + expected_machine = { + 'addressing_mode': 'dhcp', + 'driver_info': { + 'ipmi_address': '1.2.3.4', + }, + 'name': 'node1', + 'nics': [ + { + 'mac': '00:11:22:33:44:55', + }, + ], + 'properties': { + 'cpus': 42, + }, + 'uuid': 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45', + } + self.assertEqual(hostvars['node1'], expected_machine) + + def test__process_shade_multiple_nics(self): + inventory.shade = mock_shade = mock.Mock() + inventory.SHADE_LOADED = True + (groups, hostvars) = inventory._prepare_inventory() + mock_cloud = mock_shade.operator_cloud.return_value + mock_cloud.list_machines.return_value = [ + { + 'driver_info': { + 'ipmi_address': '1.2.3.4', + }, + 'links': [], + 'name': 'node1', + 'ports': [], + 'properties': { + 'cpus': 42, + }, + 'uuid': 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45', + }, + ] + mock_cloud.list_nics_for_machine.return_value = [ + { + 'address': '00:11:22:33:44:55', + 'uuid': 'e2be93b5-a8f6-46a2-bec7-571b8ecf2938', + }, + { + 'address': '00:11:22:33:44:56', + 'uuid': '59e8cd37-4f71-4ca1-a264-93c2ca7de0f7', + }, + ] + (groups, hostvars) = inventory._process_shade(groups, hostvars) + mock_shade.operator_cloud.assert_called_once_with( + auth_type='None', auth={'endpoint': 'http://localhost:6385/'}) + mock_cloud.list_machines.assert_called_once_with() + mock_cloud.list_nics_for_machine.assert_called_once_with( + 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45') + self.assertIn('baremetal', groups) + self.assertIn('hosts', groups['baremetal']) + self.assertEqual(groups['baremetal'], {'hosts': ['node1']}) + self.assertIn('node1', hostvars) + expected_machine = { + 'addressing_mode': 'dhcp', + 'driver_info': { + 'ipmi_address': '1.2.3.4', + }, + 'name': 'node1', + 'nics': [ + { + 'mac': '00:11:22:33:44:55', + }, + { + 'mac': '00:11:22:33:44:56', + }, + ], + 'properties': { + 'cpus': 42, + }, + 'uuid': 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45', + } + self.assertEqual(hostvars['node1'], expected_machine) diff --git a/test-requirements.txt b/test-requirements.txt index c96b2ac4f..3fb3201b7 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -14,3 +14,4 @@ testrepository>=0.0.18 # Apache-2.0/BSD testscenarios>=0.4 # Apache-2.0/BSD testtools>=1.4.0 # MIT PyYAML>=3.10.0 # MIT +mock>=2.0 # BSD