awstagdeprovision: add support for deleting V2 LBs and LB Target groups #62
Conversation
openshift-ci-robot
added
the
size/L
This is doubling down on not allowing clusters to get launched into pre-existing, shared VPCs. But we can always revisit the logic here if we need to return to allowing shared VPCs, so I'm cautiously ok with this approach ;). |
| if err != nil { | ||
| logger.Debugf("Error deleting V2 load balancer %v: %v", *lb.LoadBalancerName, err) | ||
| } else { | ||
| logger.WithField("name", *lb.LoadBalancerName).Info("Deleted load balancer") |
There was a problem hiding this comment.
You're probably just following an existing pattern, but it's a bit strange to me that we use WithField for name here but : %v for the name a few lines up in Deleting V2 load balancer: %v. Should we use WithField there too? If so, do we want to do that in this PR for your additions, or punt it to a whole-file-fix later PR?
There was a problem hiding this comment.
punt it to a whole-file-fix later PR is what I want :)
|
|
||
| complete = deleteV2LBs(vpc, awsSession, logger) | ||
| if !complete { | ||
| logger.Debugf("not finished deleting V2 load balancers, will need to retry") |
There was a problem hiding this comment.
This whole block seems inefficient to me. Can we try all of the children (deleteLBs, deleteV2LBs, and deleteRouteTablesWithVPC) even if one of them errors or is unfinished? You could do that with something like:
v1LBComplete := deleteLBs(vpc, awsSession, logger)
v2LBComplete := deleteV2LBs(vpc, awsSession, logger)
err := deleteRouteTablesWithVPC(vpc, ec2Client, logger)
if err != nil {
logger.Debugf("error deleting route tables: %v", err)
return false, nil
}
if !v1LBComplete {
logger.Debugf("not finished deleting load balancers, will need to retry")
return false, nil
}
if !v2LBComplete {
logger.Debugf("not finished deleting V2 load balancers, will need to retry")
return false, nil
}That's still serial, but at least we try them all. You could use goroutines or some such to make those parallel. I'm also fine punting either my snippet above or parallel calls to follow-up work.
There was a problem hiding this comment.
deleteRouteTablesWithVPC will not succeed unless all the consumers of the route tables are deleted; So it might have been setup to prevent waste of api calls.
There was a problem hiding this comment.
deleteRouteTablesWithVPCwill not succeed unless all the consumers of the route tables are deleted; So it might have been setup to prevent waste of api calls.
Ok. And we don't expect any clusters with both v1 and v2 load balancers in the same VPC. So I'm fine with this as you have it, then :).
There was a problem hiding this comment.
If I spun up a cluster with these new LBs and then used Service type LoadBalancer, would I end up with V1 LBs as well?
There was a problem hiding this comment.
And we don't expect any clusters with both v1 and v2 load balancers in the same VPC
we do expect that there be both v1 and v2 LBs in the same VPC.
@wking would there be any problems in that case?
There was a problem hiding this comment.
It looks like it would work fine to me.
There was a problem hiding this comment.
we do expect that there be both v1 and v2 LBs in the same VPC.
Oh, in that case I'm back in favor of something like this to get at least v1 and v2 LB deletion attempts (but not route table deletions?) in the same round of attempted-VPC-deletion. Otherwise we' could go through a few rounds trying to delete the v1 LBs, and then another few rounds trying to delete the v2 LBs, and end up deeper in the exponential backoff making teardown slower than it needs to be.
|
Ok, @abhinavdahiya's addressed all of my questions. Anyone else want to weigh in before I |
|
Looks fine to me, go for it @wking as soon as the V1+V2 thing is resolved. Suspecting not an issue. |
To delete these load balancers:
* deleting V2 load balancers
```console
aws elbv2 describe-load-balancers
{
"LoadBalancers": [
{
"DNSName": "crawford-aws-int-a85653d6d18c082c.elb.us-east-1.amazonaws.com",
"VpcId": "vpc-06de9aca2fabbf1b7",
"State": {
"Code": "active"
},
"Type": "network",
"AvailabilityZones": [
{
"ZoneName": "us-east-1c",
"SubnetId": "subnet-061048a5f17adb00b",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1d",
"SubnetId": "subnet-03506a45cf76bbb78",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1a",
"SubnetId": "subnet-0d36ad7ab10e8a721",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1f",
"SubnetId": "subnet-00f19af286407f964",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1e",
"SubnetId": "subnet-0adbb874efe59de3f",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1b",
"SubnetId": "subnet-0122ba9d35b720814",
"LoadBalancerAddresses": [
{}
]
}
],
"IpAddressType": "ipv4"
},
{
"DNSName": "crawford-aws-ext-e443ac9111e22219.elb.us-east-1.amazonaws.com",
"CanonicalHostedZoneId": "Z26RNL4JYFTOTI",
"CreatedTime": "2018-11-01T15:30:22.577Z",
"LoadBalancerName": "crawford-aws-ext",
"Scheme": "internet-facing",
"VpcId": "vpc-06de9aca2fabbf1b7",
"State": {
"Code": "active"
},
"Type": "network",
"AvailabilityZones": [
{
"ZoneName": "us-east-1e",
"SubnetId": "subnet-0adbb874efe59de3f",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1a",
"SubnetId": "subnet-0d36ad7ab10e8a721",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1b",
"SubnetId": "subnet-0122ba9d35b720814",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1f",
"SubnetId": "subnet-00f19af286407f964",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1d",
"SubnetId": "subnet-03506a45cf76bbb78",
"LoadBalancerAddresses": [
{}
]
},
{
"ZoneName": "us-east-1c",
"SubnetId": "subnet-061048a5f17adb00b",
"LoadBalancerAddresses": [
{}
]
}
],
"IpAddressType": "ipv4"
}
]
}
```
These load balancers are tagged, but the api does not allow filters based on tags, so we do that same as
the V1 Lbs, filter on VpcID
```console
aws elbv2 delete-load-balancer --load-balancer-arn
```
* deleting target groups
```console
aws elbv2 describe-target-groups
{
"TargetGroups": [
{
"TargetGroupName": "crawford-aws-api-external",
"Protocol": "TCP",
"Port": 6443,
"VpcId": "vpc-06de9aca2fabbf1b7",
"HealthCheckProtocol": "TCP",
"HealthCheckPort": "6443",
"HealthCheckIntervalSeconds": 10,
"HealthCheckTimeoutSeconds": 10,
"HealthyThresholdCount": 3,
"UnhealthyThresholdCount": 3,
"LoadBalancerArns": [],
"TargetType": "ip"
},
{
"TargetGroupName": "crawford-aws-api-internal",
"Protocol": "TCP",
"Port": 6443,
"VpcId": "vpc-06de9aca2fabbf1b7",
"HealthCheckProtocol": "TCP",
"HealthCheckPort": "6443",
"HealthCheckIntervalSeconds": 10,
"HealthCheckTimeoutSeconds": 10,
"HealthyThresholdCount": 3,
"UnhealthyThresholdCount": 3,
"LoadBalancerArns": [
"arn:aws:elasticloadbalancing:us-east-1:816138690521:loadbalancer/net/crawford-aws-int/a85653d6d18c082c"
],
"TargetType": "ip"
},
{
"TargetGroupName": "crawford-aws-services",
"Protocol": "TCP",
"Port": 49500,
"VpcId": "vpc-06de9aca2fabbf1b7",
"HealthCheckProtocol": "TCP",
"HealthCheckPort": "49500",
"HealthCheckIntervalSeconds": 10,
"HealthCheckTimeoutSeconds": 10,
"HealthyThresholdCount": 3,
"UnhealthyThresholdCount": 3,
"LoadBalancerArns": [
"arn:aws:elasticloadbalancing:us-east-1:816138690521:loadbalancer/net/crawford-aws-int/a85653d6d18c082c"
],
"TargetType": "ip"
}
]
}
```
These target groups can be tagged, but i couldn't find a way to get those tags, and the describe api doesn't allow for
filtering on tags anyways, so we filter on our VpcId
```console
aws elbv2 delete-target-group --target-group-arn
```
deleting target groups without deleting all the load balancers that have listeners to this target groups returns
```console
An error occurred (ResourceInUse) when calling the DeleteTargetGroup operation: Target group 'REDACTED' is currently in use by a listener or a rule
```
So we call delete target groups after we have completed delete of all v2 LBs.
5 participants
To delete these load balancers:
These load balancers are tagged, but the api does not allow filters based on tags, so we do that same as
the V1 Lbs, filter on VpcID
aws elbv2 delete-load-balancer --load-balancer-arnThese target groups can be tagged, but i couldn't find a way to get those tags, and the describe api doesn't allow for
filtering on tags anyways, so we filter on our VpcId
aws elbv2 delete-target-group --target-group-arndeleting target groups without deleting all the load balancers that have listeners to this target groups returns
An error occurred (ResourceInUse) when calling the DeleteTargetGroup operation: Target group 'REDACTED' is currently in use by a listener or a ruleSo we call delete target groups after we have completed delete of all v2 LBs.
/cc @crawford @wking @dgoodwin