From abc36a8f189f6b408f5ad41aea256cdaa9d91f70 Mon Sep 17 00:00:00 2001 From: Lili Nie Date: Sun, 15 Dec 2024 10:05:29 -0500 Subject: [PATCH 1/5] Support `--workdir-root` in the `tmt clean images` command (#3426) --- docs/releases.rst | 7 +++++++ tmt/base.py | 3 ++- tmt/cli/_root.py | 9 +++++++-- tmt/steps/provision/__init__.py | 7 ++----- tmt/steps/provision/testcloud.py | 14 +++++++------- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/docs/releases.rst b/docs/releases.rst index abf59b7895..01be45c68f 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -4,6 +4,13 @@ Releases ====================== +tmt-1.43 +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Add the ``--workdir-root`` option for the ``tmt clean images`` +command so that users can specify the directory they want. + + tmt-1.42.1 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tmt/base.py b/tmt/base.py index 5a9a0ac280..49c88a5ebc 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -4318,7 +4318,8 @@ def images(self) -> bool: successful = True for method in tmt.steps.provision.ProvisionPlugin.methods(): # FIXME: ignore[union-attr]: https://github.com/teemtee/tmt/issues/1599 - if not method.class_.clean_images(self, self.is_dry_run): # type: ignore[union-attr] + if not method.class_.clean_images( # type: ignore[union-attr] + self, self.is_dry_run, self.workdir_root): successful = False return successful diff --git a/tmt/cli/_root.py b/tmt/cli/_root.py index c374f164e0..c5ab3bba58 100644 --- a/tmt/cli/_root.py +++ b/tmt/cli/_root.py @@ -1850,9 +1850,10 @@ def clean_guests( # inference. See Context and ContextObjects above. @clean.command(name='images') # type: ignore[arg-type] @pass_context +@workdir_root_options @verbosity_options @dry_options -def clean_images(context: Context, **kwargs: Any) -> None: +def clean_images(context: Context, _workdir_root: Optional[str], **kwargs: Any) -> None: """ Remove images of supported provision methods. @@ -1865,13 +1866,17 @@ def clean_images(context: Context, **kwargs: Any) -> None: # cleaned, similarly to guests. assert context.obj.clean_logger is not None # narrow type + workdir_root = Path(_workdir_root) if _workdir_root is not None else None + if workdir_root and not workdir_root.exists(): + raise tmt.utils.GeneralError(f"Path '{workdir_root}' doesn't exist.") + clean_obj = tmt.Clean( logger=context.obj.clean_logger.descend( logger_name='clean-images', extra_shift=0 ).apply_verbosity_options(**kwargs), parent=context.obj.clean, cli_invocation=CliInvocation.from_context(context), - ) + workdir_root=effective_workdir_root(workdir_root)) context.obj.clean_partials["images"].append(clean_obj.images) diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index e4a6b9e0d1..46425c28ad 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -2619,11 +2619,8 @@ def options(cls, how: Optional[str] = None) -> list[tmt.options.ClickOptionDecor return super().options(how) + cls._guest_class.options(how) @classmethod - def clean_images(cls, clean: 'tmt.base.Clean', dry: bool) -> bool: - """ - Remove the images of one particular plugin - """ - + def clean_images(cls, clean: 'tmt.base.Clean', dry: bool, workdir_root: Path) -> bool: + """ Remove the images of one particular plugin """ return True def show(self, keys: Optional[list[str]] = None) -> None: diff --git a/tmt/steps/provision/testcloud.py b/tmt/steps/provision/testcloud.py index 74095f7946..6cc13027bf 100644 --- a/tmt/steps/provision/testcloud.py +++ b/tmt/steps/provision/testcloud.py @@ -1293,17 +1293,17 @@ def _print_local_images(self) -> None: click.echo(f"{TESTCLOUD_IMAGES / filename}") @classmethod - def clean_images(cls, clean: 'tmt.base.Clean', dry: bool) -> bool: - """ - Remove the testcloud images - """ + def clean_images(cls, clean: 'tmt.base.Clean', dry: bool, workdir_root: Path) -> bool: + """ Remove the testcloud images """ + testcloud_images = workdir_root / 'testcloud/images' clean.info('testcloud', shift=1, color='green') - if not TESTCLOUD_IMAGES.exists(): - clean.warn(f"Directory '{TESTCLOUD_IMAGES}' does not exist.", shift=2) + if not testcloud_images.exists(): + clean.warn( + f"Directory '{testcloud_images}' does not exist.", shift=2) return True successful = True - for image in TESTCLOUD_IMAGES.iterdir(): + for image in testcloud_images.iterdir(): if dry: clean.verbose(f"Would remove '{image}'.", shift=2) else: From 6fb9a1f42d8dd08ef921e77c913578ae56d3323a Mon Sep 17 00:00:00 2001 From: Lili Nie Date: Mon, 10 Feb 2025 14:48:19 +0800 Subject: [PATCH 2/5] squash:extend test --- tests/clean/images/main.fmf | 1 + tests/clean/images/test.sh | 23 +++++++++++++++++++++++ tmt/cli/_root.py | 3 +-- 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 tests/clean/images/main.fmf create mode 100755 tests/clean/images/test.sh diff --git a/tests/clean/images/main.fmf b/tests/clean/images/main.fmf new file mode 100644 index 0000000000..d3e4a25ac0 --- /dev/null +++ b/tests/clean/images/main.fmf @@ -0,0 +1 @@ +summary: Checks whether tmt clean images works correctly diff --git a/tests/clean/images/test.sh b/tests/clean/images/test.sh new file mode 100755 index 0000000000..ad27519819 --- /dev/null +++ b/tests/clean/images/test.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# vim: dict+=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k +. /usr/share/beakerlib/beakerlib.sh || exit 1 + +rlJournalStart + rlPhaseStartSetup + rlRun "tmp=\$(mktemp -d)" 0 "Create tmp directory" + rlRun "pushd $tmp" + rlRun "set -o pipefail" + rlRun "tmt init" + rlRun "mkdir -p $tmp/testcloud/images" + rlRun "touch $tmp/testcloud/images/dummy" + + rlPhaseStartTest "clean images " + rlRun "tmt clean images -v --workdir-root $tmp" + rlAssertNotExists "$tmp/testcloud/images/dummy" + rlPhaseEnd + + rlPhaseStartCleanup + rlRun "popd" + rlRun "rm -r $tmp" 0 "Remove tmp directory" + rlPhaseEnd +rlJournalEnd diff --git a/tmt/cli/_root.py b/tmt/cli/_root.py index c5ab3bba58..25ab8e1712 100644 --- a/tmt/cli/_root.py +++ b/tmt/cli/_root.py @@ -1853,7 +1853,7 @@ def clean_guests( @workdir_root_options @verbosity_options @dry_options -def clean_images(context: Context, _workdir_root: Optional[str], **kwargs: Any) -> None: +def clean_images(context: Context, workdir_root: Optional[Path], **kwargs: Any) -> None: """ Remove images of supported provision methods. @@ -1866,7 +1866,6 @@ def clean_images(context: Context, _workdir_root: Optional[str], **kwargs: Any) # cleaned, similarly to guests. assert context.obj.clean_logger is not None # narrow type - workdir_root = Path(_workdir_root) if _workdir_root is not None else None if workdir_root and not workdir_root.exists(): raise tmt.utils.GeneralError(f"Path '{workdir_root}' doesn't exist.") From 3e8d59811e8b37ffb16f729f9882b7fc4af4dc08 Mon Sep 17 00:00:00 2001 From: Lili Nie Date: Wed, 12 Feb 2025 11:39:31 +0800 Subject: [PATCH 3/5] squash:cleanup redudant code --- tmt/base.py | 3 ++- tmt/cli/_root.py | 22 ++++++++++------------ tmt/steps/provision/__init__.py | 2 +- tmt/steps/provision/testcloud.py | 2 +- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/tmt/base.py b/tmt/base.py index 49c88a5ebc..2e99e89ac0 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -4319,7 +4319,8 @@ def images(self) -> bool: for method in tmt.steps.provision.ProvisionPlugin.methods(): # FIXME: ignore[union-attr]: https://github.com/teemtee/tmt/issues/1599 if not method.class_.clean_images( # type: ignore[union-attr] - self, self.is_dry_run, self.workdir_root): + self, self.is_dry_run, self.workdir_root + ): successful = False return successful diff --git a/tmt/cli/_root.py b/tmt/cli/_root.py index 25ab8e1712..e40e0a8176 100644 --- a/tmt/cli/_root.py +++ b/tmt/cli/_root.py @@ -1644,8 +1644,6 @@ def clean( if last and id_: raise tmt.utils.GeneralError("Options --last and --id cannot be used together.") - if workdir_root and not workdir_root.exists(): - raise tmt.utils.GeneralError(f"Path '{workdir_root}' doesn't exist.") context.obj.clean_logger = context.obj.logger.descend( logger_name='clean', extra_shift=0 @@ -1662,6 +1660,10 @@ def clean( if context.invoked_subcommand is None: assert context.obj.clean_logger is not None # narrow type workdir_root = effective_workdir_root(workdir_root) + if not workdir_root.exists(): + raise tmt.utils.GeneralError( + f"Path '{workdir_root}' does not exist, skipping guest, run and image cleanup." + ) # Create another level to the hierarchy so that logging indent is # consistent between the command and subcommands clean_obj = tmt.Clean( @@ -1672,15 +1674,10 @@ def clean( cli_invocation=CliInvocation.from_context(context), workdir_root=workdir_root, ) - if workdir_root.exists(): - if 'guests' not in skip and not clean_obj.guests(id_, keep): - exit_code = 1 - if 'runs' not in skip and not clean_obj.runs(id_, keep): - exit_code = 1 - else: - clean_obj.warn( - f"Directory '{workdir_root}' does not exist, skipping guest and run cleanup." - ) + if 'guests' not in skip and not clean_obj.guests(id_, keep): + exit_code = 1 + if 'runs' not in skip and not clean_obj.runs(id_, keep): + exit_code = 1 if 'images' not in skip and not clean_obj.images(): exit_code = 1 raise SystemExit(exit_code) @@ -1875,7 +1872,8 @@ def clean_images(context: Context, workdir_root: Optional[Path], **kwargs: Any) ).apply_verbosity_options(**kwargs), parent=context.obj.clean, cli_invocation=CliInvocation.from_context(context), - workdir_root=effective_workdir_root(workdir_root)) + workdir_root=effective_workdir_root(workdir_root), + ) context.obj.clean_partials["images"].append(clean_obj.images) diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index 46425c28ad..c8b8f71e08 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -2620,7 +2620,7 @@ def options(cls, how: Optional[str] = None) -> list[tmt.options.ClickOptionDecor @classmethod def clean_images(cls, clean: 'tmt.base.Clean', dry: bool, workdir_root: Path) -> bool: - """ Remove the images of one particular plugin """ + """Remove the images of one particular plugin""" return True def show(self, keys: Optional[list[str]] = None) -> None: diff --git a/tmt/steps/provision/testcloud.py b/tmt/steps/provision/testcloud.py index 6cc13027bf..8cd02ca114 100644 --- a/tmt/steps/provision/testcloud.py +++ b/tmt/steps/provision/testcloud.py @@ -1294,7 +1294,7 @@ def _print_local_images(self) -> None: @classmethod def clean_images(cls, clean: 'tmt.base.Clean', dry: bool, workdir_root: Path) -> bool: - """ Remove the testcloud images """ + """Remove the testcloud images""" testcloud_images = workdir_root / 'testcloud/images' clean.info('testcloud', shift=1, color='green') From d2ef96c91604897f504f3683dff3953bebd0f2e4 Mon Sep 17 00:00:00 2001 From: Lili Nie Date: Thu, 13 Feb 2025 23:17:06 +0800 Subject: [PATCH 4/5] squash:ruff-clean --- tmt/steps/provision/testcloud.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tmt/steps/provision/testcloud.py b/tmt/steps/provision/testcloud.py index 8cd02ca114..cc07de038d 100644 --- a/tmt/steps/provision/testcloud.py +++ b/tmt/steps/provision/testcloud.py @@ -1299,8 +1299,7 @@ def clean_images(cls, clean: 'tmt.base.Clean', dry: bool, workdir_root: Path) -> testcloud_images = workdir_root / 'testcloud/images' clean.info('testcloud', shift=1, color='green') if not testcloud_images.exists(): - clean.warn( - f"Directory '{testcloud_images}' does not exist.", shift=2) + clean.warn(f"Directory '{testcloud_images}' does not exist.", shift=2) return True successful = True for image in testcloud_images.iterdir(): From d06f6cda358839872a109bc3b4fafcae7433fa06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Thu, 13 Feb 2025 21:16:35 +0100 Subject: [PATCH 5/5] squash: post-rebase fixes --- tmt/steps/provision/__init__.py | 5 ++++- tmt/steps/provision/testcloud.py | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index c8b8f71e08..770e9c3a5c 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -2620,7 +2620,10 @@ def options(cls, how: Optional[str] = None) -> list[tmt.options.ClickOptionDecor @classmethod def clean_images(cls, clean: 'tmt.base.Clean', dry: bool, workdir_root: Path) -> bool: - """Remove the images of one particular plugin""" + """ + Remove the images of one particular plugin + """ + return True def show(self, keys: Optional[list[str]] = None) -> None: diff --git a/tmt/steps/provision/testcloud.py b/tmt/steps/provision/testcloud.py index cc07de038d..7bf02d9a0b 100644 --- a/tmt/steps/provision/testcloud.py +++ b/tmt/steps/provision/testcloud.py @@ -1294,7 +1294,9 @@ def _print_local_images(self) -> None: @classmethod def clean_images(cls, clean: 'tmt.base.Clean', dry: bool, workdir_root: Path) -> bool: - """Remove the testcloud images""" + """ + Remove the testcloud images + """ testcloud_images = workdir_root / 'testcloud/images' clean.info('testcloud', shift=1, color='green')