launchpad-reviewers team mailing list archive

[Artem Streltsov <mp+493520@xxxxxxxxxxxxxxxxxx>]

Artem Streltsov has proposed merging ~artemstreltsov/launchpad-buildd:revert_snap_retry_workaround into launchpad-buildd:master.

Commit message:
Remove snap retry workaround

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~artemstreltsov/launchpad-buildd/+git/launchpad-buildd/+merge/493520
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~artemstreltsov/launchpad-buildd:revert_snap_retry_workaround into launchpad-buildd:master.

diff --git a/lpbuildd/target/lxd.py b/lpbuildd/target/lxd.py
index ebf4392..d48a769 100644
--- a/lpbuildd/target/lxd.py
+++ b/lpbuildd/target/lxd.py
@@ -695,29 +695,6 @@ class LXD(Backend):
             except subprocess.CalledProcessError:
                 pass
 
-    def _run_command(
-        self, cmd, input_text, get_output, echo, return_process, **kwargs
-    ):
-        if input_text is None and not get_output:
-            subprocess.check_call(cmd, **kwargs)
-        else:
-            if get_output:
-                kwargs["stdout"] = subprocess.PIPE
-            proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, **kwargs)
-            if return_process:
-                return proc
-            output, _ = proc.communicate(input_text)
-            if proc.returncode:
-                raise subprocess.CalledProcessError(proc.returncode, cmd)
-            if get_output:
-                if echo:
-                    print("Output:")
-                    output_text = output
-                    if isinstance(output_text, bytes):
-                        output_text = output_text.decode("UTF-8", "replace")
-                    print(output_text)
-                return output
-
     def run(
         self,
         args,
@@ -755,28 +732,25 @@ class LXD(Backend):
         # pylxd's Container.execute doesn't support sending stdin, and it's
         # tedious to implement ourselves.
         cmd = ["lxc", "exec", self.name] + env_params + ["--"] + args
-
-        # XXX tushar5526 2025-01-16: Installing a snap fails on first
-        # attempt due to https://bugs.launchpad.net/snapd/+bug/1731519 and
-        # runs into udev issues.
-        # This retry mechanism needs to be removed once the snapd team
-        # has fixed the udev issue which is curently in progress.
-        if "snap" not in args or "install" not in args:
-            return self._run_command(
-                cmd, input_text, get_output, echo, return_process, **kwargs
-            )
-        max_retries = 3
-        e = None
-        for retry in range(max_retries):
-            print("Running '%s'. Attempt %d" % (args, retry + 1))
-            try:
-                return self._run_command(
-                    cmd, input_text, get_output, echo, return_process, **kwargs
-                )
-            except subprocess.CalledProcessError as exc:
-                e = exc
-                pass
-        raise e
+        if input_text is None and not get_output:
+            subprocess.check_call(cmd, **kwargs)
+        else:
+            if get_output:
+                kwargs["stdout"] = subprocess.PIPE
+            proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, **kwargs)
+            if return_process:
+                return proc
+            output, _ = proc.communicate(input_text)
+            if proc.returncode:
+                raise subprocess.CalledProcessError(proc.returncode, cmd)
+            if get_output:
+                if echo:
+                    print("Output:")
+                    output_text = output
+                    if isinstance(output_text, bytes):
+                        output_text = output_text.decode("UTF-8", "replace")
+                    print(output_text)
+                return output
 
     def copy_in(self, source_path, target_path):
         """See `Backend`."""
diff --git a/lpbuildd/target/tests/test_lxd.py b/lpbuildd/target/tests/test_lxd.py
index cce92d5..6907fa4 100644
--- a/lpbuildd/target/tests/test_lxd.py
+++ b/lpbuildd/target/tests/test_lxd.py
@@ -1400,43 +1400,3 @@ class TestLXD(TestCase):
                 ]
             ),
         )
-
-    def test_fail_after_3_retries_on_snap_install(self):
-        processes_fixture = self.useFixture(FakeProcesses())
-        processes_fixture.add(lambda _: {}, name="lxc")
-
-        with mock.patch(
-            "lpbuildd.target.lxd.LXD._run_command"
-        ) as mock_run_command:
-            mock_run_command.side_effect = subprocess.CalledProcessError(1, "")
-
-            self.assertRaises(
-                subprocess.CalledProcessError,
-                LXD("1", "noble", "amd64").run,
-                ["snap", "install", "lxd"],
-                env={"LANG": "C"},
-            )
-
-        # max retries is hardcode to 3
-        assert mock_run_command.call_count == 3
-
-    def test_retry_on_snap_install(self):
-        processes_fixture = self.useFixture(FakeProcesses())
-        processes_fixture.add(lambda _: {}, name="lxc")
-
-        with mock.patch(
-            "lpbuildd.target.lxd.LXD._run_command"
-        ) as mock_run_command:
-            # Make the first call to _run_command fail and let
-            # the second one pass
-            mock_run_command.side_effect = [
-                subprocess.CalledProcessError(1, ""),
-                None,
-            ]
-
-            LXD("1", "noble", "amd64").run(
-                ["snap", "install", "lxd"],
-                env={"LANG": "C"},
-            )
-
-        assert mock_run_command.call_count == 2