Include tags in RunInstances dry run

This change is motivated by users whose organizational security policies require certain tags to be present when they're attempting to launch instances. Without this change, the dry run performed as part of validation would fail, and the CLI would incorrectly tell the customer that there's an issue with their configuration file. Signed-off-by: Tim Lane <[email protected]>
aws · Oct 22, 2021 · 8b6a1bb · 8b6a1bb
1 parent d87b768
commit 8b6a1bb
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,9 @@ CHANGELOG
 3.0.1
 ------
 
+**CHANGES**
+- Include tags from cluster configuration file in the RunInstances dry runs performed during configuration validation.
+
 **ENHANCEMENTS**
 - Add `pcluster3-config-converter` CLI command to convert cluster configuration from ParallelCluster 2 to ParallelCluster 3 version.
 - The region parameter is now retrieved from the provider chain, thus supporting the use of profiles and defaults

diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py
@@ -972,7 +972,9 @@ def _register_validators(self):
             SubnetsValidator, subnet_ids=self.compute_subnet_ids + [self.head_node.networking.subnet_id]
         )
         self._register_storage_validators()
-        self._register_validator(HeadNodeLaunchTemplateValidator, head_node=self.head_node, ami_id=self.head_node_ami)
+        self._register_validator(
+            HeadNodeLaunchTemplateValidator, head_node=self.head_node, ami_id=self.head_node_ami, tags=self.tags
+        )
 
         if self.head_node.dcv:
             self._register_validator(
@@ -1520,7 +1522,7 @@ def _register_validators(self):
 
         for queue in self.scheduling.queues:
             self._register_validator(
-                ComputeResourceLaunchTemplateValidator, queue=queue, ami_id=self.image_dict[queue.name]
+                ComputeResourceLaunchTemplateValidator, queue=queue, ami_id=self.image_dict[queue.name], tags=self.tags
             )
             queue_image = self.image_dict[queue.name]
             if queue_image not in checked_images and queue.queue_ami:

diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py
@@ -799,11 +799,21 @@ def _ec2_run_instance(self, availability_zone: str, **kwargs):  # noqa: C901 FIX
                     FailureLevel.ERROR,
                 )
 
+    @staticmethod
+    def _generate_tag_specifications(tags):
+        """Turn list of Tag objects into tag specifications required by RunInstances."""
+        tag_specifications = []
+        if tags:
+            tag_specifications.append(
+                {"ResourceType": "instance", "Tags": [{"Key": tag.key, "Value": tag.value} for tag in tags]}
+            )
+        return tag_specifications
+
 
 class HeadNodeLaunchTemplateValidator(_LaunchTemplateValidator):
     """Try to launch the requested instance (in dry-run mode) to verify configuration parameters."""
 
-    def _validate(self, head_node, ami_id):
+    def _validate(self, head_node, ami_id, tags):
         try:
             head_node_security_groups = []
             if head_node.networking.security_groups:
@@ -835,6 +845,7 @@ def _validate(self, head_node, ami_id):
                 CpuOptions=head_node_cpu_options,
                 NetworkInterfaces=head_node_network_interfaces,
                 DryRun=True,
+                TagSpecifications=self._generate_tag_specifications(tags),
             )
         except Exception as e:
             self._add_failure(
@@ -865,7 +876,7 @@ def _validate(self, imds_secured: bool, scheduler: str):
 class ComputeResourceLaunchTemplateValidator(_LaunchTemplateValidator):
     """Try to launch the requested instances (in dry-run mode) to verify configuration parameters."""
 
-    def _validate(self, queue, ami_id):
+    def _validate(self, queue, ami_id, tags):
         try:
             # Retrieve network parameters
             queue_subnet_id = queue.networking.subnet_ids[0]
@@ -893,14 +904,15 @@ def _validate(self, queue, ami_id):
                 subnet_id=queue_subnet_id,
                 security_groups_ids=queue_security_groups,
                 placement_group=queue_placement_group,
+                tags=tags,
             )
         except Exception as e:
             self._add_failure(
                 f"Unable to validate configuration parameters for queue {queue.name}.\n{str(e)}", FailureLevel.ERROR
             )
 
     def _test_compute_resource(
-        self, compute_resource, use_public_ips, ami_id, subnet_id, security_groups_ids, placement_group
+        self, compute_resource, use_public_ips, ami_id, subnet_id, security_groups_ids, placement_group, tags
     ):
         """Test Compute Resource Instance Configuration."""
         compute_cpu_options = (
@@ -925,6 +937,7 @@ def _test_compute_resource(
             Placement=placement_group,
             NetworkInterfaces=network_interfaces,
             DryRun=True,
+            TagSpecifications=self._generate_tag_specifications(tags),
         )
 
 

diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py
@@ -9,8 +9,10 @@
 # OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and
 # limitations under the License.
 import pytest
+from assertpy import assert_that
 
 from pcluster.aws.aws_resources import InstanceTypeInfo
+from pcluster.config.cluster_config import Tag
 from pcluster.constants import PCLUSTER_NAME_MAX_LENGTH
 from pcluster.validators.cluster_validators import (
     FSX_MESSAGES,
@@ -39,6 +41,7 @@
     RegionValidator,
     SchedulerOsValidator,
     SharedStorageNameValidator,
+    _LaunchTemplateValidator,
 )
 from tests.pcluster.aws.dummy_aws_api import mock_aws_api
 from tests.pcluster.validators.utils import assert_failure_messages
@@ -885,3 +888,22 @@ def test_hosted_zone_validator(mocker, vpcs, is_private_zone, domain_name, expec
         cluster_name="ThisClusterNameShouldBeRightSize-ContainAHyphen-AndANumber12",
     )
     assert_failure_messages(actual_failures, expected_message)
+
+
+@pytest.mark.parametrize(
+    "input_tags",
+    [
+        [],
+        [{"key": "SomeKey", "value": "SomeValue"}],
+    ],
+)
+def test_generate_tag_specifications(input_tags):
+    """Verify function to generate tag specifications for dry runs of RunInstances works as expected."""
+    input_tags = [Tag(tag.get("key"), tag.get("value")) for tag in input_tags]
+    if input_tags:
+        expected_output_tags = [
+            {"ResourceType": "instance", "Tags": [{"Key": tag.key, "Value": tag.value} for tag in input_tags]}
+        ]
+    else:
+        expected_output_tags = []
+    assert_that(_LaunchTemplateValidator._generate_tag_specifications(input_tags)).is_equal_to(expected_output_tags)