Merge "Add functionality for individual cleaning on nodes"

ironic/common/release_mappings.py

@@ -134,7 +134,7 @@ RELEASE_MAPPING = {
         'api': '1.46',
         'rpc': '1.47',
         'objects': {
-            'Node': ['1.27'],
+            'Node': ['1.28'],
             'Conductor': ['1.3'],
             'Chassis': ['1.3'],
             'Port': ['1.8'],

ironic/conductor/manager.py

@@ -1200,7 +1200,7 @@ class ConductorManager(base_manager.BaseConductorManager):
         LOG.debug('Starting %(type)s cleaning for node %(node)s',
                   {'type': clean_type, 'node': node.uuid})
 
-        if not manual_clean and not CONF.conductor.automated_clean:
+        if not manual_clean and utils.skip_automated_cleaning(node):
             # Skip cleaning, move to AVAILABLE.
             node.clean_step = None
             node.save()

ironic/conductor/utils.py

@@ -990,3 +990,11 @@ def notify_conductor_resume_clean(task):
 def notify_conductor_resume_deploy(task):
     _notify_conductor_resume_operation(task, 'deploying',
                                        'continue_node_deploy')
+
+
+def skip_automated_cleaning(node):
+    """Checks if node cleaning needs to be skipped for an specific node.
+
+    :param node: the node to consider
+    """
+    return not CONF.conductor.automated_clean and not node.automated_clean

ironic/drivers/modules/oneview/deploy.py

@@ -22,6 +22,7 @@ import six
 
 from ironic.common import exception
 from ironic.common import states
+from ironic.conductor import utils
 from ironic.conf import CONF
 from ironic.drivers.modules import agent
 from ironic.drivers.modules import iscsi_deploy
@@ -243,7 +244,10 @@ class OneViewIscsiDeploy(iscsi_deploy.ISCSIDeploy, OneViewPeriodicTasks):
 
     @METRICS.timer('OneViewIscsiDeploy.tear_down')
     def tear_down(self, task):
-        if not CONF.conductor.automated_clean:
+        # teardown if automated clean is disabled on the node
+        # or if general automated clean is not enabled generally
+        # and not on the node as well
+        if utils.skip_automated_cleaning(task.node):
             deploy_utils.tear_down(task)
         return super(OneViewIscsiDeploy, self).tear_down(task)
 
@@ -287,7 +291,9 @@ class OneViewAgentDeploy(agent.AgentDeploy, OneViewPeriodicTasks):
 
     @METRICS.timer('OneViewAgentDeploy.tear_down')
     def tear_down(self, task):
-        if not CONF.conductor.automated_clean:
+        # if node specifically has cleanup disabled, or general cleanup
+        # is disabled and node has not it enabled
+        if utils.skip_automated_cleaning(task.node):
             deploy_utils.tear_down(task)
         return super(OneViewAgentDeploy, self).tear_down(task)
 

ironic/objects/node.py

@@ -64,7 +64,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
     # Version 1.25: Add fault field
     # Version 1.26: Add deploy_step field
     # Version 1.27: Add conductor_group field
-    VERSION = '1.27'
+    # Version 1.28: Add automated_clean field
+    VERSION = '1.28'
 
     dbapi = db_api.get_instance()
 
@@ -130,7 +131,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
         'inspection_started_at': object_fields.DateTimeField(nullable=True),
 
         'extra': object_fields.FlexibleDictField(nullable=True),
-
+        'automated_clean': objects.fields.BooleanField(nullable=True),
         'bios_interface': object_fields.StringField(nullable=True),
         'boot_interface': object_fields.StringField(nullable=True),
         'console_interface': object_fields.StringField(nullable=True),
@@ -529,6 +530,26 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
             elif self.conductor_group:
                 self.conductor_group = ''
 
+    # NOTE (yolanda): new method created to avoid repeating code in
+    # _convert_to_version, and to avoid pep8 too complex error
+    def _adjust_field_to_version(self, field_name, field_default_value,
+                                 target_version, major, minor,
+                                 remove_unavailable_fields):
+        field_is_set = self.obj_attr_is_set(field_name)
+        if target_version >= (major, minor):
+            # target version supports the major/minor specified
+            if not field_is_set:
+                # set it to its default value if it is not set
+                setattr(self, field_name, field_default_value)
+        elif field_is_set:
+            # target version does not support the field, and it is set
+            if remove_unavailable_fields:
+                # (De)serialising: remove unavailable fields
+                delattr(self, field_name)
+            elif getattr(self, field_name) is not field_default_value:
+                # DB: set unavailable field to the default value
+                setattr(self, field_name, field_default_value)
+
     def _convert_to_version(self, target_version,
                             remove_unavailable_fields=True):
         """Convert to the target version.
@@ -552,6 +573,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
             this, it should be removed.
         Version 1.27: conductor_group field was added. For versions prior to
             this, it should be removed.
+        Version 1.28: automated_clean was added. For versions prior to this, it
+            should be set to None (or removed).
 
         :param target_version: the desired version of the object
         :param remove_unavailable_fields: True to remove fields that are
@@ -560,47 +583,13 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
             values; set this to False for DB interactions.
         """
         target_version = versionutils.convert_version_to_tuple(target_version)
-        # Convert the rescue_interface field.
-        rescue_iface_is_set = self.obj_attr_is_set('rescue_interface')
-        if target_version >= (1, 22):
-            # Target version supports rescue_interface.
-            if not rescue_iface_is_set:
-                # Set it to its default value if it is not set.
-                self.rescue_interface = None
-        elif rescue_iface_is_set:
-            # Target version does not support rescue_interface, and it is set.
-            if remove_unavailable_fields:
-                # (De)serialising: remove unavailable fields.
-                delattr(self, 'rescue_interface')
-            elif self.rescue_interface is not None:
-                # DB: set unavailable field to the default of None.
-                self.rescue_interface = None
 
-        traits_is_set = self.obj_attr_is_set('traits')
+        # Convert the different fields depending on version
-        if target_version >= (1, 23):
+        fields = [('rescue_interface', 22), ('traits', 23),
-            # Target version supports traits.
+                  ('bios_interface', 24), ('automated_clean', 28)]
-            if not traits_is_set:
+        for name, minor in fields:
-                self.traits = None
+            self._adjust_field_to_version(name, None, target_version,
-        elif traits_is_set:
+                                          1, minor, remove_unavailable_fields)
-            if remove_unavailable_fields:
-                delattr(self, 'traits')
-            elif self.traits is not None:
-                self.traits = None
-
-        bios_iface_is_set = self.obj_attr_is_set('bios_interface')
-        if target_version >= (1, 24):
-            # Target version supports bios_interface.
-            if not bios_iface_is_set:
-                # Set it to its default value if it is not set.
-                self.bios_interface = None
-        elif bios_iface_is_set:
-            # Target version does not support bios_interface, and it is set.
-            if remove_unavailable_fields:
-                # (De)serialising: remove unavailable fields.
-                delattr(self, 'bios_interface')
-            elif self.bios_interface is not None:
-                # DB: set unavailable field to the default of None.
-                self.bios_interface = None
 
         self._convert_fault_field(target_version, remove_unavailable_fields)
         self._convert_deploy_step_field(target_version,

ironic/tests/unit/api/utils.py

@@ -112,6 +112,8 @@ def node_post_data(**kw):
         node.pop('resource_class')
     if 'fault' not in kw:
         node.pop('fault')
+    if 'automated_clean' not in kw:
+        node.pop('automated_clean')
 
     internal = node_controller.NodePatchType.internal_attrs()
     return remove_internal(node, internal)

ironic/tests/unit/conductor/test_manager.py

@@ -3304,6 +3304,128 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertNotIn('clean_steps', node.driver_internal_info)
         self.assertNotIn('clean_step_index', node.driver_internal_info)
 
+    @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
+                autospec=True)
+    @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
+                autospec=True)
+    def test__do_node_clean_automated_disabled_individual_enabled(
+            self, mock_network, mock_validate):
+        self.config(automated_clean=False, group='conductor')
+
+        self._start_service()
+        node = obj_utils.create_test_node(
+            self.context, driver='fake-hardware',
+            provision_state=states.CLEANING,
+            target_provision_state=states.AVAILABLE,
+            last_error=None, automated_clean=True)
+        with task_manager.acquire(
+                self.context, node.uuid, shared=False) as task:
+            self.service._do_node_clean(task)
+        self._stop_service()
+        node.refresh()
+
+        # Assert that the node clean was called
+        self.assertTrue(mock_validate.called)
+        self.assertIn('clean_steps', node.driver_internal_info)
+
+    @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
+                autospec=True)
+    def test__do_node_clean_automated_disabled_individual_disabled(
+            self, mock_validate):
+        self.config(automated_clean=False, group='conductor')
+
+        self._start_service()
+        node = obj_utils.create_test_node(
+            self.context, driver='fake-hardware',
+            provision_state=states.CLEANING,
+            target_provision_state=states.AVAILABLE,
+            last_error=None, automated_clean=False)
+        with task_manager.acquire(
+                self.context, node.uuid, shared=False) as task:
+            self.service._do_node_clean(task)
+        self._stop_service()
+        node.refresh()
+
+        # Assert that the node was moved to available without cleaning
+        self.assertFalse(mock_validate.called)
+        self.assertEqual(states.AVAILABLE, node.provision_state)
+        self.assertEqual(states.NOSTATE, node.target_provision_state)
+        self.assertEqual({}, node.clean_step)
+        self.assertNotIn('clean_steps', node.driver_internal_info)
+        self.assertNotIn('clean_step_index', node.driver_internal_info)
+
+    @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
+                autospec=True)
+    @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
+                autospec=True)
+    def test__do_node_clean_automated_enabled(self, mock_validate,
+                                              mock_network):
+        self.config(automated_clean=True, group='conductor')
+
+        self._start_service()
+        node = obj_utils.create_test_node(
+            self.context, driver='fake-hardware',
+            provision_state=states.CLEANING,
+            target_provision_state=states.AVAILABLE,
+            last_error=None)
+        with task_manager.acquire(
+                self.context, node.uuid, shared=False) as task:
+            self.service._do_node_clean(task)
+        self._stop_service()
+        node.refresh()
+
+        # Assert that the node was cleaned
+        self.assertTrue(mock_validate.called)
+        self.assertIn('clean_steps', node.driver_internal_info)
+
+    @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
+                autospec=True)
+    @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
+                autospec=True)
+    def test__do_node_clean_automated_enabled_individual_enabled(
+            self, mock_network, mock_validate):
+        self.config(automated_clean=True, group='conductor')
+
+        self._start_service()
+        node = obj_utils.create_test_node(
+            self.context, driver='fake-hardware',
+            provision_state=states.CLEANING,
+            target_provision_state=states.AVAILABLE,
+            last_error=None, automated_clean=True)
+        with task_manager.acquire(
+                self.context, node.uuid, shared=False) as task:
+            self.service._do_node_clean(task)
+        self._stop_service()
+        node.refresh()
+
+        # Assert that the node was cleaned
+        self.assertTrue(mock_validate.called)
+        self.assertIn('clean_steps', node.driver_internal_info)
+
+    @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
+                autospec=True)
+    @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
+                autospec=True)
+    def test__do_node_clean_automated_enabled_individual_none(
+            self, mock_validate, mock_network):
+        self.config(automated_clean=True, group='conductor')
+
+        self._start_service()
+        node = obj_utils.create_test_node(
+            self.context, driver='fake-hardware',
+            provision_state=states.CLEANING,
+            target_provision_state=states.AVAILABLE,
+            last_error=None, automated_clean=None)
+        with task_manager.acquire(
+                self.context, node.uuid, shared=False) as task:
+            self.service._do_node_clean(task)
+        self._stop_service()
+        node.refresh()
+
+        # Assert that the node was cleaned
+        self.assertTrue(mock_validate.called)
+        self.assertIn('clean_steps', node.driver_internal_info)
+
     @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate',
                 autospec=True)
     @mock.patch('ironic.drivers.modules.fake.FakeDeploy.prepare_cleaning',

ironic/tests/unit/db/utils.py

@@ -214,6 +214,7 @@ def get_test_node(**kw):
         'tags': kw.get('tags', []),
         'resource_class': kw.get('resource_class'),
         'traits': kw.get('traits', []),
+        'automated_clean': kw.get('automated_clean', None),
     }
 
     for iface in drivers_base.ALL_INTERFACES:

ironic/tests/unit/objects/test_node.py

@@ -706,6 +706,69 @@ class TestConvertToVersion(db_base.DbTestCase):
         self.assertEqual('', node.conductor_group)
         self.assertEqual({'conductor_group': ''}, node.obj_get_changes())
 
+    def test_automated_clean_supported_missing(self):
+        # automated_clean_interface not set, should be set to default.
+        node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
+        delattr(node, 'automated_clean')
+        node.obj_reset_changes()
+
+        node._convert_to_version("1.28")
+
+        self.assertIsNone(node.automated_clean)
+        self.assertEqual({'automated_clean': None},
+                         node.obj_get_changes())
+
+    def test_automated_clean_supported_set(self):
+        # automated_clean set, no change required.
+        node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
+
+        node.automated_clean = True
+        node.obj_reset_changes()
+        node._convert_to_version("1.28")
+        self.assertEqual(True, node.automated_clean)
+        self.assertEqual({}, node.obj_get_changes())
+
+    def test_automated_clean_unsupported_missing(self):
+        # automated_clean not set, no change required.
+        node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
+
+        delattr(node, 'automated_clean')
+        node.obj_reset_changes()
+        node._convert_to_version("1.27")
+        self.assertNotIn('automated_clean', node)
+        self.assertEqual({}, node.obj_get_changes())
+
+    def test_automated_clean_unsupported_set_remove(self):
+        # automated_clean set, should be removed.
+        node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
+
+        node.automated_clean = True
+        node.obj_reset_changes()
+        node._convert_to_version("1.27")
+        self.assertNotIn('automated_clean', node)
+        self.assertEqual({}, node.obj_get_changes())
+
+    def test_automated_clean_unsupported_set_no_remove_non_default(self):
+        # automated_clean set, should be set to default.
+        node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
+
+        node.automated_clean = True
+        node.obj_reset_changes()
+        node._convert_to_version("1.27", False)
+        self.assertIsNone(node.automated_clean)
+        self.assertEqual({'automated_clean': None},
+                         node.obj_get_changes())
+
+    def test_automated_clean_unsupported_set_no_remove_default(self):
+        # automated_clean set, no change required.
+        node = obj_utils.get_test_node(self.ctxt, **self.fake_node)
+
+        node.automated_clean = None
+        node.obj_reset_changes()
+        node._convert_to_version("1.27", False)
+        self.assertIsNone(node.automated_clean)
+        self.assertEqual({}, node.obj_get_changes())
+
 
 class TestNodePayloads(db_base.DbTestCase):
 

ironic/tests/unit/objects/test_objects.py

@@ -677,7 +677,7 @@ class TestObject(_LocalTest, _TestObject):
 # version bump. It is an MD5 hash of the object fields and remotable methods.
 # The fingerprint values should only be changed if there is a version bump.
 expected_object_fingerprints = {
-    'Node': '1.27-129323d486c03a99de27053503b2cae3',
+    'Node': '1.28-d4aba1f583774326903f7366fbaae752',
     'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6',
     'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905',
     'Port': '1.8-898a47921f4a1f53fcdddd4eeb179e0b',