Revert "[Bug] Use subnet hash for ELB target group so that a new target group is created whenever we update the LoginNode's subnetID"

This reverts commit 11ab3d4.

Author: Himani Anil Deshpande

CHANGELOG.md

@@ -13,7 +13,6 @@ CHANGELOG
 - Add validation to block updates that change tag order. Blocking such change prevents update failures.
 - Fix LoginNodes NLB not being publicly accessible when using public subnet with implicit main route table association. 
   See https://github.com/aws/aws-parallelcluster/issues/7173
-- Fix cluster update failure when changing LoginNodes subnet by including subnet hash in target group logical ID and name.
 - Fix cluster creation failure without Internet access when GPU instances and DCV are used.
 
 3.14.1

cli/src/pcluster/templates/login_nodes_stack.py

@@ -404,10 +404,9 @@ def _get_launching_lifecycle_hook_specification(self):
         )
 
     def _add_login_nodes_pool_target_group(self):
-        subnet_hash = create_hash_suffix(self._pool.networking.subnet_ids[0])[:8]
         return elbv2.NetworkTargetGroup(
             self,
-            f"{self._pool.name}TargetGroup{subnet_hash}",
+            f"{self._pool.name}TargetGroup",
             health_check=elbv2.HealthCheck(
                 port="22",
                 protocol=elbv2.Protocol.TCP,
@@ -416,14 +415,11 @@ def _add_login_nodes_pool_target_group(self):
             protocol=elbv2.Protocol.TCP,
             target_type=elbv2.TargetType.INSTANCE,
             vpc=self._vpc,
-            # AWS Target Group name limit is 32 characters.
-            # With 3 args: (7 chars * 3) + (2 hyphens) + (1 hyphen) + (8 char hash) = 32 chars
             target_group_name=_get_resource_combination_name(
                 self._config.cluster_name,
                 self._pool.name,
-                subnet_hash,
                 partial_length=7,
-                hash_length=8,
+                hash_length=16,
             ),
         )
 

cli/tests/pcluster/templates/test_cluster_stack.py

@@ -876,7 +876,7 @@ def test_login_nodes_traffic_management_resources_values_properties(
 
     auto_scaling_group_id = "clusternametestloginnodespool1clusternametestloginnodespool1AutoScalingGroup5EBA3937"
     load_balancer_id = "clusternametestloginnodespool1testloginnodespool1LoadBalancerE1D4FCC7"
-    target_group_id = "clusternametestloginnodespool1testloginnodespool1TargetGroup4592cff378B1099C"
+    target_group_id = "clusternametestloginnodespool1testloginnodespool1TargetGroup713F5EC5"
     listener_id = (
         "clusternametestloginnodespool1testloginnodespool1LoadBalancerLoginNodesListenertestloginnodespool165B4D3DC"
     )
@@ -1357,18 +1357,19 @@ def test_custom_munge_key_iam_policy(mocker, test_datadir, config_file_name):
 
 
 @pytest.mark.parametrize(
-    "resource_names, partial_length, hash_length, expected_combination_name",
+    "resource_name_1, resource_name_2, partial_length, hash_length, expected_combination_name",
     [
-        (["test-cluster", "test-pool"], 7, 16, "test-cl-test-po-18c74b16dfbc78ac"),
-        (["abcdefghijk", "lmnopqrst"], 8, 14, "abcdefgh-lmnopqrs-dd65eea0329dcb"),
-        (["a", "b"], 7, 16, "a-b-fb8e20fc2e4c3f24"),
-        # Test with 3 resource names (like target group: cluster_name, pool_name, subnet_hash)
-        (["clustername", "loginpool", "092512032a1df0b2c"], 7, 8, "cluster-loginpo-0925120-fde21041"),
+        ("test-cluster", "test-pool", 7, 16, "test-cl-test-po-18c74b16dfbc78ac"),
+        ("abcdefghijk", "lmnopqrst", 8, 14, "abcdefgh-lmnopqrs-dd65eea0329dcb"),
+        ("a", "b", 7, 16, "a-b-fb8e20fc2e4c3f24"),
     ],
 )
-def test_resource_combination_name(resource_names, partial_length, hash_length, expected_combination_name):
+def test_resource_combination_name(
+    resource_name_1, resource_name_2, partial_length, hash_length, expected_combination_name
+):
     combination_name = _get_resource_combination_name(
-        *resource_names,
+        resource_name_1,
+        resource_name_2,
         partial_length=partial_length,
         hash_length=hash_length,
     )

cli/tests/pcluster/templates/test_login_nodes_stack.py

@@ -78,86 +78,3 @@ def render_join(elem: dict):
         else:
             raise ValueError("Found unsupported item type while rendering Fn::Join")
     return sep.join(rendered_body)
-
-
-@pytest.mark.parametrize(
-    "config_file_name, keep_original_subnet",
-    [
-        ("config-1.yaml", True),
-        ("config-2.yaml", False),
-    ],
-)
-def test_target_group_with_config_files(mocker, test_datadir, config_file_name, keep_original_subnet):
-    """
-    Test target group logical ID and name change when subnet is modified.
-
-    This test verifies that:
-    1. Target group logical ID and name are generated correctly
-    2. When subnet changes, both logical ID and target group name change
-    3. Target group name stays within AWS 32 char limit
-
-    This ensures that cluster updates with subnet changes create new target groups,
-    avoiding "target group cannot be associated with more than one load balancer"
-    and "target group name already exists" errors.
-    """
-    mock_aws_api(mocker)
-    mock_bucket_object_utils(mocker)
-
-    def get_target_group_info(cdk_assets):
-        """Extract target group logical ID and name from CDK assets."""
-        for asset in cdk_assets:
-            asset_content = asset.get("content")
-            if asset_content and "Resources" in asset_content:
-                for resource_name, resource in asset_content["Resources"].items():
-                    if resource.get("Type") == "AWS::ElasticLoadBalancingV2::TargetGroup":
-                        return resource_name, resource["Properties"]["Name"]
-        return None, None
-
-    def build_template(config_yaml):
-        """Build CDK template from config yaml."""
-        cluster_config = ClusterSchema(cluster_name="clustername").load(config_yaml)
-        _, cdk_assets = CDKTemplateBuilder().build_cluster_template(
-            cluster_config=cluster_config, bucket=dummy_cluster_bucket(), stack_name="clustername"
-        )
-        return cdk_assets
-
-    def assert_target_group_info(logical_id, tg_name):
-        """Verify target group logical ID and name are not None and within AWS 32 char limit."""
-        assert_that(logical_id).is_not_none()
-        assert_that(tg_name).is_not_none()
-        assert_that(len(tg_name)).is_less_than_or_equal_to(32)
-
-    # Read yaml config
-    input_yaml = load_yaml_dict(test_datadir / config_file_name)
-    original_subnet = input_yaml["LoginNodes"]["Pools"][0]["Networking"]["SubnetIds"][0]
-
-    # Build template with original subnet
-    cdk_assets_original = build_template(input_yaml)
-    logical_id_original, tg_name_original = get_target_group_info(cdk_assets_original)
-
-    # Verify original target group was created correctly
-    assert_target_group_info(logical_id_original, tg_name_original)
-
-    # Modify subnet to a new value
-    new_subnet = "subnet-12345678901234567"
-    new_pool_count = 0
-    if keep_original_subnet:
-        new_subnet = original_subnet
-        new_pool_count = 1
-    input_yaml["LoginNodes"]["Pools"][0]["Networking"]["SubnetIds"][0] = new_subnet
-    input_yaml["LoginNodes"]["Pools"][0]["Count"] = new_pool_count
-
-    # Build template with new subnet
-    cdk_assets_new = build_template(input_yaml)
-    logical_id_new, tg_name_new = get_target_group_info(cdk_assets_new)
-
-    if keep_original_subnet:
-        assert_that(logical_id_new).is_equal_to(logical_id_original)
-        assert_that(tg_name_new).is_equal_to(tg_name_original)
-    else:
-        # Verify that both logical ID and target group name changed when subnet changed
-        assert_that(logical_id_new).is_not_equal_to(logical_id_original)
-        assert_that(tg_name_new).is_not_equal_to(tg_name_original)
-
-    # Verify new target group was created correctly
-    assert_target_group_info(logical_id_new, tg_name_new)