Split standard plugin tests from other processing tests

Allows for much cleaner and complete testing, also simplifies future
changes.

Change-Id: Ia586c21ff8e6e6449c6048d4764d1d12259ab0ba
Partial-Bug: #1492946

ironic_inspector/test/base.py

@@ -68,6 +68,11 @@ class BaseTest(unittest.TestCase):
             # 'p=patch' magic is due to how closures work
             self.addCleanup(lambda p=patch: p.stop())
 
+    def assertPatchEqual(self, expected, actual):
+        expected = sorted(expected, key=lambda p: p['path'])
+        actual = sorted(actual, key=lambda p: p['path'])
+        self.assertEqual(expected, actual)
+
 
 class NodeTest(BaseTest):
     def setUp(self):

ironic_inspector/test/test_plugins_standard.py

@@ -16,8 +16,11 @@ import os
 import shutil
 import tempfile
 
+import mock
 from oslo_config import cfg
 
+from ironic_inspector import node_cache
+from ironic_inspector.plugins import base
 from ironic_inspector.plugins import standard as std_plugins
 from ironic_inspector import process
 from ironic_inspector.test import base as test_base
@@ -26,6 +29,181 @@ from ironic_inspector import utils
 CONF = cfg.CONF
 
 
+class TestSchedulerHook(test_base.NodeTest):
+    def setUp(self):
+        super(TestSchedulerHook, self).setUp()
+        self.hook = std_plugins.SchedulerHook()
+        self.data = {
+            'local_gb': 20,
+            'memory_mb': 1024,
+            'cpus': 2,
+            'cpu_arch': 'x86_64'
+        }
+        self.node_patches = []
+        self.ports_patches = {}
+        self.node_info = node_cache.NodeInfo(uuid=self.uuid, started_at=0,
+                                             node=self.node)
+
+    def test_hook_loadable_by_name(self):
+        CONF.set_override('processing_hooks', 'scheduler', 'processing')
+        ext = base.processing_hooks_manager()['scheduler']
+        self.assertIsInstance(ext.obj, std_plugins.SchedulerHook)
+
+    def test_before_processing(self):
+        self.hook.before_processing(self.data)
+
+    def test_before_processing_missing(self):
+        for key in self.data:
+            new_data = self.data.copy()
+            del new_data[key]
+            self.assertRaisesRegexp(utils.Error, key,
+                                    self.hook.before_processing, new_data)
+
+    def test_before_update(self):
+        patch = [
+            {'path': '/properties/cpus', 'value': '2', 'op': 'add'},
+            {'path': '/properties/cpu_arch', 'value': 'x86_64', 'op': 'add'},
+            {'path': '/properties/memory_mb', 'value': '1024', 'op': 'add'},
+            {'path': '/properties/local_gb', 'value': '20', 'op': 'add'}
+        ]
+
+        self.hook.before_update(self.data, self.node_info,
+                                self.node_patches, self.ports_patches)
+        self.assertPatchEqual(patch, self.node_patches)
+        self.assertFalse(self.ports_patches)
+
+    def test_before_update_no_overwrite(self):
+        CONF.set_override('overwrite_existing', False, 'processing')
+        self.node.properties = {
+            'memory_mb': '4096',
+            'cpu_arch': 'i686'
+        }
+        patch = [
+            {'path': '/properties/cpus', 'value': '2', 'op': 'add'},
+            {'path': '/properties/local_gb', 'value': '20', 'op': 'add'}
+        ]
+
+        self.hook.before_update(self.data, self.node_info,
+                                self.node_patches, self.ports_patches)
+        self.assertPatchEqual(patch, self.node_patches)
+        self.assertFalse(self.ports_patches)
+
+
+class TestValidateInterfacesHook(test_base.NodeTest):
+    def setUp(self):
+        super(TestValidateInterfacesHook, self).setUp()
+        self.hook = std_plugins.ValidateInterfacesHook()
+        self.data = {
+            'interfaces': {
+                'em1': {'mac': '11:11:11:11:11:11', 'ip': '1.1.1.1'},
+                'em2': {'mac': '22:22:22:22:22:22', 'ip': '2.2.2.2'},
+                'em3': {'mac': '33:33:33:33:33:33'}
+            },
+            'boot_interface': '01-22-22-22-22-22-22',
+        }
+        self.orig_interfaces = self.data['interfaces'].copy()
+        self.pxe_interface = self.data['interfaces']['em2']
+        self.active_interfaces = {
+            'em1': {'mac': '11:11:11:11:11:11', 'ip': '1.1.1.1'},
+            'em2': {'mac': '22:22:22:22:22:22', 'ip': '2.2.2.2'},
+        }
+
+        self.existing_ports = [mock.Mock(spec=['address', 'uuid'],
+                                         address=a)
+                               for a in ('11:11:11:11:11:11',
+                                         '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_hook_loadable_by_name(self):
+        CONF.set_override('processing_hooks', 'validate_interfaces',
+                          'processing')
+        ext = base.processing_hooks_manager()['validate_interfaces']
+        self.assertIsInstance(ext.obj, std_plugins.ValidateInterfacesHook)
+
+    def test_wrong_add_ports(self):
+        CONF.set_override('add_ports', 'foobar', 'processing')
+        self.assertRaises(SystemExit, std_plugins.ValidateInterfacesHook)
+
+    def test_wrong_keep_ports(self):
+        CONF.set_override('keep_ports', 'foobar', 'processing')
+        self.assertRaises(SystemExit, std_plugins.ValidateInterfacesHook)
+
+    def test_no_interfaces(self):
+        self.assertRaisesRegexp(utils.Error, 'No interfaces',
+                                self.hook.before_processing, {})
+
+    def test_only_pxe(self):
+        self.hook.before_processing(self.data)
+
+        self.assertEqual({'em2': self.pxe_interface}, self.data['interfaces'])
+        self.assertEqual([self.pxe_interface['mac']], self.data['macs'])
+        self.assertEqual(self.orig_interfaces, self.data['all_interfaces'])
+
+    def test_only_pxe_mac_format(self):
+        self.data['boot_interface'] = '22:22:22:22:22:22'
+        self.hook.before_processing(self.data)
+
+        self.assertEqual({'em2': self.pxe_interface}, self.data['interfaces'])
+        self.assertEqual([self.pxe_interface['mac']], self.data['macs'])
+        self.assertEqual(self.orig_interfaces, self.data['all_interfaces'])
+
+    def test_only_pxe_not_found(self):
+        self.data['boot_interface'] = 'aa:bb:cc:dd:ee:ff'
+        self.assertRaisesRegexp(utils.Error, 'No valid interfaces',
+                                self.hook.before_processing, self.data)
+
+    def test_only_pxe_no_boot_interface(self):
+        del self.data['boot_interface']
+        self.hook.before_processing(self.data)
+
+        self.assertEqual(self.active_interfaces, self.data['interfaces'])
+        self.assertEqual([i['mac'] for i in self.active_interfaces.values()],
+                         self.data['macs'])
+        self.assertEqual(self.orig_interfaces, self.data['all_interfaces'])
+
+    def test_only_active(self):
+        CONF.set_override('add_ports', 'active', 'processing')
+        self.hook.before_processing(self.data)
+
+        self.assertEqual(self.active_interfaces, self.data['interfaces'])
+        self.assertEqual([i['mac'] for i in self.active_interfaces.values()],
+                         self.data['macs'])
+        self.assertEqual(self.orig_interfaces, self.data['all_interfaces'])
+
+    def test_all(self):
+        CONF.set_override('add_ports', 'all', 'processing')
+        self.hook.before_processing(self.data)
+
+        self.assertEqual(self.orig_interfaces, self.data['interfaces'])
+        self.assertEqual([i['mac'] for i in self.orig_interfaces.values()],
+                         self.data['macs'])
+        self.assertEqual(self.orig_interfaces, self.data['all_interfaces'])
+
+    @mock.patch.object(node_cache.NodeInfo, 'delete_port', autospec=True)
+    def test_keep_all(self, mock_delete_port):
+        self.hook.before_update(self.data, self.node_info, None, None)
+        self.assertFalse(mock_delete_port.called)
+
+    @mock.patch.object(node_cache.NodeInfo, 'delete_port')
+    def test_keep_present(self, mock_delete_port):
+        CONF.set_override('keep_ports', 'present', 'processing')
+        self.data['all_interfaces'] = self.orig_interfaces
+        self.hook.before_update(self.data, self.node_info, None, None)
+
+        mock_delete_port.assert_called_once_with(self.existing_ports[1])
+
+    @mock.patch.object(node_cache.NodeInfo, 'delete_port')
+    def test_keep_added(self, mock_delete_port):
+        CONF.set_override('keep_ports', 'added', 'processing')
+        self.data['macs'] = [self.pxe_interface['mac']]
+        self.hook.before_update(self.data, self.node_info, None, None)
+
+        mock_delete_port.assert_any_call(self.existing_ports[0])
+        mock_delete_port.assert_any_call(self.existing_ports[1])
+
+
 class TestRamdiskError(test_base.BaseTest):
     def setUp(self):
         super(TestRamdiskError, self).setUp()

ironic_inspector/test/test_process.py

@@ -24,7 +24,6 @@ from ironic_inspector import firewall
 from ironic_inspector import node_cache
 from ironic_inspector.plugins import base as plugins_base
 from ironic_inspector.plugins import example as example_plugin
-from ironic_inspector.plugins import standard as std_plugins
 from ironic_inspector import process
 from ironic_inspector.test import base as test_base
 from ironic_inspector import utils
@@ -36,7 +35,6 @@ class BaseTest(test_base.NodeTest):
     def setUp(self):
         super(BaseTest, self).setUp()
         self.started_at = time.time()
-        self.all_macs = self.macs + ['DE:AD:BE:EF:DE:AD']
         self.pxe_mac = self.macs[1]
         self.data = {
             'ipmi_address': self.bmc_address,
@@ -47,7 +45,7 @@ class BaseTest(test_base.NodeTest):
             'interfaces': {
                 'em1': {'mac': self.macs[0], 'ip': '1.2.0.1'},
                 'em2': {'mac': self.macs[1], 'ip': '1.2.0.2'},
-                'em3': {'mac': self.all_macs[2]},
+                'em3': {'mac': 'DE:AD:BE:EF:DE:AD'},
             },
             'boot_interface': '01-' + self.pxe_mac.replace(':', '-'),
         }
@@ -89,8 +87,6 @@ class TestProcess(BaseTest):
 
         # Only boot interface is added by default
         self.assertEqual(['em2'], sorted(self.data['interfaces']))
-        self.assertEqual(['em1', 'em2', 'em3'],
-                         sorted(self.data['all_interfaces']))
         self.assertEqual([self.pxe_mac], self.data['macs'])
 
         pop_mock.assert_called_once_with(bmc_address=self.bmc_address,
@@ -99,87 +95,6 @@ class TestProcess(BaseTest):
         process_mock.assert_called_once_with(cli.node.get.return_value,
                                              self.data, pop_mock.return_value)
 
-    @prepare_mocks
-    def test_boot_interface_as_mac(self, cli, pop_mock, process_mock):
-        self.data['boot_interface'] = self.pxe_mac
-
-        res = process.process(self.data)
-
-        self.assertEqual(self.fake_result_json, res)
-
-        # Only boot interface is added by default
-        self.assertEqual(['em2'], sorted(self.data['interfaces']))
-        self.assertEqual(['em1', 'em2', 'em3'],
-                         sorted(self.data['all_interfaces']))
-        self.assertEqual([self.pxe_mac], self.data['macs'])
-
-        pop_mock.assert_called_once_with(bmc_address=self.bmc_address,
-                                         mac=self.data['macs'])
-        cli.node.get.assert_called_once_with(self.uuid)
-        process_mock.assert_called_once_with(cli.node.get.return_value,
-                                             self.data, pop_mock.return_value)
-
-    @prepare_mocks
-    def test_no_boot_interface(self, cli, pop_mock, process_mock):
-        del self.data['boot_interface']
-
-        res = process.process(self.data)
-
-        self.assertEqual(self.fake_result_json, res)
-
-        # By default interfaces w/o IP are dropped
-        self.assertEqual(['em1', 'em2'], sorted(self.data['interfaces']))
-        self.assertEqual(['em1', 'em2', 'em3'],
-                         sorted(self.data['all_interfaces']))
-        self.assertEqual(self.macs, sorted(self.data['macs']))
-
-        pop_mock.assert_called_once_with(bmc_address=self.bmc_address,
-                                         mac=self.data['macs'])
-        cli.node.get.assert_called_once_with(self.uuid)
-        process_mock.assert_called_once_with(cli.node.get.return_value,
-                                             self.data, pop_mock.return_value)
-
-    @prepare_mocks
-    def test_add_ports_active(self, cli, pop_mock, process_mock):
-        CONF.set_override('add_ports', 'active', 'processing')
-
-        res = process.process(self.data)
-
-        self.assertEqual(self.fake_result_json, res)
-
-        self.assertEqual(['em1', 'em2'],
-                         sorted(self.data['interfaces']))
-        self.assertEqual(['em1', 'em2', 'em3'],
-                         sorted(self.data['all_interfaces']))
-        self.assertEqual(self.macs, sorted(self.data['macs']))
-
-        pop_mock.assert_called_once_with(bmc_address=self.bmc_address,
-                                         mac=self.data['macs'])
-        cli.node.get.assert_called_once_with(self.uuid)
-        process_mock.assert_called_once_with(cli.node.get.return_value,
-                                             self.data, pop_mock.return_value)
-
-    @prepare_mocks
-    def test_add_ports_all(self, cli, pop_mock, process_mock):
-        CONF.set_override('add_ports', 'all', 'processing')
-
-        res = process.process(self.data)
-
-        self.assertEqual(self.fake_result_json, res)
-
-        # By default interfaces w/o IP are dropped
-        self.assertEqual(['em1', 'em2', 'em3'],
-                         sorted(self.data['interfaces']))
-        self.assertEqual(['em1', 'em2', 'em3'],
-                         sorted(self.data['all_interfaces']))
-        self.assertEqual(self.all_macs, sorted(self.data['macs']))
-
-        pop_mock.assert_called_once_with(bmc_address=self.bmc_address,
-                                         mac=self.data['macs'])
-        cli.node.get.assert_called_once_with(self.uuid)
-        process_mock.assert_called_once_with(cli.node.get.return_value,
-                                             self.data, pop_mock.return_value)
-
     @prepare_mocks
     def test_no_ipmi(self, cli, pop_mock, process_mock):
         del self.data['ipmi_address']
@@ -191,50 +106,6 @@ class TestProcess(BaseTest):
         process_mock.assert_called_once_with(cli.node.get.return_value,
                                              self.data, pop_mock.return_value)
 
-    @prepare_mocks
-    def test_no_interfaces(self, cli, pop_mock, process_mock):
-        del self.data['interfaces']
-        self.assertRaises(utils.Error, process.process, self.data)
-
-    @prepare_mocks
-    def test_invalid_interfaces_all(self, cli, pop_mock, process_mock):
-        self.data['interfaces'] = {
-            'br1': {'mac': 'broken', 'ip': '1.2.0.1'},
-            'br2': {'mac': '', 'ip': '1.2.0.2'},
-            'br3': {},
-        }
-
-        self.assertRaises(utils.Error, process.process, self.data)
-
-    @prepare_mocks
-    def test_invalid_interfaces(self, cli, pop_mock, process_mock):
-        self.data['interfaces'] = {
-            'br1': {'mac': 'broken', 'ip': '1.2.0.1'},
-            'br2': {'mac': '', 'ip': '1.2.0.2'},
-            'br3': {},
-            'em1': {'mac': self.macs[1], 'ip': '1.2.3.4'},
-        }
-
-        process.process(self.data)
-
-        self.assertEqual(['em1'], list(self.data['interfaces']))
-        self.assertEqual([self.macs[1]], self.data['macs'])
-
-        pop_mock.assert_called_once_with(bmc_address=self.bmc_address,
-                                         mac=[self.macs[1]])
-        cli.node.get.assert_called_once_with(self.uuid)
-        process_mock.assert_called_once_with(cli.node.get.return_value,
-                                             self.data, pop_mock.return_value)
-
-    @prepare_mocks
-    def test_missing_required(self, cli, pop_mock, process_mock):
-        del self.data['cpus']
-
-        self.assertRaisesRegexp(utils.Error,
-                                'missing',
-                                process.process, self.data)
-        self.assertFalse(process_mock.called)
-
     @prepare_mocks
     def test_not_found_in_cache(self, cli, pop_mock, process_mock):
         pop_mock.side_effect = iter([utils.Error('not found')])
@@ -537,47 +408,6 @@ class TestProcessNode(BaseTest):
             error='Failed to power off node %s, check it\'s power management'
             ' configuration: boom' % self.uuid)
 
-    @mock.patch.object(utils, 'get_client')
-    def test_keep_ports_present(self, client_mock, filters_mock,
-                                post_hook_mock):
-        CONF.set_override('keep_ports', 'present', 'processing')
-
-        # 2 MACs valid, one invalid, one not present in data
-        all_macs = self.all_macs + ['01:09:02:08:03:07']
-        all_ports = [
-            mock.Mock(uuid='port_uuid%d' % i, address=mac)
-            for i, mac in enumerate(all_macs)
-        ]
-
-        client_mock.return_value = self.cli
-        self.cli.node.list_ports.return_value = all_ports
-
-        self.call()
-
-        self.cli.node.list_ports.assert_called_once_with(self.uuid, limit=0)
-        self.cli.port.delete.assert_called_once_with(all_ports[-1].uuid)
-
-    @mock.patch.object(utils, 'get_client')
-    def test_keep_ports_added(self, client_mock, filters_mock, post_hook_mock):
-        CONF.set_override('keep_ports', 'added', 'processing')
-
-        # 2 MACs valid, one invalid, one not present in data
-        all_macs = self.all_macs + ['01:09:02:08:03:07']
-        all_ports = [
-            mock.Mock(uuid='port_uuid%d' % i, address=mac)
-            for i, mac in enumerate(all_macs)
-        ]
-
-        client_mock.return_value = self.cli
-        self.cli.node.list_ports.return_value = all_ports
-
-        self.call()
-
-        self.cli.node.list_ports.assert_called_once_with(self.uuid, limit=0)
-        for port in all_ports[2:]:
-            self.cli.port.delete.assert_any_call(port.uuid)
-        self.assertEqual(2, self.cli.port.delete.call_count)
-
     @mock.patch.object(process.swift, 'SwiftAPI', autospec=True)
     def test_store_data(self, swift_mock, filters_mock, post_hook_mock):
         CONF.set_override('store_data', 'swift', 'processing')
@@ -611,13 +441,3 @@ class TestProcessNode(BaseTest):
         swift_conn.create_object.assert_called_once_with(name, expected)
         self.cli.node.update.assert_called_once_with(self.uuid,
                                                      self.patch_props)
-
-
-class TestValidateInterfacesHook(test_base.BaseTest):
-    def test_wrong_add_ports(self):
-        CONF.set_override('add_ports', 'foobar', 'processing')
-        self.assertRaises(SystemExit, std_plugins.ValidateInterfacesHook)
-
-    def test_wrong_keep_ports(self):
-        CONF.set_override('keep_ports', 'foobar', 'processing')
-        self.assertRaises(SystemExit, std_plugins.ValidateInterfacesHook)