From 1d76b1034a56bc0738d18a7e0c64442b7bf91519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Mon, 22 Jul 2024 09:52:35 +0200 Subject: [PATCH] omnibus: fail the task when omnibus can't be installed (#27730) Co-authored-by: Alex Lopez --- tasks/omnibus.py | 13 ++++++++----- tasks/unit_tests/omnibus_tests.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/tasks/omnibus.py b/tasks/omnibus.py index 48aec2f6f1e60..860c15e34606e 100644 --- a/tasks/omnibus.py +++ b/tasks/omnibus.py @@ -69,12 +69,15 @@ def bundle_install_omnibus(ctx, gem_path=None, env=None, max_try=2): with gitlab_section("Bundle install omnibus", collapsed=True): for trial in range(max_try): - res = ctx.run(cmd, env=env, warn=True, err_stream=sys.stdout) - if res.ok: + try: + ctx.run(cmd, env=env, err_stream=sys.stdout) return - if not should_retry_bundle_install(res): - return - print(f"Retrying bundle install, attempt {trial + 1}/{max_try}") + except UnexpectedExit as e: + if not should_retry_bundle_install(e.result): + print(f'Fatal error while installing omnibus: {e.result.stdout}. Cannot continue.') + raise + print(f"Retrying bundle install, attempt {trial + 1}/{max_try}") + raise Exit('Too many failures while installing omnibus, giving up') def get_omnibus_env( diff --git a/tasks/unit_tests/omnibus_tests.py b/tasks/unit_tests/omnibus_tests.py index e9b4e1d70b070..1d7438303eefd 100644 --- a/tasks/unit_tests/omnibus_tests.py +++ b/tasks/unit_tests/omnibus_tests.py @@ -4,7 +4,7 @@ from unittest import mock from invoke.context import MockContext -from invoke.exceptions import UnexpectedExit +from invoke.exceptions import Exit, UnexpectedExit from invoke.runners import Result from tasks import omnibus @@ -175,3 +175,31 @@ def test_cache_is_disabled_by_unsetting_env_var(self): self.assertRunLines(['bundle exec omnibus build agent']) commands = _run_calls_to_string(self.mock_ctx.run.mock_calls) self.assertNotIn('omnibus-git-cache', commands) + + +class TestOmnibusInstall(unittest.TestCase): + def setUp(self): + self.mock_ctx = MockContextRaising(run={}) + + def test_success(self): + self.mock_ctx.set_result_for('run', 'bundle install', Result()) + omnibus.bundle_install_omnibus(self.mock_ctx) + self.assertEqual(len(self.mock_ctx.run.mock_calls), 1) + + def test_failure(self): + self.mock_ctx.set_result_for('run', 'bundle install', Result(exited=1)) + with self.assertRaises(UnexpectedExit): + omnibus.bundle_install_omnibus(self.mock_ctx) + self.assertEqual(len(self.mock_ctx.run.mock_calls), 1) + + def test_transient(self): + self.mock_ctx = MockContextRaising(run=[Result(exited=1, stderr='Net::HTTPNotFound: something'), Result()]) + omnibus.bundle_install_omnibus(self.mock_ctx) + self.assertEqual(len(self.mock_ctx.run.mock_calls), 2) + + def test_transient_repeated(self): + self.mock_ctx.set_result_for('run', 'bundle install', Result(exited=1, stderr='Net::HTTPNotFound: something')) + max_try = 2 + with self.assertRaises(Exit): + omnibus.bundle_install_omnibus(self.mock_ctx) + self.assertEqual(len(self.mock_ctx.run.mock_calls), max_try)