diff --git a/tests/unit/artifact/test_file.py b/tests/unit/artifact/test_file.py index 0e89e2e5a0..d546f28936 100644 --- a/tests/unit/artifact/test_file.py +++ b/tests/unit/artifact/test_file.py @@ -60,7 +60,7 @@ def test_download_artifact(tmp_path, artifact_provider): guest, Path("/remote/foo-1.0-1.fc43.x86_64.rpm"), ) - guest.execute.assert_called_once() + guest.download.assert_called_once() @pytest.mark.parametrize( diff --git a/tests/unit/artifact/test_repository.py b/tests/unit/artifact/test_repository.py index 93a781b102..6328a53071 100644 --- a/tests/unit/artifact/test_repository.py +++ b/tests/unit/artifact/test_repository.py @@ -230,24 +230,11 @@ def test_url_no_path(root_logger): # ================================================================================ -@pytest.fixture -def mock_repo_file_fetch(): - with patch( - 'tmt.steps.prepare.artifact.providers.tmt.utils.retry_session' - ) as mock_retry_session: - mock_session = MagicMock() - mock_response = MagicMock() - mock_response.text = VALID_REPO_CONTENT - mock_response.raise_for_status.return_value = None - mock_session.get.return_value = mock_response - mock_retry_session.return_value.__enter__.return_value = mock_session - yield mock_session - - @pytest.fixture def mock_guest_and_pm(mock_package_manager): mock_guest = MagicMock() mock_guest.package_manager = mock_package_manager + mock_guest.execute.return_value.stdout = VALID_REPO_CONTENT return mock_guest, mock_package_manager @@ -260,7 +247,7 @@ def test_id_extraction(artifact_provider): assert provider.id == "https://download.docker.com/linux/centos/docker-ce.repo" -def test_artifacts_before_fetch(mock_repo_file_fetch, artifact_provider): +def test_artifacts_before_fetch(artifact_provider): """Test that repository provider artifacts returns empty list""" provider = artifact_provider("repository-file:https://example.com/test.repo") @@ -270,7 +257,7 @@ def test_artifacts_before_fetch(mock_repo_file_fetch, artifact_provider): assert provider.artifacts == [] -def test_fetch_contents(mock_repo_file_fetch, mock_guest_and_pm, artifact_provider, tmppath): +def test_fetch_contents(mock_guest_and_pm, artifact_provider, tmppath): """Test that fetch_contents is a no-op for repository providers""" mock_guest, _ = mock_guest_and_pm @@ -291,9 +278,7 @@ def test_fetch_contents(mock_repo_file_fetch, mock_guest_and_pm, artifact_provid assert len(artifacts) == 0 -def test_contribute_to_shared_repo( - mock_repo_file_fetch, mock_guest_and_pm, artifact_provider, tmppath -): +def test_contribute_to_shared_repo(mock_guest_and_pm, artifact_provider, tmppath): """Test that contribute_to_shared_repo does nothing for repository providers""" mock_guest, mock_package_manager = mock_guest_and_pm @@ -316,7 +301,7 @@ def test_contribute_to_shared_repo( assert provider.artifacts == [] -def test_get_repositories(mock_repo_file_fetch, mock_guest_and_pm, artifact_provider, tmppath): +def test_get_repositories(mock_guest_and_pm, artifact_provider, tmppath): """Test that get_repositories returns the initialized repository""" mock_guest, _ = mock_guest_and_pm diff --git a/tmt/guest/__init__.py b/tmt/guest/__init__.py index fa636c4772..38a5c4c992 100644 --- a/tmt/guest/__init__.py +++ b/tmt/guest/__init__.py @@ -114,6 +114,11 @@ def default_connect_waiting() -> Waiting: #: ``TMT_REBOOT_TIMEOUT``. REBOOT_TIMEOUT: int = configure_constant(DEFAULT_REBOOT_TIMEOUT, 'TMT_REBOOT_TIMEOUT') +#: Number of attempts to download a file from a URL. +DOWNLOAD_ATTEMPTS = 3 +#: Number of seconds to wait between download attempts. +DOWNLOAD_INTERVAL = 5 + def default_reboot_waiting() -> Waiting: """ @@ -459,6 +464,12 @@ class RebootMode(enum.Enum): HardRebootModes = Literal[RebootMode.HARD] +class DownloadError(tmt.utils.GeneralError): + """ + Raised when download fails. + """ + + class RebootModeNotSupportedError(ProvisionError): """A requested reboot mode is not supported by the guest""" @@ -2389,6 +2400,35 @@ def pull( raise NotImplementedError + def download(self, url: str, destination: Path) -> None: + """ + Download a file from a URL to a path on the guest. + + :param url: URL to download from. + :param destination: path on the guest to save the file to. + :raises DownloadError: if the download fails after all attempts. + """ + + def _do_download() -> None: + self.execute( + ShellScript( + f"{self.facts.sudo_prefix} curl -L --fail" + f" -o {quote(str(destination))} {quote(url)}" + ), + silent=True, + ) + + try: + tmt.utils.retry( + _do_download, + DOWNLOAD_ATTEMPTS, + DOWNLOAD_INTERVAL, + f"Download '{url}'", + self._logger, + ) + except tmt.utils.RetryError as error: + raise DownloadError(f"Failed to download '{url}' to '{destination}'.") from error + @abc.abstractmethod def stop(self) -> None: """ diff --git a/tmt/package_managers/__init__.py b/tmt/package_managers/__init__.py index 2ef2ca8f44..4cc709072e 100644 --- a/tmt/package_managers/__init__.py +++ b/tmt/package_managers/__init__.py @@ -24,6 +24,10 @@ PackageOrigin: TypeAlias = Union[str, 'SpecialPackageOrigin'] +#: Directory where DNF/YUM repository files are stored. +YUM_REPOS_DIR = Path("/etc/yum.repos.d") + + @container(frozen=True) class Version: """ diff --git a/tmt/package_managers/dnf.py b/tmt/package_managers/dnf.py index a426ee172c..987b457ebf 100644 --- a/tmt/package_managers/dnf.py +++ b/tmt/package_managers/dnf.py @@ -4,6 +4,7 @@ from tmt._compat.pathlib import Path from tmt.package_managers import ( + YUM_REPOS_DIR, FileSystemPath, Installable, Options, @@ -179,7 +180,7 @@ def install_debuginfo( return script def install_repository(self, repository: Repository) -> ShellScript: - repo_path = f"/etc/yum.repos.d/{repository.filename}" + repo_path = YUM_REPOS_DIR / repository.filename return ShellScript( f"{self.guest.facts.sudo_prefix} tee {repo_path} <<'EOF'\n{repository.content}\nEOF" ) diff --git a/tmt/steps/prepare/artifact/providers/__init__.py b/tmt/steps/prepare/artifact/providers/__init__.py index 5ff0b12421..4734f8d759 100644 --- a/tmt/steps/prepare/artifact/providers/__init__.py +++ b/tmt/steps/prepare/artifact/providers/__init__.py @@ -14,7 +14,7 @@ import tmt.utils from tmt._compat.typing import TypeAlias from tmt.container import container, simple_field -from tmt.guest import Guest +from tmt.guest import DownloadError, Guest from tmt.package_managers import Repository, Version from tmt.plugins import PluginRegistry from tmt.utils import Path, ShellScript @@ -24,12 +24,6 @@ ) -class DownloadError(tmt.utils.GeneralError): - """ - Raised when download fails. - """ - - class UnsupportedOperationError(RuntimeError): """ Raised when an operation is intentionally unsupported by a provider. @@ -141,18 +135,22 @@ def artifacts(self) -> Sequence[ArtifactInfo]: return self._artifacts - @abstractmethod def _download_artifact( self, artifact: ArtifactInfo, guest: Guest, destination: tmt.utils.Path ) -> None: """ Download a single artifact to the specified destination on a given guest. + :param artifact: the artifact to download. :param guest: the guest on which the artifact should be downloaded. :param destination: path into which the artifact should be downloaded. + :raises DownloadError: if the download fails. """ - raise NotImplementedError + try: + guest.download(artifact.location, destination) + except tmt.utils.GeneralError as error: + raise DownloadError(f"Failed to download '{artifact}'.") from error def fetch_contents( self, diff --git a/tmt/steps/prepare/artifact/providers/copr_build.py b/tmt/steps/prepare/artifact/providers/copr_build.py index b993b42cd5..7ca131dae5 100644 --- a/tmt/steps/prepare/artifact/providers/copr_build.py +++ b/tmt/steps/prepare/artifact/providers/copr_build.py @@ -19,7 +19,6 @@ ArtifactInfo, ArtifactProvider, ArtifactProviderId, - DownloadError, provides_artifact_provider, ) from tmt.utils import ShellScript @@ -136,20 +135,6 @@ def _extract_provider_id(cls, raw_id: str) -> ArtifactProviderId: ) from error return value - def _download_artifact( - self, artifact: ArtifactInfo, guest: Guest, destination: tmt.utils.Path - ) -> None: - try: - # Destination directory is guaranteed to exist, download the artifact - guest.execute( - tmt.utils.ShellScript( - f"curl -L --fail -o {quote(str(destination))} {quote(artifact.location)}" - ), - silent=True, - ) - except Exception as error: - raise DownloadError(f"Failed to download '{artifact}'.") from error - @cached_property def result_url(self) -> str: """ diff --git a/tmt/steps/prepare/artifact/providers/copr_repository.py b/tmt/steps/prepare/artifact/providers/copr_repository.py index dba8ff1562..e6e4c77844 100644 --- a/tmt/steps/prepare/artifact/providers/copr_repository.py +++ b/tmt/steps/prepare/artifact/providers/copr_repository.py @@ -10,6 +10,7 @@ from tmt._compat.typing import Self from tmt.container import container from tmt.guest import Guest +from tmt.package_managers import YUM_REPOS_DIR from tmt.steps.prepare.artifact.providers import ( ArtifactInfo, ArtifactProvider, @@ -114,9 +115,7 @@ def fetch_contents( repo_filename = f"_copr:copr.fedorainfracloud.org:{owner}:{repo.project}.repo" try: - output = guest.execute( - tmt.utils.Command("cat", Path(f"/etc/yum.repos.d/{repo_filename}")) - ) + output = guest.execute(tmt.utils.Command("cat", YUM_REPOS_DIR / repo_filename)) except tmt.utils.RunError as error: raise tmt.utils.PrepareError( f"Failed to read '{repo_filename}' from the guest." diff --git a/tmt/steps/prepare/artifact/providers/file.py b/tmt/steps/prepare/artifact/providers/file.py index 9869b1ab92..480e4a1288 100644 --- a/tmt/steps/prepare/artifact/providers/file.py +++ b/tmt/steps/prepare/artifact/providers/file.py @@ -109,12 +109,7 @@ def _download_artifact( ) -> None: try: if self._is_url: # Remote file, download it - guest.execute( - tmt.utils.ShellScript( - f"curl -L --fail -o {quote(str(destination))} {quote(artifact.location)}" - ), - silent=True, - ) + guest.download(artifact.location, destination) else: # Local file, push it to the guest # When pushing a single file, use recursive=False. The default recursive=True # treats the source as a directory (appending "/."), which only works for diff --git a/tmt/steps/prepare/artifact/providers/koji.py b/tmt/steps/prepare/artifact/providers/koji.py index 5fc501d525..2dc98b5551 100644 --- a/tmt/steps/prepare/artifact/providers/koji.py +++ b/tmt/steps/prepare/artifact/providers/koji.py @@ -20,7 +20,6 @@ ArtifactInfo, ArtifactProvider, ArtifactProviderId, - DownloadError, provides_artifact_provider, ) from tmt.utils import ShellScript @@ -178,27 +177,6 @@ def _extract_provider_id(cls, raw_id: str) -> ArtifactProviderId: ) from exc return value - def _download_artifact( - self, artifact: ArtifactInfo, guest: Guest, destination: tmt.utils.Path - ) -> None: - """ - Download the specified artifact to the given destination on the guest. - - :param artifact: The artifact to download - :param guest: The guest where the artifact should be downloaded - :param destination: The destination path on the guest - """ - try: - # Destination directory is guaranteed to exist, download the artifact - guest.execute( - tmt.utils.ShellScript( - f"curl -L --fail -o {quote(str(destination))} {quote(artifact.location)}" - ), - silent=True, - ) - except Exception as error: - raise DownloadError(f"Failed to download '{artifact}'.") from error - def contribute_to_shared_repo( self, guest: Guest, diff --git a/tmt/steps/prepare/artifact/providers/repository.py b/tmt/steps/prepare/artifact/providers/repository.py index fc975f435a..87cda4ff10 100644 --- a/tmt/steps/prepare/artifact/providers/repository.py +++ b/tmt/steps/prepare/artifact/providers/repository.py @@ -9,7 +9,8 @@ import tmt.log import tmt.utils from tmt.container import container, simple_field -from tmt.guest import Guest +from tmt.guest import DownloadError, Guest +from tmt.package_managers import YUM_REPOS_DIR from tmt.steps import DefaultNameGenerator from tmt.steps.prepare.artifact.providers import ( ArtifactInfo, @@ -75,18 +76,28 @@ def fetch_contents( self.logger.info(f"Initializing repository provider with URL: {self.id}") parsed = urlparse(self.id) + parsed_path = Path(parsed.path) if parsed.scheme == 'file': # Read .repo file from the local (controller) filesystem. - self.logger.info(f"Reading repository file from local path: {parsed.path}") - self.logger.debug( - f"Absolute path of the repository file: '{Path(parsed.path).resolve()}'" - ) - self.repository = Repository.from_file_path( - file_path=Path(parsed.path), logger=self.logger - ) + self.logger.info(f"Reading repository file from local path: {parsed_path}") + self.logger.debug(f"Absolute path of the repository file: '{parsed_path.resolve()}'") + self.repository = Repository.from_file_path(file_path=parsed_path, logger=self.logger) else: - # TODO: This should not be using Repository.from_url - self.repository = Repository.from_url(url=self.id, logger=self.logger) + repo_filename = parsed_path.name + remote_path = YUM_REPOS_DIR / repo_filename + try: + guest.download(self.id, remote_path) + except DownloadError as error: + raise PrepareError( + f"Failed to download repository file '{self.id}' to the guest." + ) from error + try: + output = guest.execute(tmt.utils.Command("cat", remote_path)) + except tmt.utils.RunError as error: + raise PrepareError(f"Failed to read '{repo_filename}' from the guest.") from error + self.repository = Repository.from_content( + output.stdout or '', repo_filename.removesuffix('.repo'), self.logger + ) self.logger.info( f"Repository initialized: {self.repository.name} "