omnibus: fail the task when omnibus can't be installed (#27730)

Co-authored-by: Alex Lopez <alex.lopez.zorzano@gmail.com>

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(

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)