AWS NLB linger after they're orphaned

[voor]

/kind bug

What steps did you take and what happened:
Workload clusters are able to create Network Load Balancers instead of Classic ELBs, you just add an annotation service.beta.kubernetes.io/aws-load-balancer-type: nlb (read more), we need to identify those Load Balancers by tag and delete them as well when the associated workload cluster is destroyed.

Referenced here in a conversation on the topic.

What did you expect to happen:
NLB are deleted alongside the cluster.

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

• Cluster-api-provider-aws version: v0.5.2
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

[vincepri]

/milestone v0.5.x
/help

[k8s-ci-robot]

@vincepri:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/milestone v0.5.x
/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bagnaram]

/assign

[bagnaram]

It is probably safe to assume that we will need an additional elbv2 service to handle the cleanup of the NLB resources

[randomvariable]

The issue is that you essentially need to drain the workload cluster of services, so that the cloud provider tears down the NLBs. It's not specifically a CAPA problem IMHO, or you add logic to CAPA to handle resources created by the cloud provider integration. Worth adding to the agenda for the meeting, as I'm a bit weary of crossing responsibility boundaries here.

[detiber]

@randomvariable that is a good point, we could remove the current ELB Classic cleanup we do in favor of moving the core functionality into core cluster api, where we could delete all Services w/ Type=LoadBalancer prior to deletion of a given Cluster. That would then cover any similar issues that would arise with other infrastructure providers as well.

[randomvariable]

That could be good. Did suggest to @nckturner, @justinsb, @andrewsykim that we could use the test framework to set up CI for the cloud provider repo. Having CAPI take care of auto-deleting service type="load balancers" would be a neat trick.

[randomvariable]

We forgot to discuss this in yesterday's meeting. I'll file an issue to Cluster API and add it to the agenda.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[voor]

/remove-lifecycle stale
/remove-lifecycle frozen

[voor]

/lifecycle frozen

[sedefsavas]

Closing in favor of kubernetes-sigs/cluster-api#3075

[richardcase]

@sedefsavas - this issue is impacting some customers so going to reopen with this plan in mind:

• Fix this in CAPA for the time being, so that we can get this fixed quicker for the people its impacting.
• Will put the functionality behind a feature flag.
• Then i'll help with working this into upstream via proposal/change based on Drain workload clusters of service Type=Loadbalancer on teardown cluster-api#3075
• Once its available upstream we can deprecate

/reopen
/assign
/lifecycle active

[k8s-ci-robot]

@richardcase: Reopened this issue.

In response to this:

@sedefsavas - this issue is impacting some customers so going to reopen with this plan in mind:

• Fix this in CAPA for the time being, so that we can get this fixed quicker for the people its impacting.
• Will put the functionality behind a feature flag.
• Then i'll help with working this into upstream via proposal/change based on Drain workload clusters of service Type=Loadbalancer on teardown cluster-api#3075
• Once its available upstream we can deprecate

/reopen
/assign
/lifecycle active

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[richardcase]

/priority critical-urgent

[richardcase]

/triage accepted

[richardcase]

Just tested this scenario and it does occur. The delete of the cluster gets stuck because of

E0609 15:49:16.022022      14 awsmanagedcontrolplane_controller.go:292] controller/awsmanagedcontrolplane "msg"="error deleting network for AWSManagedControlPlane" "error"="failed  │
│ to detach internet gateway \"igw-0f81d9e12a5a97bf2\": DependencyViolation: Network vpc-0b06fcdbbc37ab172 has some mapped public address(es). Please unmap those public address(es) b │
│ efore detaching the gateway.\n\tstatus code: 400, request id: 65dc0fa0-584f-4256-baf5-a2aac2d2dde4" "reconciler group"="controlplane.cluster.x-k8s.io" "reconciler kind"="AWSManaged │
│ ControlPlane" "name"="capi-managed-test-control-plane" "namespace"="default"                                                                                                         │
│ I0609 15:49:16.022130      14 recorder.go:103] events "msg"="Warning"  "message"="Failed to detach Internet Gateway \"igw-0f81d9e12a5a97bf2\" from VPC \"vpc-0b06fcdbbc37ab172\": De │
│ pendencyViolation: Network vpc-0b06fcdbbc37ab172 has some mapped public address(es). Please unmap those public address(es) before detaching the gateway.\n\tstatus code: 400, reques │
│ t id: 65dc0fa0-584f-4256-baf5-a2aac2d2dde4" "object"={"kind":"AWSManagedControlPlane","namespace":"default","name":"capi-managed-test-control-plane","uid":"adefde7f-760d-453d-b81e- │
│ cde2461ccdd6","apiVersion":"controlplane.cluster.x-k8s.io/v1beta1","resourceVersion":"20624"} "reason"="FailedDetachInternetGateway"

And the load balancer still exists.

[sedefsavas]

Looks like we only clean up the CCM created ELBs but not NLBs.

[richardcase]

Looks like we only clean up the CCM created ELBs but not NLBs.

@sedefsavas - spot on :)

[richardcase]

/milestone v1.5.0