diff --git a/cli/pcluster/config/hit_converter.py b/cli/pcluster/config/hit_converter.py index 76561e4d98..241e62fa51 100644 --- a/cli/pcluster/config/hit_converter.py +++ b/cli/pcluster/config/hit_converter.py @@ -25,7 +25,7 @@ class HitConverter: def __init__(self, pcluster_config): self.pcluster_config = pcluster_config - def convert(self): + def convert(self, prepare_to_file=False): """ Convert the pcluster_config instance from pre-HIT to HIT configuration model. @@ -138,6 +138,9 @@ def convert(self): self.pcluster_config.auto_refresh = auto_refresh self.clean_config_parser(hit_cluster_section) + if prepare_to_file: + self._prepare_to_file() + LOGGER.debug("Conversion to HIT completed successfully.") conversion_done = True @@ -147,6 +150,28 @@ def _copy_param_value(self, old_param, new_param, new_value=None): """Copy the value from the old param to the new one.""" new_param.value = new_value if new_value is not None else old_param.value + @staticmethod + def _reset_config_params(section, parameters_to_remove): + for param_key in parameters_to_remove: + param = section.get_param(param_key) + param.value = param.get_default_value() + + def _prepare_to_file(self): + """Reset some parameters from config file since their values can be inferred at runtime.""" + # disable_hyperthreading will get its value from cluster section + queue_section = self.pcluster_config.get_section("queue", "compute") + self._reset_config_params(queue_section, ["disable_hyperthreading"]) + + # Remove initial_count if not needed + compute_section = self.pcluster_config.get_section("compute_resource", "default") + if compute_section.get_param_value("initial_count") == compute_section.get_param_value("min_count"): + self._reset_config_params(compute_section, ["initial_count"]) + + # cluster's disable_hyperthreading's HIT default is None instead of False + cluster_section = self.pcluster_config.get_section("cluster") + if not cluster_section.get_param_value("disable_hyperthreading"): + self._reset_config_params(cluster_section, ["disable_hyperthreading"]) + def _store_original_sections(self): """ Store original default sections from configuration. diff --git a/cli/pcluster/configure/easyconfig.py b/cli/pcluster/configure/easyconfig.py index c9413d8266..c037ccf963 100644 --- a/cli/pcluster/configure/easyconfig.py +++ b/cli/pcluster/configure/easyconfig.py @@ -186,7 +186,7 @@ def configure(args): param.value = param.get_value_from_string(param_value) # Convert file if needed - _convert_config(pcluster_config) + HitConverter(pcluster_config).convert(prepare_to_file=True) # Update config file by overriding changed settings pcluster_config.to_file() @@ -318,28 +318,6 @@ def _prompt_for_subnet(default_subnet, all_subnets, qualified_subnets, message): return prompt_iterable(message, qualified_subnets, default_value=default_subnet) -def _convert_config(pcluster_config): - """Convert the generated SIT configuration to HIT model if scheduler is Slurm.""" - if pcluster_config.cluster_model == ClusterModel.SIT: - HitConverter(pcluster_config).convert() - - if pcluster_config.cluster_model == ClusterModel.HIT: - # Conversion occurred: reset some parameters from config file since their values can be inferred at runtime - - # enable_efa and disable_hyperthreading will get their value from cluster section - queue_section = pcluster_config.get_section("queue", "compute") - _reset_config_params(queue_section, ("enable_efa", "disable_hyperthreading")) - - # initial_count will take its value from min_count - compute_resource_section = pcluster_config.get_section("compute_resource", "default") - _reset_config_params(compute_resource_section, ["initial_count"]) - - # cluster's disable_hyperthreading's HIT default is None instead of False - cluster_section = pcluster_config.get_section("cluster") - if not cluster_section.get_param_value("disable_hyperthreading"): - _reset_config_params(cluster_section, ["disable_hyperthreading"]) - - class ClusterConfigureHelper: """Handle prompts for cluster section.""" diff --git a/cli/pcluster_config/cli.py b/cli/pcluster_config/cli.py index 6f55e49e2c..6befd5e2ac 100644 --- a/cli/pcluster_config/cli.py +++ b/cli/pcluster_config/cli.py @@ -81,7 +81,7 @@ def convert(args=None): ) # Automatic SIT -> HIT conversion, if needed - conversion_done, reason = HitConverter(pcluster_config).convert() + conversion_done, reason = HitConverter(pcluster_config).convert(prepare_to_file=True) if conversion_done: if args.output_file: if os.path.isfile(args.output_file): diff --git a/cli/tests/pcluster/configure/test_pcluster_configure.py b/cli/tests/pcluster/configure/test_pcluster_configure.py index 24118058e9..a4ea800460 100644 --- a/cli/tests/pcluster/configure/test_pcluster_configure.py +++ b/cli/tests/pcluster/configure/test_pcluster_configure.py @@ -334,6 +334,7 @@ def _assert_configurations_are_equal(path_config_expected, path_config_after_inp config_expected = ConfigParser() config_expected.read(path_config_expected) config_expected_dict = {s: dict(config_expected.items(s)) for s in config_expected.sections()} + config_actual = ConfigParser() config_actual.read(path_config_after_input) config_actual_dict = {s: dict(config_actual.items(s)) for s in config_actual.sections()} diff --git a/cli/tests/pcluster/configure/test_pcluster_configure/test_vpc_automation_no_vpc_in_region/pcluster.config.ini b/cli/tests/pcluster/configure/test_pcluster_configure/test_vpc_automation_no_vpc_in_region/pcluster.config.ini index 6221b39c8e..655f78442e 100644 --- a/cli/tests/pcluster/configure/test_pcluster_configure/test_vpc_automation_no_vpc_in_region/pcluster.config.ini +++ b/cli/tests/pcluster/configure/test_pcluster_configure/test_vpc_automation_no_vpc_in_region/pcluster.config.ini @@ -24,6 +24,7 @@ compute_subnet_id = subnet-23456789 use_public_ips = false [queue compute] +enable_efa = false compute_resource_settings = default [compute_resource default] diff --git a/cli/tests/pcluster/configure/test_pcluster_configure/test_vpc_automation_no_vpc_in_region_public/pcluster.config.ini b/cli/tests/pcluster/configure/test_pcluster_configure/test_vpc_automation_no_vpc_in_region_public/pcluster.config.ini index 2300cf9e89..ffbc5c1d00 100644 --- a/cli/tests/pcluster/configure/test_pcluster_configure/test_vpc_automation_no_vpc_in_region_public/pcluster.config.ini +++ b/cli/tests/pcluster/configure/test_pcluster_configure/test_vpc_automation_no_vpc_in_region_public/pcluster.config.ini @@ -22,6 +22,7 @@ vpc_id = vpc-12345678 master_subnet_id = subnet-12345678 [queue compute] +enable_efa = false compute_resource_settings = default [compute_resource default] diff --git a/cli/tests/pcluster/configure/test_pcluster_configure/test_with_input_no_automation_no_errors_with_config_file/pcluster.config.ini b/cli/tests/pcluster/configure/test_pcluster_configure/test_with_input_no_automation_no_errors_with_config_file/pcluster.config.ini index 669c5687ba..a407e27f02 100644 --- a/cli/tests/pcluster/configure/test_pcluster_configure/test_with_input_no_automation_no_errors_with_config_file/pcluster.config.ini +++ b/cli/tests/pcluster/configure/test_pcluster_configure/test_with_input_no_automation_no_errors_with_config_file/pcluster.config.ini @@ -24,6 +24,7 @@ sanity_check = true ssh = ssh {CFN_USER}@{MASTER_IP} {ARGS} [queue compute] +enable_efa = false compute_resource_settings = default [compute_resource default] diff --git a/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_sit_full/expected_output.ini b/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_sit_full/expected_output.ini index 6bb8e3095e..77fe6a193b 100644 --- a/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_sit_full/expected_output.ini +++ b/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_sit_full/expected_output.ini @@ -78,12 +78,10 @@ raid_settings = settings1 fsx_settings = settings1 additional_iam_policies = arn:aws:iam::aws:policy/CloudWatchFullAccess,arn:aws:iam::aws:policy/CloudWatchReadAccess queue_settings = compute -disable_hyperthreading = false [queue compute] compute_type = spot enable_efa = true -disable_hyperthreading = false placement_group = DYNAMIC compute_resource_settings = default @@ -91,6 +89,5 @@ compute_resource_settings = default instance_type = t2.large min_count = 1 max_count = 2 -initial_count = 1 spot_price = 5.0 diff --git a/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_sit_simple/expected_output.ini b/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_sit_simple/expected_output.ini index 9f9d1b1467..74655ece08 100644 --- a/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_sit_simple/expected_output.ini +++ b/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_sit_simple/expected_output.ini @@ -22,15 +22,12 @@ shared_dir = /test vpc_settings = public additional_iam_policies = arn:aws:iam::aws:policy/CloudWatchFullAccess queue_settings = compute -disable_hyperthreading = false [queue compute] enable_efa = false -disable_hyperthreading = false compute_resource_settings = default [compute_resource default] instance_type = g4dn.12xlarge max_count = 100 -initial_count = 0 diff --git a/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_unrelated_sections/expected_output.ini b/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_unrelated_sections/expected_output.ini index 6bb8e3095e..70a80b746e 100644 --- a/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_unrelated_sections/expected_output.ini +++ b/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_unrelated_sections/expected_output.ini @@ -78,18 +78,15 @@ raid_settings = settings1 fsx_settings = settings1 additional_iam_policies = arn:aws:iam::aws:policy/CloudWatchFullAccess,arn:aws:iam::aws:policy/CloudWatchReadAccess queue_settings = compute -disable_hyperthreading = false [queue compute] compute_type = spot enable_efa = true -disable_hyperthreading = false placement_group = DYNAMIC compute_resource_settings = default [compute_resource default] instance_type = t2.large -min_count = 1 max_count = 2 initial_count = 1 spot_price = 5.0 diff --git a/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_unrelated_sections/pcluster.config.ini b/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_unrelated_sections/pcluster.config.ini index 3608a8819a..c6c1e5b638 100644 --- a/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_unrelated_sections/pcluster.config.ini +++ b/cli/tests/pcluster_config/test_pcluster_config_convert/test_slurm_unrelated_sections/pcluster.config.ini @@ -22,7 +22,7 @@ compute_instance_type = t2.large compute_root_volume_size = 30 initial_queue_size = 1 max_queue_size = 2 -maintain_initial_size = true +maintain_initial_size = false cluster_type = spot spot_price = 5 proxy_server = proxy @@ -48,6 +48,7 @@ ebs_settings = settings1,settings2 efs_settings = settings1 raid_settings = settings1 fsx_settings = settings1 +disable_hyperthreading = compute [scaling settings1] scaledown_idletime = 12