diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py index 0a26df6d9..520de8e64 100644 --- a/ironic_python_agent/config.py +++ b/ironic_python_agent/config.py @@ -306,6 +306,13 @@ cli_opts = [ 'ipa-image-download-connection-retry-interval', 10), help='Interval (in seconds) between two attempts to establish ' 'connection when downloading an image.'), + cfg.IntOpt('image_download_max_duration', min=0, + default=int(APARAMS.get( + 'ipa-image-download-max-duration', 0)), + help='Maximum total duration (in seconds) allowed for ' + 'downloading an image. If the download exceeds this ' + 'duration, it will be aborted regardless of retry or ' + 'connection success.'), cfg.StrOpt('ironic_api_version', default=APARAMS.get('ipa-ironic-api-version', None), help='Ironic API version in format "x.x". If not set, the API ' diff --git a/ironic_python_agent/errors.py b/ironic_python_agent/errors.py index c0a48e8fe..4636015aa 100644 --- a/ironic_python_agent/errors.py +++ b/ironic_python_agent/errors.py @@ -156,6 +156,19 @@ class ImageDownloadError(RESTError): super(ImageDownloadError, self).__init__(details) +class ImageDownloadTimeoutError(RESTError): + """Raised when an image download operation exceeds its allowed time limit. + + """ + status_code = 408 + message = 'Image download timeout' + + def __init__(self, image_id, msg): + details = 'Download of image {} failed: {}'.format(image_id, msg) + self.secondary_message = msg + super(ImageDownloadTimeoutError, self).__init__(details) + + class ImageDownloadOutofSpaceError(ImageDownloadError): """Raised when an image download fails due to insufficient storage.""" pass diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py index b6e2e4ee8..a4d3cd760 100644 --- a/ironic_python_agent/extensions/standby.py +++ b/ironic_python_agent/extensions/standby.py @@ -641,7 +641,21 @@ class ImageDownload(object): which is a constant in this module. """ self._last_chunk_time = None + start_time = self._time + for chunk in self._request.iter_content(IMAGE_CHUNK_SIZE): + + max_download_duration = CONF.image_download_max_duration + if max_download_duration: + elapsed = time.time() - start_time + if elapsed > max_download_duration: + LOG.error('Total download timeout (%s seconds) exceeded', + max_download_duration) + raise errors.ImageDownloadTimeoutError( + self._image_info['id'], + 'Download exceeded max allowed time (%s seconds)' % + max_download_duration) + # Per requests forum posts/discussions, iter_content should # periodically yield to the caller for the client to do things # like stopwatch and potentially interrupt the download. diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py index c7a6784f7..1fd043086 100644 --- a/ironic_python_agent/tests/unit/extensions/test_standby.py +++ b/ironic_python_agent/tests/unit/extensions/test_standby.py @@ -2214,6 +2214,31 @@ class TestImageDownload(base.IronicAgentTest): sleep_mock.assert_called_with(10) self.assertEqual(2, sleep_mock.call_count) + @mock.patch('time.time', autospec=True) + def test_download_image_exceeds_max_duration(self, time_mock, + requests_mock, hash_mock): + CONF.set_override('image_download_max_duration', 5) + + image_info = _build_fake_image_info() + hash_mock.return_value.hexdigest.return_value = image_info[ + 'os_hash_value'] + + content = ['a'] * 10 + + # simulating time passing with each chunk downloaded + time_mock.side_effect = list(range(11)) + + response = requests_mock.return_value + response.status_code = 200 + response.iter_content.return_value = content + + image_download = standby.ImageDownload(image_info) + + with self.assertRaisesRegex(errors.ImageDownloadTimeoutError, + 'Download exceeded max allowed time'): + # Iterating triggers the timeout logic inside __iter__() + list(image_download) + @mock.patch('time.sleep', autospec=True) def test_download_image_no_space_error_fatal(self, sleep_mock, requests_mock, hash_mock): diff --git a/releasenotes/notes/image-download-max-duration-49eb5de0f1c23f52.yaml b/releasenotes/notes/image-download-max-duration-49eb5de0f1c23f52.yaml new file mode 100644 index 000000000..6319b78fe --- /dev/null +++ b/releasenotes/notes/image-download-max-duration-49eb5de0f1c23f52.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Adds a new configuration option `[DEFAULT]image_download_max_duration`, + which enforces a maximum total duration (in seconds) allowed for + downloading an image. If this threshold is exceeded, the download is + aborted with a non-retryable `ImageDownloadTimeoutError` exception. + + A value of 0 (the default) disables this feature. +