From 91b3ae1b627cb4461137d60754983a03b828daa1 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Wed, 18 Jun 2025 01:40:49 +0000 Subject: [PATCH 01/12] feat: Adding a de-dupe function --- src/xpk/commands/cluster.py | 200 ++++++++++++++++++++++++++---------- 1 file changed, 148 insertions(+), 52 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 83eb2b07b..8fd0bfa99 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -77,6 +77,10 @@ from .common import set_cluster_command + + +import shlex + def cluster_adapt(args) -> None: """Function that performs cluster adaptation. @@ -790,6 +794,77 @@ def run_gke_clusters_list_command(args) -> int: return 0 +def parse_command_args_to_dict(arg_string: str) -> dict: + """Parses a command-line argument string into a dictionary of parameters. + + This function safely splits a command-line string, handling quoted arguments + and different parameter formats (e.g., --flag, --key=value, --key value). + It's designed to help convert user-provided custom arguments into a structured + format for easier merging and de-duplication. + + Args: + arg_string: A string containing command-line arguments, such as + "--master-ipv4-cidr=10.0.0.0/28 --enable-ip-alias". + + Returns: + A dictionary where keys are parameter names (e.g., "--enable-ip-alias", + "--cluster-ipv4-cidr") and values are their corresponding parsed values + (e.g., True for a boolean flag, "10.0.0.0/28" for a string value). + """ + parsed_args = {} + if not arg_string: + return parsed_args + + tokens = shlex.split(arg_string) + i = 0 + while i < len(tokens): + token = tokens[i] + if token.startswith('--'): + if '=' in token: + key, value = token.split('=', 1) + parsed_args[key] = value + else: + if i + 1 < len(tokens) and not tokens[i+1].startswith('--'): + parsed_args[token] = tokens[i+1] + i += 1 + else: + parsed_args[token] = True + elif token.startswith('-'): + pass + i += 1 + return parsed_args + +def format_params_to_command_string(params: dict) -> str: + """Formats a dictionary of parameters back into a command-line argument string. + + This function takes a dictionary of processed parameters (after de-duplication + and priority handling) and converts them into a single string suitable for + execution as part of a shell command (e.g., gcloud). It correctly formats + boolean flags, key-value pairs, and handles values containing spaces by + enclosing them in quotes. + + Args: + params: A dictionary where keys are parameter names (e.g., "--project") + and values are their corresponding values (e.g., "my-project", True). + + Returns: + A single string representing the command-line arguments, ready to be appended + to the main command. + """ + parts = [] + for key, value in params.items(): + if value is True: + parts.append(key) + elif value is False: + parts.append(key) + elif value is not None and value != '': + if ' ' in str(value): + parts.append(f"{key}=\"{value}\"") + else: + parts.append(f"{key}={value}") + return ' '.join(parts) + + def run_gke_cluster_create_command( args, gke_control_plane_version: str, system: SystemCharacteristics ) -> int: @@ -813,78 +888,99 @@ def run_gke_cluster_create_command( ) machine_type = args.cluster_cpu_machine_type - # Create the regional cluster with `num-nodes` CPU nodes in the same zone as - # TPUs. This has been tested with clusters of 300 VMs. Larger clusters will - # benefit from a larger initial `--num-nodes`. After the cluster is created, - # the auto-scaler can reduce/increase the nodes based on the load. + final_gcloud_args = {} - # If the user passes in the gke version then we use that directly instead of the rapid release. - # This allows users to directly pass a specified gke version without release channel constraints. - rapid_release_cmd = '' - if args.gke_version is not None: - rapid_release_cmd = ' --release-channel rapid' + + final_gcloud_args['--project'] = args.project + final_gcloud_args['--region'] = zone_to_region(args.zone) + final_gcloud_args['--node-locations'] = args.zone + final_gcloud_args['--cluster-version'] = gke_control_plane_version + final_gcloud_args['--machine-type'] = machine_type + final_gcloud_args['--enable-autoscaling'] = True + final_gcloud_args['--total-min-nodes'] = 1 + final_gcloud_args['--total-max-nodes'] = 1000 + final_gcloud_args['--num-nodes'] = args.default_pool_cpu_num_nodes + final_gcloud_args['--enable-dns-access'] = True + final_gcloud_args['--enable-dns-access'] = True # 預設啟用 DNS 存取 + final_gcloud_args['--master-ipv4-cidr'] = '172.16.0.32/28' # 檢查正確的預設值 + final_gcloud_args['--cluster-ipv4-cidr'] = '10.224.0.0/12' # 確保這是 /12 - command = ( - 'gcloud beta container clusters create' - f' {args.cluster} --project={args.project}' - f' --region={zone_to_region(args.zone)}' - f' --node-locations={args.zone}' - f' --cluster-version={gke_control_plane_version}' - f' --machine-type={machine_type}' - ' --enable-autoscaling' - ' --total-min-nodes 1 --total-max-nodes 1000' - f' --num-nodes {args.default_pool_cpu_num_nodes}' - f' {args.custom_cluster_arguments}' - f' {rapid_release_cmd}' - ' --enable-dns-access' - ) - enable_ip_alias = False + if args.gke_version is not None: + final_gcloud_args['--release-channel'] = 'rapid' + + conditional_params = {} if args.private or args.authorized_networks is not None: - enable_ip_alias = True - command += ' --enable-master-authorized-networks --enable-private-nodes' + conditional_params['--enable-master-authorized-networks'] = True + conditional_params['--enable-private-nodes'] = True + conditional_params['--enable-ip-alias'] = True if system.accelerator_type == AcceleratorType['GPU']: - enable_ip_alias = True - command += ( - ' --enable-dataplane-v2' - ' --enable-multi-networking --no-enable-autoupgrade' - ) + conditional_params['--enable-dataplane-v2'] = True + conditional_params['--enable-multi-networking'] = True + conditional_params['--no-enable-autoupgrade'] = True + conditional_params['--enable-ip-alias'] = True else: - command += ' --location-policy=BALANCED --scopes=storage-full,gke-default' - - if args.enable_pathways: - enable_ip_alias = True - - if enable_ip_alias: - command += ' --enable-ip-alias' + conditional_params['--location-policy'] = 'BALANCED' + conditional_params['--scopes'] = 'storage-full,gke-default' + if args.enable_pathways: + conditional_params['--enable-ip-alias'] = True if args.enable_ray_cluster: - command += ' --addons RayOperator' + conditional_params['--addons'] = 'RayOperator' if args.enable_workload_identity or args.enable_gcsfuse_csi_driver: - command += f' --workload-pool={args.project}.svc.id.goog' + conditional_params['--workload-pool'] = f'{args.project}.svc.id.goog' addons = [] if args.enable_gcsfuse_csi_driver: - addons.append('GcsFuseCsiDriver') - + addons.append('GcsFuseCsiDriver') if args.enable_gcpfilestore_csi_driver: - addons.append('GcpFilestoreCsiDriver') - + addons.append('GcpFilestoreCsiDriver') if args.enable_parallelstore_csi_driver: - addons.append('ParallelstoreCsiDriver') - + addons.append('ParallelstoreCsiDriver') if args.enable_pd_csi_driver: - addons.append('GcePersistentDiskCsiDriver') - + addons.append('GcePersistentDiskCsiDriver') if hasattr(args, 'enable_mtc') and args.enable_mtc: - addons.append('HighScaleCheckpointing') - + addons.append('HighScaleCheckpointing') if len(addons) > 0: - addons_str = ','.join(addons) - command += f' --addons={addons_str}' + conditional_params['--addons'] = ','.join(addons) + + for key, value in conditional_params.items(): + if key not in final_gcloud_args: + final_gcloud_args[key] = value + elif key == '--addons' and key in final_gcloud_args: + final_gcloud_args[key] = ','.join(list(set(final_gcloud_args[key].split(',') + value.split(',')))) + + + user_parsed_args = parse_command_args_to_dict(args.custom_cluster_arguments) + for key, value in user_parsed_args.items(): + if key.startswith('--no-') and key[5:] in final_gcloud_args: + del final_gcloud_args[key[5:]] + final_gcloud_args[key] = True + elif key.startswith('--enable-') and f'--no-{key[2:]}' in final_gcloud_args: + del final_gcloud_args[f'--no-{key[2:]}'] + final_gcloud_args[key] = value + else: + final_gcloud_args[key] = value + + + command_parts = ['gcloud beta container clusters create', args.cluster] + + + for key, value in final_gcloud_args.items(): + if value is True: + command_parts.append(key) + elif value is False: + pass + elif value is not None and value != '': + if ' ' in str(value): + command_parts.append(f"{key}=\"{value}\"") + else: + command_parts.append(f"{key}={value}") + + command = ' '.join(command_parts) return_code = run_command_with_updates(command, 'GKE Cluster Create', args) if return_code != 0: From bc4760183f37f3a6554a25b22037fcc3e8746e92 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Fri, 20 Jun 2025 08:55:23 +0000 Subject: [PATCH 02/12] Refactored GKE cluster creation to use a dictionary for argument management, and added functionalities to remove duplicate parameters and overwrite existing ones. --- src/xpk/commands/cluster.py | 132 ++++++++++++++---------------------- 1 file changed, 52 insertions(+), 80 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 8fd0bfa99..4d1e0c4b8 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -834,36 +834,6 @@ def parse_command_args_to_dict(arg_string: str) -> dict: i += 1 return parsed_args -def format_params_to_command_string(params: dict) -> str: - """Formats a dictionary of parameters back into a command-line argument string. - - This function takes a dictionary of processed parameters (after de-duplication - and priority handling) and converts them into a single string suitable for - execution as part of a shell command (e.g., gcloud). It correctly formats - boolean flags, key-value pairs, and handles values containing spaces by - enclosing them in quotes. - - Args: - params: A dictionary where keys are parameter names (e.g., "--project") - and values are their corresponding values (e.g., "my-project", True). - - Returns: - A single string representing the command-line arguments, ready to be appended - to the main command. - """ - parts = [] - for key, value in params.items(): - if value is True: - parts.append(key) - elif value is False: - parts.append(key) - elif value is not None and value != '': - if ' ' in str(value): - parts.append(f"{key}=\"{value}\"") - else: - parts.append(f"{key}={value}") - return ' '.join(parts) - def run_gke_cluster_create_command( args, gke_control_plane_version: str, system: SystemCharacteristics @@ -881,15 +851,14 @@ def run_gke_cluster_create_command( machine_type = args.default_pool_cpu_machine_type if args.cluster_cpu_machine_type != '': xpk_print( - 'Warning: Note that cluster-cpu-machine-type is soon to be', - ' deprecated. Please use --default-pool-cpu-machine-type instead,' - ' to denote the machine type of the default cpu node pool. Set' - ' the machine type of other cpu nodepools using `--device-type`.', + 'Warning: Note that cluster-cpu-machine-type is soon to be', + ' deprecated. Please use --default-pool-cpu-machine-type instead,' + ' to denote the machine type of the default cpu node pool. Set' + ' the machine type of other cpu nodepools using `--device-type`.', ) machine_type = args.cluster_cpu_machine_type final_gcloud_args = {} - final_gcloud_args['--project'] = args.project final_gcloud_args['--region'] = zone_to_region(args.zone) @@ -901,84 +870,87 @@ def run_gke_cluster_create_command( final_gcloud_args['--total-max-nodes'] = 1000 final_gcloud_args['--num-nodes'] = args.default_pool_cpu_num_nodes final_gcloud_args['--enable-dns-access'] = True - final_gcloud_args['--enable-dns-access'] = True # 預設啟用 DNS 存取 - final_gcloud_args['--master-ipv4-cidr'] = '172.16.0.32/28' # 檢查正確的預設值 - final_gcloud_args['--cluster-ipv4-cidr'] = '10.224.0.0/12' # 確保這是 /12 - + final_gcloud_args['--master-ipv4-cidr'] = '172.16.0.32/28' + final_gcloud_args['--cluster-ipv4-cidr'] = '10.224.0.0/12' if args.gke_version is not None: - final_gcloud_args['--release-channel'] = 'rapid' + final_gcloud_args['--release-channel'] = 'rapid' conditional_params = {} if args.private or args.authorized_networks is not None: - conditional_params['--enable-master-authorized-networks'] = True - conditional_params['--enable-private-nodes'] = True - conditional_params['--enable-ip-alias'] = True + conditional_params['--enable-master-authorized-networks'] = True + conditional_params['--enable-private-nodes'] = True + conditional_params['--enable-ip-alias'] = True if system.accelerator_type == AcceleratorType['GPU']: - conditional_params['--enable-dataplane-v2'] = True - conditional_params['--enable-multi-networking'] = True - conditional_params['--no-enable-autoupgrade'] = True - conditional_params['--enable-ip-alias'] = True + conditional_params['--enable-dataplane-v2'] = True + conditional_params['--enable-multi-networking'] = True + conditional_params['--no-enable-autoupgrade'] = True + conditional_params['--enable-ip-alias'] = True else: - conditional_params['--location-policy'] = 'BALANCED' - conditional_params['--scopes'] = 'storage-full,gke-default' - if args.enable_pathways: - conditional_params['--enable-ip-alias'] = True + conditional_params['--location-policy'] = 'BALANCED' + conditional_params['--scopes'] = 'storage-full,gke-default' + if args.enable_pathways: + conditional_params['--enable-ip-alias'] = True if args.enable_ray_cluster: - conditional_params['--addons'] = 'RayOperator' + conditional_params['--addons'] = 'RayOperator' if args.enable_workload_identity or args.enable_gcsfuse_csi_driver: - conditional_params['--workload-pool'] = f'{args.project}.svc.id.goog' + conditional_params['--workload-pool'] = f'{args.project}.svc.id.goog' addons = [] if args.enable_gcsfuse_csi_driver: - addons.append('GcsFuseCsiDriver') + addons.append('GcsFuseCsiDriver') + if args.enable_gcpfilestore_csi_driver: - addons.append('GcpFilestoreCsiDriver') + addons.append('GcpFilestoreCsiDriver') + if args.enable_parallelstore_csi_driver: - addons.append('ParallelstoreCsiDriver') + addons.append('ParallelstoreCsiDriver') + if args.enable_pd_csi_driver: - addons.append('GcePersistentDiskCsiDriver') + addons.append('GcePersistentDiskCsiDriver') + if hasattr(args, 'enable_mtc') and args.enable_mtc: - addons.append('HighScaleCheckpointing') + addons.append('HighScaleCheckpointing') + if len(addons) > 0: - conditional_params['--addons'] = ','.join(addons) + conditional_params['--addons'] = ','.join(addons) for key, value in conditional_params.items(): - if key not in final_gcloud_args: - final_gcloud_args[key] = value - elif key == '--addons' and key in final_gcloud_args: - final_gcloud_args[key] = ','.join(list(set(final_gcloud_args[key].split(',') + value.split(',')))) + if key not in final_gcloud_args: + final_gcloud_args[key] = value + elif key == '--addons' and key in final_gcloud_args: + final_gcloud_args[key] = ','.join(list(set(final_gcloud_args[key].split(',') + value.split(',')))) user_parsed_args = parse_command_args_to_dict(args.custom_cluster_arguments) for key, value in user_parsed_args.items(): - if key.startswith('--no-') and key[5:] in final_gcloud_args: - del final_gcloud_args[key[5:]] - final_gcloud_args[key] = True - elif key.startswith('--enable-') and f'--no-{key[2:]}' in final_gcloud_args: - del final_gcloud_args[f'--no-{key[2:]}'] - final_gcloud_args[key] = value - else: - final_gcloud_args[key] = value + if key.startswith('--no-') and key[5:] in final_gcloud_args: + del final_gcloud_args[key[5:]] + final_gcloud_args[key] = True + elif key.startswith('--enable-') and f'--no-{key[2:]}' in final_gcloud_args: + del final_gcloud_args[f'--no-{key[2:]}'] + final_gcloud_args[key] = value + else: + final_gcloud_args[key] = value command_parts = ['gcloud beta container clusters create', args.cluster] for key, value in final_gcloud_args.items(): - if value is True: - command_parts.append(key) - elif value is False: - pass - elif value is not None and value != '': - if ' ' in str(value): - command_parts.append(f"{key}=\"{value}\"") - else: - command_parts.append(f"{key}={value}") + if value is True: + command_parts.append(key) + elif value is False: + pass + elif value is not None and value != '': + if ' ' in str(value): + command_parts.append(f"{key}=\"{value}\"") + else: + command_parts.append(f"{key}={value}") command = ' '.join(command_parts) From 85f0982a3e5d500826e891a5d1848666043541a9 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Tue, 24 Jun 2025 08:28:59 +0000 Subject: [PATCH 03/12] feat: Add process_gcloud_args to handle custom gcloud arguments. --- src/xpk/commands/cluster.py | 39 ++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 4d1e0c4b8..1e4b95c68 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -835,6 +835,31 @@ def parse_command_args_to_dict(arg_string: str) -> dict: return parsed_args + +def process_gcloud_args(user_parsed_args, final_gcloud_args): + """ + Processes custom cluster arguments and updates the final gcloud arguments dictionary. + + This function handles special cases for '--no-' and '--enable-' prefixes + in custom arguments to correctly modify the gcloud arguments. + + """ + + for key, value in user_parsed_args.items(): + if key.startswith('--no-'): + opposite_key = f'--{key[5:]}' + if opposite_key in final_gcloud_args: + del final_gcloud_args[opposite_key] + final_gcloud_args[key] = True + elif key.startswith('--enable-'): + opposite_key = f'--no-{key[2:]}' + if opposite_key in final_gcloud_args: + del final_gcloud_args[opposite_key] + final_gcloud_args[key] = value + else: + # For all other arguments, simply add or update their values. + final_gcloud_args[key] = value + def run_gke_cluster_create_command( args, gke_control_plane_version: str, system: SystemCharacteristics ) -> int: @@ -870,7 +895,9 @@ def run_gke_cluster_create_command( final_gcloud_args['--total-max-nodes'] = 1000 final_gcloud_args['--num-nodes'] = args.default_pool_cpu_num_nodes final_gcloud_args['--enable-dns-access'] = True + # This value is from here: https://cloud.google.com/kubernetes-engine/docs/how-to/legacy/network-isolation final_gcloud_args['--master-ipv4-cidr'] = '172.16.0.32/28' + # This value is from here https://cloud.google.com/vpc/docs/subnets final_gcloud_args['--cluster-ipv4-cidr'] = '10.224.0.0/12' if args.gke_version is not None: @@ -927,20 +954,10 @@ def run_gke_cluster_create_command( user_parsed_args = parse_command_args_to_dict(args.custom_cluster_arguments) - for key, value in user_parsed_args.items(): - if key.startswith('--no-') and key[5:] in final_gcloud_args: - del final_gcloud_args[key[5:]] - final_gcloud_args[key] = True - elif key.startswith('--enable-') and f'--no-{key[2:]}' in final_gcloud_args: - del final_gcloud_args[f'--no-{key[2:]}'] - final_gcloud_args[key] = value - else: - final_gcloud_args[key] = value - + process_gcloud_args(user_parsed_args, final_gcloud_args) command_parts = ['gcloud beta container clusters create', args.cluster] - for key, value in final_gcloud_args.items(): if value is True: command_parts.append(key) From ab3cd89f991fbc38d8b871a9b130ad9afe4d203f Mon Sep 17 00:00:00 2001 From: DannyLi Date: Wed, 25 Jun 2025 07:25:47 +0000 Subject: [PATCH 04/12] feat: Added two unit tests. --- src/xpk/commands/cluster.py | 3 +- .../commands/tests/unit/test_arg_parser.py | 43 ++++++++++++ .../tests/unit/test_gcloud_arg_processor.py | 70 +++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 src/xpk/commands/tests/unit/test_arg_parser.py create mode 100644 src/xpk/commands/tests/unit/test_gcloud_arg_processor.py diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 1e4b95c68..3792bc5cc 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -75,6 +75,7 @@ from ..utils.file import write_tmp_file from . import cluster_gcluster from .common import set_cluster_command +import argparse @@ -955,7 +956,7 @@ def run_gke_cluster_create_command( user_parsed_args = parse_command_args_to_dict(args.custom_cluster_arguments) process_gcloud_args(user_parsed_args, final_gcloud_args) - + command_parts = ['gcloud beta container clusters create', args.cluster] for key, value in final_gcloud_args.items(): diff --git a/src/xpk/commands/tests/unit/test_arg_parser.py b/src/xpk/commands/tests/unit/test_arg_parser.py new file mode 100644 index 000000000..27e965780 --- /dev/null +++ b/src/xpk/commands/tests/unit/test_arg_parser.py @@ -0,0 +1,43 @@ +import unittest +from ...cluster import parse_command_args_to_dict + +class TestParseCommandArgsToDict(unittest.TestCase): + + def test_empty_string(self): + self.assertEqual(parse_command_args_to_dict(''), {}) + + def test_simple_key_value_pairs(self): + result = parse_command_args_to_dict('--key1=value1 --key2=value2') + self.assertEqual(result, {'--key1': 'value1', '--key2': 'value2'}) + + def test_flag_with_space_value(self): + result = parse_command_args_to_dict('--key1 value1 --key2 value2') + self.assertEqual(result, {'--key1': 'value1', '--key2': 'value2'}) + + def test_boolean_flags(self): + result = parse_command_args_to_dict('--enable-feature --no-logs') + self.assertEqual(result, {'--enable-feature': True, '--no-logs': True}) + + def test_mixed_formats(self): + result = parse_command_args_to_dict('--project=my-project --zone us-central1 --dry-run') + self.assertEqual(result, {'--project': 'my-project', '--zone': 'us-central1', '--dry-run': True}) + + def test_quoted_values(self): + result = parse_command_args_to_dict('--description "My cluster with spaces" --name=test-cluster') + self.assertEqual(result, {'--description': 'My cluster with spaces', '--name': 'test-cluster'}) + + def test_no_double_hyphen_flags(self): + result = parse_command_args_to_dict('random-word -f --flag') + self.assertEqual(result, {'--flag': True}) # Only --flag should be parsed + + def test_duplicate_keys_last_one_wins(self): + result = parse_command_args_to_dict('--key=value1 --key=value2') + self.assertEqual(result, {'--key': 'value2'}) + + def test_hyphenated_keys(self): + result = parse_command_args_to_dict('--api-endpoint=some-url') + self.assertEqual(result, {'--api-endpoint': 'some-url'}) + +if __name__ == '__main__': + # Run python3 -m src.xpk.commands.tests.unit.test_arg_parser under the xpk folder. + unittest.main() \ No newline at end of file diff --git a/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py b/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py new file mode 100644 index 000000000..30a15f7cf --- /dev/null +++ b/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py @@ -0,0 +1,70 @@ +import unittest +from ...cluster import process_gcloud_args + +class TestProcessGcloudArgs(unittest.TestCase): + + def test_add_new_argument(self): + final_args = {'--existing-key': 'existing-value'} + user_args = {'--new-key': 'new-value'} + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, {'--existing-key': 'existing-value', '--new-key': 'new-value'}) + + def test_override_existing_argument(self): + final_args = {'--common-key': 'old-value'} + user_args = {'--common-key': 'new-value'} + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, {'--common-key': 'new-value'}) + + def test_no_enable_flag_overrides_enable(self): + final_args = {'--enable-logging': True} + user_args = {'--no-enable-logging': True} + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, {'--no-enable-logging': True}) + self.assertNotIn('--enable-logging', final_args) # Check that enable flag is removed + + def test_enable_flag_overrides_no_enable(self): + final_args = {'--no-enable-monitoring': True} + user_args = {'--enable-monitoring': True} + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, {'--enable-monitoring': True}) + self.assertNotIn('--no-enable-monitoring', final_args) # Check that no-enable flag is removed + + def test_no_conflict(self): + final_args = {'--param1': 'value1'} + user_args = {'--param2': 'value2'} + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, {'--param1': 'value1', '--param2': 'value2'}) + + def test_empty_user_args(self): + final_args = {'--param1': 'value1'} + user_args = {} + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, {'--param1': 'value1'}) + + def test_complex_overrides(self): + final_args = { + '--zone': 'us-east1-b', + '--enable-ip-alias': True, + '--machine-type': 'n1-standard-4', + '--no-enable-public-ip': True # This will be removed if --enable-public-ip is set + } + user_args = { + '--zone': 'us-central1-a', # Overrides + '--no-enable-ip-alias': True, # Overrides --enable-ip-alias + '--disk-size': '200GB', # New + '--enable-public-ip': True # Overrides --no-public-ip + } + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, { + '--zone': 'us-central1-a', + '--no-enable-ip-alias': True, + '--machine-type': 'n1-standard-4', # Not affected + '--disk-size': '200GB', + '--enable-public-ip': True + }) + self.assertNotIn('--enable-ip-alias', final_args) + self.assertNotIn('--no-enable-public-ip', final_args) + +if __name__ == '__main__': + # Run python3 -m src.xpk.commands.tests.unit.test_gcloud_arg_processor under the xpk folder. + unittest.main() \ No newline at end of file From 928f693a24d1244cce905a73dfb4472d2dc27957 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Wed, 25 Jun 2025 16:22:32 +0800 Subject: [PATCH 05/12] Update test_gcloud_arg_processor.py --- src/xpk/commands/tests/unit/test_gcloud_arg_processor.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py b/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py index 30a15f7cf..832f70108 100644 --- a/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py +++ b/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py @@ -20,14 +20,14 @@ def test_no_enable_flag_overrides_enable(self): user_args = {'--no-enable-logging': True} process_gcloud_args(user_args, final_args) self.assertEqual(final_args, {'--no-enable-logging': True}) - self.assertNotIn('--enable-logging', final_args) # Check that enable flag is removed + self.assertNotIn('--enable-logging', final_args) def test_enable_flag_overrides_no_enable(self): final_args = {'--no-enable-monitoring': True} user_args = {'--enable-monitoring': True} process_gcloud_args(user_args, final_args) self.assertEqual(final_args, {'--enable-monitoring': True}) - self.assertNotIn('--no-enable-monitoring', final_args) # Check that no-enable flag is removed + self.assertNotIn('--no-enable-monitoring', final_args) def test_no_conflict(self): final_args = {'--param1': 'value1'} @@ -52,7 +52,7 @@ def test_complex_overrides(self): '--zone': 'us-central1-a', # Overrides '--no-enable-ip-alias': True, # Overrides --enable-ip-alias '--disk-size': '200GB', # New - '--enable-public-ip': True # Overrides --no-public-ip + '--enable-public-ip': True # Overrides --no-enable-public-ip } process_gcloud_args(user_args, final_args) self.assertEqual(final_args, { @@ -67,4 +67,4 @@ def test_complex_overrides(self): if __name__ == '__main__': # Run python3 -m src.xpk.commands.tests.unit.test_gcloud_arg_processor under the xpk folder. - unittest.main() \ No newline at end of file + unittest.main() From e95b91fe04078b934d17f7496783e1e9f67c83e3 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Thu, 26 Jun 2025 03:45:51 +0000 Subject: [PATCH 06/12] feat: Updated the functionality of the process_gcloud_args function. Also, made minor code fixes. --- src/xpk/commands/cluster.py | 25 +++++++------ .../tests/unit/test_gcloud_arg_processor.py | 36 +++++++++++++++++++ 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 3792bc5cc..c6a57535e 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -75,11 +75,6 @@ from ..utils.file import write_tmp_file from . import cluster_gcluster from .common import set_cluster_command -import argparse - - - - import shlex def cluster_adapt(args) -> None: @@ -794,7 +789,6 @@ def run_gke_clusters_list_command(args) -> int: return 0 - def parse_command_args_to_dict(arg_string: str) -> dict: """Parses a command-line argument string into a dictionary of parameters. @@ -817,6 +811,8 @@ def parse_command_args_to_dict(arg_string: str) -> dict: return parsed_args tokens = shlex.split(arg_string) + # After shlex.split: Print the tokens list + xpk_print(f"Shlex-split tokens: {tokens}") i = 0 while i < len(tokens): token = tokens[i] @@ -833,10 +829,11 @@ def parse_command_args_to_dict(arg_string: str) -> dict: elif token.startswith('-'): pass i += 1 + # After parsing: Print the final parsed dictionary + xpk_print(f"Final parsed_args: {parsed_args}") + xpk_print(f"-------------------------------------------") return parsed_args - - def process_gcloud_args(user_parsed_args, final_gcloud_args): """ Processes custom cluster arguments and updates the final gcloud arguments dictionary. @@ -845,7 +842,6 @@ def process_gcloud_args(user_parsed_args, final_gcloud_args): in custom arguments to correctly modify the gcloud arguments. """ - for key, value in user_parsed_args.items(): if key.startswith('--no-'): opposite_key = f'--{key[5:]}' @@ -854,13 +850,23 @@ def process_gcloud_args(user_parsed_args, final_gcloud_args): final_gcloud_args[key] = True elif key.startswith('--enable-'): opposite_key = f'--no-{key[2:]}' + opposite_disable_key = f'--disable-{key[9:]}' if opposite_key in final_gcloud_args: del final_gcloud_args[opposite_key] + if opposite_disable_key in final_gcloud_args: + del final_gcloud_args[opposite_disable_key] final_gcloud_args[key] = value + elif key.startswith('--disable-'): + feature_name = key[10:] + opposite_enable_key = f'--enable-{feature_name}' + if opposite_enable_key in final_gcloud_args: + del final_gcloud_args[opposite_enable_key] + final_gcloud_args[key] = True else: # For all other arguments, simply add or update their values. final_gcloud_args[key] = value + def run_gke_cluster_create_command( args, gke_control_plane_version: str, system: SystemCharacteristics ) -> int: @@ -953,7 +959,6 @@ def run_gke_cluster_create_command( elif key == '--addons' and key in final_gcloud_args: final_gcloud_args[key] = ','.join(list(set(final_gcloud_args[key].split(',') + value.split(',')))) - user_parsed_args = parse_command_args_to_dict(args.custom_cluster_arguments) process_gcloud_args(user_parsed_args, final_gcloud_args) diff --git a/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py b/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py index 832f70108..ccf3f7131 100644 --- a/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py +++ b/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py @@ -64,6 +64,42 @@ def test_complex_overrides(self): }) self.assertNotIn('--enable-ip-alias', final_args) self.assertNotIn('--no-enable-public-ip', final_args) + + def test_disable_flag_is_added(self): + """ + Tests that a --disable- flag from user_args is simply added to final_args + when no conflicting --enable- or --no-enable- flag exists. + """ + final_args = {'--existing-flag': 'value'} + user_args = {'--disable-dataplane-v2': True} + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, { + '--existing-flag': 'value', + '--disable-dataplane-v2': True + }) + + def test_enable_flag_overrides_disable(self): + """ + Tests that an --enable- flag from user_args overrides a --disable- flag + present in final_args. + """ + final_args = {'--disable-logging': True} # Existing disable flag + user_args = {'--enable-logging': True} # User wants to enable + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, {'--enable-logging': True}) + self.assertNotIn('--disable-logging', final_args) + + def test_disable_flag_overrides_enable_from_user_args_order(self): + """ + Tests that if --enable- is in final_args and --disable- is in user_args, + the --disable- from user_args takes precedence and removes the --enable-. + This is implied by the order of processing (user_args overwrite/remove final_args). + """ + final_args = {'--enable-some-feature': True} + user_args = {'--disable-some-feature': True} + process_gcloud_args(user_args, final_args) + self.assertEqual(final_args, {'--disable-some-feature': True}) + self.assertNotIn('--enable-some-feature', final_args) if __name__ == '__main__': # Run python3 -m src.xpk.commands.tests.unit.test_gcloud_arg_processor under the xpk folder. From 1b7442ce9f135cafe46f70bf3b336f47c8bd5b08 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Thu, 10 Jul 2025 08:11:22 +0000 Subject: [PATCH 07/12] Resolve lint issue --- src/xpk/commands/cluster.py | 74 ++++++++++--------- .../commands/tests/unit/test_arg_parser.py | 29 ++++++-- .../tests/unit/test_gcloud_arg_processor.py | 56 ++++++++------ 3 files changed, 94 insertions(+), 65 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index c6a57535e..f64e14d8d 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -77,6 +77,7 @@ from .common import set_cluster_command import shlex + def cluster_adapt(args) -> None: """Function that performs cluster adaptation. @@ -789,6 +790,7 @@ def run_gke_clusters_list_command(args) -> int: return 0 + def parse_command_args_to_dict(arg_string: str) -> dict: """Parses a command-line argument string into a dictionary of parameters. @@ -812,28 +814,29 @@ def parse_command_args_to_dict(arg_string: str) -> dict: tokens = shlex.split(arg_string) # After shlex.split: Print the tokens list - xpk_print(f"Shlex-split tokens: {tokens}") + xpk_print(f'Shlex-split tokens: {tokens}') i = 0 while i < len(tokens): token = tokens[i] if token.startswith('--'): - if '=' in token: + if '=' in token: key, value = token.split('=', 1) parsed_args[key] = value - else: - if i + 1 < len(tokens) and not tokens[i+1].startswith('--'): - parsed_args[token] = tokens[i+1] - i += 1 - else: - parsed_args[token] = True - elif token.startswith('-'): + else: + if i + 1 < len(tokens) and not tokens[i + 1].startswith('--'): + parsed_args[token] = tokens[i + 1] + i += 1 + else: + parsed_args[token] = True + elif token.startswith('-'): pass i += 1 # After parsing: Print the final parsed dictionary - xpk_print(f"Final parsed_args: {parsed_args}") - xpk_print(f"-------------------------------------------") + xpk_print(f'Final parsed_args: {parsed_args}') + xpk_print('-------------------------------------------') return parsed_args + def process_gcloud_args(user_parsed_args, final_gcloud_args): """ Processes custom cluster arguments and updates the final gcloud arguments dictionary. @@ -883,29 +886,28 @@ def run_gke_cluster_create_command( machine_type = args.default_pool_cpu_machine_type if args.cluster_cpu_machine_type != '': xpk_print( - 'Warning: Note that cluster-cpu-machine-type is soon to be', - ' deprecated. Please use --default-pool-cpu-machine-type instead,' - ' to denote the machine type of the default cpu node pool. Set' - ' the machine type of other cpu nodepools using `--device-type`.', + 'Warning: Note that cluster-cpu-machine-type is soon to be', + ' deprecated. Please use --default-pool-cpu-machine-type instead,' + ' to denote the machine type of the default cpu node pool. Set' + ' the machine type of other cpu nodepools using `--device-type`.', ) machine_type = args.cluster_cpu_machine_type final_gcloud_args = {} - final_gcloud_args['--project'] = args.project final_gcloud_args['--region'] = zone_to_region(args.zone) final_gcloud_args['--node-locations'] = args.zone final_gcloud_args['--cluster-version'] = gke_control_plane_version final_gcloud_args['--machine-type'] = machine_type - final_gcloud_args['--enable-autoscaling'] = True + final_gcloud_args['--enable-autoscaling'] = True final_gcloud_args['--total-min-nodes'] = 1 final_gcloud_args['--total-max-nodes'] = 1000 final_gcloud_args['--num-nodes'] = args.default_pool_cpu_num_nodes - final_gcloud_args['--enable-dns-access'] = True + final_gcloud_args['--enable-dns-access'] = True # This value is from here: https://cloud.google.com/kubernetes-engine/docs/how-to/legacy/network-isolation - final_gcloud_args['--master-ipv4-cidr'] = '172.16.0.32/28' + final_gcloud_args['--master-ipv4-cidr'] = '172.16.0.32/28' # This value is from here https://cloud.google.com/vpc/docs/subnets - final_gcloud_args['--cluster-ipv4-cidr'] = '10.224.0.0/12' + final_gcloud_args['--cluster-ipv4-cidr'] = '10.224.0.0/12' if args.gke_version is not None: final_gcloud_args['--release-channel'] = 'rapid' @@ -915,21 +917,21 @@ def run_gke_cluster_create_command( if args.private or args.authorized_networks is not None: conditional_params['--enable-master-authorized-networks'] = True conditional_params['--enable-private-nodes'] = True - conditional_params['--enable-ip-alias'] = True + conditional_params['--enable-ip-alias'] = True if system.accelerator_type == AcceleratorType['GPU']: conditional_params['--enable-dataplane-v2'] = True conditional_params['--enable-multi-networking'] = True - conditional_params['--no-enable-autoupgrade'] = True - conditional_params['--enable-ip-alias'] = True + conditional_params['--no-enable-autoupgrade'] = True + conditional_params['--enable-ip-alias'] = True else: conditional_params['--location-policy'] = 'BALANCED' conditional_params['--scopes'] = 'storage-full,gke-default' if args.enable_pathways: - conditional_params['--enable-ip-alias'] = True + conditional_params['--enable-ip-alias'] = True if args.enable_ray_cluster: - conditional_params['--addons'] = 'RayOperator' + conditional_params['--addons'] = 'RayOperator' if args.enable_workload_identity or args.enable_gcsfuse_csi_driver: conditional_params['--workload-pool'] = f'{args.project}.svc.id.goog' @@ -951,30 +953,30 @@ def run_gke_cluster_create_command( addons.append('HighScaleCheckpointing') if len(addons) > 0: - conditional_params['--addons'] = ','.join(addons) + conditional_params['--addons'] = ','.join(addons) for key, value in conditional_params.items(): - if key not in final_gcloud_args: + if key not in final_gcloud_args: final_gcloud_args[key] = value - elif key == '--addons' and key in final_gcloud_args: - final_gcloud_args[key] = ','.join(list(set(final_gcloud_args[key].split(',') + value.split(',')))) + elif key == '--addons' and key in final_gcloud_args: + final_gcloud_args[key] = ','.join( + list(set(final_gcloud_args[key].split(',') + value.split(','))) + ) user_parsed_args = parse_command_args_to_dict(args.custom_cluster_arguments) process_gcloud_args(user_parsed_args, final_gcloud_args) command_parts = ['gcloud beta container clusters create', args.cluster] - for key, value in final_gcloud_args.items(): if value is True: command_parts.append(key) - elif value is False: - pass - elif value is not None and value != '': + elif value is False: + pass + elif value is not None and value != ' ': if ' ' in str(value): - command_parts.append(f"{key}=\"{value}\"") + command_parts.append(f'{key}="{value}"') else: - command_parts.append(f"{key}={value}") - + command_parts.append(f'{key}={value}') command = ' '.join(command_parts) return_code = run_command_with_updates(command, 'GKE Cluster Create', args) diff --git a/src/xpk/commands/tests/unit/test_arg_parser.py b/src/xpk/commands/tests/unit/test_arg_parser.py index 27e965780..a75b7935a 100644 --- a/src/xpk/commands/tests/unit/test_arg_parser.py +++ b/src/xpk/commands/tests/unit/test_arg_parser.py @@ -1,7 +1,11 @@ +"""Unit tests for the arg_parser module in xpk.commands.""" + import unittest -from ...cluster import parse_command_args_to_dict +from src.xpk.commands.cluster import parse_command_args_to_dict + class TestParseCommandArgsToDict(unittest.TestCase): + """Tests the parse_command_args_to_dict function from the cluster module.""" def test_empty_string(self): self.assertEqual(parse_command_args_to_dict(''), {}) @@ -19,16 +23,26 @@ def test_boolean_flags(self): self.assertEqual(result, {'--enable-feature': True, '--no-logs': True}) def test_mixed_formats(self): - result = parse_command_args_to_dict('--project=my-project --zone us-central1 --dry-run') - self.assertEqual(result, {'--project': 'my-project', '--zone': 'us-central1', '--dry-run': True}) + result = parse_command_args_to_dict( + '--project=my-project --zone us-central1 --dry-run' + ) + self.assertEqual( + result, + {'--project': 'my-project', '--zone': 'us-central1', '--dry-run': True}, + ) def test_quoted_values(self): - result = parse_command_args_to_dict('--description "My cluster with spaces" --name=test-cluster') - self.assertEqual(result, {'--description': 'My cluster with spaces', '--name': 'test-cluster'}) + result = parse_command_args_to_dict( + '--description "My cluster with spaces" --name=test-cluster' + ) + self.assertEqual( + result, + {'--description': 'My cluster with spaces', '--name': 'test-cluster'}, + ) def test_no_double_hyphen_flags(self): result = parse_command_args_to_dict('random-word -f --flag') - self.assertEqual(result, {'--flag': True}) # Only --flag should be parsed + self.assertEqual(result, {'--flag': True}) # Only --flag should be parsed def test_duplicate_keys_last_one_wins(self): result = parse_command_args_to_dict('--key=value1 --key=value2') @@ -38,6 +52,7 @@ def test_hyphenated_keys(self): result = parse_command_args_to_dict('--api-endpoint=some-url') self.assertEqual(result, {'--api-endpoint': 'some-url'}) + if __name__ == '__main__': # Run python3 -m src.xpk.commands.tests.unit.test_arg_parser under the xpk folder. - unittest.main() \ No newline at end of file + unittest.main() diff --git a/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py b/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py index ccf3f7131..efaa8ac3f 100644 --- a/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py +++ b/src/xpk/commands/tests/unit/test_gcloud_arg_processor.py @@ -1,13 +1,20 @@ +"""Unit tests for the gcloud_arg_parser module in xpk.commands.""" + import unittest -from ...cluster import process_gcloud_args +from src.xpk.commands.cluster import process_gcloud_args + class TestProcessGcloudArgs(unittest.TestCase): + """Tests the process_gcloud_args function from the cluster module.""" def test_add_new_argument(self): final_args = {'--existing-key': 'existing-value'} user_args = {'--new-key': 'new-value'} process_gcloud_args(user_args, final_args) - self.assertEqual(final_args, {'--existing-key': 'existing-value', '--new-key': 'new-value'}) + self.assertEqual( + final_args, + {'--existing-key': 'existing-value', '--new-key': 'new-value'}, + ) def test_override_existing_argument(self): final_args = {'--common-key': 'old-value'} @@ -20,14 +27,14 @@ def test_no_enable_flag_overrides_enable(self): user_args = {'--no-enable-logging': True} process_gcloud_args(user_args, final_args) self.assertEqual(final_args, {'--no-enable-logging': True}) - self.assertNotIn('--enable-logging', final_args) + self.assertNotIn('--enable-logging', final_args) def test_enable_flag_overrides_no_enable(self): final_args = {'--no-enable-monitoring': True} user_args = {'--enable-monitoring': True} process_gcloud_args(user_args, final_args) self.assertEqual(final_args, {'--enable-monitoring': True}) - self.assertNotIn('--no-enable-monitoring', final_args) + self.assertNotIn('--no-enable-monitoring', final_args) def test_no_conflict(self): final_args = {'--param1': 'value1'} @@ -46,25 +53,30 @@ def test_complex_overrides(self): '--zone': 'us-east1-b', '--enable-ip-alias': True, '--machine-type': 'n1-standard-4', - '--no-enable-public-ip': True # This will be removed if --enable-public-ip is set + '--no-enable-public-ip': ( + True # This will be removed if --enable-public-ip is set + ), } user_args = { - '--zone': 'us-central1-a', # Overrides - '--no-enable-ip-alias': True, # Overrides --enable-ip-alias - '--disk-size': '200GB', # New - '--enable-public-ip': True # Overrides --no-enable-public-ip + '--zone': 'us-central1-a', # Overrides + '--no-enable-ip-alias': True, # Overrides --enable-ip-alias + '--disk-size': '200GB', # New + '--enable-public-ip': True, # Overrides --no-enable-public-ip } process_gcloud_args(user_args, final_args) - self.assertEqual(final_args, { - '--zone': 'us-central1-a', - '--no-enable-ip-alias': True, - '--machine-type': 'n1-standard-4', # Not affected - '--disk-size': '200GB', - '--enable-public-ip': True - }) + self.assertEqual( + final_args, + { + '--zone': 'us-central1-a', + '--no-enable-ip-alias': True, + '--machine-type': 'n1-standard-4', # Not affected + '--disk-size': '200GB', + '--enable-public-ip': True, + }, + ) self.assertNotIn('--enable-ip-alias', final_args) self.assertNotIn('--no-enable-public-ip', final_args) - + def test_disable_flag_is_added(self): """ Tests that a --disable- flag from user_args is simply added to final_args @@ -73,17 +85,16 @@ def test_disable_flag_is_added(self): final_args = {'--existing-flag': 'value'} user_args = {'--disable-dataplane-v2': True} process_gcloud_args(user_args, final_args) - self.assertEqual(final_args, { - '--existing-flag': 'value', - '--disable-dataplane-v2': True - }) + self.assertEqual( + final_args, {'--existing-flag': 'value', '--disable-dataplane-v2': True} + ) def test_enable_flag_overrides_disable(self): """ Tests that an --enable- flag from user_args overrides a --disable- flag present in final_args. """ - final_args = {'--disable-logging': True} # Existing disable flag + final_args = {'--disable-logging': True} # Existing disable flag user_args = {'--enable-logging': True} # User wants to enable process_gcloud_args(user_args, final_args) self.assertEqual(final_args, {'--enable-logging': True}) @@ -101,6 +112,7 @@ def test_disable_flag_overrides_enable_from_user_args_order(self): self.assertEqual(final_args, {'--disable-some-feature': True}) self.assertNotIn('--enable-some-feature', final_args) + if __name__ == '__main__': # Run python3 -m src.xpk.commands.tests.unit.test_gcloud_arg_processor under the xpk folder. unittest.main() From 9466df48807742c69acf9543ef3785910f5c6889 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Mon, 14 Jul 2025 06:38:40 +0000 Subject: [PATCH 08/12] Added --enable-private-nodes. --- src/xpk/commands/cluster.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index f64e14d8d..b60de952f 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -908,6 +908,7 @@ def run_gke_cluster_create_command( final_gcloud_args['--master-ipv4-cidr'] = '172.16.0.32/28' # This value is from here https://cloud.google.com/vpc/docs/subnets final_gcloud_args['--cluster-ipv4-cidr'] = '10.224.0.0/12' + final_gcloud_args['--enable-private-nodes'] = True if args.gke_version is not None: final_gcloud_args['--release-channel'] = 'rapid' From 03f1fd62c6757b0fc934eec0c09fca2a7639ab48 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Mon, 21 Jul 2025 03:54:56 +0000 Subject: [PATCH 09/12] Modify the command issue. --- 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 558bec054..87716a2f5 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -957,7 +957,7 @@ def run_gke_cluster_create_command( if args.enable_lustre_csi_driver: addons.append('LustreCsiDriver') - command += ' --enable-legacy-lustre-port' + conditional_params['--enable-legacy-lustre-port'] = True if hasattr(args, 'enable_mtc') and args.enable_mtc: addons.append('HighScaleCheckpointing') From 9cac6313052a23dd21c969a66e87191716214789 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Mon, 21 Jul 2025 04:57:53 +0000 Subject: [PATCH 10/12] Add the enable-ip-alias flag. --- src/xpk/commands/cluster.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 87716a2f5..81c3e80a5 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -914,6 +914,7 @@ def run_gke_cluster_create_command( # This value is from here https://cloud.google.com/vpc/docs/subnets final_gcloud_args['--cluster-ipv4-cidr'] = '10.224.0.0/12' final_gcloud_args['--enable-private-nodes'] = True + final_gcloud_args['--enable-ip-alias'] = True if args.gke_version is not None: final_gcloud_args['--release-channel'] = 'rapid' From 3d728e3f7ce99e302681328a5db6b352c29b8b05 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Mon, 28 Jul 2025 08:50:19 +0000 Subject: [PATCH 11/12] Add the merge_conditional_params and construct_gcloud_command_string functions --- src/xpk/commands/cluster.py | 62 ++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index 81c3e80a5..b5553fe23 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -874,6 +874,45 @@ def process_gcloud_args(user_parsed_args, final_gcloud_args): # For all other arguments, simply add or update their values. final_gcloud_args[key] = value +def merge_conditional_params(conditional_params, final_gcloud_args): + """ + Merge conditional parameters into the final gcloud arguments dictionary. Specifically handle the --addons parameter by merging its values. + """ + for key, value in conditional_params.items(): + if key not in final_gcloud_args: + final_gcloud_args[key] = value + elif key == '--addons' and key in final_gcloud_args: + final_gcloud_args[key] = ','.join( + list(set(final_gcloud_args[key].split(',') + value.split(','))) + ) + +def construct_gcloud_command_string(cluster_name: str, gcloud_args: dict) -> str: + """ + Constructs the gcloud command string from a dictionary of arguments. + + Args: + cluster_name: The name of the cluster. + gcloud_args: A dictionary where keys are gcloud argument names + and values are their corresponding parsed values. + + Returns: + A complete gcloud command string. + """ + # Start with the base command and the cluster name + command_parts = ['gcloud beta container clusters create', cluster_name] + + for key, value in gcloud_args.items(): + if value is True: + command_parts.append(key) + elif value is False: + pass + elif value is not None and str(value).strip() != '': + if ' ' in str(value): + command_parts.append(f'{key}="{value}"') + else: + command_parts.append(f'{key}={value}') + + return ' '.join(command_parts) def run_gke_cluster_create_command( args, gke_control_plane_version: str, system: SystemCharacteristics @@ -966,29 +1005,10 @@ def run_gke_cluster_create_command( if len(addons) > 0: conditional_params['--addons'] = ','.join(addons) - for key, value in conditional_params.items(): - if key not in final_gcloud_args: - final_gcloud_args[key] = value - elif key == '--addons' and key in final_gcloud_args: - final_gcloud_args[key] = ','.join( - list(set(final_gcloud_args[key].split(',') + value.split(','))) - ) - + merge_conditional_params(conditional_params, final_gcloud_args) user_parsed_args = parse_command_args_to_dict(args.custom_cluster_arguments) process_gcloud_args(user_parsed_args, final_gcloud_args) - - command_parts = ['gcloud beta container clusters create', args.cluster] - for key, value in final_gcloud_args.items(): - if value is True: - command_parts.append(key) - elif value is False: - pass - elif value is not None and value != ' ': - if ' ' in str(value): - command_parts.append(f'{key}="{value}"') - else: - command_parts.append(f'{key}={value}') - command = ' '.join(command_parts) + command = construct_gcloud_command_string(args.cluster, final_gcloud_args) return_code = run_command_with_updates(command, 'GKE Cluster Create', args) if return_code != 0: From 2aae966e052b12a6606c495db9a2cb8ae18a4c04 Mon Sep 17 00:00:00 2001 From: DannyLi Date: Tue, 29 Jul 2025 08:10:07 +0000 Subject: [PATCH 12/12] fix pyink --- src/xpk/commands/cluster.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/xpk/commands/cluster.py b/src/xpk/commands/cluster.py index b5553fe23..440ff15c9 100644 --- a/src/xpk/commands/cluster.py +++ b/src/xpk/commands/cluster.py @@ -874,8 +874,9 @@ def process_gcloud_args(user_parsed_args, final_gcloud_args): # For all other arguments, simply add or update their values. final_gcloud_args[key] = value + def merge_conditional_params(conditional_params, final_gcloud_args): - """ + """ Merge conditional parameters into the final gcloud arguments dictionary. Specifically handle the --addons parameter by merging its values. """ for key, value in conditional_params.items(): @@ -883,10 +884,13 @@ def merge_conditional_params(conditional_params, final_gcloud_args): final_gcloud_args[key] = value elif key == '--addons' and key in final_gcloud_args: final_gcloud_args[key] = ','.join( - list(set(final_gcloud_args[key].split(',') + value.split(','))) + list(set(final_gcloud_args[key].split(',') + value.split(','))) ) -def construct_gcloud_command_string(cluster_name: str, gcloud_args: dict) -> str: + +def construct_gcloud_command_string( + cluster_name: str, gcloud_args: dict +) -> str: """ Constructs the gcloud command string from a dictionary of arguments. @@ -898,7 +902,6 @@ def construct_gcloud_command_string(cluster_name: str, gcloud_args: dict) -> str Returns: A complete gcloud command string. """ - # Start with the base command and the cluster name command_parts = ['gcloud beta container clusters create', cluster_name] for key, value in gcloud_args.items(): @@ -914,6 +917,7 @@ def construct_gcloud_command_string(cluster_name: str, gcloud_args: dict) -> str return ' '.join(command_parts) + def run_gke_cluster_create_command( args, gke_control_plane_version: str, system: SystemCharacteristics ) -> int: