From 0da73cdd30f6726548992090bb3626292e3518b0 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur 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. Conflicts: devstack/lib/ironic Change-Id: Ibbf90dcbcb36a5f7cf084a44a221c0c5c003b95a (cherry picked from commit 73bdebd127b3897860c7bcff3848032a2737d1ba) --- devstack/lib/ironic | 7 +++++++ ironic/conductor/deployments.py | 3 ++- ironic/tests/unit/conductor/test_deployments.py | 14 +++++++------- ...figdrive_use_object_store-93cfd7dc27d90003.yaml | 5 +++++ 4 files changed, 21 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 cc1fd2808f..9a1a5feb9d 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1424,6 +1424,13 @@ function configure_ironic { # Set requirement for agent tokens iniset $IRONIC_CONF_FILE DEFAULT require_agent_token $IRONIC_REQUIRE_AGENT_TOKEN + + # 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 22eacfe67e..33d713f4f6 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -348,7 +348,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 cef95f1882..ee11a02c47 100644 --- a/ironic/tests/unit/conductor/test_deployments.py +++ b/ironic/tests/unit/conductor/test_deployments.py @@ -242,7 +242,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) @@ -265,8 +265,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, @@ -301,7 +301,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) @@ -834,7 +834,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( @@ -862,7 +862,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( @@ -889,7 +889,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.