From 46ce095a9ca2be10585aeed3b6790f239b8f5dff Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 1 Apr 2019 16:49:21 +0200 Subject: Do not try to create temporary URLs with zero lifetime A new option is introduced for that, which defaults to the deploy timeout if it is set and to 1800 seconds if it is not. Change-Id: I10e02919e40d25bd4411f2b6f98f9317d1cfb187 Story: #1653112 Task: #9707 (cherry picked from commit 2039138cfe2854b477f06d9185eddbdb264b649f) --- ironic/conductor/manager.py | 5 +- ironic/conf/conductor.py | 7 +++ ironic/tests/unit/conductor/test_manager.py | 55 ++++++++++++++++++++++ .../notes/zero-temp-url-c21e208f8933c6f6.yaml | 7 +++ 4 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/zero-temp-url-c21e208f8933c6f6.yaml diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index ba4c97106..099062886 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -3606,7 +3606,10 @@ def _store_configdrive(node, configdrive): if CONF.deploy.configdrive_use_object_store: # NOTE(lucasagomes): No reason to use a different timeout than # the one used for deploying the node - timeout = CONF.conductor.deploy_callback_timeout + timeout = (CONF.conductor.configdrive_swift_temp_url_duration + or CONF.conductor.deploy_callback_timeout + # The documented default in ironic.conf.conductor + or 1800) container = CONF.conductor.configdrive_swift_container object_name = _get_configdrive_obj_name(node) diff --git a/ironic/conf/conductor.py b/ironic/conf/conductor.py index ae7ed983c..d5053dc01 100644 --- a/ironic/conf/conductor.py +++ b/ironic/conf/conductor.py @@ -139,6 +139,13 @@ opts = [ help=_('Name of the Swift container to store config drive ' 'data. Used when configdrive_use_object_store is ' 'True.')), + cfg.IntOpt('configdrive_swift_temp_url_duration', + min=60, + help=_('The timeout (in seconds) after which a configdrive ' + 'temporary URL becomes invalid. Defaults to ' + 'deploy_callback_timeout if it is set, otherwise to ' + '1800 seconds. Used when ' + 'configdrive_use_object_store is True.')), cfg.IntOpt('inspect_wait_timeout', default=1800, help=_('Timeout (seconds) for waiting for node inspection. ' diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index de282fb32..bf2e51dfa 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -7544,6 +7544,61 @@ class StoreConfigDriveTestCase(db_base.DbTestCase): self.node.refresh() self.assertEqual(expected_instance_info, self.node.instance_info) + def test_store_configdrive_swift_no_deploy_timeout(self, mock_swift): + container_name = 'foo_container' + expected_obj_name = 'configdrive-%s' % self.node.uuid + expected_obj_header = {'X-Delete-After': '1200'} + expected_instance_info = {'configdrive': 'http://1.2.3.4'} + + # set configs and mocks + CONF.set_override('configdrive_use_object_store', True, + group='deploy') + CONF.set_override('configdrive_swift_container', container_name, + group='conductor') + CONF.set_override('configdrive_swift_temp_url_duration', 1200, + group='conductor') + CONF.set_override('deploy_callback_timeout', 0, + group='conductor') + mock_swift.return_value.get_temp_url.return_value = 'http://1.2.3.4' + + manager._store_configdrive(self.node, b'foo') + + mock_swift.assert_called_once_with() + mock_swift.return_value.create_object.assert_called_once_with( + container_name, expected_obj_name, mock.ANY, + object_headers=expected_obj_header) + mock_swift.return_value.get_temp_url.assert_called_once_with( + container_name, expected_obj_name, 1200) + self.node.refresh() + self.assertEqual(expected_instance_info, self.node.instance_info) + + def test_store_configdrive_swift_no_deploy_timeout_fallback(self, + mock_swift): + container_name = 'foo_container' + expected_obj_name = 'configdrive-%s' % self.node.uuid + expected_obj_header = {'X-Delete-After': '1800'} + expected_instance_info = {'configdrive': 'http://1.2.3.4'} + + # set configs and mocks + CONF.set_override('configdrive_use_object_store', True, + group='deploy') + CONF.set_override('configdrive_swift_container', container_name, + group='conductor') + CONF.set_override('deploy_callback_timeout', 0, + group='conductor') + mock_swift.return_value.get_temp_url.return_value = 'http://1.2.3.4' + + manager._store_configdrive(self.node, b'foo') + + mock_swift.assert_called_once_with() + mock_swift.return_value.create_object.assert_called_once_with( + container_name, expected_obj_name, mock.ANY, + object_headers=expected_obj_header) + mock_swift.return_value.get_temp_url.assert_called_once_with( + container_name, expected_obj_name, 1800) + self.node.refresh() + self.assertEqual(expected_instance_info, self.node.instance_info) + @mgr_utils.mock_record_keepalive class NodeInspectHardware(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): diff --git a/releasenotes/notes/zero-temp-url-c21e208f8933c6f6.yaml b/releasenotes/notes/zero-temp-url-c21e208f8933c6f6.yaml new file mode 100644 index 000000000..364b94ba6 --- /dev/null +++ b/releasenotes/notes/zero-temp-url-c21e208f8933c6f6.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + No longer tries to create a temporary URL with zero lifetime if the + ``deploy_callback_timeout`` option is set to zero. The default of 1800 + seconds is used in that case. Use the new + ``configdrive_swift_temp_url_duration`` option to override. -- cgit v1.2.1