Loadbalancers created via aws-load-balancer-controller for EnvoyProxy instances are leaked once Gateway is removed

[wondersd]

Description:
When creating a Gateway instance in an EKS cluster using the aws-load-balancer-controller to provision loadbalancers, subsequent deletion of the Gateway resource can leave the underlying AWS resources (loadbalancer, targetgroup, sg rules, sgs, etc) leaked and never deleted.

The underlying reason for this appears to be EG is striping the .metadata.finalizers section of the managed Service object. As aws-load-balancer-controller (and probably any and all load balancer providers for that matter) create non k8s resources that must be cleaned up, the finalizer is injected onto the service so that it the controller is guaranteed time to reconcile the deletion of these resources before losing the Service object (and then its tracking of those resources leaving them leaked).

EnvoyProxy service object created

apiVersion: v1
  kind: Service
  metadata:
    annotations:
      service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip
      service.beta.kubernetes.io/aws-load-balancer-scheme: internal
      service.beta.kubernetes.io/aws-load-balancer-type: external
    creationTimestamp: "2023-08-23T14:18:56Z"
    labels:
      app.kubernetes.io/component: proxy
      app.kubernetes.io/managed-by: envoy-gateway
      app.kubernetes.io/name: envoy
      gateway.envoyproxy.io/owning-gateway-name: ...
      gateway.envoyproxy.io/owning-gateway-namespace:...
    name: envoy-...-72616262
    namespace: envoy-gateway
    resourceVersion: ...
    uid: ...
  spec:
    allocateLoadBalancerNodePorts: true
    clusterIP: 172.20.132.225
    clusterIPs:
    - 172.20.132.225
    externalTrafficPolicy: Local
    healthCheckNodePort: 30042
    internalTrafficPolicy: Cluster
    ipFamilies:
    - IPv4
    ipFamilyPolicy: SingleStack
    ports:
    ...
    selector:
      app.kubernetes.io/component: proxy
      app.kubernetes.io/managed-by: envoy-gateway
      app.kubernetes.io/name: envoy
      gateway.envoyproxy.io/owning-gateway-name: ...
      gateway.envoyproxy.io/owning-gateway-namespace:...
    sessionAffinity: None
    type: LoadBalancer
  status:
    loadBalancer:
      ingress:
      - hostname: ....elb.us-east-1.amazonaws.com

A service object created manually from the following yaml

apiVersion: v1
kind: Service
metadata:
  name: loadbalancer
  namespace: envoy-gateway
  annotations:
    service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip
    service.beta.kubernetes.io/aws-load-balancer-scheme: internal
    service.beta.kubernetes.io/aws-load-balancer-type: external
spec:
  type: LoadBalancer
  ports:
    - name: test
      port: 8080
      protocol: TCP

yeilds

apiVersion: v1
kind: Service
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: ...
    service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip
    service.beta.kubernetes.io/aws-load-balancer-scheme: internal
    service.beta.kubernetes.io/aws-load-balancer-type: external
  creationTimestamp: "2023-08-23T15:04:39Z"
  finalizers:
  - service.kubernetes.io/load-balancer-cleanup
  - service.k8s.aws/resources
  name: loadbalancer
  namespace: envoy-gateway
  resourceVersion: ...
  uid: ...
spec:
  allocateLoadBalancerNodePorts: true
  clusterIP: 172.20.204.23
  clusterIPs:
  - 172.20.204.23
  externalTrafficPolicy: Cluster
  internalTrafficPolicy: Cluster
  ipFamilies:
  - IPv4
  ipFamilyPolicy: SingleStack
  ports:
  - name: test
    nodePort: 30456
    port: 8080
    protocol: TCP
    targetPort: 8080
  sessionAffinity: None
  type: LoadBalancer
status:
  loadBalancer:
    ingress:
    - hostname: ....elb.us-east-1.amazonaws.com

Unlike what is observed from the EG managed service, we have the finalizer section intact for the manually created service (with it not being part of the original yaml). So it seems as though the reconciliation process is stripping that out. Without that our loadbalancer provider is unable to guarantee cleanup the cloud resources.

Repro steps:

1. setup gatewayclass, envoyproxy and gateway resource as per normal with service type LoadBalancer (default) and requisite annotations to activate aws-load-balancer-controller (should also be reproducible for any other loadbalancer providers that rely on finalizers)

   apiVersion: "config.gateway.envoyproxy.io/v1alpha1"
   kind: "EnvoyProxy"
   metadata:
     name: "aws-loadbalancer-controller-ep"
   spec:
     provider:
       type: "Kubernetes"
       kubernetes:
         envoyService:
           annotations:
             service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip
             service.beta.kubernetes.io/aws-load-balancer-scheme: internal
             service.beta.kubernetes.io/aws-load-balancer-type: external
   

2. delete Gateway object. see aws loadbalancer is not removed

Environment:

• aws based kubernetes cluster with aws-load-balancer-controller available to be used
• eg 0.5.0

Logs:
Have not been able to locate specific logs indicating finalizer is being removed.

Relates to:

• envoyproxy instance svc update error after gateway restart #1340
• Allow setting envoy proxy service spec.loadbalancerClass #1793

[arkodg]

shouldnt the aws controller use owner references here instead of finalizer to clean up itself after the linked service is deleted
https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/ , we had a similar clean up issue in EG which was solved with owner references #1706

[arkodg]

@qicz can you help triage this one ?

[wondersd]

@arkodg I think its a bit tricky here because there is no other k8s objects that represent the underlying resources. The owner reference seems to only work when both halves are represented independently as k8s resources. In EGs case there was the Gateway and then the owned rate limit Deployment, both k8s objects.

For ServiceType = LoadBalancer the representing k8s object is the Service object, theres no guarantee of another k8s resource that it could "own". I guess this would start to make the case that all such LoadBalancer providers must use CR representation for the underlying cloud resources (where these resources contain the finalizers to de-provision the underlying resources). Which might be a better overall pattern for cases like this, but would likely require changes to many LoadBalancer implementors and is not the case today.

Did some research in other LoadBalancer providers:

https://github.com/kubernetes/cloud-provider-openstack

seems to use annotations to mark provisioned resources, would make for an awkward interaction with EG without a mechanism to ingnore certain annotations

kubernetes/cloud-provider-openstack#1912

The OP here references "finalizer" but couldn't find finalizers in use in their code.

MetalLB also uses annotations to store its state about the provisioned LoadBalancer in the Service, https://github.com/metallb/metallb/blob/main/controller/service.go#L176-L182

Seems to be quite a bit of variety on how LoadBalancer providers manage their state: dynamic annotations, finalizers, status, and probably some using ownerRef+CR

So while i think it might be a cleaner approach to assume that LoadBalancer providers include CRs to represent provisioned infra and to link this with the service via ownerRef, that doesn't seem to be the case in a lot of the current implementations

To that end, it would help to have options open to account for these in EG until such a time that the ownerRef+CR pattern is widely used. This will allow EG to be cleanly usable conjunction with currently available LoadBalancer provider implementations

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

[wondersd]

Hi @arkodg @qicz,

Noticed this was marked as stale has there been any thoughts on this?

This is problem for us when trying to adopt envoy gateway in wider sense as this presents issues for dynamically created/destroyed gateways.

Thanks!

[arkodg]

hi @wondersd added the right labels to this issue to get more attention

[arkodg]

we chatting about this in the community meeting today, @davidalger pointed out that this issue exists in GCP as well today, which can can cause a churn in cloud resources, a short term solution for this could be to copy metadata.finalizers from the existing resource, before updating the service, a longer term solution could be to use server side apply