Skip to content

Commit 64d0ebb

Browse files
authored
Add guards to cluster deletion from cli (#16053)
Adds guards to cluster deletion. - If cluster has running apps -> throw an error - If cluster has stopped apps -> confirm w/ user that apps and logs will be deleted
1 parent ebe7848 commit 64d0ebb

File tree

2 files changed

+143
-1
lines changed

2 files changed

+143
-1
lines changed

src/lightning_app/cli/cmd_clusters.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import lightning_cloud
1010
from lightning_cloud.openapi import (
1111
Externalv1Cluster,
12+
Externalv1LightningappInstance,
1213
V1AWSClusterDriverSpec,
1314
V1ClusterDriver,
1415
V1ClusterPerformanceProfile,
@@ -18,13 +19,17 @@
1819
V1CreateClusterRequest,
1920
V1GetClusterResponse,
2021
V1KubernetesClusterDriver,
22+
V1LightningappInstanceState,
23+
V1ListLightningappInstancesResponse,
24+
V1Membership,
2125
)
2226
from lightning_utilities.core.enums import StrEnum
2327
from rich.console import Console
2428
from rich.table import Table
2529
from rich.text import Text
2630

2731
from lightning_app.cli.core import Formatable
32+
from lightning_app.utilities.cloud import _get_project
2833
from lightning_app.utilities.network import LightningClient
2934
from lightning_app.utilities.openapi import create_openapi_object, string2dict
3035

@@ -198,6 +203,33 @@ def delete(self, cluster_id: str, force: bool = False, do_async: bool = False) -
198203
)
199204
click.confirm("Do you want to continue?", abort=True)
200205

206+
else:
207+
apps = _list_apps(self.api_client, cluster_id=cluster_id, phase_in=[V1LightningappInstanceState.RUNNING])
208+
if apps:
209+
raise click.ClickException(
210+
dedent(
211+
"""\
212+
To delete the cluster, you must first delete the apps running on it.
213+
Use the following commands to delete the apps, then delete the cluster again:
214+
215+
"""
216+
)
217+
+ "\n".join([f"\tlightning delete app {app.name} --cluster-id {cluster_id}" for app in apps])
218+
)
219+
220+
if _list_apps(self.api_client, cluster_id=cluster_id, phase_not_in=[V1LightningappInstanceState.DELETED]):
221+
click.echo(
222+
dedent(
223+
"""\
224+
This cluster has stopped apps.
225+
Deleting this cluster will delete those apps and their logs.
226+
227+
App artifacts aren't deleted and will still be available in the S3 bucket for the cluster.
228+
"""
229+
)
230+
)
231+
click.confirm("Are you sure you want to continue?", abort=True)
232+
201233
resp: V1GetClusterResponse = self.api_client.cluster_service_get_cluster(id=cluster_id)
202234
bucket_name = resp.spec.driver.kubernetes.aws.bucket_name
203235

@@ -226,6 +258,28 @@ def delete(self, cluster_id: str, force: bool = False, do_async: bool = False) -
226258
click.echo(background_message)
227259

228260

261+
def _list_apps(
262+
api_client: LightningClient,
263+
**filters: Any,
264+
) -> List[Externalv1LightningappInstance]:
265+
"""_list_apps is a thin wrapper around lightningapp_instance_service_list_lightningapp_instances.
266+
267+
Args:
268+
api_client (LightningClient): Used for listing app instances
269+
**filters: keyword arguments passed to the list method
270+
271+
Returns:
272+
List[Externalv1LightningappInstance]: List of apps matching the filters
273+
"""
274+
project: V1Membership = _get_project(api_client)
275+
resp: V1ListLightningappInstancesResponse = api_client.lightningapp_instance_service_list_lightningapp_instances(
276+
project.project_id,
277+
**filters,
278+
)
279+
280+
return resp.lightningapps
281+
282+
229283
def _wait_for_cluster_state(
230284
api_client: LightningClient,
231285
cluster_id: str,

tests/tests_app/cli/test_cmd_clusters.py

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import click
55
import pytest
66
from lightning_cloud.openapi import (
7+
Externalv1LightningappInstance,
78
V1AWSClusterDriverSpec,
89
V1ClusterDriver,
910
V1ClusterPerformanceProfile,
@@ -14,6 +15,11 @@
1415
V1CreateClusterRequest,
1516
V1GetClusterResponse,
1617
V1KubernetesClusterDriver,
18+
V1LightningappInstanceState,
19+
V1LightningappInstanceStatus,
20+
V1ListLightningappInstancesResponse,
21+
V1ListMembershipsResponse,
22+
V1Membership,
1723
)
1824

1925
from lightning_app.cli import cmd_clusters
@@ -95,17 +101,99 @@ def test_list_clusters(api: mock.MagicMock):
95101
api.assert_called_once_with(phase_not_in=[V1ClusterState.DELETED])
96102

97103

104+
@pytest.fixture()
105+
def fixture_list_instances_empty():
106+
return V1ListLightningappInstancesResponse([])
107+
108+
98109
@mock.patch("lightning_cloud.login.Auth.authenticate", MagicMock())
110+
@mock.patch("lightning_app.utilities.network.LightningClient.projects_service_list_memberships")
111+
@mock.patch("lightning_app.utilities.network.LightningClient.lightningapp_instance_service_list_lightningapp_instances")
99112
@mock.patch("lightning_app.utilities.network.LightningClient.cluster_service_delete_cluster")
100113
@mock.patch("lightning_app.utilities.network.LightningClient.cluster_service_get_cluster")
101-
def test_delete_cluster_api(api_get: mock.MagicMock, api_delete: mock.MagicMock, async_or_interrupt, spec):
114+
def test_delete_cluster_api(
115+
api_get: mock.MagicMock,
116+
api_delete: mock.MagicMock,
117+
api_list_instances: mock.MagicMock,
118+
api_list_memberships: mock.MagicMock,
119+
async_or_interrupt,
120+
spec,
121+
fixture_list_instances_empty,
122+
):
123+
api_list_memberships.return_value = V1ListMembershipsResponse([V1Membership(project_id="test-project")])
124+
api_list_instances.return_value = fixture_list_instances_empty
102125
api_get.return_value = V1GetClusterResponse(spec=spec)
103126
cluster_manager = AWSClusterManager()
104127
cluster_manager.delete(cluster_id="test-7", do_async=async_or_interrupt)
105128

106129
api_delete.assert_called_once_with(id="test-7", force=False)
107130

108131

132+
@mock.patch("click.confirm")
133+
@mock.patch("lightning_cloud.login.Auth.authenticate", MagicMock())
134+
@mock.patch("lightning_app.utilities.network.LightningClient.projects_service_list_memberships")
135+
@mock.patch("lightning_app.utilities.network.LightningClient.lightningapp_instance_service_list_lightningapp_instances")
136+
@mock.patch("lightning_app.utilities.network.LightningClient.cluster_service_delete_cluster")
137+
@mock.patch("lightning_app.utilities.network.LightningClient.cluster_service_get_cluster")
138+
def test_delete_cluster_with_stopped_apps(
139+
api_get: mock.MagicMock,
140+
api_delete: mock.MagicMock,
141+
api_list_instances: mock.MagicMock,
142+
api_list_memberships: mock.MagicMock,
143+
click_mock: mock.MagicMock,
144+
spec,
145+
):
146+
api_list_memberships.return_value = V1ListMembershipsResponse([V1Membership(project_id="test-project")])
147+
api_list_instances.side_effect = [
148+
# when querying for running apps
149+
V1ListLightningappInstancesResponse([]),
150+
# when querying for stopped apps
151+
V1ListLightningappInstancesResponse(
152+
lightningapps=[
153+
Externalv1LightningappInstance(
154+
status=V1LightningappInstanceStatus(
155+
phase=V1LightningappInstanceState.STOPPED,
156+
)
157+
)
158+
]
159+
),
160+
]
161+
api_get.return_value = V1GetClusterResponse(spec=spec)
162+
cluster_manager = AWSClusterManager()
163+
164+
cluster_manager.delete(cluster_id="test-7", do_async=True)
165+
api_delete.assert_called_once_with(id="test-7", force=False)
166+
click_mock.assert_called_once_with("Are you sure you want to continue?", abort=True)
167+
168+
169+
@mock.patch("lightning_cloud.login.Auth.authenticate", MagicMock())
170+
@mock.patch("lightning_app.utilities.network.LightningClient.projects_service_list_memberships")
171+
@mock.patch("lightning_app.utilities.network.LightningClient.lightningapp_instance_service_list_lightningapp_instances")
172+
@mock.patch("lightning_app.utilities.network.LightningClient.cluster_service_get_cluster")
173+
def test_delete_cluster_with_running_apps(
174+
api_get: mock.MagicMock,
175+
api_list_instances: mock.MagicMock,
176+
api_list_memberships: mock.MagicMock,
177+
spec,
178+
):
179+
api_list_memberships.return_value = V1ListMembershipsResponse([V1Membership(project_id="test-project")])
180+
api_list_instances.return_value = V1ListLightningappInstancesResponse(
181+
lightningapps=[
182+
Externalv1LightningappInstance(
183+
status=V1LightningappInstanceStatus(
184+
phase=V1LightningappInstanceState.RUNNING,
185+
)
186+
)
187+
]
188+
)
189+
api_get.return_value = V1GetClusterResponse(spec=spec)
190+
cluster_manager = AWSClusterManager()
191+
192+
with pytest.raises(click.ClickException) as exception:
193+
cluster_manager.delete(cluster_id="test-7")
194+
exception.match("apps running")
195+
196+
109197
class Test_check_cluster_id_is_valid:
110198
@pytest.mark.parametrize("name", ["test-7", "0wildgoat"])
111199
def test_valid(self, name):

0 commit comments

Comments
 (0)