diff --git a/subiquity/server/controllers/cmdlist.py b/subiquity/server/controllers/cmdlist.py index be3d645505..b363e5e52c 100644 --- a/subiquity/server/controllers/cmdlist.py +++ b/subiquity/server/controllers/cmdlist.py @@ -16,7 +16,6 @@ import asyncio import os import shlex -import shutil from typing import List, Sequence, Union import attr @@ -26,7 +25,7 @@ from subiquity.server.controller import NonInteractiveController from subiquitycore.async_helpers import run_bg_task from subiquitycore.context import with_context -from subiquitycore.utils import arun_command, orig_environ +from subiquitycore.utils import arun_command @attr.s(auto_attribs=True) @@ -79,15 +78,6 @@ async def run(self, context): env = self.env() for i, cmd in enumerate(tuple(self.builtin_cmds) + tuple(self.cmds)): desc = cmd.desc() - - # If the path to the command isn't found on the snap we should - # drop the snap specific environment variables. - command = shlex.split(desc)[0] - path = shutil.which(command) - if path is not None: - if not path.startswith("/snap"): - env = orig_environ(env) - with context.child("command_{}".format(i), desc): args = cmd.as_args_list() if self.syslog_id: diff --git a/subiquity/server/controllers/tests/test_cmdlist.py b/subiquity/server/controllers/tests/test_cmdlist.py deleted file mode 100644 index 1191ea6cac..0000000000 --- a/subiquity/server/controllers/tests/test_cmdlist.py +++ /dev/null @@ -1,84 +0,0 @@ -# Copyright 2023 Canonical, Ltd. -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as -# published by the Free Software Foundation, either version 3 of the -# License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see . - -from unittest import IsolatedAsyncioTestCase, mock - -from subiquity.server.controllers import cmdlist -from subiquity.server.controllers.cmdlist import ( - CmdListController, - Command, - EarlyController, - LateController, -) -from subiquitycore.tests.mocks import make_app -from subiquitycore.utils import orig_environ - - -@mock.patch.object(cmdlist, "orig_environ", side_effect=orig_environ) -@mock.patch.object(cmdlist, "arun_command") -class TestCmdListController(IsolatedAsyncioTestCase): - controller_type = CmdListController - - def setUp(self): - self.controller = self.controller_type(make_app()) - self.controller.cmds = [Command(args="some-command", check=False)] - snap_env = { - "LD_LIBRARY_PATH": "/var/lib/snapd/lib/gl", - } - self.mocked_os_environ = mock.patch.dict("os.environ", snap_env) - - @mock.patch("shutil.which", return_value="/usr/bin/path/to/bin") - async def test_no_snap_env_on_call( - self, - mocked_shutil, - mocked_arun, - mocked_orig_environ, - ): - with self.mocked_os_environ: - await self.controller.run() - args, kwargs = mocked_arun.call_args - call_env = kwargs["env"] - - mocked_orig_environ.assert_called() - self.assertNotIn("LD_LIBRARY_PATH", call_env) - - @mock.patch("shutil.which", return_value="/snap/path/to/bin") - async def test_with_snap_env_on_call( - self, - mocked_shutil, - mocked_arun, - mocked_orig_environ, - ): - with self.mocked_os_environ: - await self.controller.run() - args, kwargs = mocked_arun.call_args - call_env = kwargs["env"] - - mocked_orig_environ.assert_not_called() - self.assertIn("LD_LIBRARY_PATH", call_env) - - -class TestEarlyController(TestCmdListController): - controller_type = EarlyController - - def setUp(self): - super().setUp() - - -class TestLateController(TestCmdListController): - controller_type = LateController - - def setUp(self): - super().setUp() diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index 4321601df1..2ef0348325 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -91,6 +91,8 @@ def setUp(self): self.app = make_app() self.app.opts.bootloader = "UEFI" self.app.command_runner = mock.AsyncMock() + self.app.report_start_event = mock.Mock() + self.app.report_finish_event = mock.Mock() self.app.prober = mock.Mock() self.app.prober.get_storage = mock.AsyncMock() self.app.block_log_dir = "/inexistent" @@ -1205,6 +1207,8 @@ def setUp(self): self.app = make_app() self.app.command_runner = mock.AsyncMock() self.app.opts.bootloader = "UEFI" + self.app.report_start_event = mock.Mock() + self.app.report_finish_event = mock.Mock() self.app.prober = mock.Mock() self.app.prober.get_storage = mock.AsyncMock() self.app.snapdapi = snapdapi.make_api_client(AsyncSnapd(get_fake_connection())) diff --git a/subiquity/server/controllers/tests/test_install.py b/subiquity/server/controllers/tests/test_install.py index 51b40412a0..00c91e01d0 100644 --- a/subiquity/server/controllers/tests/test_install.py +++ b/subiquity/server/controllers/tests/test_install.py @@ -34,6 +34,8 @@ def setUp(self): self.controller = InstallController(make_app()) self.controller.write_config = unittest.mock.Mock() self.controller.app.note_file_for_apport = Mock() + self.controller.app.report_start_event = Mock() + self.controller.app.report_finish_event = Mock() self.controller.model.target = "/target" @@ -197,6 +199,8 @@ def test_generic_config(self): class TestInstallController(unittest.IsolatedAsyncioTestCase): def setUp(self): self.controller = InstallController(make_app()) + self.controller.app.report_start_event = Mock() + self.controller.app.report_finish_event = Mock() self.controller.model.target = tempfile.mkdtemp() os.makedirs(os.path.join(self.controller.model.target, "etc/grub.d")) self.addCleanup(shutil.rmtree, self.controller.model.target) diff --git a/subiquity/server/controllers/tests/test_refresh.py b/subiquity/server/controllers/tests/test_refresh.py index 0da98fbfad..94adf24d6c 100644 --- a/subiquity/server/controllers/tests/test_refresh.py +++ b/subiquity/server/controllers/tests/test_refresh.py @@ -26,6 +26,8 @@ class TestRefreshController(SubiTestCase): def setUp(self): self.app = make_app() + self.app.report_start_event = mock.Mock() + self.app.report_finish_event = mock.Mock() self.app.note_data_for_apport = mock.Mock() self.app.prober = mock.Mock() self.app.snapdapi = snapdapi.make_api_client(AsyncSnapd(get_fake_connection())) diff --git a/subiquity/server/controllers/tests/test_snaplist.py b/subiquity/server/controllers/tests/test_snaplist.py index ba12aed7a5..a31e8e5ac5 100644 --- a/subiquity/server/controllers/tests/test_snaplist.py +++ b/subiquity/server/controllers/tests/test_snaplist.py @@ -14,7 +14,7 @@ # along with this program. If not, see . import unittest -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, Mock import requests @@ -31,6 +31,8 @@ def setUp(self): self.model = SnapListModel() self.app = make_app() self.app.snapd = AsyncMock() + self.app.report_start_event = Mock() + self.app.report_finish_event = Mock() self.loader = SnapdSnapInfoLoader( self.model, self.app.snapd, "server", self.app.context diff --git a/subiquitycore/tests/mocks.py b/subiquitycore/tests/mocks.py index 663b02b822..74f2f66bf6 100644 --- a/subiquitycore/tests/mocks.py +++ b/subiquitycore/tests/mocks.py @@ -43,9 +43,4 @@ def make_app(model=None): app.opts = mock.Mock() app.opts.dry_run = True app.scale_factor = 1000 - app.echo_syslog_id = None - app.log_syslog_id = None - app.report_start_event = mock.Mock() - app.report_finish_event = mock.Mock() - return app diff --git a/subiquitycore/utils.py b/subiquitycore/utils.py index 10027d1d16..2a283b5f0a 100644 --- a/subiquitycore/utils.py +++ b/subiquitycore/utils.py @@ -19,7 +19,7 @@ import os import random import subprocess -from typing import Any, Dict, List, Optional, Sequence +from typing import Any, Dict, List, Sequence log = logging.getLogger("subiquitycore.utils") @@ -35,18 +35,15 @@ def _clean_env(env, *, locale=True): return env -def orig_environ(env: Optional[Dict[str, str]]) -> Dict[str, str]: +def orig_environ(env): """Generate an environment dict that is suitable for use for running programs that live outside the snap.""" - if env is None: - env: Dict[str, str] = os.environ - - ret: Dict[str, str] = env.copy() - + env = os.environ + ret = env.copy() for key, val in env.items(): if key.endswith("_ORIG"): - key_to_restore: str = key[: -len("_ORIG")] + key_to_restore = key[: -len("_ORIG")] if val: ret[key_to_restore] = val else: