From bf518cd5407296e32e26686fc6e99be8f71e5731 Mon Sep 17 00:00:00 2001
From: Jim Rollenhagen <jim@jimrollenhagen.com>
Date: Mon, 23 Nov 2015 22:40:19 +0000
Subject: [PATCH] Fix bug where clean steps do not run

A bug was introduced during Liberty where Ironic transparently
ignores all clean steps and finishes cleaning. This is caused
by _get_node_next_clean_steps returning an empty list when
cleaning has just started. Fix this method to return the full
list of clean steps when cleaning begins.

This may leave previous tenants' data on disk and available to future
tenants. Deployers should apply this patch (or upgrade to a new release
with this patch) ASAP.

Depends-On: Id15cf6cc49122b08e557e44871b31a8c0d20b55d

Change-Id: If815f81d7e668244f0d434d4e2933e8f41946107
Closes-Bug: #1517277
---
 ironic/conductor/manager.py                        |  7 ++++---
 ironic/tests/unit/conductor/test_manager.py        | 14 ++++++++++++++
 ...x-clean-steps-not-running-0d065cb022bc0419.yaml | 11 +++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/fix-clean-steps-not-running-0d065cb022bc0419.yaml

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 35e5b1e03..3f75ac5ac 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -835,10 +835,11 @@ class ConductorManager(periodic_task.PeriodicTasks):
 
         """
         node = task.node
-        if not node.clean_step:
-            return []
-
         next_steps = node.driver_internal_info.get('clean_steps', [])
+        if not node.clean_step:
+            # first time through, return all steps
+            return next_steps
+
         try:
             # Trim off the last clean step (now finished) and
             # all previous steps
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 4e745b8eb..6fe754210 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -2064,6 +2064,20 @@ class DoNodeCleanTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase):
             steps = self.service._get_node_next_clean_steps(task)
             self.assertEqual(self.next_clean_steps, steps)
 
+    def test__get_node_next_clean_steps_unset_clean_step(self):
+        driver_internal_info = {'clean_steps': self.clean_steps}
+        node = obj_utils.create_test_node(
+            self.context, driver='fake',
+            provision_state=states.CLEANWAIT,
+            target_provision_state=states.AVAILABLE,
+            driver_internal_info=driver_internal_info,
+            last_error=None,
+            clean_step=None)
+
+        with task_manager.acquire(self.context, node.uuid) as task:
+            steps = self.service._get_node_next_clean_steps(task)
+            self.assertEqual(self.clean_steps, steps)
+
     def test__get_node_next_clean_steps_bad_clean_step(self):
         driver_internal_info = {'clean_steps': self.clean_steps}
         node = obj_utils.create_test_node(
diff --git a/releasenotes/notes/fix-clean-steps-not-running-0d065cb022bc0419.yaml b/releasenotes/notes/fix-clean-steps-not-running-0d065cb022bc0419.yaml
new file mode 100644
index 000000000..2ef966a18
--- /dev/null
+++ b/releasenotes/notes/fix-clean-steps-not-running-0d065cb022bc0419.yaml
@@ -0,0 +1,11 @@
+---
+prelude: >
+    A major bug was fixed where clean steps do not run.
+critical:
+  - This fixes a bug where Ironic skipped all clean steps,
+    which may leave the previous tenant's data on disk
+    available to new users.
+security:
+  - This fixes a bug where Ironic skipped all clean steps,
+    which may leave the previous tenant's data on disk
+    available to new users.