From 0967238ad47cabccd1ecc2a4ef341120f410302f Mon Sep 17 00:00:00 2001
From: Pavlo Shchelokovskyy <pshchelokovskyy@mirantis.com>
Date: Sat, 12 Sep 2015 15:50:58 +0300
Subject: [PATCH] Fix string formatting issues

if keyworded substitution is used, the formatter must be a dict
(even for single element).

Add a missing unit test for a code branch where a formatting
error was found.

Also, fix a tiny error message typo (missing space).

Change-Id: Ib1649511b2613bc1a8ba52398543be0fdfefb8db
Closes-Bug: #1494574
---
 ironic/dhcp/neutron.py                      |  2 +-
 ironic/drivers/modules/agent_base_vendor.py |  2 +-
 ironic/tests/dhcp/test_neutron.py           | 35 +++++++++++++++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/ironic/dhcp/neutron.py b/ironic/dhcp/neutron.py
index 16fe5813bc..9dddd3bf29 100644
--- a/ironic/dhcp/neutron.py
+++ b/ironic/dhcp/neutron.py
@@ -320,7 +320,7 @@ class NeutronDHCPApi(base.BaseDHCP):
             if not port.get('port') or not port['port'].get('id'):
                 self._rollback_cleaning_ports(task)
                 msg = (_('Failed to create cleaning ports for node '
-                         '%(node)s') % task.node.uuid)
+                         '%(node)s') % {'node': task.node.uuid})
                 LOG.error(msg)
                 raise exception.NodeCleaningFailure(msg)
             # Match return value of get_node_vif_ids()
diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py
index 6af61b8505..ed0f0e0d0a 100644
--- a/ironic/drivers/modules/agent_base_vendor.py
+++ b/ironic/drivers/modules/agent_base_vendor.py
@@ -510,7 +510,7 @@ class BaseAgentVendor(base.VendorInterface):
         ports = self._find_ports_by_macs(context, mac_addresses)
         if not ports:
             raise exception.NodeNotFound(_(
-                'No ports matching the given MAC addresses %sexist in the '
+                'No ports matching the given MAC addresses %s exist in the '
                 'database.') % mac_addresses)
         node_id = self._get_node_id(ports)
         try:
diff --git a/ironic/tests/dhcp/test_neutron.py b/ironic/tests/dhcp/test_neutron.py
index ccea711e2f..b30a433605 100644
--- a/ironic/tests/dhcp/test_neutron.py
+++ b/ironic/tests/dhcp/test_neutron.py
@@ -393,6 +393,41 @@ class TestNeutron(db_base.DbTestCase):
                 'admin_state_up': True, 'mac_address': self.ports[0].address}})
             rollback_mock.assert_called_once_with(task)
 
+    @mock.patch.object(neutron.NeutronDHCPApi, '_rollback_cleaning_ports')
+    @mock.patch.object(client.Client, 'create_port')
+    def test_create_cleaning_ports_fail_delayed(self, create_mock,
+                                                rollback_mock):
+        """Check ports are cleaned up on failure to create them
+
+        This test checks that the port clean-up occurs
+        when the port create call was successful,
+        but the port in fact was not created.
+
+        """
+        # NOTE(pas-ha) this is trying to emulate the complex port object
+        # with both methods and dictionary access with methods on elements
+        mockport = mock.MagicMock()
+        create_mock.return_value = mockport
+        # fail only on second 'or' branch to fool lazy eval
+        # and actually execute both expressions to assert on both mocks
+        mockport.get.return_value = True
+        mockitem = mock.Mock()
+        mockport.__getitem__.return_value = mockitem
+        mockitem.get.return_value = None
+        api = dhcp_factory.DHCPFactory().provider
+
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            self.assertRaises(exception.NodeCleaningFailure,
+                              api.create_cleaning_ports,
+                              task)
+            create_mock.assert_called_once_with({'port': {
+                'network_id': '00000000-0000-0000-0000-000000000000',
+                'admin_state_up': True, 'mac_address': self.ports[0].address}})
+            rollback_mock.assert_called_once_with(task)
+            mockport.get.assert_called_once_with('port')
+            mockitem.get.assert_called_once_with('id')
+            mockport.__getitem__.assert_called_once_with('port')
+
     @mock.patch.object(client.Client, 'create_port')
     def test_create_cleaning_ports_bad_config(self, create_mock):
         # Check an error is raised if the cleaning network is not set