[CLI] drop name column from cluster list (#15721)

* drop name column from cluster list

* change create cluster to accept id as well

* rename validator

* remove cluster name from logs

* fix merge with master

* more merge with master issues

docs/source-app/workflows/byoc/index.rst

@@ -53,7 +53,7 @@ Here's an example:
 Arguments
 ^^^^^^^^^
 
-* cluster_name: The name of the cluster to be created
+* cluster_id: The name of the cluster to be created
 
 .. note:: Cluster names can only contain lowercase letters, numbers, and periodic hyphens ( - ).
 

src/lightning_app/cli/cmd_clusters.py

@@ -56,7 +56,7 @@ def as_json(self) -> str:
         return json.dumps(self.clusters)
 
     def as_table(self) -> Table:
-        table = Table("id", "name", "type", "status", "created", show_header=True, header_style="bold green")
+        table = Table("id", "type", "status", "created", show_header=True, header_style="bold green")
         phases = {
             V1ClusterState.QUEUED: Text("queued", style="bold yellow"),
             V1ClusterState.PENDING: Text("pending", style="bold yellow"),
@@ -82,7 +82,6 @@ def as_table(self) -> Table:
 
             table.add_row(
                 cluster.id,
-                cluster.name,
                 cluster_type_lookup.get(cluster.spec.cluster_type, Text("unknown", style="red")),
                 status,
                 created_at.strftime("%Y-%m-%d") if created_at else "",
@@ -100,7 +99,7 @@ def __init__(self) -> None:
     def create(
         self,
         cost_savings: bool = False,
-        cluster_name: str = None,
+        cluster_id: str = None,
         role_arn: str = None,
         region: str = "us-east-1",
         external_id: str = None,
@@ -111,7 +110,7 @@ def create(
 
         Args:
             cost_savings: Specifies if the cluster uses cost savings mode
-            cluster_name: The name of the cluster to be created
+            cluster_id: The name of the cluster to be created
             role_arn: AWS IAM Role ARN used to provision resources
             region: AWS region containing compute resources
             external_id: AWS IAM Role external ID
@@ -125,7 +124,7 @@ def create(
             performance_profile = V1ClusterPerformanceProfile.COST_SAVING
 
         body = V1CreateClusterRequest(
-            name=cluster_name,
+            name=cluster_id,
             spec=V1ClusterSpec(
                 cluster_type=V1ClusterType.BYOC,
                 performance_profile=performance_profile,
@@ -159,10 +158,10 @@ def create(
                 lightning list clusters
 
             To view cluster logs use:
-                lightning show cluster logs {cluster_name}
+                lightning show cluster logs {cluster_id}
 
             To delete the cluster run:
-                lightning delete cluster {cluster_name}
+                lightning delete cluster {cluster_id}
             """
             )
         )
@@ -277,7 +276,7 @@ def _wait_for_cluster_state(
         )
 
 
-def _check_cluster_name_is_valid(_ctx: Any, _param: Any, value: str) -> str:
+def _check_cluster_id_is_valid(_ctx: Any, _param: Any, value: str) -> str:
     pattern = r"^(?!-)[a-z0-9-]{1,63}(?<!-)$"
     if not re.match(pattern, value):
         raise click.ClickException(
@@ -296,7 +295,7 @@ def _cluster_status_long(cluster: V1GetClusterResponse, desired_state: V1Cluster
         elapsed: Seconds since we've started polling
     """
 
-    cluster_name = cluster.name
+    cluster_id = cluster.id
     current_state = cluster.status.phase
     current_reason = cluster.status.reason
     bucket_name = cluster.spec.driver.kubernetes.aws.bucket_name
@@ -306,15 +305,15 @@ def _cluster_status_long(cluster: V1GetClusterResponse, desired_state: V1Cluster
     if current_state == V1ClusterState.FAILED:
         return dedent(
             f"""\
-            The requested cluster operation for cluster {cluster_name} has errors:
+            The requested cluster operation for cluster {cluster_id} has errors:
             {current_reason}
 
             ---
             We are automatically retrying, and an automated alert has been created
 
             WARNING: Any non-deleted cluster may be using resources.
             To avoid incuring cost on your cloud provider, delete the cluster using the following command:
-                lightning delete cluster {cluster_name}
+                lightning delete cluster {cluster_id}
 
             Contact [email protected] for additional help
             """
@@ -323,18 +322,18 @@ def _cluster_status_long(cluster: V1GetClusterResponse, desired_state: V1Cluster
     if desired_state == current_state == V1ClusterState.RUNNING:
         return dedent(
             f"""\
-                Cluster {cluster_name} is now running and ready to use.
-                To launch an app on this cluster use: lightning run app app.py --cloud --cluster-id {cluster_name}
+                Cluster {cluster_id} is now running and ready to use.
+                To launch an app on this cluster use: lightning run app app.py --cloud --cluster-id {cluster_id}
                 """
         )
 
     if desired_state == V1ClusterState.RUNNING:
-        return f"Cluster {cluster_name} is being created [elapsed={duration}]"
+        return f"Cluster {cluster_id} is being created [elapsed={duration}]"
 
     if desired_state == current_state == V1ClusterState.DELETED:
         return dedent(
             f"""\
-            Cluster {cluster_name} has been successfully deleted, and almost all AWS resources have been removed
+            Cluster {cluster_id} has been successfully deleted, and almost all AWS resources have been removed
 
             For safety purposes we kept the S3 bucket associated with the cluster: {bucket_name}
 
@@ -344,7 +343,7 @@ def _cluster_status_long(cluster: V1GetClusterResponse, desired_state: V1Cluster
         )
 
     if desired_state == V1ClusterState.DELETED:
-        return f"Cluster {cluster_name} is being deleted [elapsed={duration}]"
+        return f"Cluster {cluster_id} is being deleted [elapsed={duration}]"
 
     raise click.ClickException(f"Unknown cluster desired state {desired_state}")
 

src/lightning_app/cli/lightning_cli.py

@@ -122,7 +122,7 @@ def cluster() -> None:
 
 
 @cluster.command(name="logs")
-@click.argument("cluster_name", required=True)
+@click.argument("cluster_id", required=True)
 @click.option(
     "--from",
     "from_time",
@@ -141,7 +141,7 @@ def cluster() -> None:
 )
 @click.option("--limit", default=10000, help="The max number of log lines returned.")
 @click.option("-f", "--follow", required=False, is_flag=True, help="Wait for new logs, to exit use CTRL+C.")
-def cluster_logs(cluster_name: str, to_time: arrow.Arrow, from_time: arrow.Arrow, limit: int, follow: bool) -> None:
+def cluster_logs(cluster_id: str, to_time: arrow.Arrow, from_time: arrow.Arrow, limit: int, follow: bool) -> None:
     """Show cluster logs.
 
     Example uses:
@@ -170,26 +170,26 @@ def cluster_logs(cluster_name: str, to_time: arrow.Arrow, from_time: arrow.Arrow
     cluster_manager = AWSClusterManager()
     existing_cluster_list = cluster_manager.get_clusters()
 
-    clusters = {cluster.name: cluster.id for cluster in existing_cluster_list.clusters}
+    clusters = {cluster.id: cluster.id for cluster in existing_cluster_list.clusters}
 
     if not clusters:
         raise click.ClickException("You don't have any clusters.")
 
-    if not cluster_name:
+    if not cluster_id:
         raise click.ClickException(
             f"You have not specified any clusters. Please select one of available: [{', '.join(clusters.keys())}]"
         )
 
-    if cluster_name not in clusters:
+    if cluster_id not in clusters:
         raise click.ClickException(
-            f"The cluster '{cluster_name}' does not exist."
+            f"The cluster '{cluster_id}' does not exist."
             f" Please select one of the following: [{', '.join(clusters.keys())}]"
         )
 
     try:
         log_reader = _cluster_logs_reader(
             logs_api_client=_ClusterLogsSocketAPI(client.api_client),
-            cluster_id=clusters[cluster_name],
+            cluster_id=cluster_id,
             start=from_time.int_timestamp,
             end=to_time.int_timestamp if not follow else None,
             limit=limit,

src/lightning_app/cli/lightning_cli_create.py

@@ -5,7 +5,7 @@
 import click
 from lightning_cloud.openapi.rest import ApiException
 
-from lightning_app.cli.cmd_clusters import _check_cluster_name_is_valid, AWSClusterManager
+from lightning_app.cli.cmd_clusters import _check_cluster_id_is_valid, AWSClusterManager
 from lightning_app.cli.cmd_ssh_keys import _SSHKeyManager
 
 
@@ -16,7 +16,7 @@ def create() -> None:
 
 
 @create.command("cluster")
-@click.argument("cluster_name", callback=_check_cluster_name_is_valid)
+@click.argument("cluster_id", callback=_check_cluster_id_is_valid)
 @click.option("--provider", "provider", type=str, default="aws", help="cloud provider to be used for your cluster")
 @click.option("--external-id", "external_id", type=str, required=True)
 @click.option(
@@ -59,7 +59,7 @@ def create() -> None:
     help="This flag makes the CLI return immediately and lets the cluster creation happen in the background.",
 )
 def create_cluster(
-    cluster_name: str,
+    cluster_id: str,
     region: str,
     role_arn: str,
     external_id: str,
@@ -75,7 +75,7 @@ def create_cluster(
         return
     cluster_manager = AWSClusterManager()
     cluster_manager.create(
-        cluster_name=cluster_name,
+        cluster_id=cluster_id,
         region=region,
         role_arn=role_arn,
         external_id=external_id,

tests/tests_app/cli/test_cli.py

@@ -122,7 +122,7 @@ def test_create_cluster(create_command: mock.MagicMock):
     )
 
     create_command.assert_called_once_with(
-        cluster_name="test-7",
+        cluster_id="test-7",
         region="us-east-1",
         role_arn="arn:aws:iam::1234567890:role/lai-byoc",
         external_id="dummy",

tests/tests_app/cli/test_cmd_clusters.py

@@ -59,7 +59,7 @@ def cluster_service_get_cluster(self, id: str):
 def test_create_cluster_api(api: mock.MagicMock, async_or_interrupt):
     cluster_manager = AWSClusterManager()
     cluster_manager.create(
-        cluster_name="test-7",
+        cluster_id="test-7",
         external_id="dummy",
         role_arn="arn:aws:iam::1234567890:role/lai-byoc",
         region="us-west-2",
@@ -106,17 +106,17 @@ def test_delete_cluster_api(api_get: mock.MagicMock, api_delete: mock.MagicMock,
     api_delete.assert_called_once_with(id="test-7", force=False)
 
 
-class Test_check_cluster_name_is_valid:
+class Test_check_cluster_id_is_valid:
     @pytest.mark.parametrize("name", ["test-7", "0wildgoat"])
     def test_valid(self, name):
-        assert cmd_clusters._check_cluster_name_is_valid(None, None, name)
+        assert cmd_clusters._check_cluster_id_is_valid(None, None, name)
 
     @pytest.mark.parametrize(
         "name", ["(&%)!@#", "1234567890123456789012345678901234567890123456789012345678901234567890"]
     )
     def test_invalid(self, name):
         with pytest.raises(click.ClickException) as e:
-            cmd_clusters._check_cluster_name_is_valid(None, None, name)
+            cmd_clusters._check_cluster_id_is_valid(None, None, name)
             assert "cluster name doesn't match regex pattern" in str(e.value)