From c0502649bae086a87d52fba0bda1af489b08b49d Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Mon, 20 Apr 2020 12:46:33 +0200
Subject: [PATCH] Add raid.apply_configuration deploy step

For compatibility with out-of-band RAID deploy steps, we need to have
one apply_configuration step, not a create/delete pair.

Change-Id: I55bbed96673c9fa247cafdac9a3ade3a6ff3f38d
Story: #2006963
---
 ironic_python_agent/extensions/deploy.py      |  2 +
 ironic_python_agent/hardware.py               | 52 ++++++++++++++-----
 .../tests/unit/extensions/test_deploy.py      | 28 ++++++++++
 3 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/ironic_python_agent/extensions/deploy.py b/ironic_python_agent/extensions/deploy.py
index 2fef7b1dc..31936ad49 100644
--- a/ironic_python_agent/extensions/deploy.py
+++ b/ironic_python_agent/extensions/deploy.py
@@ -70,6 +70,8 @@ class DeployExtension(base.BaseAgentExtension):
             msg = 'Malformed deploy_step, no "step" key: %s' % step
             LOG.error(msg)
             raise ValueError(msg)
+
+        kwargs.update(step.get('args') or {})
         try:
             result = hardware.dispatch_to_managers(step['step'], node, ports,
                                                    **kwargs)
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 7ae4c84a9..f58948e38 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -55,6 +55,21 @@ NODE = None
 
 SUPPORTED_SOFTWARE_RAID_LEVELS = frozenset(['0', '1', '1+0', '5', '6'])
 
+RAID_APPLY_CONFIGURATION_ARGSINFO = {
+    "raid_config": {
+        "description": "The RAID configuration to apply.",
+        "required": True,
+    },
+    "delete_existing": {
+        "description": (
+            "Setting this to 'True' indicates to delete existing RAID "
+            "configuration prior to creating the new configuration. "
+            "Default value is 'True'."
+        ),
+        "required": False,
+    }
+}
+
 
 def _get_device_info(dev, devclass, field):
     """Get the device info according to device class and field."""
@@ -738,6 +753,7 @@ class HardwareManager(object, metaclass=abc.ABCMeta):
            'reboot_requested': Whether the agent should request Ironic reboots
                                the node via the power driver after the
                                operation completes.
+           'argsinfo': arguments specification.
           }
 
 
@@ -1537,19 +1553,29 @@ class GenericHardwareManager(HardwareManager):
     def get_deploy_steps(self, node, ports):
         return [
             {
-                'step': 'delete_configuration',
-                'priority': 0,
-                'interface': 'raid',
-                'reboot_requested': False,
-            },
-            {
-                'step': 'create_configuration',
+                'step': 'apply_configuration',
                 'priority': 0,
                 'interface': 'raid',
                 'reboot_requested': False,
+                'argsinfo': RAID_APPLY_CONFIGURATION_ARGSINFO,
             },
         ]
 
+    def apply_configuration(self, node, ports, raid_config,
+                            delete_existing=True):
+        """Apply RAID configuration.
+
+        :param node: A dictionary of the node object.
+        :param ports: A list of dictionaries containing information
+                      of ports for the node.
+        :param raid_config: The configuration to apply.
+        :param delete_existing: Whether to delete the existing configuration.
+        """
+        self.validate_configuration(raid_config, node)
+        if delete_existing:
+            self.delete_configuration(node, ports)
+        self._do_create_configuration(node, ports, raid_config)
+
     def create_configuration(self, node, ports):
         """Create a RAID configuration.
 
@@ -1565,18 +1591,20 @@ class GenericHardwareManager(HardwareManager):
                  valid or if there was an error when creating the RAID
                  devices.
         """
+        raid_config = node.get('target_raid_config', {})
+        if not raid_config:
+            LOG.debug("No target_raid_config found")
+            return {}
 
+        return self._do_create_configuration(node, ports, raid_config)
+
+    def _do_create_configuration(self, node, ports, raid_config):
         # incr starts to 1
         # It means md0 is on the partition 1, md1 on 2...
         # incr could be incremented if we ever decide, for example to create
         # some additional partitions here (boot partitions)
         incr = 1
 
-        raid_config = node.get('target_raid_config', {})
-        if not raid_config:
-            LOG.debug("No target_raid_config found")
-            return {}
-
         # No 'software' controller: do nothing. If 'controller' is
         # set to 'software' on only one of the drives, the validation
         # code will catch it.
diff --git a/ironic_python_agent/tests/unit/extensions/test_deploy.py b/ironic_python_agent/tests/unit/extensions/test_deploy.py
index a282ecf95..32cd0f997 100644
--- a/ironic_python_agent/tests/unit/extensions/test_deploy.py
+++ b/ironic_python_agent/tests/unit/extensions/test_deploy.py
@@ -163,6 +163,34 @@ class TestDeployExtension(base.IronicAgentTest):
         self.assertEqual(expected_result, async_result.command_result)
         mock_cache_node.assert_called_once_with(self.node)
 
+    @mock.patch('ironic_python_agent.hardware.dispatch_to_managers',
+                autospec=True)
+    @mock.patch('ironic_python_agent.hardware.check_versions',
+                autospec=True)
+    def test_execute_deploy_step_with_args(self, mock_version, mock_dispatch,
+                                           mock_cache_node):
+        result = 'deployed'
+        mock_dispatch.return_value = result
+
+        step = self.step['GenericHardwareManager'][0]
+        step['args'] = {'foo': 'bar'}
+        expected_result = {
+            'deploy_step': step,
+            'deploy_result': result
+        }
+        async_result = self.agent_extension.execute_deploy_step(
+            step=self.step['GenericHardwareManager'][0],
+            node=self.node, ports=self.ports,
+            deploy_version=self.version)
+        async_result.join()
+
+        mock_version.assert_called_once_with(self.version)
+        mock_dispatch.assert_called_once_with(
+            self.step['GenericHardwareManager'][0]['step'],
+            self.node, self.ports, foo='bar')
+        self.assertEqual(expected_result, async_result.command_result)
+        mock_cache_node.assert_called_once_with(self.node)
+
     @mock.patch('ironic_python_agent.hardware.dispatch_to_managers',
                 autospec=True)
     @mock.patch('ironic_python_agent.hardware.check_versions',