Fix: ENI's prevent SecGrps from being destroyed on tf destroy

[whiskeyjimbo]

Fix: ENI's prevent SecGrps from being destroyed on tf destroy

Description

terraform destroy fails on security group deletion due to the SG's being attached to ENI's
Setting destroy on term allows for the cluster to be fully cleaned up.

Checklist

• terraform fmt and terraform validate both work from the root and examples/eks_test_fixture directories (look in CI for an example)
• Tests for the changes have been added and passing (for bug fixes/features)
• Test results are pasted in this PR (in lieu of CI)
• I've added my change to CHANGELOG.md
• Any breaking changes are highlighted above

[mrtheb]

Thanks! I was looking for this too. Just did the same thing w/o the variable in a branch of my own.

[max-rocket-internet]

Can we think of any reason or situation where this should NOT default to true? Maybe like accidental destroy or something like that?

[whiskeyjimbo]

The interfaces get orphaned once the ASG's get deleted by the destroy, this wont prevent an accidental destroy, but it will allow it to be a clean one.

[mrtheb]

I was actually wondering if it is even necessary to have the eni_delete variable to prevent deleting the ENI on destroy.

The question is really in which cases would someone purposely want to leave ENI dangling for the EKS cluster. The case I know and use outside of EKS is to be able to replace an instance of a service but keeping the same IP than the previous instance it is replacing. I don't think this is really necessary for EKS nodes.

The default to true is the sensible choice otherwise.

[max-rocket-internet]

The question is really in which cases would someone purposely want to leave ENI dangling for the EKS cluster

Is this definitely desired? So we can have a clean destroy when people don't bother scaling ASG to 0 before running destroy, but we end up with leftover ENIs?

[mrtheb]

@max-rocket-internet I am definitely confused now. My PoV is it's definitely not desired. I was commenting about this because I don't think anyone would want to set eni_delete=false, to a point that I wouldn't even add the variable.

I am regularly creating and destroying EKS clusters w/o scaling down the ASG to 0 because I don't bother for a test environment. The problem this solves is not only the dangling ENIs but the fact the cluster won't be destroyed if we attach external SG to it.

[max-rocket-internet]

OK cool! Looks like everyone wants this so let's do it.

[haofeif]

Hi, experiencing similar things at version 11.0.0.

I have an additional security group added :
cluster_security_group_id = aws_security_group.additional_cluster_sg.id

resource "aws_security_group" "additional_cluster_sg" {
name_prefix = "${var.cluster_name}-additional_cluster_sg"
vpc_id = var.vpc_id
lifecycle {
create_before_destroy = true
}
}

When i added the lifecycle {} part into my security groups, i did 20 destroy, only 1 fails.
I am not sure when the fix will be available in the next release ?

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.