diff --git a/ironic_inspector/plugins/standard.py b/ironic_inspector/plugins/standard.py index 8ad68afd3..7033e4c42 100644 --- a/ironic_inspector/plugins/standard.py +++ b/ironic_inspector/plugins/standard.py @@ -252,7 +252,10 @@ class ValidateInterfacesHook(base.ProcessingHook): introspection_data['macs'] = valid_macs def before_update(self, introspection_data, node_info, **kwargs): - """Drop ports that are not present in the data.""" + """Create new ports and drop ports that are not present in the data.""" + interfaces = introspection_data.get('interfaces') + node_info.create_ports(list(interfaces.values())) + if CONF.processing.keep_ports == 'present': expected_macs = { iface['mac'] diff --git a/ironic_inspector/process.py b/ironic_inspector/process.py index b1e00ce8a..03d2dbf5a 100644 --- a/ironic_inspector/process.py +++ b/ironic_inspector/process.py @@ -268,8 +268,6 @@ def _run_post_hooks(node_info, introspection_data): def _process_node(node_info, node, introspection_data): # NOTE(dtantsur): repeat the check in case something changed ir_utils.check_provision_state(node) - interfaces = introspection_data.get('interfaces') - node_info.create_ports(list(interfaces.values())) _run_post_hooks(node_info, introspection_data) _store_data(node_info, introspection_data) @@ -435,8 +433,6 @@ def _reapply_with_data(node_info, introspection_data): 'introspection on stored data:\n%s') % '\n'.join(failures), node_info=node_info) - interfaces = introspection_data.get('interfaces') - node_info.create_ports(list(interfaces.values())) _run_post_hooks(node_info, introspection_data) _store_data(node_info, introspection_data) node_info.invalidate_cache() diff --git a/ironic_inspector/test/unit/test_plugins_standard.py b/ironic_inspector/test/unit/test_plugins_standard.py index 4e595ae10..ae058b56e 100644 --- a/ironic_inspector/test/unit/test_plugins_standard.py +++ b/ironic_inspector/test/unit/test_plugins_standard.py @@ -96,18 +96,7 @@ class TestSchedulerHook(test_base.NodeTest): self.assertCalledWithPatch(patch, mock_patch) -class TestValidateInterfacesHook(test_base.NodeTest): - def setUp(self): - super(TestValidateInterfacesHook, self).setUp() - self.hook = std_plugins.ValidateInterfacesHook() - self.existing_ports = [mock.Mock(spec=['address', 'uuid'], - address=a) - for a in (self.macs[1], - '44:44:44:44:44:44')] - self.node_info = node_cache.NodeInfo(uuid=self.uuid, started_at=0, - node=self.node, - ports=self.existing_ports) - +class TestValidateInterfacesHookLoad(test_base.NodeTest): def test_hook_loadable_by_name(self): CONF.set_override('processing_hooks', 'validate_interfaces', 'processing') @@ -122,6 +111,12 @@ class TestValidateInterfacesHook(test_base.NodeTest): CONF.set_override('keep_ports', 'foobar', 'processing') self.assertRaises(SystemExit, std_plugins.ValidateInterfacesHook) + +class TestValidateInterfacesHookBeforeProcessing(test_base.NodeTest): + def setUp(self): + super(TestValidateInterfacesHookBeforeProcessing, self).setUp() + self.hook = std_plugins.ValidateInterfacesHook() + def test_no_interfaces(self): self.assertRaisesRegex(utils.Error, 'Hardware inventory is empty or missing', @@ -212,27 +207,61 @@ class TestValidateInterfacesHook(test_base.NodeTest): self.assertRaisesRegex(utils.Error, 'No suitable interfaces found', self.hook.before_processing, self.data) - @mock.patch.object(node_cache.NodeInfo, 'delete_port', autospec=True) - def test_keep_all(self, mock_delete_port): + +@mock.patch.object(node_cache.NodeInfo, 'delete_port', autospec=True) +@mock.patch.object(node_cache.NodeInfo, 'create_ports', autospec=True) +class TestValidateInterfacesHookBeforeUpdate(test_base.NodeTest): + def setUp(self): + super(TestValidateInterfacesHookBeforeUpdate, self).setUp() + self.hook = std_plugins.ValidateInterfacesHook() + self.interfaces_to_create = sorted(self.valid_interfaces.values(), + key=lambda i: i['mac']) + self.existing_ports = [mock.Mock(spec=['address', 'uuid'], + address=a) + for a in (self.macs[1], + '44:44:44:44:44:44')] + self.node_info = node_cache.NodeInfo(uuid=self.uuid, started_at=0, + node=self.node, + ports=self.existing_ports) + + def test_keep_all(self, mock_create_ports, mock_delete_port): self.hook.before_update(self.data, self.node_info) + + # NOTE(dtantsur): dictionary ordering is not defined + mock_create_ports.assert_called_once_with(self.node_info, mock.ANY) + self.assertEqual(self.interfaces_to_create, + sorted(mock_create_ports.call_args[0][1], + key=lambda i: i['mac'])) + self.assertFalse(mock_delete_port.called) - @mock.patch.object(node_cache.NodeInfo, 'delete_port') - def test_keep_present(self, mock_delete_port): + def test_keep_present(self, mock_create_ports, mock_delete_port): CONF.set_override('keep_ports', 'present', 'processing') self.data['all_interfaces'] = self.all_interfaces self.hook.before_update(self.data, self.node_info) - mock_delete_port.assert_called_once_with(self.existing_ports[1]) + mock_create_ports.assert_called_once_with(self.node_info, mock.ANY) + self.assertEqual(self.interfaces_to_create, + sorted(mock_create_ports.call_args[0][1], + key=lambda i: i['mac'])) - @mock.patch.object(node_cache.NodeInfo, 'delete_port') - def test_keep_added(self, mock_delete_port): + mock_delete_port.assert_called_once_with(self.node_info, + self.existing_ports[1]) + + def test_keep_added(self, mock_create_ports, mock_delete_port): CONF.set_override('keep_ports', 'added', 'processing') self.data['macs'] = [self.pxe_mac] self.hook.before_update(self.data, self.node_info) - mock_delete_port.assert_any_call(self.existing_ports[0]) - mock_delete_port.assert_any_call(self.existing_ports[1]) + mock_create_ports.assert_called_once_with(self.node_info, mock.ANY) + self.assertEqual(self.interfaces_to_create, + sorted(mock_create_ports.call_args[0][1], + key=lambda i: i['mac'])) + + mock_delete_port.assert_any_call(self.node_info, + self.existing_ports[0]) + mock_delete_port.assert_any_call(self.node_info, + self.existing_ports[1]) class TestRootDiskSelection(test_base.NodeTest): diff --git a/releasenotes/notes/port-creation-plugin-c0405ec646b1051d.yaml b/releasenotes/notes/port-creation-plugin-c0405ec646b1051d.yaml new file mode 100644 index 000000000..03ae9ea8e --- /dev/null +++ b/releasenotes/notes/port-creation-plugin-c0405ec646b1051d.yaml @@ -0,0 +1,7 @@ +--- +upgrade: + - | + Ports creating logic was moved from core processing code to the + ``validate_interfaces`` processing hook. This may affect deployments + that disable this hook or replace it with something else. Also make + sure to place this hook before any hooks expecting ports to be created.