Repository: skypilot-org/skypilot
Pull request: Terminate stopped gcp instances
Fixed #318.

Tested:
- [x] `sky launch -c min --cloud gcp examples/env_check.yaml; sky stop min; sky down min`

Problem statement:
Sky down not working for stopped GCP
Ray is buggy for deleting stopped clusters of all clouds. We call the `awscli` for AWS to solve the problem. For now we just skip the stopped GCP clusters during terminating, we need to support deleting them as well.

To repro
```bash
sky launch --cloud GCP -c debug examples/minimal.yaml           

# Wait until UI changes from Stopping to Stopped.
sky stop debug

# This has no actual effect, as seen in the AWS UI -- cluster remains in Stopped.
# `sky status` removed the cluster entry, so it is now leaked.
sky down debug
```

Hints from issue discussion:

Commit message: lint

Patch to review:
diff --git a/prototype/sky/backends/cloud_vm_ray_backend.py b/prototype/sky/backends/cloud_vm_ray_backend.py
index ff0b66967de..325b9153d97 100644
--- a/prototype/sky/backends/cloud_vm_ray_backend.py
+++ b/prototype/sky/backends/cloud_vm_ray_backend.py
@@ -1580,6 +1580,16 @@ def teardown(self, handle: ResourceHandle, terminate: bool) -> None:
                     f'aws ec2 terminate-instances --region {region} '
                     f'--instance-ids $({query_cmd})')
                 backend_utils.run(terminate_cmd, check=True)
+            elif isinstance(cloud, clouds.GCP):
+                zone = config['provider']['availability_zone']
+                query_cmd = (
+                    f'gcloud compute instances list '
+                    f'--filter=\\(labels.ray-cluster-name={cluster_name}\\) '
+                    f'--zones={zone} --format=value\\(name\\)')
+                terminate_cmd = (
+                    f'gcloud compute instances delete --zone={zone} --quiet '
+                    f'$({query_cmd})')
+                backend_utils.run(terminate_cmd, check=True)
             elif isinstance(cloud, clouds.Azure):
                 # Special handling because `ray down` is buggy with Azure.
                 # Set check=False to not error out on not found VMs.
@@ -1591,7 +1601,7 @@ def teardown(self, handle: ResourceHandle, terminate: bool) -> None:
                 # TODO(suquark,zongheng): Support deleting stopped GCP clusters.
                 # Tracked in issue #318.
                 logger.info(
-                    f'Cannot terminate GCP cluster {cluster_name!r} '
+                    f'Cannot terminate {cloud} cluster {cluster_name!r} '
                     'because it is STOPPED. \nTo fix: manually terminate in '
                     'the cloud\'s UI or '
                     f'`sky start {cluster_name}; sky down {cluster_name}` '


Code review comments:
diff_hunk: @@ -1591,7 +1601,7 @@ def teardown(self, handle: ResourceHandle, terminate: bool) -> None:
                 # TODO(suquark,zongheng): Support deleting stopped GCP clusters.
                 # Tracked in issue #318.
                 logger.info(
-                    f'Cannot terminate GCP cluster {cluster_name!r} '
+                    f'Cannot terminate {cloud} cluster {cluster_name!r} '
path: prototype/sky/backends/cloud_vm_ray_backend.py
text: @user1:
 L1600-1609 can be turned into raising invalid cloud found?

@author:
Good point! Fixed. Thanks!