openstack/ironic
Code Issues Proposed changes

Fix broken configdrive_use_object_store

Browse Source 
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
This commit is contained in:
Dmitry Tantsur 2021-02-16 20:50:58 +01:00
parent 17952a9756
commit 73bdebd127
4 changed files with 20 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
devstack/lib/ironic
View File

@ -1445,6 +1445,12 @@ function configure_ironic {
# Set fast track options # Set fast track options
iniset $IRONIC_CONF_FILE deploy fast_track $IRONIC_DEPLOY_FAST_TRACK 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 # No need to check if RabbitMQ is enabled, this call does it in a smart way
if [[ "$IRONIC_RPC_TRANSPORT" == "oslo" ]]; then if [[ "$IRONIC_RPC_TRANSPORT" == "oslo" ]]; then
iniset_rpc_backend ironic $IRONIC_CONF_FILE iniset_rpc_backend ironic $IRONIC_CONF_FILE

3
ironic/conductor/deployments.py
View File

@ -415,7 +415,8 @@ def _store_configdrive(node, configdrive):
object_headers = {'X-Delete-After': str(timeout)} 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.write(configdrive)
fileobj.flush() fileobj.flush()

14
ironic/tests/unit/conductor/test_deployments.py
View File

@ -248,7 +248,7 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertRaises(exception.SwiftOperationError, self.assertRaises(exception.SwiftOperationError,
deployments.do_node_deploy, task, deployments.do_node_deploy, task,
self.service.conductor.id, self.service.conductor.id,
configdrive=b'fake config drive') configdrive='fake config drive')
node.refresh() node.refresh()
self.assertEqual(states.DEPLOYFAIL, node.provision_state) self.assertEqual(states.DEPLOYFAIL, node.provision_state)
self.assertEqual(states.ACTIVE, node.target_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, self.assertRaises(db_exception.DBDataError,
deployments.do_node_deploy, task, deployments.do_node_deploy, task,
self.service.conductor.id, self.service.conductor.id,
configdrive=b'fake config drive') configdrive='fake config drive')
expected_instance_info.update(configdrive=b'fake config drive') expected_instance_info.update(configdrive='fake config drive')
expected_calls = [ expected_calls = [
mock.call(node.uuid, mock.call(node.uuid,
{'version': mock.ANY, {'version': mock.ANY,
@ -310,7 +310,7 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertRaises(RuntimeError, self.assertRaises(RuntimeError,
deployments.do_node_deploy, task, deployments.do_node_deploy, task,
self.service.conductor.id, self.service.conductor.id,
configdrive=b'fake config drive') configdrive='fake config drive')
node.refresh() node.refresh()
self.assertEqual(states.DEPLOYFAIL, node.provision_state) self.assertEqual(states.DEPLOYFAIL, node.provision_state)
self.assertEqual(states.ACTIVE, node.target_provision_state) self.assertEqual(states.ACTIVE, node.target_provision_state)
@ -964,7 +964,7 @@ class StoreConfigDriveTestCase(db_base.DbTestCase):
group='conductor') group='conductor')
mock_swift.return_value.get_temp_url.return_value = 'http://1.2.3.4' 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.assert_called_once_with()
mock_swift.return_value.create_object.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') group='conductor')
mock_swift.return_value.get_temp_url.return_value = 'http://1.2.3.4' 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.assert_called_once_with()
mock_swift.return_value.create_object.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') group='conductor')
mock_swift.return_value.get_temp_url.return_value = 'http://1.2.3.4' 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.assert_called_once_with()
mock_swift.return_value.create_object.assert_called_once_with( mock_swift.return_value.create_object.assert_called_once_with(

5
releasenotes/notes/configdrive_use_object_store-93cfd7dc27d90003.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- |
Fixes the ``[deploy]configdrive_use_object_store`` option that was
broken during the Python 3 transition.