compose-x / ecs_composex

Manage, Configure and Deploy your services and AWS services and applications from your docker-compose definitions
https://docs.compose-x.io
Mozilla Public License 2.0
158 stars 16 forks source link

Lookup an existing loadbalancer #728

Closed slootmaekersdirk closed 6 months ago

slootmaekersdirk commented 6 months ago

Is your feature request related to a problem? Please describe. I have a setup where I create a compose-x stack 1 and in my compose-x stack 2 I want to reuse the VPC and loadbalancer I created with compose-x stack 1

Describe the solution you'd like We should be able to lookup a x-elbv2 (in my case application lb) the same way as we can lookup a x-vpc

JohnPreston commented 6 months ago

Thanks for the FR. This is a duplicate of #687 In progress

slootmaekersdirk commented 6 months ago

Hello John,

Sorry for the duplicate, do you have a time frame when it is going to be available?

Thx Dirk

From: John Preston @.> Date: Tuesday, 12 December 2023 at 14:36 To: compose-x/ecs_composex @.> Cc: Dirk Slootmaekers @.>, Author @.> Subject: Re: [compose-x/ecs_composex] Lookup an existing loadbalancer (Issue #728)

Closed #728https://github.com/compose-x/ecs_composex/issues/728 as completed.

— Reply to this email directly, view it on GitHubhttps://github.com/compose-x/ecs_composex/issues/728#event-11223683872, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACNLQBU7FHFLY6JC57FG4YLYJBMXPAVCNFSM6AAAAABARPFJPKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGIZDGNRYGM4DOMQ. You are receiving this because you authored the thread.Message ID: @.***>

JohnPreston commented 6 months ago

I don't have a timeframe no, but happy to prioritize :)

PS: it's one of these things that you just dread doing :D

slootmaekersdirk commented 6 months ago

we will be looking forward to this FR, for us it is the missing piece to deploy our infra using compose-x

JohnPreston commented 6 months ago

we will be looking forward to this FR, for us it is the missing piece to deploy our infra using compose-x

No worries. Would you mind describing your use-case (what type of LB? Do you want to add listeners? Listener rules? This ticket or the other one, whichever :)

slootmaekersdirk commented 6 months ago

sure... here is our use-case: In our environment (using compose-ecs) we have 1 ecs cluster, using a VPC and ALB loadbalancer. In the future we want to add an extra ecs cluster, attaching to the same VPC and ALB loadbalancer and adding new listeners for this new cluster. This is how we do it at the moment with compose-ecs. We have some 'default' setup and adapt it on the fly by adding additional clusters when needed

So In short we need to lookup an ALB (application loadbalancer) and add new listener + rules to it. updating existing rules is a nice feature but for the moment we don't need that.

Dirk

JohnPreston commented 6 months ago

Hey :) Thanks for the details :+1: Just for my own clarity, you are using this compose-ecs? https://github.com/docker/compose-ecs

JohnPreston commented 6 months ago

So In short we need to lookup an ALB (application loadbalancer) and add new listener + rules to it. updating existing rules is a nice feature but for the moment we don't need that.

That is very useful thank you. Updating rules is quite a bit of a giant pain TBH even when you evolve them through compose-x due to how the rule ID is generated etc.

I think I might be able to deal with adding a new Listener and its rules quite easily, but time will tell how "not ready" the code is for this.

slootmaekersdirk commented 6 months ago

Yes, that is what we use.Sent from my iPhoneOn 12 Dec 2023, at 21:47, John Preston @.***> wrote: Hey :) Just for my own clarity, you are using this compose-ecs? https://github.com/docker/compose-ecs

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

slootmaekersdirk commented 6 months ago

hello John,

can you give us an update on the progress?

thank you , Dirk

JohnPreston commented 6 months ago

Hi @slootmaekersdirk This is most likely going to be my today/tomorrow next thing :)

slootmaekersdirk commented 5 months ago

Hello John,

As I need to start our deployment (UAT), when can we expect this fix?

and if available, how should I use it?

thx Dirk

JohnPreston commented 5 months ago

@slootmaekersdirk Hey Assuming you are going to create new listeners, the code in the branch for #734 If you could please git it a try and provide some feedback as to the workings? Thanks,

slootmaekersdirk commented 5 months ago

Hello John,

thx,

I installed it but can you give me an example on how to use it? I have something like this: is this correct? (I receive errors)

x-elbv2: lbA: Lookup: Name: existing_name_of_loadbalancer Services:

so to recap:

with the above config i get : UserWarning: x-elbv2 - Lookup not supported. You can only create new resources

JohnPreston commented 5 months ago

I have been this, with my own real domain names though. This would work to add a new listener. You need to install it from git from the branch of the PR

pip install git+https://github.com/compose-x/ecs_composex@feature/x-elbv2-lookup


services:
  proxy:
    image: nginx
    ports:
      - 6969
    deploy:
      labels:
        ecs.task.family: proxy

x-elbv2:
  public-ingress:
    DnsAliases:
      - Route53Zone: x-route53::PublicZone
        Names:
          - "proxy.tld"

    Lookup:
      loadbalancer:
        Tags:
          Name: proxy-publicingress
    Listeners:
      - Port: 6969
        Protocol: TCP
        Targets:
          - name: proxy:proxy:6969
            access: /
    Services:
      - name: proxy:proxy
        port: 6969
        protocol: TCP
        healthcheck: 6969:TCP:2:2:15:5
        TargetGroupAttributes:
          - Key: deregistration_delay.timeout_seconds
            Value: "15"
          - Key: deregistration_delay.connection_termination.enabled
            Value: "false"
          - Key: preserve_client_ip.enabled
            Value: "true"

x-route53:
  PublicZone:
    Name: proxy
    Lookup: true
slootmaekersdirk commented 5 months ago

Hello John,

after some testing (Lookup works !, add Listeners works) I saw that there is no inbound rule added to the Existing security group of the existing loadbalancer (in the below case there is no inbound rule added for port 82):

with the below example (2nd part) I see a new SG ( lbA: ) but I just want to reuse the existing LB SG for that (the new SG is not linked to the existing LB).

is this the correct syntax in the yml file? ? I just want to lookup an existing LB add some listeners and that's it (no other stuff)

This is how I created the original (first) LB: -> creates SG lbA attached to LB x-elbv2: lbA: Properties: Scheme: internet-facing Type: application MacroParameters: Ingress: ExtSources:

and this is how I want to link the new services to the exsting LB: -> creates another SG "LBA" not linked to the existing LB

x-vpc: Lookup: VpcId: Tags:

x-cluster: Lookup: ClusterName: modev02

networks: application: x-vpc: AppSubnets

x-elbv2: lbA: Properties: Scheme: internet-facing Type: application MacroParameters: Ingress: ExtSources:

thx Dirk

JohnPreston commented 5 months ago

Interesting conflicts on the dependencies. Bizarre really. Try again with creating a new venv first maybe?. python3 -m venv compose-x-dev; source compose-x-dev/bin/activate then re run the pip install from git branch

slootmaekersdirk commented 5 months ago

Interesting conflicts on the dependencies. Bizarre really. Try again with creating a new venv first maybe?. python3 -m venv compose-x-dev; source compose-x-dev/bin/activate then re run the pip install from git branch

indeed; that fixes the issue. after wards I did additional tests (see latest comment on the creation of the SG when performing a lookup) (I have updated the existing comment a few times, so you should receive some emails on that, just look at the latest one)

slootmaekersdirk commented 5 months ago

Hey John,

just to summarize:

issue is that a new SG is created without any inbound rules and that it is not linked to the LB.

I suggest that the existing security group of the LB is used and add the inbound rules to open up the ports to that one

Dirk

JohnPreston commented 5 months ago

Oh okay. Interesting. I tried with a NLB mostly so far so I need to check on the ALB to create the ingress rules indeed. Thanks a lot for the feedback. Thoughts on maybe what could be done differently? Thanks

slootmaekersdirk commented 5 months ago

Hello John,

I suggest that when a lookup of an LB is done, in stead of the default creation of the SG, you lookup the associated SG of the LB as well and add the additional ingress rules to that one

Dirk

JohnPreston commented 5 months ago

Yeah already do find the SG. Need to re-use it :)

slootmaekersdirk commented 5 months ago

Hello John,

Do you have an update on this?

Thx Dirk

JohnPreston commented 5 months ago

Yes, so I can either release it as-is with only adding new listeners, or add listeners. I have need for existing listeners myself, but happy to split the work into two. Thoughts?

slootmaekersdirk commented 5 months ago

Hello John,

unfortunately I'm not a python programmer, so I can't help you on this one. The only part missing for me is to use the existing SG of the LB and add the ingress rules to that one.

can you add this functionality?

Dirk

JohnPreston commented 5 months ago

unfortunately I'm not a python programmer, so I can't help you on this one.

No worries, wasn't what I meant ;)

The only part missing for me is to use the existing SG of the LB and add the ingress rules to that one.

I'll do that and make a release, add more functionalities later I suppose.

JohnPreston commented 5 months ago

@slootmaekersdirk I just tested with ALB and ports, and I can see in the CFN template of the service itself that is has the Security Group ingress rules already. Did that not appear from your tests? Are you trying with a NLB or ALB? Thanks,

slootmaekersdirk commented 5 months ago

Hello,

I'm also trying with ALB but when I open the SG of the LB then no new ports are added.

In the below example it should add port 82 to the existing group of the LB. What I see is that a new SG is created, not linked to the LB and not containing port 82

x-vpc: Lookup: VpcId: Tags:

x-cluster: Lookup: ClusterName: modev02

networks: application: x-vpc: AppSubnets

x-elbv2: modevLB: Properties: Scheme: internet-facing Type: application MacroParameters: Ingress: ExtSources:

services: modev02-apigateway: x-iam: ManagedPolicyArns:

JohnPreston commented 5 months ago

Oh I see, my bad. I thought you meant LB -> Service

JohnPreston commented 5 months ago

@slootmaekersdirk That's done in the latest commit of the feature branch. Let me know if that works for you too :)

slootmaekersdirk commented 5 months ago

Hello John,

I did pip install git+https://github.com/compose-x/ecs_composex@feature/x-elbv2-lookup

Still a new SG (also here no ports are added) is created and the SG attached to the LB does not have the new ingress ports

So the fix didn't make any difference Dirk

slootmaekersdirk commented 5 months ago

sorry, my mistake.

it is adding the ingress port to the SG attached to the LB ! but it also creates a new one, which is not used.

So the fix works

Thx

I will perform some additional tests

JohnPreston commented 5 months ago

Let me fix that new SG created. This might be a good feature though :thinking: For now, will take it out.

EDIT: I pushed the change to not create the SG when looking up.

slootmaekersdirk commented 5 months ago

ok, thanks for the support. You can merge it with the trunk

Dirk

slootmaekersdirk commented 5 months ago

Hello John,

when are you performing the merge ?

thx Dirk

JohnPreston commented 5 months ago

Hey @slootmaekersdirk I have worked on the lookup for listeners this weekend, it's nearly done. I need to do actual tests. Then I will merge the branch in.

In the meantime installing it from git+https://@branch should work. I will make sure not to delete the branch until I have updated you in case you have that somewhere in your CICD process.

JohnPreston commented 4 months ago

@slootmaekersdirk new version will be released this week :) Sorry for the delay, it has been a very busy start of the year, but my biggest deliveries are now done, I will be able to give my projects some love again :)

JohnPreston commented 4 months ago

@slootmaekersdirk I published 0.25.0-rc1 today This has the lookup changes including re-using existing listeners (ALB only, NLBs don't have rules)

slootmaekersdirk commented 1 month ago

Hello John,

it seems this is broke in in the latest version. I don't want to lookup any Listeners for an existing LB, but just look it up and add new onces. this is my yml -> x-elbv2: modevLB: Properties: Scheme: internet-facing Type: application MacroParameters: Ingress: ExtSources:

services: modev03-apigateway: ....

but it fails with the error -> ('You must specify at least one Listener for a LB.', 'modevLB')

this was not the case in the RC

what am i doing wrong here?

Dirk

slootmaekersdirk commented 1 month ago

when I look at the rule, you are obliged to enter a 'Listener' under the Lookup ! that is not what I want, I don't want to lookup listners

-> or (self.lookup and not keyisset("Listeners", self.lookup))

Dirk

slootmaekersdirk commented 1 month ago

Hello John,

did you find time to give some feedback?

thx Dirk

JohnPreston commented 1 month ago

Hello @slootmaekersdirk Sorry this totally flew under the radar. Let me open up an issue for it.

EDIT: https://github.com/compose-x/ecs_composex/issues/758