Merge "Allow disabling automated_clean per node"

6 files changed, 72 insertions, 2 deletions

diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py
index 96a5642e0..a436d29be 100644
--- a/ironic/api/controllers/v1/node.py
+++ b/ironic/api/controllers/v1/node.py
@@ -2093,6 +2093,10 @@ class NodesController(rest.RestController):
                 policy_checks.append('baremetal:node:update_instance_info')
             elif p['path'].startswith('/extra'):
                 policy_checks.append('baremetal:node:update_extra')
+            elif (p['path'].startswith('/automated_clean')
+                  and strutils.bool_from_string(p['value'], default=None)
+                  is False):
+                policy_checks.append('baremetal:node:disable_cleaning')
             else:
                 generic_update = True
 
diff --git a/ironic/common/policy.py b/ironic/common/policy.py
index 240797731..811198206 100644
--- a/ironic/common/policy.py
+++ b/ironic/common/policy.py
@@ -256,7 +256,13 @@ node_policies = [
         'rule:is_admin or rule:is_observer',
         'Retrieve Node BIOS information',
         [{'path': '/nodes/{node_ident}/bios', 'method': 'GET'},
-         {'path': '/nodes/{node_ident}/bios/{setting}', 'method': 'GET'}])
+         {'path': '/nodes/{node_ident}/bios/{setting}', 'method': 'GET'}]),
+
+    policy.DocumentedRuleDefault(
+        'baremetal:node:disable_cleaning',
+        'rule:baremetal:node:update',
+        'Disable Node disk cleaning',
+        [{'path': '/nodes/{node_ident}', 'method': 'PATCH'}]),
 ]
 
 port_policies = [
diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py
index 678de11af..081c6ce42 100644
--- a/ironic/conductor/utils.py
+++ b/ironic/conductor/utils.py
@@ -876,7 +876,15 @@ def skip_automated_cleaning(node):
 
     :param node: the node to consider
     """
-    return not CONF.conductor.automated_clean and not node.automated_clean
+    if node.automated_clean:
+        return False
+    elif node.automated_clean is None:
+        return not CONF.conductor.automated_clean
+    else:
+        LOG.info("Automated cleaning is disabled via the API for "
+                 "node %(node)s",
+                 {'node': node.uuid})
+        return True
 
 
 def power_on_node_if_needed(task):
diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py
index 68c0e73ad..14cb0d3c8 100644
--- a/ironic/tests/unit/api/controllers/v1/test_node.py
+++ b/ironic/tests/unit/api/controllers/v1/test_node.py
@@ -3347,6 +3347,27 @@ class TestPatch(test_api_base.BaseApiTest):
         self.assertEqual('application/json', response.content_type)
         self.assertEqual(http_client.OK, response.status_code)
 
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_update_automated_clean_with_false(self, mock_authorize):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:disable_cleaning':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+
+        node = obj_utils.create_test_node(self.context,
+                                          uuid=uuidutils.generate_uuid())
+        self.mock_update_node.return_value = node
+        headers = {api_base.Version.string: '1.47'}
+        response = self.patch_json('/nodes/%s' % node.uuid,
+                                   [{'path': '/automated_clean',
+                                     'value': False,
+                                     'op': 'replace'}],
+                                   headers=headers,
+                                   expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.FORBIDDEN, response.status_code)
+
     def test_update_automated_clean_old_api(self):
         node = obj_utils.create_test_node(self.context,
                                           uuid=uuidutils.generate_uuid())
diff --git a/ironic/tests/unit/conductor/test_cleaning.py b/ironic/tests/unit/conductor/test_cleaning.py
index 0ed561201..2fba81687 100644
--- a/ironic/tests/unit/conductor/test_cleaning.py
+++ b/ironic/tests/unit/conductor/test_cleaning.py
@@ -195,6 +195,30 @@ class DoNodeCleanTestCase(db_base.DbTestCase):
 
     @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
                 autospec=True)
+    def test__do_node_clean_automated_enabled_individual_disabled(
+            self, mock_validate):
+        self.config(automated_clean=True, group='conductor')
+
+        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:
+            cleaning.do_node_clean(task)
+        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)
     def test__do_node_clean_automated_disabled_individual_disabled(
             self, mock_validate):
         self.config(automated_clean=False, group='conductor')
diff --git a/releasenotes/notes/allow_to_disable_automated_clean-a3ccb1e19940a7a4.yaml b/releasenotes/notes/allow_to_disable_automated_clean-a3ccb1e19940a7a4.yaml
new file mode 100644
index 000000000..aa9f937d5
--- /dev/null
+++ b/releasenotes/notes/allow_to_disable_automated_clean-a3ccb1e19940a7a4.yaml
@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Allows disabling automated cleaning per node if it is enabled globally.
+    An existing ``automated_clean`` field will allow disabling of automated
+    cleaning on the node object. A new ``baremetal:node:disable_cleaning``
+    policy is added which defaults to ``baremetal:node:update``.
\ No newline at end of file