From 9b6d7a4512d579b30c14361ca5e047f9d8f89b09 Mon Sep 17 00:00:00 2001 From: Thomas Heinen Date: Fri, 19 Jul 2024 18:31:24 +0200 Subject: [PATCH 1/2] Add EFS access point support (aws-parallelcluster#2337) Signed-off-by: Thomas Heinen --- cli/src/pcluster/config/cluster_config.py | 3 +++ cli/src/pcluster/schemas/cluster_schema.py | 5 ++++- cli/src/pcluster/templates/cluster_stack.py | 5 +++++ cli/src/pcluster/templates/login_nodes_stack.py | 4 ++++ cli/src/pcluster/templates/queues_stack.py | 4 ++++ cli/src/pcluster/validators/efs_validators.py | 2 +- .../pcluster3_config_converter/pcluster3_config_converter.py | 1 + 7 files changed, 22 insertions(+), 2 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 2479eff30c..24007b7056 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -371,6 +371,7 @@ def __init__( deletion_policy: str = None, encryption_in_transit: bool = None, iam_authorization: bool = None, + accesspoint_id: str = None, ): super().__init__() self.mount_dir = Resource.init_param(mount_dir) @@ -387,6 +388,7 @@ def __init__( ) self.encryption_in_transit = Resource.init_param(encryption_in_transit, default=False) self.iam_authorization = Resource.init_param(iam_authorization, default=False) + self.accesspoint_id = Resource.init_param(accesspoint_id) def _register_validators(self, context: ValidatorContext = None): # noqa: D102 #pylint: disable=unused-argument self._register_validator(SharedStorageNameValidator, name=self.name) @@ -398,6 +400,7 @@ def _register_validators(self, context: ValidatorContext = None): # noqa: D102 EfsMountOptionsValidator, encryption_in_transit=self.encryption_in_transit, iam_authorization=self.iam_authorization, + accesspoint_id=self.accesspoint_id, name=self.name, ) diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 8f3eec5930..29fecaf30c 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -321,6 +321,9 @@ class EfsSettingsSchema(BaseSchema): deletion_policy = fields.Str( validate=validate.OneOf(DELETION_POLICIES), metadata={"update_policy": UpdatePolicy.SUPPORTED} ) + accesspoint_id = fields.Str( + validate=validate.Regexp(r"^fsap-[0-9a-z]{17}$"), + ) encryption_in_transit = fields.Bool(metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) iam_authorization = fields.Bool(metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) @@ -331,7 +334,7 @@ def validate_file_system_id_ignored_parameters(self, data, **kwargs): messages = [] if data.get("file_system_id") is not None: for key in data: - if key is not None and key not in ["encryption_in_transit", "iam_authorization", "file_system_id"]: + if key is not None and key not in ["encryption_in_transit", "iam_authorization", "file_system_id", "accesspoint_id"]: messages.append(EFS_MESSAGES["errors"]["ignored_param_with_efs_fs_id"].format(efs_param=key)) if messages: raise ValidationError(message=messages) diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py index 3c3dccc046..25c5642032 100644 --- a/cli/src/pcluster/templates/cluster_stack.py +++ b/cli/src/pcluster/templates/cluster_stack.py @@ -1109,6 +1109,7 @@ def _add_efs_storage(self, id: str, shared_efs: SharedEfs): shared_efs.encryption_in_transit ) self.shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"].append(shared_efs.iam_authorization) + self.shared_storage_attributes[SharedStorageType.EFS]["AccesspointIds"].append(shared_efs.accesspoint_id) return efs_id @@ -1288,6 +1289,10 @@ def _add_head_node(self): "efs_iam_authorizations": to_comma_separated_string( self.shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"], use_lower_case=True ), + "efs_accesspoint_ids": to_comma_separated_string( + self.shared_storage_attributes[SharedStorageType.EFS]["AccesspointIds"], + use_lower_case=True, + ), "fsx_fs_ids": get_shared_storage_ids_by_type(self.shared_storage_infos, SharedStorageType.FSX), "fsx_mount_names": to_comma_separated_string( self.shared_storage_attributes[SharedStorageType.FSX]["MountNames"] diff --git a/cli/src/pcluster/templates/login_nodes_stack.py b/cli/src/pcluster/templates/login_nodes_stack.py index 41ffab59a9..299c9bc99b 100644 --- a/cli/src/pcluster/templates/login_nodes_stack.py +++ b/cli/src/pcluster/templates/login_nodes_stack.py @@ -235,6 +235,10 @@ def _add_login_nodes_pool_launch_template(self): self._shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"], use_lower_case=True, ), + "efs_accesspoint_ids": to_comma_separated_string( + self._shared_storage_attributes[SharedStorageType.EFS]["AccesspointIds"], + use_lower_case=True, + ), "enable_intel_hpc_platform": "true" if self._config.is_intel_hpc_platform_enabled else "false", "ephemeral_dir": DEFAULT_EPHEMERAL_DIR, "fsx_fs_ids": get_shared_storage_ids_by_type(self._shared_storage_infos, SharedStorageType.FSX), diff --git a/cli/src/pcluster/templates/queues_stack.py b/cli/src/pcluster/templates/queues_stack.py index 9f2a824a6e..fa67a3d692 100644 --- a/cli/src/pcluster/templates/queues_stack.py +++ b/cli/src/pcluster/templates/queues_stack.py @@ -335,6 +335,10 @@ def _add_compute_resource_launch_template( self._shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"], use_lower_case=True, ), + "efs_accesspoint_ids": to_comma_separated_string( + self._shared_storage_attributes[SharedStorageType.EFS]["AccesspointIds"], + use_lower_case=True, + ), "fsx_fs_ids": get_shared_storage_ids_by_type(self._shared_storage_infos, SharedStorageType.FSX), "fsx_mount_names": to_comma_separated_string( self._shared_storage_attributes[SharedStorageType.FSX]["MountNames"] diff --git a/cli/src/pcluster/validators/efs_validators.py b/cli/src/pcluster/validators/efs_validators.py index 35ca33c4df..8ba24865f5 100644 --- a/cli/src/pcluster/validators/efs_validators.py +++ b/cli/src/pcluster/validators/efs_validators.py @@ -18,7 +18,7 @@ class EfsMountOptionsValidator(Validator): IAM Authorization requires Encryption in Transit. """ - def _validate(self, encryption_in_transit: bool, iam_authorization: bool, name: str): + def _validate(self, encryption_in_transit: bool, iam_authorization: bool, accesspoint_id: str, name: str): if iam_authorization and not encryption_in_transit: self._add_failure( "EFS IAM authorization cannot be enabled when encryption in-transit is disabled. " diff --git a/cli/src/pcluster3_config_converter/pcluster3_config_converter.py b/cli/src/pcluster3_config_converter/pcluster3_config_converter.py index 5a26fcade3..e0f148d7a0 100644 --- a/cli/src/pcluster3_config_converter/pcluster3_config_converter.py +++ b/cli/src/pcluster3_config_converter/pcluster3_config_converter.py @@ -296,6 +296,7 @@ def convert_efs_settings(self, section_name): ("efs_kms_key_id", "KmsKeyId"), ("provisioned_throughput", "ProvisionedThroughput", "getint"), ("throughput_mode", "ThroughputMode"), + ("accesspoint_id", "AccesspointId"), ] efs_section, efs_dict, _section_label = self.convert_storage_base( "efs", efs_label.strip(), additional_items From a752e2e68e899f8926bb16912c283b8ce81119be Mon Sep 17 00:00:00 2001 From: Thomas Heinen Date: Fri, 2 Aug 2024 13:19:50 +0000 Subject: [PATCH 2/2] Rename new setting, Add changelog, Fix style, fix some tests Signed-off-by: Thomas Heinen --- CHANGELOG.md | 1 + cli/src/pcluster/aws/efs.py | 11 +++++++++++ cli/src/pcluster/config/cluster_config.py | 6 +++--- cli/src/pcluster/schemas/cluster_schema.py | 11 ++++++++--- cli/src/pcluster/templates/cluster_stack.py | 6 +++--- cli/src/pcluster/templates/login_nodes_stack.py | 4 ++-- cli/src/pcluster/templates/queues_stack.py | 4 ++-- .../pcluster3_config_converter.py | 2 +- .../head_node_default.dna.json | 1 + .../test_login_nodes_dna_json/dna-1.json | 1 + .../test_login_nodes_dna_json/dna-2.json | 1 + .../test_compute_nodes_dna_json/dna-1.json | 1 + .../test_compute_nodes_dna_json/dna-2.json | 1 + 13 files changed, 36 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee4bbce7f8..3513d35eec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ CHANGELOG **ENHANCEMENTS** - Add support for custom actions on login nodes. +- Add new configuration `SharedStorage/EfsSettings/AccessPointId` to specify an optional EFS access point for a mount **BUG FIXES** - Fix validator `EfaPlacementGroupValidator` so that it does not suggest to configure a Placement Group when Capacity Blocks are used. diff --git a/cli/src/pcluster/aws/efs.py b/cli/src/pcluster/aws/efs.py index ed55457416..5dd711e365 100644 --- a/cli/src/pcluster/aws/efs.py +++ b/cli/src/pcluster/aws/efs.py @@ -80,3 +80,14 @@ def describe_file_system(self, efs_fs_id): :return: the mount_target_ids """ return self._client.describe_file_systems(FileSystemId=efs_fs_id) + + @AWSExceptionHandler.handle_client_exception + @Cache.cached + def describe_access_point(self, access_point_id): + """ + Describe access point attributes for the given EFS access point id. + + :param efaccess_point_ids_ap_id: EFS access point Id + :return: the access_point details + """ + return self._client.describe_access_points(AccessPointId=access_point_id) \ No newline at end of file diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 24007b7056..4c55ad3b62 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -371,7 +371,7 @@ def __init__( deletion_policy: str = None, encryption_in_transit: bool = None, iam_authorization: bool = None, - accesspoint_id: str = None, + access_point_id: str = None, ): super().__init__() self.mount_dir = Resource.init_param(mount_dir) @@ -388,7 +388,7 @@ def __init__( ) self.encryption_in_transit = Resource.init_param(encryption_in_transit, default=False) self.iam_authorization = Resource.init_param(iam_authorization, default=False) - self.accesspoint_id = Resource.init_param(accesspoint_id) + self.access_point_id = Resource.init_param(access_point_id) def _register_validators(self, context: ValidatorContext = None): # noqa: D102 #pylint: disable=unused-argument self._register_validator(SharedStorageNameValidator, name=self.name) @@ -400,7 +400,7 @@ def _register_validators(self, context: ValidatorContext = None): # noqa: D102 EfsMountOptionsValidator, encryption_in_transit=self.encryption_in_transit, iam_authorization=self.iam_authorization, - accesspoint_id=self.accesspoint_id, + access_point_id=self.access_point_id, name=self.name, ) diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 29fecaf30c..c2bad76bb6 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -321,8 +321,8 @@ class EfsSettingsSchema(BaseSchema): deletion_policy = fields.Str( validate=validate.OneOf(DELETION_POLICIES), metadata={"update_policy": UpdatePolicy.SUPPORTED} ) - accesspoint_id = fields.Str( - validate=validate.Regexp(r"^fsap-[0-9a-z]{17}$"), + access_point_id = fields.Str( + validate=validate.Regexp(r"^fsap-[0-9a-z]{17}$"), metadata={"update_policy": UpdatePolicy.SUPPORTED} ) encryption_in_transit = fields.Bool(metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) iam_authorization = fields.Bool(metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) @@ -334,7 +334,12 @@ def validate_file_system_id_ignored_parameters(self, data, **kwargs): messages = [] if data.get("file_system_id") is not None: for key in data: - if key is not None and key not in ["encryption_in_transit", "iam_authorization", "file_system_id", "accesspoint_id"]: + if key is not None and key not in [ + "encryption_in_transit", + "iam_authorization", + "file_system_id", + "access_point_id" + ]: messages.append(EFS_MESSAGES["errors"]["ignored_param_with_efs_fs_id"].format(efs_param=key)) if messages: raise ValidationError(message=messages) diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py index 25c5642032..a89a264e0d 100644 --- a/cli/src/pcluster/templates/cluster_stack.py +++ b/cli/src/pcluster/templates/cluster_stack.py @@ -1109,7 +1109,7 @@ def _add_efs_storage(self, id: str, shared_efs: SharedEfs): shared_efs.encryption_in_transit ) self.shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"].append(shared_efs.iam_authorization) - self.shared_storage_attributes[SharedStorageType.EFS]["AccesspointIds"].append(shared_efs.accesspoint_id) + self.shared_storage_attributes[SharedStorageType.EFS]["AccessPointIds"].append(shared_efs.access_point_id) return efs_id @@ -1289,8 +1289,8 @@ def _add_head_node(self): "efs_iam_authorizations": to_comma_separated_string( self.shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"], use_lower_case=True ), - "efs_accesspoint_ids": to_comma_separated_string( - self.shared_storage_attributes[SharedStorageType.EFS]["AccesspointIds"], + "efs_access_point_ids": to_comma_separated_string( + self.shared_storage_attributes[SharedStorageType.EFS]["AccessPointIds"], use_lower_case=True, ), "fsx_fs_ids": get_shared_storage_ids_by_type(self.shared_storage_infos, SharedStorageType.FSX), diff --git a/cli/src/pcluster/templates/login_nodes_stack.py b/cli/src/pcluster/templates/login_nodes_stack.py index 299c9bc99b..005bfe6d3b 100644 --- a/cli/src/pcluster/templates/login_nodes_stack.py +++ b/cli/src/pcluster/templates/login_nodes_stack.py @@ -235,8 +235,8 @@ def _add_login_nodes_pool_launch_template(self): self._shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"], use_lower_case=True, ), - "efs_accesspoint_ids": to_comma_separated_string( - self._shared_storage_attributes[SharedStorageType.EFS]["AccesspointIds"], + "efs_access_point_ids": to_comma_separated_string( + self._shared_storage_attributes[SharedStorageType.EFS]["AccessPointIds"], use_lower_case=True, ), "enable_intel_hpc_platform": "true" if self._config.is_intel_hpc_platform_enabled else "false", diff --git a/cli/src/pcluster/templates/queues_stack.py b/cli/src/pcluster/templates/queues_stack.py index fa67a3d692..19cb4a357f 100644 --- a/cli/src/pcluster/templates/queues_stack.py +++ b/cli/src/pcluster/templates/queues_stack.py @@ -335,8 +335,8 @@ def _add_compute_resource_launch_template( self._shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"], use_lower_case=True, ), - "efs_accesspoint_ids": to_comma_separated_string( - self._shared_storage_attributes[SharedStorageType.EFS]["AccesspointIds"], + "efs_access_point_ids": to_comma_separated_string( + self._shared_storage_attributes[SharedStorageType.EFS]["AccessPointIds"], use_lower_case=True, ), "fsx_fs_ids": get_shared_storage_ids_by_type(self._shared_storage_infos, SharedStorageType.FSX), diff --git a/cli/src/pcluster3_config_converter/pcluster3_config_converter.py b/cli/src/pcluster3_config_converter/pcluster3_config_converter.py index e0f148d7a0..d1ccb32b0c 100644 --- a/cli/src/pcluster3_config_converter/pcluster3_config_converter.py +++ b/cli/src/pcluster3_config_converter/pcluster3_config_converter.py @@ -296,7 +296,7 @@ def convert_efs_settings(self, section_name): ("efs_kms_key_id", "KmsKeyId"), ("provisioned_throughput", "ProvisionedThroughput", "getint"), ("throughput_mode", "ThroughputMode"), - ("accesspoint_id", "AccesspointId"), + ("access_point_id", "AccessPointId"), ] efs_section, efs_dict, _section_label = self.convert_storage_base( "efs", efs_label.strip(), additional_items diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json b/cli/tests/pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json index 0862440334..30db9a7a44 100644 --- a/cli/tests/pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json +++ b/cli/tests/pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json @@ -20,6 +20,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_intel_hpc_platform": "false", "ephemeral_dir": "/scratch", "fsx_dns_names": "", diff --git a/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-1.json b/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-1.json index f1b89346fd..7cb4ec7324 100644 --- a/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-1.json +++ b/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-1.json @@ -22,6 +22,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_intel_hpc_platform": "false", "ephemeral_dir": "/scratch", "fsx_dns_names": "", diff --git a/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-2.json b/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-2.json index 3899f16910..8ef858ddba 100644 --- a/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-2.json +++ b/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-2.json @@ -22,6 +22,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_intel_hpc_platform": "false", "ephemeral_dir": "/scratch", "fsx_dns_names": "", diff --git a/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-1.json b/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-1.json index 69a56fa080..18140d9c0c 100644 --- a/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-1.json +++ b/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-1.json @@ -20,6 +20,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_efa": "NONE", "enable_efa_gdr": "NONE", "enable_intel_hpc_platform": "false", diff --git a/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-2.json b/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-2.json index 56d0da1a2c..2f4f04c400 100644 --- a/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-2.json +++ b/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-2.json @@ -20,6 +20,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_efa": "NONE", "enable_efa_gdr": "NONE", "enable_intel_hpc_platform": "false",