From 0ecf53cfb9b1ba7322eacce561626c7fbd7faab5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Wed, 23 Oct 2024 09:03:28 +0000 Subject: [PATCH 01/11] add flag parser and enable GCPFilestore on cluster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/commands/cluster.py | 15 +++++++- src/xpk/core/core.py | 75 +++++++++++++++++++++++++++++++------ src/xpk/parser/cluster.py | 9 +++++ src/xpk/parser/storage.py | 7 +++- 4 files changed, 91 insertions(+), 15 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index c580344a..79cbfdaf 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -89,7 +89,11 @@ def cluster_create(args) -> None: xpk_exit(create_cluster_command_code) # Enable WorkloadIdentity if not enabled already. - if args.enable_workload_identity or args.enable_gcsfuse_csi_driver: + if ( + args.enable_workload_identity + or args.enable_gcsfuse_csi_driver + or args.enable_gcpfilestore_csi_driver + ): update_cluster_command_code = ( update_cluster_with_workload_identity_if_necessary(args) ) @@ -486,12 +490,19 @@ def run_gke_cluster_create_command( f' --cluster-dns-domain={args.cluster}-domain' ) - if args.enable_workload_identity or args.enable_gcsfuse_csi_driver: + if ( + args.enable_workload_identity + or args.enable_gcsfuse_csi_driver + or args.enable_gcpfilestore_csi_driver + ): command += f' --workload-pool={args.project}.svc.id.goog' if args.enable_gcsfuse_csi_driver: command += ' --addons GcsFuseCsiDriver' + if args.enable_gcpfilestore_csi_driver: + command += '--addons GcpFilestoreCsiDriver' + return_code = run_command_with_updates(command, 'GKE Cluster Create', args) if return_code != 0: xpk_print(f'GKE Cluster Create request returned ERROR {return_code}') diff --git a/src/xpk/core/core.py b/src/xpk/core/core.py index bea45972..70a008db 100644 --- a/src/xpk/core/core.py +++ b/src/xpk/core/core.py @@ -436,8 +436,8 @@ def update_gke_cluster_with_workload_identity_enabled(args) -> int: return 0 -def update_gke_cluster_with_gcsfuse_driver_enabled(args) -> int: - """Run the GKE cluster update command for existing cluster and enable GCSFuse CSI driver. +def update_gke_cluster_with_addon(addon: str, args) -> int: + """Run the GKE cluster update command for existing cluster and enabling passed addon. Args: args: user provided arguments for running the command. Returns: @@ -447,14 +447,12 @@ def update_gke_cluster_with_gcsfuse_driver_enabled(args) -> int: 'gcloud container clusters update' f' {args.cluster} --project={args.project}' f' --region={zone_to_region(args.zone)}' - ' --update-addons GcsFuseCsiDriver=ENABLED' + ' --update-addons {addon}=ENABLED' ' --quiet' ) - xpk_print( - 'Updating GKE cluster to enable GCSFuse CSI driver, may take a while!' - ) + xpk_print('Updating GKE cluster to enable {addon}, may take a while!') return_code = run_command_with_updates( - command, 'GKE Cluster Update to enable GCSFuse CSI driver', args + command, f'GKE Cluster Update to enable {addon}', args ) if return_code != 0: xpk_print(f'GKE Cluster Update request returned ERROR {return_code}') @@ -1200,6 +1198,33 @@ def is_gcsfuse_driver_enabled_on_cluster(args) -> bool: return False +def is_gcpfilestore_driver_enabled_on_cluster(args) -> bool: + """Checks if GCPFilestore CSI driver is enabled on the cluster. + Args: + args: user provided arguments for running the command. + Returns: + True if GCPFilestore CSI driver is enabled on the cluster and False otherwise. + """ + command = ( + f'gcloud container clusters describe {args.cluster}' + f' --project={args.project} --region={zone_to_region(args.zone)}' + ' --format="value(addonsConfig.gcpFilestoreCsiDriverConfig.enabled)"' + ) + return_code, gcpfilestore_driver_enabled = run_command_for_value( + command, + 'Checks if GCPFilestore CSI driver is enabled in cluster describe.', + args, + ) + if return_code != 0: + xpk_exit(return_code) + if gcpfilestore_driver_enabled.lower() == 'true': + xpk_print( + 'GCPFilestore CSI driver is enabled on the cluster, no update needed.' + ) + return True + return False + + def update_cluster_with_clouddns_if_necessary(args) -> int: """Updates a GKE cluster to use CloudDNS, if not enabled already. @@ -1275,8 +1300,8 @@ def update_cluster_with_gcsfuse_driver_if_necessary(args) -> int: if is_gcsfuse_driver_enabled_on_cluster(args): return 0 - cluster_update_return_code = update_gke_cluster_with_gcsfuse_driver_enabled( - args + cluster_update_return_code = update_gke_cluster_with_addon( + 'GcsFuseCsiDriver', args ) if cluster_update_return_code > 0: xpk_print('Updating GKE cluster to enable GCSFuse CSI driver failed!') @@ -1285,6 +1310,26 @@ def update_cluster_with_gcsfuse_driver_if_necessary(args) -> int: return 0 +def update_cluster_with_gcpfilestore_driver_if_necessary(args) -> int: + """Updates a GKE cluster to enable GCPFilestore CSI driver, if not enabled already. + Args: + args: user provided arguments for running the command. + Returns: + 0 if successful and error code otherwise. + """ + + if is_gcpfilestore_driver_enabled_on_cluster(args): + return 0 + cluster_update_return_code = update_gke_cluster_with_addon( + 'GcpFilestoreCsiDriver', args + ) + if cluster_update_return_code > 0: + xpk_print('Updating GKE cluster to enable GCPFilestore CSI driver failed!') + return cluster_update_return_code + + return 0 + + def get_nodepool_zone(args, nodepool_name) -> tuple[int, str]: """Return zone in which nodepool exists in the cluster. @@ -1603,7 +1648,11 @@ def run_gke_node_pool_create_command( node_pools_to_remain.append(node_pool_name) # Workload Identity for existing nodepools - if args.enable_workload_identity or args.enable_gcsfuse_csi_driver: + if ( + args.enable_workload_identity + or args.enable_gcsfuse_csi_driver + or args.enable_gcpfilestore_csi_driver + ): for node_pool_name in existing_node_pool_names: if not node_pool_name in node_pools_to_delete: # Check if workload identity is not already enabled: @@ -1774,7 +1823,11 @@ def run_gke_node_pool_create_command( command += f' --num-nodes={system.vms_per_slice}' command += ' --scopes=storage-full,gke-default' - if args.enable_workload_identity or args.enable_gcsfuse_csi_driver: + if ( + args.enable_workload_identity + or args.enable_gcsfuse_csi_driver + or args.enable_gcp_filestore_csi_driver + ): command += ' --workload-metadata=GKE_METADATA' task = f'NodepoolCreate-{node_pool_name}' diff --git a/src/xpk/parser/cluster.py b/src/xpk/parser/cluster.py index 7ac89977..348f9bae 100644 --- a/src/xpk/parser/cluster.py +++ b/src/xpk/parser/cluster.py @@ -465,6 +465,15 @@ def add_shared_cluster_create_optional_arguments(args_parsers): ), ) + custom_parser.add_argument( + '--enable-gcpfilestore-csi-driver', + action='store_true', + help=( + 'Enable GCPFilestore driver on the cluster. This enables Workload' + ' Identity Federation.' + ), + ) + def add_shared_cluster_create_tensorboard_arguments(args_parsers): """Add shared tensorboard arguments in cluster create and Pathways cluster create. diff --git a/src/xpk/parser/storage.py b/src/xpk/parser/storage.py index 8404ff4b..3b275b65 100644 --- a/src/xpk/parser/storage.py +++ b/src/xpk/parser/storage.py @@ -57,8 +57,11 @@ def add_storage_create_parser( req_args.add_argument( '--type', type=str, - help='The type of storage. Currently supported types: ["gcsfuse"]', - choices=['gcsfuse'], + help=( + 'The type of storage. Currently supported types: ["gcsfuse",' + ' "filestore"]' + ), + choices=['gcsfuse', 'filestore'], required=True, ) req_args.add_argument( From 8491a43c5d61d1e31a2cd5423013b4550c150208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Wed, 23 Oct 2024 09:21:32 +0000 Subject: [PATCH 02/11] update cluster with gcp filestore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/commands/storage.py | 9 +++++++++ src/xpk/core/storage.py | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/xpk/commands/storage.py b/src/xpk/commands/storage.py index 0dcdf0b0..5f5caeeb 100644 --- a/src/xpk/commands/storage.py +++ b/src/xpk/commands/storage.py @@ -23,9 +23,11 @@ setup_k8s_env, update_cluster_with_gcsfuse_driver_if_necessary, update_cluster_with_workload_identity_if_necessary, + update_cluster_with_gcpfilestore_driver_if_necessary ) from ..core.storage import ( GCS_FUSE_TYPE, + GCP_FILESTORE_TYPE, STORAGE_CRD_KIND, XPK_API_GROUP_NAME, XPK_API_GROUP_VERSION, @@ -49,6 +51,13 @@ def storage_create(args: Namespace) -> None: xpk_exit(return_code) apply_kubectl_manifest(k8s_api_client, args.manifest) + if args.type == GCP_FILESTORE_TYPE: + return_code = update_cluster_with_workload_identity_if_necessary(args) + if return_code > 0: + xpk_exit(return_code) + return_code = update_cluster_with_gcpfilestore_driver_if_necessary(args) + if return_code > 0: + xpk_exit(return_code) def storage_list(args: Namespace) -> None: k8s_api_client = setup_k8s_env(args) diff --git a/src/xpk/core/storage.py b/src/xpk/core/storage.py index 861be750..655ffacb 100644 --- a/src/xpk/core/storage.py +++ b/src/xpk/core/storage.py @@ -38,7 +38,7 @@ XPK_API_GROUP_NAME = "xpk.x-k8s.io" XPK_API_GROUP_VERSION = "v1" GCS_FUSE_TYPE = "gcsfuse" - +GCP_FILESTORE_TYPE = "gcpfilestore" @dataclass class Storage: From f121fa917a68ec504dd8431121f54f275c483cc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Wed, 23 Oct 2024 09:23:32 +0000 Subject: [PATCH 03/11] fix linter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/commands/storage.py | 3 ++- src/xpk/core/storage.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/xpk/commands/storage.py b/src/xpk/commands/storage.py index 5f5caeeb..9f5bcc0b 100644 --- a/src/xpk/commands/storage.py +++ b/src/xpk/commands/storage.py @@ -23,7 +23,7 @@ setup_k8s_env, update_cluster_with_gcsfuse_driver_if_necessary, update_cluster_with_workload_identity_if_necessary, - update_cluster_with_gcpfilestore_driver_if_necessary + update_cluster_with_gcpfilestore_driver_if_necessary, ) from ..core.storage import ( GCS_FUSE_TYPE, @@ -59,6 +59,7 @@ def storage_create(args: Namespace) -> None: if return_code > 0: xpk_exit(return_code) + def storage_list(args: Namespace) -> None: k8s_api_client = setup_k8s_env(args) storages = list_storages(k8s_api_client) diff --git a/src/xpk/core/storage.py b/src/xpk/core/storage.py index 655ffacb..c9a8f0b4 100644 --- a/src/xpk/core/storage.py +++ b/src/xpk/core/storage.py @@ -40,6 +40,7 @@ GCS_FUSE_TYPE = "gcsfuse" GCP_FILESTORE_TYPE = "gcpfilestore" + @dataclass class Storage: """ From 0bc8636ac19f111d297f449f3543e950e7a21a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Wed, 23 Oct 2024 09:28:56 +0000 Subject: [PATCH 04/11] filestore -> gcpfilestore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/parser/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xpk/parser/storage.py b/src/xpk/parser/storage.py index 3b275b65..7ef929da 100644 --- a/src/xpk/parser/storage.py +++ b/src/xpk/parser/storage.py @@ -61,7 +61,7 @@ def add_storage_create_parser( 'The type of storage. Currently supported types: ["gcsfuse",' ' "filestore"]' ), - choices=['gcsfuse', 'filestore'], + choices=['gcsfuse', 'gcpfilestore'], required=True, ) req_args.add_argument( From 9679b7b0aacf98677b3fc82e372e8859e02b3067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Wed, 23 Oct 2024 09:30:01 +0000 Subject: [PATCH 05/11] filetore -> gcpfilestorage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/parser/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xpk/parser/storage.py b/src/xpk/parser/storage.py index 7ef929da..ce981951 100644 --- a/src/xpk/parser/storage.py +++ b/src/xpk/parser/storage.py @@ -59,7 +59,7 @@ def add_storage_create_parser( type=str, help=( 'The type of storage. Currently supported types: ["gcsfuse",' - ' "filestore"]' + ' "gcpfilestore"]' ), choices=['gcsfuse', 'gcpfilestore'], required=True, From 80ae221ae3bc865393357af6a04c7bfa23609486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Thu, 24 Oct 2024 10:17:50 +0000 Subject: [PATCH 06/11] enable gcpfilestore on cluster creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/commands/cluster.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 79cbfdaf..6b13ae4e 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -36,6 +36,7 @@ update_cluster_with_clouddns_if_necessary, update_cluster_with_gcsfuse_driver_if_necessary, update_cluster_with_workload_identity_if_necessary, + update_cluster_with_gcpfilestore_driver_if_necessary, zone_to_region, ) from ..core.kueue import ( @@ -107,6 +108,13 @@ def cluster_create(args) -> None: ) if update_cluster_command_code != 0: xpk_exit(update_cluster_command_code) + + if args.enable_gcpfilestore_csi_driver: + update_cluster_command_code = ( + update_cluster_with_gcpfilestore_driver_if_necessary(args) + ) + if update_cluster_command_code != 0: + xpk_exit(update_cluster_command_code) # Update Pathways clusters with CloudDNS if not enabled already. if args.enable_pathways: @@ -501,7 +509,7 @@ def run_gke_cluster_create_command( command += ' --addons GcsFuseCsiDriver' if args.enable_gcpfilestore_csi_driver: - command += '--addons GcpFilestoreCsiDriver' + command += ' --addons GcpFilestoreCsiDriver' return_code = run_command_with_updates(command, 'GKE Cluster Create', args) if return_code != 0: From 89c3cd057603dbcc9749872214441fdb6c3927de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Thu, 24 Oct 2024 10:34:21 +0000 Subject: [PATCH 07/11] pyink MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/commands/cluster.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 6b13ae4e..e606a667 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -108,7 +108,7 @@ def cluster_create(args) -> None: ) if update_cluster_command_code != 0: xpk_exit(update_cluster_command_code) - + if args.enable_gcpfilestore_csi_driver: update_cluster_command_code = ( update_cluster_with_gcpfilestore_driver_if_necessary(args) From 729546c24e4a2772196734c09b61f85fede599c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Thu, 24 Oct 2024 11:59:40 +0000 Subject: [PATCH 08/11] fix string formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/core/core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xpk/core/core.py b/src/xpk/core/core.py index 70a008db..2c7ff93b 100644 --- a/src/xpk/core/core.py +++ b/src/xpk/core/core.py @@ -447,10 +447,10 @@ def update_gke_cluster_with_addon(addon: str, args) -> int: 'gcloud container clusters update' f' {args.cluster} --project={args.project}' f' --region={zone_to_region(args.zone)}' - ' --update-addons {addon}=ENABLED' + f' --update-addons {addon}=ENABLED' ' --quiet' ) - xpk_print('Updating GKE cluster to enable {addon}, may take a while!') + xpk_print(f'Updating GKE cluster to enable {addon}, may take a while!') return_code = run_command_with_updates( command, f'GKE Cluster Update to enable {addon}', args ) From 80f8e637e173180e216e0d6d84be30b17fbac00c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Mon, 4 Nov 2024 09:32:12 +0000 Subject: [PATCH 09/11] fix naming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/core/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xpk/core/core.py b/src/xpk/core/core.py index 2c7ff93b..5645f8fb 100644 --- a/src/xpk/core/core.py +++ b/src/xpk/core/core.py @@ -1826,7 +1826,7 @@ def run_gke_node_pool_create_command( if ( args.enable_workload_identity or args.enable_gcsfuse_csi_driver - or args.enable_gcp_filestore_csi_driver + or args.enable_gcpfilestore_csi_driver ): command += ' --workload-metadata=GKE_METADATA' From e18f6969df472b85613b45f4be9deb45c2befd1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Mon, 4 Nov 2024 11:37:46 +0000 Subject: [PATCH 10/11] fix multiple addons usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/commands/cluster.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index e606a667..19400b90 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -505,11 +505,16 @@ def run_gke_cluster_create_command( ): command += f' --workload-pool={args.project}.svc.id.goog' + addons = [] if args.enable_gcsfuse_csi_driver: - command += ' --addons GcsFuseCsiDriver' + addons += 'GcsFuseCsiDriver' if args.enable_gcpfilestore_csi_driver: - command += ' --addons GcpFilestoreCsiDriver' + addons += 'GcpFilestoreCsiDriver' + + if len(addons) > 0: + addons_str = ','.join(addons) + command += f' --addons={addons_str}' return_code = run_command_with_updates(command, 'GKE Cluster Create', args) if return_code != 0: From 8b2940feb60145175d3010d9ff27c6b1e03048d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Paw=C5=82owski?= Date: Mon, 4 Nov 2024 11:52:02 +0000 Subject: [PATCH 11/11] += -> append MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Piotr Pawłowski --- src/xpk/commands/cluster.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 19400b90..0439b57a 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -507,10 +507,10 @@ def run_gke_cluster_create_command( addons = [] if args.enable_gcsfuse_csi_driver: - addons += 'GcsFuseCsiDriver' + addons.append('GcsFuseCsiDriver') if args.enable_gcpfilestore_csi_driver: - addons += 'GcpFilestoreCsiDriver' + addons.append('GcpFilestoreCsiDriver') if len(addons) > 0: addons_str = ','.join(addons)