feat: add prune instances by Aeonoi · Pull Request #1058 · canonical/craft-application

Aeonoi · 2026-04-10T16:32:24Z

Have you followed the guidelines for contributing?
Have you signed the CLA?
Have you successfully run make lint && make test?
Have you added an entry to the changelog (docs/reference/changelog.rst)?

closes #991

…tency

Copilot

Pull request overview

This PR implements the new prune-instances CLI command requested in #991, wiring it through ProviderService so users can prune managed provider instances (optionally including base/template instances).

Changes:

Add PruneInstancesCommand and register it in the “Other” command group and command exports.
Add ProviderService.prune_instances() to prune instances for a selected provider or across providers.
Add unit tests for the new command and service behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`craft_application/services/provider.py`	Adds `prune_instances()` implementation to call into craft-providers’ prune functionality.
`craft_application/commands/prune_instances.py`	Introduces the new `prune-instances` command and CLI flags.
`craft_application/commands/other.py`	Registers the new command in the “Other” command group.
`craft_application/commands/__init__.py`	Exports `PruneInstancesCommand`.
`tests/unit/services/test_provider.py`	Adds unit tests for provider-service pruning paths.
`tests/unit/commands/test_prune_instances.py`	Adds unit test for command-to-service wiring.
`tests/unit/commands/test_other.py`	Updates “Other commands” group test to include the new command.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mr-cal · 2026-04-24T14:43:55Z

+    parsed_args = argparse.Namespace(
+        all_providers=True,
+        provider="lxd",
+        prune_templates=True,


mr-cal · 2026-04-24T14:57:55Z

+def test_prune_instances_all_providers(monkeypatch, provider_service):
+    """Prune for all supported providers."""
+    mock_lxd = mock.MagicMock()
+    mock_lxd.name = "lxd"
+    mock_multipass = mock.MagicMock()
+    mock_multipass.name = "multipass"
+
+    def mock_get_by_name(name):
+        if name == "lxd":
+            return mock_lxd
+        if name == "multipass":
+            return mock_multipass
+        raise RuntimeError(f"Unknown provider: {name}")
+
+    monkeypatch.setattr(provider_service, "_get_provider_by_name", mock_get_by_name)
+
+    provider_service.prune_instances(all_providers=True, prune_templates=False)
+
+    mock_lxd.prune.assert_called_once_with(prune_templates=False)
+    mock_multipass.prune.assert_called_once_with(prune_templates=False)


I don't think we need to care about this - we're able to assume commands.prune_instances is only providing lowercase provider names to this function.

mr-cal · 2026-04-15T12:15:54Z

+                providers.append(self._get_provider_by_name("LXD"))
+            except RuntimeError:
+                emit.debug("LXD provider not available, skipping.")
+
+            try:
+                providers.append(self._get_provider_by_name("multipass"))
+                providers.append(self._get_provider_by_name("Multipass"))


Agreed, _get_provider_by_name runs lower().strip(), so you can just call it once.

mr-cal · 2026-04-15T12:20:43Z

+            try:
+                providers.append(self._get_provider_by_name("lxd"))
+                providers.append(self._get_provider_by_name("LXD"))
+            except RuntimeError:
+                emit.debug("LXD provider not available, skipping.")
+
+            try:
+                providers.append(self._get_provider_by_name("multipass"))
+                providers.append(self._get_provider_by_name("Multipass"))
+            except RuntimeError:
+                emit.debug("Multipass provider not available, skipping.")
+


The reasoning here is correct, if you decide to do _add_provider_if_available, I make it a class level function rather than a function within a function.

mr-cal · 2026-04-15T12:54:17Z

+    """Prune instances for the active provider."""
+
+    name = "prune-instances"
+    help_msg = "Prune instances for the active provider"
+    overview = textwrap.dedent(
+        """
+        Prune instances for the active provider.
+        """
+    )
+
+    def _fill_parser(self, parser: argparse.ArgumentParser) -> None:
+        parser.add_argument(
+            "--all-providers",
+            action="store_true",
+            help="Prune instances from all providers",
+        )
+        parser.add_argument(
+            "--provider",
+            help="Prune for a specific provider",


+1 on the mutual exclusion

mr-cal

Nice start, I left most of my feedback on top of the copilot review.

mr-cal · 2026-04-15T12:58:19Z

+            action="store_true",
+            help="Prune instances from all providers",
+        )
+        parser.add_argument(


Can you add choices? That will improve the UX by giving quick errors up front if a user provides something besides lxd or multipass.

It will also make your life easier in the prune_instances command, because you'll know the user declared a valid provider name.

lengau · 2026-04-21T21:18:44Z

@copilot create a child PR that fixes the linting issues

Copilot

Pull request overview

Adds a new prune-instances CLI command and supporting service logic to remove managed provider instances (optionally across providers), plus accompanying unit and spread coverage to exercise the behavior end-to-end (per #991).

Changes:

Introduce PruneInstancesCommand and register it under the “Other” command group.
Add ProviderService.prune_instances() (with helper to skip unavailable providers) and unit tests for provider pruning behavior.
Add a spread test scenario for testcraft prune-instances.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`craft_application/services/provider.py`	Adds `prune_instances()` service API and availability-aware provider selection helper.
`craft_application/commands/prune_instances.py`	Implements the new `prune-instances` CLI command and argument parsing.
`craft_application/commands/other.py`	Registers `PruneInstancesCommand` in the “Other” command group.
`craft_application/commands/__init__.py`	Exports `PruneInstancesCommand` from the commands package.
`tests/unit/services/test_provider.py`	Adds unit tests covering provider-service pruning scenarios.
`tests/unit/commands/test_prune_instances.py`	Adds a unit test ensuring the command calls the provider service.
`tests/unit/commands/test_other.py`	Updates command-group expectations to include the new command.
`tests/spread/testcraft/prune-instances/testcraft.yaml`	Adds a minimal testcraft project used by the spread scenario.
`tests/spread/testcraft/prune-instances/task.yaml`	Adds a spread task that creates an LXD instance then prunes it.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mr-cal · 2026-04-24T14:38:47Z

+        providers: list[craft_providers.Provider] = []
+
+        if provider_name:
+            providers.append(self.get_provider(name=provider_name))


Agreed. Nice to see copilot is making the same suggestions I was going to make.

mr-cal · 2026-04-24T14:36:01Z

+    def run(self, parsed_args: argparse.Namespace) -> None:
+        """Run the prune-instances command."""
+        self._services.provider.prune_instances(
+            all_providers=parsed_args.all_providers,
+            provider_name=parsed_args.provider,
+            prune_templates=parsed_args.prune_templates,
+        )


Agreed, I think include_templates accidentally got removed when making the other args mutually exclusive.

Hmm, would the _fill_parser() method add the include_templates or prune_templates flag?

mr-cal · 2026-04-24T14:54:57Z

+    parsed_args = argparse.Namespace(
+        all_providers=True,
+        provider="lxd",
+        prune_templates=True,
+    )


Agreed - these argparse values don't reflect what a user would be able to provide at the command line.

IIRC, manually defining parsed_args = argparse.Namespace(...) in the test bypasses code like the mutually exclusive args. To test that code, you have to mock argv and assert the error with capsys (example).

I think you can do this in 2 tests. One capsys-based test that ensures the args are mutually exclusive and another test that verifies the namespace args are successfully passed to provider.prune_instances(...).

You already have the second test written, but I recommend adding parametrizing it with @pytest.mark.parameterize() such that the test ensures what you set in parsed_args = argparse.Namespace(...) actually get passed to provider.prune_instances(...).

mr-cal · 2026-04-24T15:01:49Z

+
+    mock_lxd.prune.assert_called_once_with(prune_templates=True)
+
+


I think you can ignore this, as this test needs to be updated once https://github.com/canonical/craft-application/pull/1058/changes#r3120374251 is accomdated.

mr-cal · 2026-04-24T14:34:26Z

+  fi
+
+restore: |
+  testcraft prune-instances --all


Here too:

Suggested change

testcraft prune-instances --all

testcraft prune-instances --all-providers

mr-cal · 2026-04-24T14:36:01Z

+    def run(self, parsed_args: argparse.Namespace) -> None:
+        """Run the prune-instances command."""
+        self._services.provider.prune_instances(
+            all_providers=parsed_args.all_providers,
+            provider_name=parsed_args.provider,
+            prune_templates=parsed_args.prune_templates,
+        )


Agreed, I think include_templates accidentally got removed when making the other args mutually exclusive.

mr-cal · 2026-04-24T14:38:47Z

+        providers: list[craft_providers.Provider] = []
+
+        if provider_name:
+            providers.append(self.get_provider(name=provider_name))


Agreed. Nice to see copilot is making the same suggestions I was going to make.

mr-cal · 2026-04-24T14:43:55Z

+    parsed_args = argparse.Namespace(
+        all_providers=True,
+        provider="lxd",
+        prune_templates=True,


mr-cal · 2026-04-24T14:54:57Z

+    parsed_args = argparse.Namespace(
+        all_providers=True,
+        provider="lxd",
+        prune_templates=True,
+    )


Agreed - these argparse values don't reflect what a user would be able to provide at the command line.

IIRC, manually defining parsed_args = argparse.Namespace(...) in the test bypasses code like the mutually exclusive args. To test that code, you have to mock argv and assert the error with capsys (example).

I think you can do this in 2 tests. One capsys-based test that ensures the args are mutually exclusive and another test that verifies the namespace args are successfully passed to provider.prune_instances(...).

You already have the second test written, but I recommend adding parametrizing it with @pytest.mark.parameterize() such that the test ensures what you set in parsed_args = argparse.Namespace(...) actually get passed to provider.prune_instances(...).

mr-cal · 2026-04-24T14:57:55Z

+def test_prune_instances_all_providers(monkeypatch, provider_service):
+    """Prune for all supported providers."""
+    mock_lxd = mock.MagicMock()
+    mock_lxd.name = "lxd"
+    mock_multipass = mock.MagicMock()
+    mock_multipass.name = "multipass"
+
+    def mock_get_by_name(name):
+        if name == "lxd":
+            return mock_lxd
+        if name == "multipass":
+            return mock_multipass
+        raise RuntimeError(f"Unknown provider: {name}")
+
+    monkeypatch.setattr(provider_service, "_get_provider_by_name", mock_get_by_name)
+
+    provider_service.prune_instances(all_providers=True, prune_templates=False)
+
+    mock_lxd.prune.assert_called_once_with(prune_templates=False)
+    mock_multipass.prune.assert_called_once_with(prune_templates=False)


I don't think we need to care about this - we're able to assume commands.prune_instances is only providing lowercase provider names to this function.

mr-cal · 2026-04-24T15:01:49Z

+
+    mock_lxd.prune.assert_called_once_with(prune_templates=True)
+
+


I think you can ignore this, as this test needs to be updated once https://github.com/canonical/craft-application/pull/1058/changes#r3120374251 is accomdated.

mr-cal · 2026-04-24T15:02:43Z

Can you add a third unit test for when provider_name = None? It should call get_provider(), which you can mock to return lxd.

Co-authored-by: Callahan Kovacs <callahan.kovacs@canonical.com> Signed-off-by: Dylan <143847987+Aeonoi@users.noreply.github.com>

…tion into add-prune-instances

Aeonoi · 2026-04-24T17:25:17Z

How would I go about testing for the prune method since the current craft-provider version doesn't have the prune method. So I'm not sure if the prune-instances method is actually done right when running it locally.

mr-cal · 2026-04-24T20:07:20Z

@Aeonoi, you can update the pyproject.toml and run uv sync to update the lockfile.

Feel free to push that change to this PR. If the tests succeed in this PR, then we can make a new craft-providers release and you can update the pyproject.toml to the new release.

Here's an example: f155f77

Aeonoi · 2026-04-29T17:47:09Z

Here's an example: f155f77

This worked, thanks.

mr-cal · 2026-05-05T14:47:00Z

Great! By the way, there is now a 3.6.0 release of craft-providers.

https://canonical-craft-providers.readthedocs-hosted.com/en/latest/reference/changelog/

Aeonoi and others added 5 commits April 7, 2026 20:54

feat: add prune-instances command

45bd767

Merge branch 'canonical:main' into add-prune-instances

4cade65

tests: add test cases for prune_instances

43a8250

refactor: use prune_templates instead of include_templates for consis…

91c8ffb

…tency

Merge branch 'main' into add-prune-instances

c3999ec

Aeonoi marked this pull request as ready for review April 14, 2026 16:35

Aeonoi requested a review from a team as a code owner April 14, 2026 16:35

Copilot AI review requested due to automatic review settings April 14, 2026 16:35

Copilot started reviewing on behalf of Aeonoi April 14, 2026 16:36 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

mr-cal reviewed Apr 15, 2026

View reviewed changes

Aeonoi added 4 commits April 15, 2026 13:35

refactor prune_instances command

fef7796

give choices for prune_instances command for selecting provider

5b38cc6

add test spread for prune instances

dbc0068

fix test spread for prune instances

1449184

lengau requested a review from Copilot April 21, 2026 21:18

Copilot started reviewing on behalf of lengau April 21, 2026 21:18 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

mr-cal reviewed Apr 24, 2026

View reviewed changes

Aeonoi and others added 4 commits April 24, 2026 12:14

Update tests/spread/testcraft/prune-instances/task.yaml

1c06d19

Co-authored-by: Callahan Kovacs <callahan.kovacs@canonical.com> Signed-off-by: Dylan <143847987+Aeonoi@users.noreply.github.com>

fix up testcraft task.yaml for prune-instances

5b7dad5

Merge branch 'add-prune-instances' of github.com:aeonoi/craft-applica…

3deae89

…tion into add-prune-instances

add unit test for prune-instance with provider_name is None

51fc6d5


		mock_lxd.prune.assert_called_once_with(prune_templates=True)

	testcraft prune-instances --all
	testcraft prune-instances --all-providers

Conversation

Aeonoi commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mr-cal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lengau commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aeonoi commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mr-cal commented Apr 24, 2026

Uh oh!

Aeonoi commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mr-cal commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Aeonoi commented Apr 10, 2026 •

edited

Loading

Aeonoi commented Apr 24, 2026 •

edited

Loading

Aeonoi commented Apr 29, 2026 •

edited

Loading