diff --git a/src/lightning_app/CHANGELOG.md b/src/lightning_app/CHANGELOG.md index 1b38b2d35546d..7f0000d1e448d 100644 --- a/src/lightning_app/CHANGELOG.md +++ b/src/lightning_app/CHANGELOG.md @@ -25,6 +25,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed and improvements of login flow ([#16052](https://github.com/Lightning-AI/lightning/pull/16052)) - Fixed the debugger detection mechanism for lightning App in VSCode ([#16068](https://github.com/Lightning-AI/lightning/pull/16068)) +- Fixed a bug where apps that had previously been deleted could not be run again from the CLI ([#16082](https://github.com/Lightning-AI/lightning/pull/16082)) + ## [1.8.4] - 2022-12-08 diff --git a/src/lightning_app/runners/cloud.py b/src/lightning_app/runners/cloud.py index 8a7170ee0244a..0011c29ce0e6b 100644 --- a/src/lightning_app/runners/cloud.py +++ b/src/lightning_app/runners/cloud.py @@ -320,52 +320,58 @@ def dispatch( self._ensure_cluster_project_binding(project.project_id, cluster_id) # Resolve the app name, instance, and cluster ID + existing_app = None existing_instance = None app_name = app_config.name - # List existing instances + # List existing apps # TODO: Add pagination, otherwise this could break if users have a lot of apps. - find_instances_resp = self.backend.client.lightningapp_instance_service_list_lightningapp_instances( + all_apps = self.backend.client.lightningapp_v2_service_list_lightningapps_v2( project_id=project.project_id - ) + ).lightningapps - # Seach for instances with the given name (possibly with some random characters appended) + # Seach for apps with the given name (possibly with some random characters appended) pattern = re.escape(f"{app_name}-") + ".{4}" - instances = [ + all_apps = [ lightningapp - for lightningapp in find_instances_resp.lightningapps + for lightningapp in all_apps if lightningapp.name == app_name or (re.fullmatch(pattern, lightningapp.name) is not None) ] - # If instances exist and cluster is None, mimic cluster selection logic to choose a default - if cluster_id is None and len(instances) > 0: + # If apps exist and cluster is None, mimic cluster selection logic to choose a default + if cluster_id is None and len(all_apps) > 0: # Determine the cluster ID cluster_id = self._get_default_cluster(project.project_id) # If an instance exists on the cluster with the same base name - restart it - for instance in instances: - if instance.spec.cluster_id == cluster_id: - existing_instance = instance + for app in all_apps: + instances = self.backend.client.lightningapp_instance_service_list_lightningapp_instances( + project_id=project.project_id, + app_id=app.id, + ).lightningapps + if instances and instances[0].spec.cluster_id == cluster_id: + existing_app = app + existing_instance = instances[0] break - # If instances exist but not on the cluster - choose a randomised name - if len(instances) > 0 and existing_instance is None: + # If apps exist but not on the cluster - choose a randomised name + if len(all_apps) > 0 and existing_app is None: name_exists = True while name_exists: random_name = self._randomise_name(app_name) - name_exists = any([instance.name == random_name for instance in instances]) + name_exists = any([app.name == random_name for app in all_apps]) app_name = random_name # Create the app if it doesn't exist - if existing_instance is None: + if existing_app is None: app_body = Body7(name=app_name, can_download_source_code=True) lit_app = self.backend.client.lightningapp_v2_service_create_lightningapp_v2( project_id=project.project_id, body=app_body ) app_id = lit_app.id else: - app_id = existing_instance.spec.app_id + app_id = existing_app.id # check if user has sufficient credits to run an app # if so set the desired state to running otherwise, create the app in stopped state, diff --git a/tests/tests_app/cli/test_cloud_cli.py b/tests/tests_app/cli/test_cloud_cli.py index 598a90368da88..3b9317a3a9613 100644 --- a/tests/tests_app/cli/test_cloud_cli.py +++ b/tests/tests_app/cli/test_cloud_cli.py @@ -11,6 +11,7 @@ from lightning_cloud.openapi import ( V1LightningappV2, V1ListLightningappInstancesResponse, + V1ListLightningappsV2Response, V1ListMembershipsResponse, V1Membership, ) @@ -36,6 +37,9 @@ class FakeResponse: class FakeLightningClient: + def lightningapp_v2_service_list_lightningapps_v2(self, *args, **kwargs): + return V1ListLightningappsV2Response(lightningapps=[]) + def lightningapp_instance_service_list_lightningapp_instances(self, *args, **kwargs): return V1ListLightningappInstancesResponse(lightningapps=[]) @@ -182,7 +186,7 @@ def __init__(self, *args, message, **kwargs): super().__init__() self.message = message - def lightningapp_instance_service_list_lightningapp_instances(self, *args, **kwargs): + def lightningapp_v2_service_list_lightningapps_v2(self, *args, **kwargs): raise ApiException( http_resp=HttpHeaderDict( data=self.message, diff --git a/tests/tests_app/runners/test_cloud.py b/tests/tests_app/runners/test_cloud.py index 4d1ebda6ccdc5..b5d05c3e7c5cd 100644 --- a/tests/tests_app/runners/test_cloud.py +++ b/tests/tests_app/runners/test_cloud.py @@ -31,6 +31,7 @@ V1LightningworkSpec, V1ListClustersResponse, V1ListLightningappInstancesResponse, + V1ListLightningappsV2Response, V1ListMembershipsResponse, V1ListProjectClusterBindingsResponse, V1Membership, @@ -210,6 +211,13 @@ def test_new_instance_on_different_cluster(self, cloud_backend, project_id, old_ app.flows = [] app.frontend = {} + existing_app = MagicMock() + existing_app.name = app_name + existing_app.id = "test-id" + mock_client.lightningapp_v2_service_list_lightningapps_v2.return_value = V1ListLightningappsV2Response( + lightningapps=[existing_app] + ) + existing_instance = MagicMock() existing_instance.name = app_name existing_instance.status.phase = V1LightningappInstanceState.STOPPED @@ -234,6 +242,67 @@ def test_new_instance_on_different_cluster(self, cloud_backend, project_id, old_ assert args[1]["body"].name.startswith(app_name) assert args[1]["body"].cluster_id == new_cluster + def test_running_deleted_app(self, cloud_backend, project_id): + """Deleted apps show up in list apps but not in list instances. + + This tests that we don't try to reacreate a previously deleted app. + """ + app_name = "test-app" + + mock_client = mock.MagicMock() + mock_client.projects_service_list_memberships.return_value = V1ListMembershipsResponse( + memberships=[V1Membership(name="Default Project", project_id=project_id)] + ) + mock_client.lightningapp_v2_service_create_lightningapp_release.return_value = V1LightningappRelease( + cluster_id=DEFAULT_CLUSTER + ) + + mock_client.cluster_service_list_clusters.return_value = V1ListClustersResponse( + [ + Externalv1Cluster(id=DEFAULT_CLUSTER), + ] + ) + + mock_client.projects_service_list_project_cluster_bindings.return_value = V1ListProjectClusterBindingsResponse( + clusters=[ + V1ProjectClusterBinding(cluster_id=DEFAULT_CLUSTER), + ] + ) + + # Mock all clusters as global clusters + mock_client.cluster_service_get_cluster.side_effect = lambda cluster_id: V1GetClusterResponse( + id=cluster_id, spec=V1ClusterSpec(cluster_type=V1ClusterType.GLOBAL) + ) + + cloud_backend.client = mock_client + + app = mock.MagicMock() + app.flows = [] + app.frontend = {} + + existing_app = MagicMock() + existing_app.name = app_name + existing_app.id = "test-id" + mock_client.lightningapp_v2_service_list_lightningapps_v2.return_value = V1ListLightningappsV2Response( + lightningapps=[existing_app] + ) + + # Simulate the app as deleted so no instance to return + mock_client.lightningapp_instance_service_list_lightningapp_instances.return_value = ( + V1ListLightningappInstancesResponse(lightningapps=[]) + ) + + cloud_runtime = cloud.CloudRuntime(app=app, entrypoint_file="entrypoint.py") + cloud_runtime._check_uploaded_folder = mock.MagicMock() + + cloud_runtime.dispatch(name=app_name) + + # Check that a new name was used which starts with and does not equal the old name + mock_client.lightningapp_v2_service_create_lightningapp_release_instance.assert_called_once() + args = mock_client.lightningapp_v2_service_create_lightningapp_release_instance.call_args + assert args[1]["body"].name != app_name + assert args[1]["body"].name.startswith(app_name) + @pytest.mark.parametrize("flow_cloud_compute", [None, CloudCompute(name="t2.medium")]) @mock.patch("lightning_app.runners.backends.cloud.LightningClient", mock.MagicMock()) def test_run_with_default_flow_compute_config(self, monkeypatch, flow_cloud_compute): @@ -458,6 +527,9 @@ def test_call_with_work_app(self, lightningapps, start_with_flow, monkeypatch, t lightningapps[0].name = "myapp" lightningapps[0].status.phase = V1LightningappInstanceState.STOPPED lightningapps[0].spec.cluster_id = "test" + mock_client.lightningapp_v2_service_list_lightningapps_v2.return_value = V1ListLightningappsV2Response( + lightningapps=lightningapps + ) mock_client.lightningapp_instance_service_list_lightningapp_instances.return_value = ( V1ListLightningappInstancesResponse(lightningapps=lightningapps) ) @@ -632,6 +704,9 @@ def test_call_with_work_app_and_attached_drives(self, lightningapps, monkeypatch lightningapps[0].name = "myapp" lightningapps[0].status.phase = V1LightningappInstanceState.STOPPED lightningapps[0].spec.cluster_id = "test" + mock_client.lightningapp_v2_service_list_lightningapps_v2.return_value = V1ListLightningappsV2Response( + lightningapps=lightningapps + ) mock_client.lightningapp_instance_service_list_lightningapp_instances.return_value = ( V1ListLightningappInstancesResponse(lightningapps=lightningapps) ) @@ -786,6 +861,9 @@ def test_call_with_work_app_and_app_comment_command_execution_set(self, lightnin mock_client.cluster_service_get_cluster.side_effect = lambda cluster_id: V1GetClusterResponse( id=cluster_id, spec=V1ClusterSpec(cluster_type=V1ClusterType.GLOBAL) ) + mock_client.lightningapp_v2_service_list_lightningapps_v2.return_value = V1ListLightningappsV2Response( + lightningapps=lightningapps + ) mock_client.lightningapp_instance_service_list_lightningapp_instances.return_value = ( V1ListLightningappInstancesResponse(lightningapps=lightningapps) ) @@ -912,6 +990,9 @@ def test_call_with_work_app_and_multiple_attached_drives(self, lightningapps, mo mock_client.cluster_service_get_cluster.side_effect = lambda cluster_id: V1GetClusterResponse( id=cluster_id, spec=V1ClusterSpec(cluster_type=V1ClusterType.GLOBAL) ) + mock_client.lightningapp_v2_service_list_lightningapps_v2.return_value = V1ListLightningappsV2Response( + lightningapps=lightningapps + ) mock_client.lightningapp_instance_service_list_lightningapp_instances.return_value = ( V1ListLightningappInstancesResponse(lightningapps=lightningapps) ) @@ -1127,6 +1208,9 @@ def test_call_with_work_app_and_attached_mount_and_drive(self, lightningapps, mo mock_client.cluster_service_get_cluster.side_effect = lambda cluster_id: V1GetClusterResponse( id=cluster_id, spec=V1ClusterSpec(cluster_type=V1ClusterType.GLOBAL) ) + mock_client.lightningapp_v2_service_list_lightningapps_v2.return_value = V1ListLightningappsV2Response( + lightningapps=lightningapps + ) mock_client.lightningapp_instance_service_list_lightningapp_instances.return_value = ( V1ListLightningappInstancesResponse(lightningapps=lightningapps) )