Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ecs-patterns]: load balancer name of ApplicationLoadBalancerProps not working when creating ApplicationMultipleTargetGroupsEc2Service #23535

Closed
Puskin2911 opened this issue Jan 3, 2023 · 4 comments · Fixed by #28394
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@Puskin2911
Copy link

Puskin2911 commented Jan 3, 2023

Describe the bug

I'm creating an ecs service with multiple target groups using ecs_pattern module.

alb_props = ecs_patterns.ApplicationLoadBalancerProps(
            name="custom-alb-name",
            domain_name="*.example.com",
            domain_zone=route53.HostedZone.from_hosted_zone_attributes(
                self, "HostedZone",
                hosted_zone_id="xxxxxx",
                zone_name="example.com"
            ),
            listeners=[
                listener_1, listener2
            ],
            public_load_balancer=False
        )

ecs_service = ecs_patterns.ApplicationMultipleTargetGroupsEc2Service(
            self, "ECS-ID",
            service_name="service-name",
            cluster=cluster-name-ref,
            task_definition=task_definition,
            load_balancers=[alb_props],
            target_groups=[target_props_1, target_prop_2],
            cpu=1024,
            memory_reservation_mib=2048,
            desired_count=1
        )

Expected Behavior

The expected name of load balancer name should be "custom-alb-name"

Current Behavior

Load balancer name is auto-generated

Reproduction Steps

  1. Create cdk
  2. Wiring code like snip above
  3. Run cdk deploy

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.54.0

Framework Version

No response

Node.js Version

v16.15.0

OS

Window

Language

Python

Language Version

python 3.11.1

Other information

No response

@Puskin2911 Puskin2911 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 3, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Jan 3, 2023
@Puskin2911
Copy link
Author

I'm not sure about this, but i found the issue in this snip code:

  private createLoadBalancer(name: string, publicLoadBalancer?: boolean, idleTimeout?: Duration): ApplicationLoadBalancer {
    const internetFacing = publicLoadBalancer ?? true;
    const lbProps = {
      vpc: this.cluster.vpc,
      internetFacing,
      idleTimeout: idleTimeout,
    };

    return new ApplicationLoadBalancer(this, name, lbProps);
  }

Line 596-695 in this file
https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts

I think it should be

  private createLoadBalancer(id: string, name?: string, publicLoadBalancer?: boolean, idleTimeout?: Duration): ApplicationLoadBalancer {
    const internetFacing = publicLoadBalancer ?? true;
    const lbProps = {
      vpc: this.cluster.vpc,
      internetFacing,
      idleTimeout: idleTimeout,
      loadBalancerName: name
    };

    return new ApplicationLoadBalancer(this, id, lbProps);
  }

@pahud
Copy link
Contributor

pahud commented Feb 16, 2023

Yes this seems to be a bug. Are you interested to submit a PR for that?

@pahud pahud added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 16, 2023
@Puskin2911
Copy link
Author

@pahud I want but i have no test environment and i have no idea how to verify it's works. @@

sumupitchayan added a commit to lpizzinidev/aws-cdk that referenced this issue Dec 26, 2023
sumupitchayan added a commit to lpizzinidev/aws-cdk that referenced this issue Dec 26, 2023
@mergify mergify bot closed this as completed in #28394 Dec 26, 2023
mergify bot pushed a commit that referenced this issue Dec 26, 2023
…s load balancer name (#28394)

Fixes by adding the `loadBalancerName` property to the generated `ApplicationLoadBalancer`.

Closes #23535.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

paulhcsun pushed a commit to paulhcsun/aws-cdk that referenced this issue Jan 5, 2024
…s load balancer name (aws#28394)

Fixes by adding the `loadBalancerName` property to the generated `ApplicationLoadBalancer`.

Closes aws#23535.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
2 participants