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.96k stars 1.47k forks source link

EIP allocations and subnet ID order is not respected #3915

Open dgadodia opened 4 weeks ago

dgadodia commented 4 weeks ago

Describe the bug We are using service.beta.kubernetes.io/aws-load-balancer-eip-allocations and service.beta.kubernetes.io/aws-load-balancer-subnets annotations to create a mapping between EIP and subnets. As per docs , EIP allocations has this side note Length/order must match subnets

However this order is not respected because we end up sorting the list of subnets. https://github.com/kubernetes-sigs/aws-load-balancer-controller/blob/990ee2e98224e96ef8ec443e027dcb6ada3bb4f8/pkg/networking/subnet_resolver.go#L461

As a result, we end up mapping EIP to subnets which we don't intend (want) to

Steps to reproduce

Deployed k8s manifest example

apiVersion: v1
kind: Service
metadata:
  annotations:
    kubernetes.io/ingress.class: alb
    service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags: service=myapp-gateway,vertical=platform,cost_center=infrastructure
    service.beta.kubernetes.io/aws-load-balancer-alpn-policy: HTTP2Preferred
    service.beta.kubernetes.io/aws-load-balancer-attributes: deletion_protection.enabled=true,access_logs.s3.enabled=true,access_logs.s3.bucket=myapp-logs-internal-us-west-2,access_logs.s3.prefix=myapp-gateway-access-logs
    service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled: 'false'
    service.beta.kubernetes.io/aws-load-balancer-eip-allocations: eip-w,eip-x,eip-y,eip-z
    service.beta.kubernetes.io/aws-load-balancer-ip-address-type: dualstack
    service.beta.kubernetes.io/aws-load-balancer-name: myapp-gateway
    service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip
    service.beta.kubernetes.io/aws-load-balancer-proxy-protocol: '*'
    service.beta.kubernetes.io/aws-load-balancer-scheme: internet-facing
    service.beta.kubernetes.io/aws-load-balancer-ssl-cert: arn:aws:acm:us-west-2:1234567890:certificate/some-random-uuid
    service.beta.kubernetes.io/aws-load-balancer-ssl-negotiation-policy: ELBSecurityPolicy-FS-1-2-Res-2020-10
    service.beta.kubernetes.io/aws-load-balancer-ssl-ports: https
    service.beta.kubernetes.io/aws-load-balancer-subnets: subnet-random-ewewe, subnet-random-wewe, subnet-random-qrqr, subnet-random-rqrq
    service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: proxy_protocol_v2.enabled=true,deregistration_delay.timeout_seconds=240,deregistration_delay.connection_termination.enabled=true
    service.beta.kubernetes.io/aws-load-balancer-type: external
  labels:
    app: myapp-gateway
    install.operator.istio.io/owning-resource: unknown
    istio: ingressgateway
    istio.io/rev: 1-22
    operator.istio.io/component: IngressGateways
    release: istio
  name: myapp-gateway
  namespace: istio-system
spec:
  loadBalancerSourceRanges:
  - 0.0.0.0/0
  ports:
  - name: https
    port: 443
    protocol: TCP
    targetPort: 8080
  selector:
    app: myapp-gateway
    istio: ingressgateway
  type: LoadBalancer

Expected outcome A concise description of what you expected to happen.

When we set these annotations

service.beta.kubernetes.io/aws-load-balancer-eip-allocations: eip-w, eip-x, eip-y, eip-z
service.beta.kubernetes.io/aws-load-balancer-subnets: subnet-random-ewewe, subnet-random-wewe, subnet-random-qrqr, subnet-random-rqrq

The mapping of the NLB created should be eip-w -> subnet-random-ewewe eip-x -> subnet-random-wewe eip-y -> subnet-random-qrqr, eip-z -> subnet-random-rqrq

Current outcome The mapping which ends up being created is

>>> sorted(["subnet-random-ewewe", "subnet-random-wewe", "subnet-random-qrqr", "subnet-random-rqrq"])
['subnet-random-ewewe', 'subnet-random-qrqr', 'subnet-random-rqrq', 'subnet-random-wewe']

eip-w -> subnet-random-ewewe eip-x -> subnet-random-qrqr eip-y -> subnet-random-rqrq eip-z -> subnet-random-wewe

Environment

Additional Context: