From 93a40e3fc7a8187c0444c1aae93a52beb5c68b4b Mon Sep 17 00:00:00 2001
From: Riccardo Pittau <elfosardo@gmail.com>
Date: Thu, 21 Mar 2019 12:42:26 +0100
Subject: [PATCH] Fix capabilities passed as string in agent prepare

The value of capabilities can be either a string or a dict.
There is a function to parse its value and provide a correct
output as expected and it should always be used when reading
node capabilities.

Change-Id: I789fffd5f67eddbe5058581f13d75320ca712897
Story: 2005280
Task: 30133
---
 ironic/drivers/modules/agent.py               |  2 +-
 .../tests/unit/drivers/modules/test_agent.py  | 50 +++++++++++++++++++
 ...ties-as-string-agent-7c5c7975560ce280.yaml |  5 ++
 3 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/fix-capabilities-as-string-agent-7c5c7975560ce280.yaml

diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py
index 1d3d552e7d..b4f640e30f 100644
--- a/ironic/drivers/modules/agent.py
+++ b/ironic/drivers/modules/agent.py
@@ -550,7 +550,7 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
                 # option is local.
                 with excutils.save_and_reraise_exception(reraise=False) as ctx:
                     instance_info = node.instance_info
-                    capabilities = instance_info.get('capabilities', {})
+                    capabilities = utils.parse_instance_info_capabilities(node)
                     if 'boot_option' not in capabilities:
                         capabilities['boot_option'] = 'local'
                         instance_info['capabilities'] = capabilities
diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py
index 3f2353045e..e804f3d176 100644
--- a/ironic/tests/unit/drivers/modules/test_agent.py
+++ b/ironic/tests/unit/drivers/modules/test_agent.py
@@ -487,6 +487,56 @@ class TestAgentDeploy(db_base.DbTestCase):
         self.node.refresh()
         self.assertEqual('bar', self.node.instance_info['foo'])
 
+    @mock.patch.object(noop_storage.NoopStorage, 'attach_volumes',
+                       autospec=True)
+    @mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info')
+    @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk')
+    @mock.patch.object(deploy_utils, 'build_agent_options')
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    @mock.patch.object(neutron_network.NeutronNetwork,
+                       'add_provisioning_network',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(neutron_network.NeutronNetwork,
+                       'unconfigure_tenant_networks',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(neutron_network.NeutronNetwork, 'validate',
+                       spec_set=True, autospec=True)
+    def test_prepare_with_neutron_net_capabilities_as_string(
+            self, validate_net_mock,
+            unconfigure_tenant_net_mock, add_provisioning_net_mock,
+            validate_href_mock, build_options_mock,
+            pxe_prepare_ramdisk_mock, storage_driver_info_mock,
+            storage_attach_volumes_mock):
+        node = self.node
+        node.network_interface = 'neutron'
+        instance_info = node.instance_info
+        instance_info['capabilities'] = '{"lion": "roar"}'
+        node.instance_info = instance_info
+        node.save()
+        validate_net_mock.side_effect = [
+            exception.InvalidParameterValue('invalid'), None]
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=False) as task:
+            task.node.provision_state = states.DEPLOYING
+            build_options_mock.return_value = {'a': 'b'}
+            self.driver.prepare(task)
+            storage_driver_info_mock.assert_called_once_with(task)
+            self.assertEqual(2, validate_net_mock.call_count)
+            add_provisioning_net_mock.assert_called_once_with(mock.ANY, task)
+            unconfigure_tenant_net_mock.assert_called_once_with(mock.ANY, task)
+            storage_attach_volumes_mock.assert_called_once_with(
+                task.driver.storage, task)
+            validate_href_mock.assert_called_once_with(mock.ANY, 'fake-image',
+                                                       secret=False)
+            build_options_mock.assert_called_once_with(task.node)
+            pxe_prepare_ramdisk_mock.assert_called_once_with(
+                task, {'a': 'b'})
+        self.node.refresh()
+        capabilities = self.node.instance_info['capabilities']
+        self.assertEqual('local', capabilities['boot_option'])
+        self.assertEqual('roar', capabilities['lion'])
+
     @mock.patch.object(noop_storage.NoopStorage, 'attach_volumes',
                        autospec=True)
     @mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info')
diff --git a/releasenotes/notes/fix-capabilities-as-string-agent-7c5c7975560ce280.yaml b/releasenotes/notes/fix-capabilities-as-string-agent-7c5c7975560ce280.yaml
new file mode 100644
index 0000000000..f38b71e08d
--- /dev/null
+++ b/releasenotes/notes/fix-capabilities-as-string-agent-7c5c7975560ce280.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes an issue where deploy fails during node preparation if the
+    node ``capabilities`` are passed as string.
\ No newline at end of file