diff --git a/coriolis/db/api.py b/coriolis/db/api.py index 81d4c2ea2..730a518ca 100644 --- a/coriolis/db/api.py +++ b/coriolis/db/api.py @@ -898,11 +898,62 @@ def _get_task(context, task_id): return task +def _fit_exception_details(task_id, exception_details): + """Fit exception details into the task.exception_details column. + + Command output embedded in a task error, such as a yum progress bar, + can exceed the MySQL TEXT limit. The status UPDATE then fails and the + task stays in a non-terminal state. The beginning and the end are kept: + Coriolis describes the failure first, and the underlying command reports + the actual error last. + """ + if not exception_details: + return exception_details + + max_length = models.MAX_EXCEPTION_DETAILS_LENGTH + original_length = len(exception_details) + if original_length <= max_length: + return exception_details + + LOG.warn( + f"Exception details for task '{task_id}' are too long " + f"({original_length} characters) to store in the database. " + f"Truncating to {max_length} characters so the task status can " + f"be recorded." + ) + + # The marker states how many characters were removed, and that count + # depends on the marker's own length. Grow the omitted count until the + # marker and the kept text fit together. + omitted = original_length - max_length + while True: + marker = f"\n... ({omitted} characters truncated) ...\n" + keep = max_length - len(marker) + if keep < 1: + return exception_details[:max_length] + required_omitted = original_length - keep + if omitted >= required_omitted: + break + omitted = required_omitted + + head_length = keep // 2 + tail_length = keep - head_length + return ( + f"{exception_details[:head_length]}{marker}{exception_details[-tail_length:]}" + ) + + @enginefacade.writer def set_task_status(context, task_id, status, exception_details=None): + """Set a task's status and, when given, its exception details. + + Exception details are shortened to fit the database column. A value + that exceeds the column makes the status update fail and leaves the + task stuck. + """ task = _get_task(context, task_id) task.status = status - task.exception_details = exception_details + task.exception_details = _fit_exception_details(task_id, exception_details) @enginefacade.writer diff --git a/coriolis/db/sqlalchemy/models.py b/coriolis/db/sqlalchemy/models.py index 2cd6bb6a4..199a11a87 100644 --- a/coriolis/db/sqlalchemy/models.py +++ b/coriolis/db/sqlalchemy/models.py @@ -15,6 +15,11 @@ MAX_EVENT_MESSAGE_LENGHT = 1024 +# MySQL TEXT holds at most 65,535 bytes. utf8mb4 can use 4 bytes per +# character, so this is the longest exception_details value guaranteed +# to fit in the column. +MAX_EXCEPTION_DETAILS_LENGTH = 65535 // 4 + class TaskEvent(BASE, models.TimestampMixin, models.SoftDeleteMixin, models.ModelBase): __tablename__ = 'task_event' diff --git a/coriolis/tests/db/test_api.py b/coriolis/tests/db/test_api.py index 46e7bd985..0bd57ded8 100644 --- a/coriolis/tests/db/test_api.py +++ b/coriolis/tests/db/test_api.py @@ -880,6 +880,83 @@ def test_set_execution_status_update_action_status(self): ) self.assertEqual(self.valid_transfer.last_execution_status, new_status) + def test_set_task_status_stores_short_exception_details(self): + task = self._new_task() + details = "Failed to install required packages." + + api.set_task_status(self.context, task.id, constants.TASK_STATUS_ERROR, details) + + stored = api.get_task(self.context, task.id) + self.assertEqual(constants.TASK_STATUS_ERROR, stored.status) + self.assertEqual(details, stored.exception_details) + + def test_set_task_status_records_status_when_exception_details_are_too_long(self): + task = self._new_task() + max_length = models.MAX_EXCEPTION_DETAILS_LENGTH + details = "start-" + ("x" * (max_length * 3)) + "-end" + + api.set_task_status(self.context, task.id, constants.TASK_STATUS_ERROR, details) + + stored = api.get_task(self.context, task.id) + self.assertEqual(constants.TASK_STATUS_ERROR, stored.status) + self.assertLessEqual(len(stored.exception_details), max_length) + self.assertLessEqual(len(stored.exception_details.encode("utf-8")), 65535) + self.assertTrue(stored.exception_details.startswith("start-")) + self.assertTrue(stored.exception_details.endswith("-end")) + self.assertIn("characters truncated", stored.exception_details) + + def _new_task(self): + task = models.Task() + task.id = str(uuid.uuid4()) + task.execution = self.valid_tasks_execution + task.instance = DEFAULT_INSTANCE + task.status = constants.TASK_STATUS_RUNNING + task.task_type = constants.TASK_TYPE_VALIDATE_TRANSFER_SOURCE_INPUTS + task.index = 1000 + len(self.valid_tasks_execution.tasks) + task.on_error = False + self.session.add(task) + self.session.commit() + return task + + +class ExceptionDetailsFitTestCase(test_base.CoriolisBaseTestCase): + def test_fit_exception_details_leaves_short_values_unchanged(self): + self.assertIsNone(api._fit_exception_details("task-id", None)) + self.assertEqual("", api._fit_exception_details("task-id", "")) + details = "a" * models.MAX_EXCEPTION_DETAILS_LENGTH + self.assertEqual(details, api._fit_exception_details("task-id", details)) + + @mock.patch.object(api.models, "MAX_EXCEPTION_DETAILS_LENGTH", 80) + def test_fit_exception_details_keeps_head_and_tail(self): + details = ("HEAD-" * 20) + ("x" * 200) + ("-TAIL" * 20) + + fitted = api._fit_exception_details("task-id", details) + + self.assertLessEqual(len(fitted), 80) + self.assertTrue(fitted.startswith("HEAD-")) + self.assertTrue(fitted.endswith("-TAIL")) + marker_start = fitted.index("\n... (") + marker_end = fitted.index(") ...\n") + len(") ...\n") + marker = fitted[marker_start:marker_end] + head, tail = fitted.split(marker) + self.assertEqual(details[: len(head)], head) + self.assertEqual(details[-len(tail) :], tail) + omitted = len(details) - len(head) - len(tail) + self.assertIn(f"({omitted} characters truncated)", marker) + self.assertLessEqual(len(fitted.encode("utf-8")), 65535) + + def test_fit_exception_details_stays_within_mysql_text_for_multibyte_text(self): + # U+10000 encodes to 4 bytes in UTF-8, the utf8mb4 worst case. + four_byte_char = "\U00010000" + details = four_byte_char * (models.MAX_EXCEPTION_DETAILS_LENGTH + 500) + + fitted = api._fit_exception_details("task-id", details) + + self.assertLessEqual(len(fitted), models.MAX_EXCEPTION_DETAILS_LENGTH) + self.assertLessEqual(len(fitted.encode("utf-8")), 65535) + self.assertTrue(fitted.startswith(four_byte_char)) + self.assertTrue(fitted.endswith(four_byte_char)) + class TransferSchedulesDBAPITestCase(BaseDBAPITestCase): @classmethod