kubernetes-sigs / aws-load-balancer-controller

A Kubernetes controller for Elastic Load Balancers
https://kubernetes-sigs.github.io/aws-load-balancer-controller/
Apache License 2.0
3.91k stars 1.45k forks source link

Overwrite the configuration of NLB using the same NLB name #2533

Closed davidshtian closed 2 years ago

davidshtian commented 2 years ago

Describe the bug The original idea it to reuse NLB for multiple services for easier management, while target group binding might be a workaround for this, there is still risk for this that if some team member using the same NLB for a new service, all the configuration of NLB will be overwrited and thus impact the business, as there may not a name verify for existing NLB.

Steps to reproduce Create 2 services with same service.beta.kubernetes.io/aws-load-balancer-name annotation but different configurations, and the second one will overwrite the first one.

Expected outcome It will check if the NLB exists or not, and it either throws an error or append the new listener to NLB.

Environment

Additional Context: Similar issues: #1707 , #1608

kishorj commented 2 years ago

@davidshtian, you should be able to specify the same name for two services and end up with two separate NLB. We will verify it on our side and fix it if this is not the case. EDIT: two services cannot have the same name via annotation. The second one will fail.

haouc commented 2 years ago

/assign

davidshtian commented 2 years ago

@davidshtian, you should be able to specify the same name for two services and end up with two separate NLB. We will verify it on our side and fix it if this is not the case.

I've tried using v2.2+ controller and it will only create one NLB with same name annotation for two or more different services, and the last created conf will overwrite the previous ones, but not append new listener to NLB.

davidshtian commented 2 years ago

Besides, from logs of controller pod it seems that to deploy the model, the controller first clean up the previous listener. Hope the information is helpful. Thanks

{"level":"info","ts":1646284569.4988031,"logger":"controllers.service","msg":"creating loadBalancer","stackID":"default/my-nginx","resourceID":"LoadBalancer"}
{"level":"info","ts":1646284569.9104574,"logger":"controllers.service","msg":"created loadBalancer","stackID":"default/my-nginx","resourceID":"LoadBalancer","arn":"arn:aws:elasticloadbalancing:us-east-1::loadbalancer/net/my-nginx/1839f8b3b12b8"}
{"level":"info","ts":1646284570.0128255,"logger":"controllers.service","msg":"deleting listener","arn":"arn:aws:elasticloadbalancing:us-east-1::listener/net/my-nginx/1839f8b3b12b8/3d2437a701582"}
{"level":"info","ts":1646284570.1205678,"logger":"controllers.service","msg":"deleted listener","arn":"arn:aws:elasticloadbalancing:us-east-1::listener/net/my-nginx/1839f8b3b12b8/3d2437a701582"}
{"level":"info","ts":1646284570.1206074,"logger":"controllers.service","msg":"creating listener","stackID":"default/my-nginx","resourceID":"9376"}
{"level":"info","ts":1646284570.2031991,"logger":"controllers.service","msg":"created listener","stackID":"default/my-nginx","resourceID":"9376","arn":"arn:aws:elasticloadbalancing:us-east-1::listener/net/my-nginx/1839f8b3b12b8/b24ae68ddc8f1"}
{"level":"info","ts":1646284570.2378979,"logger":"controllers.service","msg":"successfully deployed model","service":{"namespace":"default","name":"my-nginx"}}
rverma-dev commented 2 years ago

Faced same issue, unknowingly created a duplicate https://github.com/kubernetes-sigs/aws-load-balancer-controller/issues/2536

M00nF1sh commented 2 years ago

@davidshtian @rverma-nsl The names of LoadBalancers must be unique within region. So the correct behavior should be the 2nd Service will failed to be reconciled due to the name is already token by another existing NLB.

Hasn't tested this yet, but I guess it's due to ELB's API idempotency, thus when we try create the 2nd NLB with same name, it returned the ARN of previous created one. We'll validate above and fix if that's the case (the fix might be validate the tags on the returned NLB, and issue a k8s event about NLB with same name already exists)

For customer use cases, we should assign different names per service, or let the controller auto-generate LB names by not specify the service.beta.kubernetes.io/aws-load-balancer-name annotation

/kind bug

walkley commented 2 years ago

@M00nF1sh You are right, I run into the same issue and confirmed that the root cause is that CreateLoadBalancer will return the existed NLB ARN if there already have NLB with the same name.

Just running following command twice, the output will be the same, no errors in the second call:

$ aws elbv2 create-load-balancer --name whe-lb-test --type network --subnet-mappings SubnetId=subnet-0123456789 
{
    "LoadBalancers": [
        {
            "LoadBalancerArn": "arn:aws:elasticloadbalancing:ap-northeast-1:0123456789:loadbalancer/net/whe-lb-test/124880a5d69e957a",
            "DNSName": "whe-lb-test-0123456789.elb.ap-northeast-1.amazonaws.com",
            "CanonicalHostedZoneId": "Z31USIVHYNEOWT",
            "CreatedTime": "2022-04-14T09:35:20.815Z",
            "LoadBalancerName": "whe-lb-test",
            "Scheme": "internet-facing",
            "VpcId": "vpc-0123456789",
            "State": {
                "Code": "provisioning"
            },
            "Type": "network",
            "AvailabilityZones": [
                {
                    "ZoneName": "ap-northeast-1a",
                    "SubnetId": "subnet-0123456789",
                    "LoadBalancerAddresses": []
                }
            ],
            "IpAddressType": "ipv4"
        }
    ]
}
k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot commented 2 years ago

@k8s-triage-robot: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/aws-load-balancer-controller/issues/2533#issuecomment-1242937328): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues and PRs according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue or PR with `/reopen` >- Mark this issue or PR as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
eslam-gomaa commented 1 year ago

Since the new NLB will override the existing one with the same name, the expected behaviour then should be to fail the NLB creation if the load balancer name is already used.

k8s-ci-robot commented 1 year ago

@abohne: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to [this](https://github.com/kubernetes-sigs/aws-load-balancer-controller/issues/2533#issuecomment-1577260513): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
abohne commented 1 year ago

This issue should not be closed. It is still an existing bug.

4rth4S commented 1 year ago

I'm facing this issue using v2.6 of aws-load-balancer-controller, the weird thing is that two weeks ago I tried to create 3 services attached to the same NLB using different ports in v2.5.4 of aws-load-balancer-controller and it worked. Particularly I used these annotations at this moment.

    service.beta.kubernetes.io/aws-load-balancer-name:
    service.beta.kubernetes.io/aws-load-balancer-nlb-target-type:
    service.beta.kubernetes.io/aws-load-balancer-scheme:
    service.beta.kubernetes.io/aws-load-balancer-subnets:
    service.beta.kubernetes.io/aws-load-balancer-type:

Hint 💡 Uninstall actual version with helm delete aws-load-balancer-controller -n kube-system Install the v2.5.4 that works with the helm chart v1.5.5 with

helm install aws-load-balancer-controller eks/aws-load-balancer-controller --version 1.5.5 -n kube-system --set clusterName=YOUR-CLUSTER-NAME --set serviceAccount.create=false --set serviceAccount.name=YOUR-SA-CREATED-BEFORE-FOR-THIS