From 73bdebd127b3897860c7bcff3848032a2737d1ba Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Tue, 16 Feb 2021 20:50:58 +0100
Subject: [PATCH] Fix broken configdrive_use_object_store

When it is set to True, we try to write text data to a binary file,
which is not possible in Python 3. The issue has been "helpfully"
hidden by the fact that we use bytes in unit tests, as well as
by lack of CI coverage.

Change-Id: Ibbf90dcbcb36a5f7cf084a44a221c0c5c003b95a
---
 devstack/lib/ironic                                |  6 ++++++
 ironic/conductor/deployments.py                    |  3 ++-
 ironic/tests/unit/conductor/test_deployments.py    | 14 +++++++-------
 ...figdrive_use_object_store-93cfd7dc27d90003.yaml |  5 +++++
 4 files changed, 20 insertions(+), 8 deletions(-)
 create mode 100644 releasenotes/notes/configdrive_use_object_store-93cfd7dc27d90003.yaml

diff --git a/devstack/lib/ironic b/devstack/lib/ironic
index 3eaea3d882..51846ecb28 100644
--- a/devstack/lib/ironic
+++ b/devstack/lib/ironic
@@ -1445,6 +1445,12 @@ function configure_ironic {
     # Set fast track options
     iniset $IRONIC_CONF_FILE deploy fast_track $IRONIC_DEPLOY_FAST_TRACK
 
+    # FIXME(dtantsur): configdrive downloading code does not respect IPA TLS
+    # configuration, not even ipa-insecure.
+    if is_service_enabled swift && ! is_service_enabled tls-proxy; then
+        iniset $IRONIC_CONF_FILE deploy configdrive_use_object_store True
+    fi
+
     # No need to check if RabbitMQ is enabled, this call does it in a smart way
     if [[ "$IRONIC_RPC_TRANSPORT" == "oslo" ]]; then
         iniset_rpc_backend ironic $IRONIC_CONF_FILE
diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py
index a37ecd29ab..6608e5c9ad 100644
--- a/ironic/conductor/deployments.py
+++ b/ironic/conductor/deployments.py
@@ -415,7 +415,8 @@ def _store_configdrive(node, configdrive):
 
         object_headers = {'X-Delete-After': str(timeout)}
 
-        with tempfile.NamedTemporaryFile(dir=CONF.tempdir) as fileobj:
+        with tempfile.NamedTemporaryFile(dir=CONF.tempdir,
+                                         mode="wt") as fileobj:
             fileobj.write(configdrive)
             fileobj.flush()
 
diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py
index d029b7994c..f0d09952b9 100644
--- a/ironic/tests/unit/conductor/test_deployments.py
+++ b/ironic/tests/unit/conductor/test_deployments.py
@@ -248,7 +248,7 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertRaises(exception.SwiftOperationError,
                           deployments.do_node_deploy, task,
                           self.service.conductor.id,
-                          configdrive=b'fake config drive')
+                          configdrive='fake config drive')
         node.refresh()
         self.assertEqual(states.DEPLOYFAIL, node.provision_state)
         self.assertEqual(states.ACTIVE, node.target_provision_state)
@@ -273,8 +273,8 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
             self.assertRaises(db_exception.DBDataError,
                               deployments.do_node_deploy, task,
                               self.service.conductor.id,
-                              configdrive=b'fake config drive')
-            expected_instance_info.update(configdrive=b'fake config drive')
+                              configdrive='fake config drive')
+            expected_instance_info.update(configdrive='fake config drive')
             expected_calls = [
                 mock.call(node.uuid,
                           {'version': mock.ANY,
@@ -310,7 +310,7 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertRaises(RuntimeError,
                           deployments.do_node_deploy, task,
                           self.service.conductor.id,
-                          configdrive=b'fake config drive')
+                          configdrive='fake config drive')
         node.refresh()
         self.assertEqual(states.DEPLOYFAIL, node.provision_state)
         self.assertEqual(states.ACTIVE, node.target_provision_state)
@@ -964,7 +964,7 @@ class StoreConfigDriveTestCase(db_base.DbTestCase):
                           group='conductor')
         mock_swift.return_value.get_temp_url.return_value = 'http://1.2.3.4'
 
-        deployments._store_configdrive(self.node, b'foo')
+        deployments._store_configdrive(self.node, 'foo')
 
         mock_swift.assert_called_once_with()
         mock_swift.return_value.create_object.assert_called_once_with(
@@ -992,7 +992,7 @@ class StoreConfigDriveTestCase(db_base.DbTestCase):
                           group='conductor')
         mock_swift.return_value.get_temp_url.return_value = 'http://1.2.3.4'
 
-        deployments._store_configdrive(self.node, b'foo')
+        deployments._store_configdrive(self.node, 'foo')
 
         mock_swift.assert_called_once_with()
         mock_swift.return_value.create_object.assert_called_once_with(
@@ -1019,7 +1019,7 @@ class StoreConfigDriveTestCase(db_base.DbTestCase):
                           group='conductor')
         mock_swift.return_value.get_temp_url.return_value = 'http://1.2.3.4'
 
-        deployments._store_configdrive(self.node, b'foo')
+        deployments._store_configdrive(self.node, 'foo')
 
         mock_swift.assert_called_once_with()
         mock_swift.return_value.create_object.assert_called_once_with(
diff --git a/releasenotes/notes/configdrive_use_object_store-93cfd7dc27d90003.yaml b/releasenotes/notes/configdrive_use_object_store-93cfd7dc27d90003.yaml
new file mode 100644
index 0000000000..b5779bb881
--- /dev/null
+++ b/releasenotes/notes/configdrive_use_object_store-93cfd7dc27d90003.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes the ``[deploy]configdrive_use_object_store`` option that was
+    broken during the Python 3 transition.