From b4d99e3cc127b04d47b3ecaed9cc42f05cc14c35 Mon Sep 17 00:00:00 2001 From: Rick Izzo Date: Mon, 5 Dec 2022 16:51:32 -0500 Subject: [PATCH] Add CLI Command to Delete Lightning App (#15783) * initial work on deleting apps * after PR review * delete CLI working * restructred to make tests easier * revert manifest changes * added changelog, fix mypy issue * updates * Update src/lightning_app/cli/cmd_apps.py Co-authored-by: Jirka Borovec <6035284+Borda@users.noreply.github.com> * Update src/lightning_app/cli/lightning_cli_delete.py Co-authored-by: Jirka Borovec <6035284+Borda@users.noreply.github.com> * Update src/lightning_app/cli/lightning_cli_delete.py Co-authored-by: Jirka Borovec <6035284+Borda@users.noreply.github.com> * Update src/lightning_app/cli/lightning_cli_delete.py Co-authored-by: Sherin Thomas * Update src/lightning_app/cli/lightning_cli_delete.py Co-authored-by: Sherin Thomas * import typing * adding tests * finished adding tests * addressed code review comments * fix mypy error * make mypy happy * make mypy happy * make mypy happy * make mypy happy * fix windows cli Co-authored-by: Jirka Borovec <6035284+Borda@users.noreply.github.com> Co-authored-by: Sherin Thomas --- src/lightning_app/CHANGELOG.md | 1 + src/lightning_app/cli/cmd_apps.py | 7 + src/lightning_app/cli/cmd_clusters.py | 4 + src/lightning_app/cli/lightning_cli_delete.py | 169 ++++++++++++++++++ tests/tests_app/cli/test_cmd_apps.py | 15 ++ tests/tests_app/cli/test_cmd_cli_delete.py | 84 +++++++++ 6 files changed, 280 insertions(+) create mode 100644 tests/tests_app/cli/test_cmd_cli_delete.py diff --git a/src/lightning_app/CHANGELOG.md b/src/lightning_app/CHANGELOG.md index b7e01e77224cb..a1bfe36a09d8d 100644 --- a/src/lightning_app/CHANGELOG.md +++ b/src/lightning_app/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Added - Added the CLI command `lightning run model` to launch a `LightningLite` accelerated script ([#15506](https://github.com/Lightning-AI/lightning/pull/15506)) +- Added the CLI command `lightning delete app` to delete a lightning app on the cloud ([#15783](https://github.com/Lightning-AI/lightning/pull/15783)) - Show a message when `BuildConfig(requirements=[...])` is passed but a `requirements.txt` file is already present in the Work ([#15799](https://github.com/Lightning-AI/lightning/pull/15799)) - Show a message when `BuildConfig(dockerfile="...")` is passed but a `Dockerfile` file is already present in the Work ([#15799](https://github.com/Lightning-AI/lightning/pull/15799)) diff --git a/src/lightning_app/cli/cmd_apps.py b/src/lightning_app/cli/cmd_apps.py index 3edc97449836b..2f9943054e00e 100644 --- a/src/lightning_app/cli/cmd_apps.py +++ b/src/lightning_app/cli/cmd_apps.py @@ -67,6 +67,13 @@ def list(self, cluster_id: str = None, limit: int = 100) -> None: console = Console() console.print(_AppList(self.list_apps(cluster_id=cluster_id, limit=limit)).as_table()) + def delete(self, app_id: str) -> None: + project = _get_project(self.api_client) + self.api_client.lightningapp_instance_service_delete_lightningapp_instance( + project_id=project.project_id, + id=app_id, + ) + class _AppList(Formatable): def __init__(self, apps: List[Externalv1LightningappInstance]) -> None: diff --git a/src/lightning_app/cli/cmd_clusters.py b/src/lightning_app/cli/cmd_clusters.py index 839e8ca306b9c..ac491609052f8 100644 --- a/src/lightning_app/cli/cmd_clusters.py +++ b/src/lightning_app/cli/cmd_clusters.py @@ -174,6 +174,10 @@ def create( except KeyboardInterrupt: click.echo(background_message) + def list_clusters(self) -> List[Externalv1Cluster]: + resp = self.api_client.cluster_service_list_clusters(phase_not_in=[V1ClusterState.DELETED]) + return resp.clusters + def get_clusters(self) -> ClusterList: resp = self.api_client.cluster_service_list_clusters(phase_not_in=[V1ClusterState.DELETED]) return ClusterList(resp.clusters) diff --git a/src/lightning_app/cli/lightning_cli_delete.py b/src/lightning_app/cli/lightning_cli_delete.py index cf9915cd6f832..675749c66dbd3 100644 --- a/src/lightning_app/cli/lightning_cli_delete.py +++ b/src/lightning_app/cli/lightning_cli_delete.py @@ -1,5 +1,12 @@ +from typing import Optional + import click +import inquirer +from inquirer.themes import GreenPassion +from lightning_cloud.openapi import V1ClusterType +from rich.console import Console +from lightning_app.cli.cmd_apps import _AppManager from lightning_app.cli.cmd_clusters import AWSClusterManager from lightning_app.cli.cmd_ssh_keys import _SSHKeyManager @@ -40,6 +47,168 @@ def delete_cluster(cluster: str, force: bool = False, do_async: bool = False) -> cluster_manager.delete(cluster_id=cluster, force=force, do_async=do_async) +def _find_cluster_for_user(app_name: str, cluster_id: Optional[str]) -> str: + console = Console() + cluster_manager = AWSClusterManager() + + default_cluster, valid_cluster_list = None, [] + for cluster in cluster_manager.list_clusters(): + valid_cluster_list.append(cluster.id) + if cluster.spec.cluster_type == V1ClusterType.GLOBAL and default_cluster is None: + default_cluster = cluster.id + break + + # when no cluster-id is passed in, use the default (Lightning Cloud) cluster + if cluster_id is None: + cluster_id = default_cluster + + if cluster_id not in valid_cluster_list: + console.print(f'[b][yellow]You don\'t have access to cluster "{cluster_id}"[/yellow][/b]') + if len(valid_cluster_list) == 1: + # if there are no BYOC clusters, then confirm that + # the user wants to fall back to the Lightning Cloud. + try: + ask = [ + inquirer.Confirm( + "confirm", + message=f'Did you mean to specify the default Lightning Cloud cluster "{default_cluster}"?', + default=True, + ), + ] + if inquirer.prompt(ask, theme=GreenPassion(), raise_keyboard_interrupt=True)["confirm"] is False: + console.print("[b][red]Aborted![/b][/red]") + raise InterruptedError + except KeyboardInterrupt: + console.print("[b][red]Cancelled by user![/b][/red]") + raise InterruptedError + cluster_id = default_cluster + else: + # When there are BYOC clusters, have them select which cluster to use from all available. + try: + ask = [ + inquirer.List( + "cluster", + message=f'Please select which cluster app "{app_name}" should be deleted from', + choices=valid_cluster_list, + default=default_cluster, + ), + ] + cluster_id = inquirer.prompt(ask, theme=GreenPassion(), raise_keyboard_interrupt=True)["cluster"] + except KeyboardInterrupt: + console.print("[b][red]Cancelled by user![/b][/red]") + raise InterruptedError + + if cluster_id is None: + # stupid mypy thing... + cluster_id = "" + + return cluster_id + + +def _find_selected_app_instance_id(app_name: str, cluster_id: str) -> str: + console = Console() + app_manager = _AppManager() + + all_app_names_and_ids = {} + selected_app_instance_id = None + + for app in app_manager.list_apps(cluster_id=cluster_id): + all_app_names_and_ids[app.name] = app.id + # figure out the ID of some app_name on the cluster. + if app_name == app.name or app_name == app.id: + selected_app_instance_id = app.id + break + + if selected_app_instance_id is None: + # when there is no app with the given app_name on the cluster, + # ask the user which app they would like to delete. + console.print(f'[b][yellow]Cluster "{cluster_id}" does not have an app named "{app_name}"[/yellow][/b]') + try: + ask = [ + inquirer.List( + "app_name", + message="Select the app name to delete", + choices=list(all_app_names_and_ids.keys()), + ), + ] + app_name = inquirer.prompt(ask, theme=GreenPassion(), raise_keyboard_interrupt=True)["app_name"] + selected_app_instance_id = all_app_names_and_ids[app_name] + except KeyboardInterrupt: + console.print("[b][red]Cancelled by user![/b][/red]") + raise InterruptedError + + return selected_app_instance_id + + +def _delete_app_confirmation_prompt(app_name: str, cluster_id: str) -> None: + console = Console() + + # when the --yes / -y flags were not passed, do a final + # confirmation that the user wants to delete the app. + try: + ask = [ + inquirer.Confirm( + "confirm", + message=f'Are you sure you want to delete app "{app_name}" on cluster "{cluster_id}"?', + default=False, + ), + ] + if inquirer.prompt(ask, theme=GreenPassion(), raise_keyboard_interrupt=True)["confirm"] is False: + console.print("[b][red]Aborted![/b][/red]") + raise InterruptedError + except KeyboardInterrupt: + console.print("[b][red]Cancelled by user![/b][/red]") + raise InterruptedError + + +@delete.command("app") +@click.argument("app-name", type=str) +@click.option( + "--cluster-id", + type=str, + default=None, + help="Delete the Lighting App from a specific Lightning AI BYOC compute cluster", +) +@click.option( + "skip_user_confirm_prompt", + "--yes", + "-y", + is_flag=True, + default=False, + help="Do not prompt for confirmation.", +) +def delete_app(app_name: str, cluster_id: str, skip_user_confirm_prompt: bool) -> None: + """Delete a Lightning app. + + Deleting an app also deletes all app websites, works, artifacts, and logs. This permanently removes any record of + the app as well as all any of its associated resources and data. This does not affect any resources and data + associated with other Lightning apps on your account. + """ + console = Console() + + try: + cluster_id = _find_cluster_for_user(app_name=app_name, cluster_id=cluster_id) + selected_app_instance_id = _find_selected_app_instance_id(app_name=app_name, cluster_id=cluster_id) + if not skip_user_confirm_prompt: + _delete_app_confirmation_prompt(app_name=app_name, cluster_id=cluster_id) + except InterruptedError: + return + + try: + # Delete the app! + app_manager = _AppManager() + app_manager.delete(app_id=selected_app_instance_id) + except Exception as e: + console.print( + f'[b][red]An issue occurred while deleting app "{app_name}. If the issue persists, please ' + "reach out to us at [link=mailto:support@lightning.ai]support@lightning.ai[/link][/b][/red]." + ) + raise click.ClickException(str(e)) + + console.print(f'[b][green]App "{app_name}" has been successfully deleted from cluster "{cluster_id}"![/green][/b]') + return + + @delete.command("ssh-key") @click.argument("key_id") def remove_ssh_key(key_id: str) -> None: diff --git a/tests/tests_app/cli/test_cmd_apps.py b/tests/tests_app/cli/test_cmd_apps.py index e579c673ac3d6..150d7aa077ab4 100644 --- a/tests/tests_app/cli/test_cmd_apps.py +++ b/tests/tests_app/cli/test_cmd_apps.py @@ -141,3 +141,18 @@ def test_list_apps_on_cluster(list_memberships: mock.MagicMock, list_instances: list_memberships.assert_called_once() list_instances.assert_called_once_with(project_id="default-project", cluster_id="12345", limit=100, phase_in=[]) + + +@mock.patch("lightning_cloud.login.Auth.authenticate", MagicMock()) +@mock.patch( + "lightning_app.utilities.network.LightningClient.lightningapp_instance_service_delete_lightningapp_instance" +) +@mock.patch("lightning_app.cli.cmd_apps._get_project") +def test_delete_app_on_cluster(get_project_mock: mock.MagicMock, delete_app_mock: mock.MagicMock): + get_project_mock.return_value = V1Membership(project_id="default-project") + + cluster_manager = _AppManager() + cluster_manager.delete(app_id="12345") + + delete_app_mock.assert_called() + delete_app_mock.assert_called_once_with(project_id="default-project", id="12345") diff --git a/tests/tests_app/cli/test_cmd_cli_delete.py b/tests/tests_app/cli/test_cmd_cli_delete.py new file mode 100644 index 0000000000000..438ef064abad4 --- /dev/null +++ b/tests/tests_app/cli/test_cmd_cli_delete.py @@ -0,0 +1,84 @@ +import sys +from unittest import mock + +import pytest +from lightning_cloud.openapi import Externalv1Cluster, Externalv1LightningappInstance, V1ClusterSpec, V1ClusterType + +from lightning_app.cli.lightning_cli_delete import _find_cluster_for_user, _find_selected_app_instance_id + + +@pytest.mark.skipif(sys.platform == "win32", reason="currently not supported for windows.") +@mock.patch("lightning_app.cli.lightning_cli_delete.AWSClusterManager.list_clusters") +def test_find_cluster_for_user_when_provided_valid_cluster_id(list_clusters_mock: mock.MagicMock): + list_clusters_mock.return_value = [ + Externalv1Cluster( + id="default", + spec=V1ClusterSpec( + cluster_type=V1ClusterType.GLOBAL, + ), + ), + Externalv1Cluster( + id="custom", + spec=V1ClusterSpec( + cluster_type=V1ClusterType.BYOC, + ), + ), + ] + returned_cluster_id = _find_cluster_for_user(app_name="whatever", cluster_id="custom") + assert returned_cluster_id == "custom" + + +@pytest.mark.skipif(sys.platform == "win32", reason="currently not supported for windows.") +@mock.patch("lightning_app.cli.lightning_cli_delete.AWSClusterManager.list_clusters") +def test_find_cluster_for_user_without_cluster_id_uses_default(list_clusters_mock: mock.MagicMock): + list_clusters_mock.return_value = [ + Externalv1Cluster( + id="default", + spec=V1ClusterSpec( + cluster_type=V1ClusterType.GLOBAL, + ), + ) + ] + returned_cluster_id = _find_cluster_for_user(app_name="whatever", cluster_id=None) + assert returned_cluster_id == "default" + + +@pytest.mark.skipif(sys.platform == "win32", reason="currently not supported for windows.") +@mock.patch("lightning_app.cli.lightning_cli_delete.AWSClusterManager.list_clusters") +@mock.patch("lightning_app.cli.lightning_cli_delete.inquirer") +def test_find_cluster_for_user_without_valid_cluster_id_asks_if_they_meant_to_use_valid( + list_clusters_mock: mock.MagicMock, + inquirer_mock: mock.MagicMock, +): + list_clusters_mock.return_value = [ + Externalv1Cluster( + id="default", + spec=V1ClusterSpec( + cluster_type=V1ClusterType.GLOBAL, + ), + ) + ] + _find_cluster_for_user(app_name="whatever", cluster_id="does-not-exist") + inquirer_mock.assert_called() + + +@pytest.mark.skipif(sys.platform == "win32", reason="currently not supported for windows.") +@mock.patch("lightning_app.cli.lightning_cli_delete._AppManager.list_apps") +def test_app_find_selected_app_instance_id_when_app_name_exists(list_apps_mock: mock.MagicMock): + list_apps_mock.return_value = [ + Externalv1LightningappInstance(name="app-name", id="app-id"), + ] + returned_app_instance_id = _find_selected_app_instance_id(app_name="app-name", cluster_id="default-cluster") + assert returned_app_instance_id == "app-id" + list_apps_mock.assert_called_once_with(cluster_id="default-cluster") + + +@pytest.mark.skipif(sys.platform == "win32", reason="currently not supported for windows.") +@mock.patch("lightning_app.cli.lightning_cli_delete._AppManager.list_apps") +def test_app_find_selected_app_instance_id_when_app_id_exists(list_apps_mock: mock.MagicMock): + list_apps_mock.return_value = [ + Externalv1LightningappInstance(name="app-name", id="app-id"), + ] + returned_app_instance_id = _find_selected_app_instance_id(app_name="app-id", cluster_id="default-cluster") + assert returned_app_instance_id == "app-id" + list_apps_mock.assert_called_once_with(cluster_id="default-cluster")