diff --git a/ironic_inspector/plugins/standard.py b/ironic_inspector/plugins/standard.py index 33b47eafc..123d0622c 100644 --- a/ironic_inspector/plugins/standard.py +++ b/ironic_inspector/plugins/standard.py @@ -38,7 +38,8 @@ class RootDiskSelectionHook(base.ProcessingHook): might not be updated. """ - def before_update(self, introspection_data, node_info, **kwargs): + def _process_root_device_hints(self, introspection_data, node_info, + inventory): """Detect root disk from root device hints and IPA inventory.""" hints = node_info.node().properties.get('root_device') if not hints: @@ -46,8 +47,6 @@ class RootDiskSelectionHook(base.ProcessingHook): node_info=node_info, data=introspection_data) return - inventory = utils.get_inventory(introspection_data, - node_info=node_info) try: device = il_utils.match_root_device_hints(inventory['disks'], hints) @@ -68,11 +67,36 @@ class RootDiskSelectionHook(base.ProcessingHook): node_info=node_info, data=introspection_data) introspection_data['root_disk'] = device + def before_update(self, introspection_data, node_info, **kwargs): + """Process root disk information.""" + inventory = utils.get_inventory(introspection_data, + node_info=node_info) + self._process_root_device_hints(introspection_data, node_info, + inventory) + + root_disk = introspection_data.get('root_disk') + if root_disk: + local_gb = root_disk['size'] // units.Gi + if CONF.processing.disk_partitioning_spacing: + local_gb -= 1 + LOG.info('Root disk %(disk)s, local_gb %(local_gb)s GiB', + {'disk': root_disk, 'local_gb': local_gb}, + node_info=node_info, data=introspection_data) + else: + local_gb = 0 + LOG.info('No root device found, assuming a diskless node', + node_info=node_info, data=introspection_data) + + introspection_data['local_gb'] = local_gb + if (CONF.processing.overwrite_existing or not + node_info.node().properties.get('local_gb')): + node_info.update_properties(local_gb=str(local_gb)) + class SchedulerHook(base.ProcessingHook): """Nova scheduler required properties.""" - KEYS = ('cpus', 'cpu_arch', 'memory_mb', 'local_gb') + KEYS = ('cpus', 'cpu_arch', 'memory_mb') def before_update(self, introspection_data, node_info, **kwargs): """Update node with scheduler properties.""" @@ -80,14 +104,6 @@ class SchedulerHook(base.ProcessingHook): node_info=node_info) errors = [] - root_disk = introspection_data.get('root_disk') - if root_disk: - introspection_data['local_gb'] = root_disk['size'] // units.Gi - if CONF.processing.disk_partitioning_spacing: - introspection_data['local_gb'] -= 1 - else: - introspection_data['local_gb'] = 0 - try: introspection_data['cpus'] = int(inventory['cpu']['count']) introspection_data['cpu_arch'] = six.text_type( @@ -110,7 +126,7 @@ class SchedulerHook(base.ProcessingHook): node_info=node_info, data=introspection_data) LOG.info('Discovered data: CPUs: %(cpus)s %(cpu_arch)s, ' - 'memory %(memory_mb)s MiB, disk %(local_gb)s GiB', + 'memory %(memory_mb)s MiB', {key: introspection_data.get(key) for key in self.KEYS}, node_info=node_info, data=introspection_data) diff --git a/ironic_inspector/test/functional.py b/ironic_inspector/test/functional.py index 821ab11ed..d378e552a 100644 --- a/ironic_inspector/test/functional.py +++ b/ironic_inspector/test/functional.py @@ -247,7 +247,7 @@ class Test(Base): self.assertEqual({'uuid': self.uuid}, res) eventlet.greenthread.sleep(DEFAULT_SLEEP) - self.cli.node.update.assert_called_once_with(self.uuid, mock.ANY) + self.cli.node.update.assert_called_with(self.uuid, mock.ANY) self.assertCalledWithPatch(self.patch, self.cli.node.update) self.cli.port.create.assert_called_once_with( node_uuid=self.uuid, address='11:22:33:44:55:66', extra={}, @@ -286,7 +286,7 @@ class Test(Base): self.assertEqual({'uuid': self.uuid}, res) eventlet.greenthread.sleep(DEFAULT_SLEEP) - self.cli.node.update.assert_called_once_with(self.uuid, mock.ANY) + self.cli.node.update.assert_called_with(self.uuid, mock.ANY) self.assertCalledWithPatch(self.patch, self.cli.node.update) calls = [ mock.call(node_uuid=self.uuid, address=self.macs[0], @@ -689,7 +689,7 @@ class Test(Base): self.assertEqual({'uuid': self.uuid}, res) eventlet.greenthread.sleep(DEFAULT_SLEEP) - self.cli.node.update.assert_called_once_with(self.uuid, mock.ANY) + self.cli.node.update.assert_called_with(self.uuid, mock.ANY) self.assertCalledWithPatch(self.patch, self.cli.node.update) self.cli.port.create.assert_called_once_with( node_uuid=self.uuid, extra={}, address='11:22:33:44:55:66', diff --git a/ironic_inspector/test/unit/test_plugins_standard.py b/ironic_inspector/test/unit/test_plugins_standard.py index 3a579b182..0b32f6e4d 100644 --- a/ironic_inspector/test/unit/test_plugins_standard.py +++ b/ironic_inspector/test/unit/test_plugins_standard.py @@ -39,29 +39,12 @@ class TestSchedulerHook(test_base.NodeTest): ext = base.processing_hooks_manager()['scheduler'] self.assertIsInstance(ext.obj, std_plugins.SchedulerHook) - @mock.patch.object(node_cache.NodeInfo, 'patch') - def test_no_root_disk(self, mock_patch): - del self.inventory['disks'] - del self.data['root_disk'] - - patch = [ - {'path': '/properties/cpus', 'value': '4', 'op': 'add'}, - {'path': '/properties/cpu_arch', 'value': 'x86_64', 'op': 'add'}, - {'path': '/properties/memory_mb', 'value': '12288', 'op': 'add'}, - {'path': '/properties/local_gb', 'value': '0', 'op': 'add'} - ] - - self.hook.before_update(self.data, self.node_info) - self.assertCalledWithPatch(patch, mock_patch) - self.assertEqual(0, self.data['local_gb']) - @mock.patch.object(node_cache.NodeInfo, 'patch') def test_ok(self, mock_patch): patch = [ {'path': '/properties/cpus', 'value': '4', 'op': 'add'}, {'path': '/properties/cpu_arch', 'value': 'x86_64', 'op': 'add'}, {'path': '/properties/memory_mb', 'value': '12288', 'op': 'add'}, - {'path': '/properties/local_gb', 'value': '999', 'op': 'add'} ] self.hook.before_update(self.data, self.node_info) @@ -76,20 +59,6 @@ class TestSchedulerHook(test_base.NodeTest): } patch = [ {'path': '/properties/cpus', 'value': '4', 'op': 'add'}, - {'path': '/properties/local_gb', 'value': '999', 'op': 'add'} - ] - - self.hook.before_update(self.data, self.node_info) - self.assertCalledWithPatch(patch, mock_patch) - - @mock.patch.object(node_cache.NodeInfo, 'patch') - def test_root_disk_no_spacing(self, mock_patch): - CONF.set_override('disk_partitioning_spacing', False, 'processing') - patch = [ - {'path': '/properties/cpus', 'value': '4', 'op': 'add'}, - {'path': '/properties/cpu_arch', 'value': 'x86_64', 'op': 'add'}, - {'path': '/properties/memory_mb', 'value': '12288', 'op': 'add'}, - {'path': '/properties/local_gb', 'value': '1000', 'op': 'add'} ] self.hook.before_update(self.data, self.node_info) @@ -332,8 +301,19 @@ class TestRootDiskSelection(test_base.NodeTest): self.hook.before_update(self.data, self.node_info) - self.assertNotIn('local_gb', self.data) + self.assertEqual(0, self.data['local_gb']) self.assertNotIn('root_disk', self.data) + self.node_info.update_properties.assert_called_once_with(local_gb='0') + + def test_no_hints_no_overwrite(self): + CONF.set_override('overwrite_existing', False, 'processing') + del self.data['root_disk'] + + self.hook.before_update(self.data, self.node_info) + + self.assertEqual(0, self.data['local_gb']) + self.assertNotIn('root_disk', self.data) + self.assertFalse(self.node_info.update_properties.called) def test_no_inventory(self): self.node.properties['root_device'] = {'model': 'foo'} @@ -347,6 +327,7 @@ class TestRootDiskSelection(test_base.NodeTest): self.assertNotIn('local_gb', self.data) self.assertNotIn('root_disk', self.data) + self.assertFalse(self.node_info.update_properties.called) def test_no_disks(self): self.node.properties['root_device'] = {'size': 10} @@ -357,12 +338,27 @@ class TestRootDiskSelection(test_base.NodeTest): self.hook.before_update, self.data, self.node_info) + self.assertNotIn('local_gb', self.data) + self.assertFalse(self.node_info.update_properties.called) + def test_one_matches(self): self.node.properties['root_device'] = {'size': 10} self.hook.before_update(self.data, self.node_info) self.assertEqual(self.matched, self.data['root_disk']) + self.assertEqual(9, self.data['local_gb']) + self.node_info.update_properties.assert_called_once_with(local_gb='9') + + def test_local_gb_without_spacing(self): + CONF.set_override('disk_partitioning_spacing', False, 'processing') + self.node.properties['root_device'] = {'size': 10} + + self.hook.before_update(self.data, self.node_info) + + self.assertEqual(self.matched, self.data['root_disk']) + self.assertEqual(10, self.data['local_gb']) + self.node_info.update_properties.assert_called_once_with(local_gb='10') def test_all_match(self): self.node.properties['root_device'] = {'size': 10, @@ -371,6 +367,8 @@ class TestRootDiskSelection(test_base.NodeTest): self.hook.before_update(self.data, self.node_info) self.assertEqual(self.matched, self.data['root_disk']) + self.assertEqual(9, self.data['local_gb']) + self.node_info.update_properties.assert_called_once_with(local_gb='9') def test_one_fails(self): self.node.properties['root_device'] = {'size': 10, @@ -384,11 +382,14 @@ class TestRootDiskSelection(test_base.NodeTest): self.assertNotIn('local_gb', self.data) self.assertNotIn('root_disk', self.data) + self.assertFalse(self.node_info.update_properties.called) def test_size_string(self): self.node.properties['root_device'] = {'size': '10'} self.hook.before_update(self.data, self.node_info) self.assertEqual(self.matched, self.data['root_disk']) + self.assertEqual(9, self.data['local_gb']) + self.node_info.update_properties.assert_called_once_with(local_gb='9') def test_size_invalid(self): for bad_size in ('foo', None, {}): @@ -398,6 +399,9 @@ class TestRootDiskSelection(test_base.NodeTest): self.hook.before_update, self.data, self.node_info) + self.assertNotIn('local_gb', self.data) + self.assertFalse(self.node_info.update_properties.called) + class TestRamdiskError(test_base.InventoryTest): def setUp(self): diff --git a/releasenotes/notes/local_gb-250bd415684a7855.yaml b/releasenotes/notes/local_gb-250bd415684a7855.yaml new file mode 100644 index 000000000..3bf8ec12a --- /dev/null +++ b/releasenotes/notes/local_gb-250bd415684a7855.yaml @@ -0,0 +1,5 @@ +--- +upgrade: + - | + Handling of ``local_gb`` property was moved from the ``scheduler`` hook + to ``root_disk_selection``.