BIOS Settings: add sync_node_setting

sync_node_setting takes a list of bios settings as input
and sorts out a tuple of lists of create, update, delete
and nochange settings by comparing the given settings with
node 'bios_settings' database table.

This commit also modifies fake BIOS interface to use
sync_node_setting for testing purpose.

Change-Id: I831b3db8f4da24d88a81b4d85889f7fd6f83ffdb
Story: #1712032

ironic/common/exception.py

@@ -785,3 +785,7 @@ class BIOSSettingAlreadyExists(Conflict):
 
 class BIOSSettingNotFound(NotFound):
     _msg_fmt = _("Node %(node)s doesn't have a BIOS setting '%(name)s'")
+
+
+class BIOSSettingListNotFound(NotFound):
+    _msg_fmt = _("Node %(node)s doesn't have BIOS settings '%(names)s'")

ironic/db/api.py

@@ -1050,13 +1050,13 @@ class Connection(object):
         """
 
     @abc.abstractmethod
-    def delete_bios_setting(self, node_id, name):
+    def delete_bios_setting_list(self, node_id, names):
-        """Delete BIOS setting.
+        """Delete a list of BIOS settings.
 
         :param node_id: The node id.
-        :param name: String containing name of bios setting to be deleted.
+        :param names: List of BIOS setting names to be deleted.
         :raises: NodeNotFound if the node is not found.
-        :raises: BIOSSettingNotFound if the bios setting is not found.
+        :raises: BIOSSettingNotFound if any of BIOS setting name is not found.
         """
 
     @abc.abstractmethod
@@ -1064,10 +1064,10 @@ class Connection(object):
         """Retrieve BIOS setting value.
 
         :param node_id: The node id.
-        :param name: String containing name of bios setting to be retrieved.
+        :param name: String containing name of BIOS setting to be retrieved.
         :returns: The BIOSSetting object.
         :raises: NodeNotFound if the node is not found.
-        :raises: BIOSSettingNotFound if the bios setting is not found.
+        :raises: BIOSSettingNotFound if the BIOS setting is not found.
         """
 
     @abc.abstractmethod

ironic/db/sqlalchemy/api.py

@@ -1431,14 +1431,18 @@ class Connection(api.Connection):
         return bios_settings
 
     @oslo_db_api.retry_on_deadlock
-    def delete_bios_setting(self, node_id, name):
+    def delete_bios_setting_list(self, node_id, names):
         self._check_node_exists(node_id)
+        missing_bios_settings = []
         with _session_for_write():
-            count = model_query(models.BIOSSetting).filter_by(
+            for name in names:
-                node_id=node_id, name=name).delete()
+                count = model_query(models.BIOSSetting).filter_by(
-            if count == 0:
+                    node_id=node_id, name=name).delete()
-                raise exception.BIOSSettingNotFound(
+                if count == 0:
-                    node=node_id, name=name)
+                    missing_bios_settings.append(name)
+        if len(missing_bios_settings) > 0:
+            raise exception.BIOSSettingListNotFound(
+                node=node_id, names=','.join(missing_bios_settings))
 
     def get_bios_setting(self, node_id, name):
         self._check_node_exists(node_id)

ironic/drivers/modules/fake.py

@@ -238,7 +238,7 @@ class FakeRAID(base.RAIDInterface):
 
 
 class FakeBIOS(base.BIOSInterface):
-    """Example implementation of simple BIOSInterface."""
+    """Fake implementation of simple BIOSInterface."""
 
     def get_properties(self):
         return {}
@@ -247,13 +247,35 @@ class FakeBIOS(base.BIOSInterface):
         pass
 
     def apply_configuration(self, task, settings):
+        # Note: the implementation of apply_configuration in fake interface
+        # is just for testing purpose, for real driver implementation, please
+        # refer to develop doc at https://docs.openstack.org/ironic/latest/
+        # contributor/bios_develop.html.
         node_id = task.node.id
-        try:
+        create_list, update_list, delete_list, nochange_list = (
-            objects.BIOSSettingList.create(task.context, node_id, settings)
+            objects.BIOSSettingList.sync_node_setting(task.context, node_id,
-        except exception.BIOSSettingAlreadyExists:
+                                                      settings))
-            objects.BIOSSettingList.save(task.context, node_id, settings)
+
+        if len(create_list) > 0:
+            objects.BIOSSettingList.create(task.context, node_id, create_list)
+        if len(update_list) > 0:
+            objects.BIOSSettingList.save(task.context, node_id, update_list)
+        if len(delete_list) > 0:
+            delete_names = [setting['name'] for setting in delete_list]
+            objects.BIOSSettingList.delete(task.context, node_id,
+                                           delete_names)
+
+        # nochange_list is part of return of sync_node_setting and it might be
+        # useful to the drivers to give a message if no change is required
+        # during application of settings.
+        if len(nochange_list) > 0:
+            pass
 
     def factory_reset(self, task):
+        # Note: the implementation of factory_reset in fake interface is
+        # just for testing purpose, for real driver implementation, please
+        # refer to develop doc at https://docs.openstack.org/ironic/latest/
+        # contributor/bios_develop.html.
         node_id = task.node.id
         setting_objs = objects.BIOSSettingList.get_by_node_id(
             task.context, node_id)
@@ -261,6 +283,10 @@ class FakeBIOS(base.BIOSInterface):
             objects.BIOSSetting.delete(task.context, node_id, setting.name)
 
     def cache_bios_settings(self, task):
+        # Note: the implementation of cache_bios_settings in fake interface
+        # is just for testing purpose, for real driver implementation, please
+        # refer to develop doc at https://docs.openstack.org/ironic/latest/
+        # contributor/bios_develop.html.
         pass
 
 

ironic/objects/bios.py

@@ -87,7 +87,7 @@ class BIOSSetting(base.IronicObject):
 
         :param context: Security context.
         :param node_id: The node id.
-        :param name: Bios setting name to be retrieved.
+        :param name: BIOS setting name to be retrieved.
         :raises: NodeNotFound if the node id is not found.
         :raises: BIOSSettingNotFound if the bios setting name is not found.
         :returns: A :class:'BIOSSetting' object.
@@ -105,11 +105,11 @@ class BIOSSetting(base.IronicObject):
 
         :param context: Security context.
         :param node_id: The node id.
-        :param name: Bios setting name to be deleted.
+        :param name: BIOS setting name to be deleted.
         :raises: NodeNotFound if the node id is not found.
         :raises: BIOSSettingNotFound if the bios setting name is not found.
         """
-        cls.dbapi.delete_bios_setting(node_id, name)
+        cls.dbapi.delete_bios_setting_list(node_id, [name])
 
 
 @base.IronicObjectRegistry.register
@@ -177,6 +177,22 @@ class BIOSSettingList(base.IronicObjectListBase, base.IronicObject):
         return object_base.obj_make_list(
             context, cls(), BIOSSetting, updated_setting_list)
 
+    # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
+    # methods can be used in the future to replace current explicit RPC calls.
+    # Implications of calling new remote procedures should be thought through.
+    # @object_base.remotable_classmethod
+    @classmethod
+    def delete(cls, context, node_id, names):
+        """Delete BIOS Settings based on node_id and names.
+
+        :param context: Security context.
+        :param node_id: The node id.
+        :param names: List of BIOS setting names to be deleted.
+        :raises: NodeNotFound if the node id is not found.
+        :raises: BIOSSettingNotFound if any of BIOS setting fails to delete.
+        """
+        cls.dbapi.delete_bios_setting_list(node_id, names)
+
     # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
     # methods can be used in the future to replace current explicit RPC calls.
     # Implications of calling new remote procedures should be thought through.
@@ -193,3 +209,48 @@ class BIOSSettingList(base.IronicObjectListBase, base.IronicObject):
         node_bios_setting = cls.dbapi.get_bios_setting_list(node_id)
         return object_base.obj_make_list(
             context, cls(), BIOSSetting, node_bios_setting)
+
+    # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
+    # methods can be used in the future to replace current explicit RPC calls.
+    # Implications of calling new remote procedures should be thought through.
+    # @object_base.remotable_classmethod
+    @classmethod
+    def sync_node_setting(cls, context, node_id, settings):
+        """Returns lists of create/update/delete/unchanged settings.
+
+        This method sync with 'bios_settings' database table and sorts
+        out four lists of create/update/delete/unchanged settings.
+
+        :param context: Security context.
+        :param node_id: The node id.
+        :param settings: BIOS settings to be synced.
+        :returns: A 4-tuple of lists of BIOS settings to be created,
+            updated, deleted and unchanged.
+        """
+        create_list = []
+        update_list = []
+        delete_list = []
+        nochange_list = []
+        current_settings_dict = {}
+
+        given_setting_names = [setting['name'] for setting in settings]
+        current_settings = cls.get_by_node_id(context, node_id)
+
+        for setting in current_settings:
+            current_settings_dict[setting.name] = setting.value
+
+        for setting in settings:
+            if setting['name'] in current_settings_dict:
+                if setting['value'] != current_settings_dict[setting['name']]:
+                    update_list.append(setting)
+                else:
+                    nochange_list.append(setting)
+            else:
+                create_list.append(setting)
+
+        for setting in current_settings:
+            if setting.name not in given_setting_names:
+                delete_list.append({'name': setting.name,
+                                    'value': setting.value})
+
+        return (create_list, update_list, delete_list, nochange_list)

ironic/tests/unit/db/test_bios_settings.py

@@ -58,24 +58,6 @@ class DbBIOSSettingTestCase(base.DbTestCase):
                           self.dbapi.get_bios_setting_list,
                           '456')
 
-    def test_delete_bios_setting(self):
-        db_utils.create_test_bios_setting(node_id=self.node.id)
-        self.dbapi.delete_bios_setting(self.node.id, 'virtualization')
-        self.assertRaises(exception.BIOSSettingNotFound,
-                          self.dbapi.get_bios_setting,
-                          self.node.id, 'virtualization')
-
-    def test_delete_bios_setting_node_not_exist(self):
-        self.assertRaises(exception.NodeNotFound,
-                          self.dbapi.delete_bios_setting,
-                          '456', 'virtualization')
-
-    def test_delete_bios_setting_setting_not_exist(self):
-        db_utils.create_test_bios_setting(node_id=self.node.id)
-        self.assertRaises(exception.BIOSSettingNotFound,
-                          self.dbapi.delete_bios_setting,
-                          self.node.id, 'hyperthread')
-
     def test_create_bios_setting_list(self):
         settings = db_utils.get_test_bios_setting_setting_list()
         result = self.dbapi.create_bios_setting_list(
@@ -121,3 +103,30 @@ class DbBIOSSettingTestCase(base.DbTestCase):
         self.assertRaises(exception.NodeNotFound,
                           self.dbapi.update_bios_setting_list,
                           '456', [], '1.0')
+
+    def test_delete_bios_setting_list(self):
+        settings = db_utils.get_test_bios_setting_setting_list()
+        self.dbapi.create_bios_setting_list(self.node.id, settings, '1.0')
+        name_list = [setting['name'] for setting in settings]
+        self.dbapi.delete_bios_setting_list(self.node.id, name_list)
+        self.assertRaises(exception.BIOSSettingNotFound,
+                          self.dbapi.get_bios_setting,
+                          self.node.id, 'virtualization')
+        self.assertRaises(exception.BIOSSettingNotFound,
+                          self.dbapi.get_bios_setting,
+                          self.node.id, 'hyperthread')
+        self.assertRaises(exception.BIOSSettingNotFound,
+                          self.dbapi.get_bios_setting,
+                          self.node.id, 'numlock')
+
+    def test_delete_bios_setting_list_node_not_exist(self):
+        self.assertRaises(exception.NodeNotFound,
+                          self.dbapi.delete_bios_setting_list,
+                          '456', ['virtualization'])
+
+    def test_delete_bios_setting_list_setting_not_exist(self):
+        settings = db_utils.get_test_bios_setting_setting_list()
+        self.dbapi.create_bios_setting_list(self.node.id, settings, '1.0')
+        self.assertRaises(exception.BIOSSettingListNotFound,
+                          self.dbapi.delete_bios_setting_list,
+                          self.node.id, ['fake-bios-option'])

ironic/tests/unit/objects/test_bios.py

@@ -10,6 +10,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import types
+
 import mock
 
 from ironic.common import context
@@ -141,9 +143,56 @@ class TestBIOSSettingObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn):
         self.assertEqual(bios_setting2['name'], bios_obj_list[1].name)
         self.assertEqual(bios_setting2['value'], bios_obj_list[1].value)
 
-    @mock.patch.object(dbapi.IMPL, 'delete_bios_setting', autospec=True)
+    @mock.patch.object(dbapi.IMPL, 'delete_bios_setting_list', autospec=True)
     def test_delete(self, mock_delete):
         objects.BIOSSetting.delete(self.context, self.node_id,
                                    self.bios_setting['name'])
         mock_delete.assert_called_once_with(self.node_id,
-                                            self.bios_setting['name'])
+                                            [self.bios_setting['name']])
+
+    @mock.patch.object(dbapi.IMPL, 'delete_bios_setting_list', autospec=True)
+    def test_list_delete(self, mock_delete):
+        bios_setting2 = db_utils.get_test_bios_setting(name='hyperthread')
+        name_list = [self.bios_setting['name'], bios_setting2['name']]
+        objects.BIOSSettingList.delete(self.context, self.node_id, name_list)
+        mock_delete.assert_called_once_with(self.node_id, name_list)
+
+    @mock.patch('ironic.objects.bios.BIOSSettingList.get_by_node_id',
+                spec_set=types.FunctionType)
+    def test_sync_node_setting_create_and_update(self, mock_get):
+        node = obj_utils.create_test_node(self.ctxt)
+        bios_obj = [obj_utils.create_test_bios_setting(
+            self.ctxt, node_id=node.id)]
+        mock_get.return_value = bios_obj
+        settings = db_utils.get_test_bios_setting_setting_list()
+        settings[0]['value'] = 'off'
+        create, update, delete, nochange = (
+            objects.BIOSSettingList.sync_node_setting(self.ctxt, node.id,
+                                                      settings))
+
+        self.assertEqual(create, settings[1:])
+        self.assertEqual(update, [settings[0]])
+        self.assertEqual(delete, [])
+        self.assertEqual(nochange, [])
+
+    @mock.patch('ironic.objects.bios.BIOSSettingList.get_by_node_id',
+                spec_set=types.FunctionType)
+    def test_sync_node_setting_delete_nochange(self, mock_get):
+        node = obj_utils.create_test_node(self.ctxt)
+        bios_obj_1 = obj_utils.create_test_bios_setting(
+            self.ctxt, node_id=node.id)
+        bios_obj_2 = obj_utils.create_test_bios_setting(
+            self.ctxt, node_id=node.id, name='numlock', value='off')
+        mock_get.return_value = [bios_obj_1, bios_obj_2]
+        settings = db_utils.get_test_bios_setting_setting_list()
+        settings[0]['name'] = 'fake-bios-option'
+        create, update, delete, nochange = (
+            objects.BIOSSettingList.sync_node_setting(self.ctxt, node.id,
+                                                      settings))
+
+        expected_delete = [{'name': bios_obj_1.name,
+                            'value': bios_obj_1.value}]
+        self.assertEqual(create, settings[:2])
+        self.assertEqual(update, [])
+        self.assertEqual(delete, expected_delete)
+        self.assertEqual(nochange, [settings[2]])