terraform-aws-eks-blueprints icon indicating copy to clipboard operation
terraform-aws-eks-blueprints copied to clipboard

[Bug]: CRDs are left orphaned after a `terraform destroy`

Open spkane opened this issue 3 years ago • 3 comments

Welcome to Amazon EKS Blueprints!

  • [X] Yes, I've searched similar issues on GitHub and didn't find any.

Amazon EKS Blueprints Release version

a421b980c5c7c665ff5c42a805dbe07ecf232d5b

What is your environment, configuration and the example used?

  • I start by applying this w/ Terraform version 1.2.3
module "k8s-addons" {
  source = "github.com/aws-ia/terraform-aws-eks-blueprints//modules/kubernetes-addons?ref=a421b980c5c7c665ff5c42a805dbe07ecf232d5b"

  eks_cluster_id               = var.eks_cluster_id
  eks_worker_security_group_id = var.eks_worker_security_group_id

  enable_amazon_eks_coredns = true
  amazon_eks_coredns_config = {
    addon_version = "v1.8.3-eksbuild.1"
  }

  enable_amazon_eks_kube_proxy = true
  amazon_eks_kube_proxy_config = {
    addon_version = "v1.20.7-eksbuild.1"
  }

  enable_amazon_eks_vpc_cni = true

  enable_argocd = true
  argocd_helm_config = {
    version = "4.9.7"
    values = [templatefile(
      "${path.module}/helm_values/argocd-values.yaml.tftpl",
      {
        nodeSelector = local.primary_node_selector
      }
    )]
  }

  argocd_manage_add_ons = true
  argocd_applications = merge(local.argocd_applications, var.additional_argocd_applications)

  enable_karpenter = true
  karpenter_helm_config = {
    version = "0.11.1"
    values = [templatefile("${path.module}/helm_values/karpenter-values.yaml.tftpl",
      {
        nodeSelector              = local.primary_node_selector,
        eks_cluster_id            = var.eks_cluster_id,
        eks_cluster_endpoint      = data.aws_eks_cluster.cluster.endpoint,
        node_iam_instance_profile = var.karpenter_node_iam_instance_profile
      }
    )]
  }

  enable_aws_load_balancer_controller = true
  aws_load_balancer_controller_helm_config = {
    version   = "1.4.2"
    namespace = "kube-system"
    values = [templatefile("${path.module}/helm_values/aws-load-balancer-controller-values.yaml.tftpl",
      {
        nodeSelector = local.primary_node_selector
      }
    )]
  }
}

What did you do and What did you see instead?

  • and then I destroy it:
$ terraform destroy
...
Destroy complete! Resources: 25 destroyed.
  • after the destruction, there are still CRDs in the cluster from things like ArgoCD and karpenter.
$ kubectl get crds

NAME                                         CREATED AT
applications.argoproj.io                     2022-06-28T17:14:22Z
applicationsets.argoproj.io                  2022-06-28T17:14:23Z
appprojects.argoproj.io                      2022-06-28T17:14:24Z
argocdextensions.argoproj.io                 2022-06-28T17:14:23Z
eniconfigs.crd.k8s.amazonaws.com             2022-06-28T16:54:08Z
ingressclassparams.elbv2.k8s.aws             2022-06-28T17:21:18Z
provisioners.karpenter.sh                    2022-06-28T17:21:09Z
securitygrouppolicies.vpcresources.k8s.aws   2022-06-28T16:54:10Z
targetgroupbindings.elbv2.k8s.aws            2022-06-28T17:21:18Z

This makes certain types of testing fail.

Additional Information

In this particular case, I am trying to solve a race condition around terraform installing ArgoCD, and then ArgoCD installing Karpenter, and then Terraform being able to install the default provisioner for Karpenter.

I am trying to use the hashicorp/terraform-provider-time to force enough of a delay before the Karpenter default provisioner is installed that this works, but I can't re-test it without removing these CRDs manually each time.

spkane avatar Jun 28 '22 17:06 spkane

First thing that comes to my mind seeing this is that this is not a bug, but more of how helm and charts working today. See https://github.com/argoproj/argo-helm/issues/1195

I've also seeing cases where CRD were not cleaned up properly because related assigned resources were not deleted first. Example that comes to my mind right away is LB controller, I've seen a case where if you delete the LB controller first before deleting the resources that using the CRD, you'll end up with such state. So in your testing for example, if you'll delete any services using the LB controller first before deleting the LB controller, and only then deleting the LB controller, can you check if the CRD still up afterwards?

This makes certain types of testing fail

Can you please elaborate on this, what type of testings are we talking here? integration testings?

Zvikan avatar Jun 28 '22 18:06 Zvikan

I generally agree. It might not be a bug, per se, but like the underlying ticket, this behavior should likely be documented.

The specific issue here is that normal terraform usage would suggest that a terraform destroy would get you back to that place that you were at before the terraform apply, but that is unfortunately not the case here.

In this specific case, I am trying to ensure that the Karpenter default provisioner (resource "kubectl_manifest" "karpenter_provisioner") gets created after ArgoCD has had enough time to do the initial sync, as depending on the "k8s-addons" module, only ensures that ArgoCD and the applications have been installed.

module "karpenter_launch_templates" {
  source = "github.com/aws-ia/terraform-aws-eks-blueprints//modules/launch-templates?ref=a421b980c5c7c665ff5c42a805dbe07ecf232d5b"

  eks_cluster_id = var.eks_cluster_id
  tags           = merge(local.tags, { Name = "karpenter" })

  launch_template_config = {
    linux = {
      ami                    = data.aws_ami.amazonlinux2eks.id
      launch_template_prefix = "karpenter"
      iam_instance_profile   = var.managed_node_groups[0]["${local.primary_node_group_name}"].managed_nodegroup_iam_instance_profile_id[0]
      vpc_security_group_ids = [var.eks_worker_security_group_id]
      block_device_mappings = [
        {
          device_name = "/dev/xvda"
          volume_type = "gp3"
          volume_size = "200"
        }
      ]
    }
  }
}

data "kubectl_path_documents" "karpenter_provisioners" {
  pattern = "${path.module}/provisioners/*.yaml.tftpl"
  vars = {
    azs                     = join(",", local.azs)
    iam-instance-profile-id = format("%s-%s", var.eks_cluster_id, local.primary_node_group_name)
    eks-cluster-id          = var.eks_cluster_id
  }
}

# Expirement to try and avoid a race condition
# We may need to increase this wait time
resource "time_sleep" "wait_60_seconds" {
  depends_on = [module.k8s-addons]

  create_duration = "60s"
}

# TODO: We might need to use ArgoCD for this as well,
# even though the templating here is useful.
#
# There is a bit of a race condition when first installing Karpenter, since
# we can't really guarantee that ArgoCD has it all installed yet.
#
# Because of this, we force this resource to wait (at least) 60 seconds
# after module.k8s-addons is created to give ArgoCD a bit of time.
resource "kubectl_manifest" "karpenter_provisioner" {
  for_each  = toset(data.kubectl_path_documents.karpenter_provisioners.documents)
  yaml_body = each.value

  depends_on = [time_sleep.wait_60_seconds]
}

At the moment, in my manual tests to get this working, this will fail the first time, but even if I run a terraform destroy, followed by a terraform apply, it will always work the second time since the CRD is already in place.

It might be that I need to use the helm or kubectl provider directly to install the provisioners.karpenter.sh CRD outside ArgoCD to make this work reliably, but I am trying to keep to the blueprints workflow.

spkane avatar Jun 28 '22 19:06 spkane

Hi @spkane, agree that minimum we should document this.

So for k8s resources being created by argocd, they need to be cleaned up first before doing the terraform destroy. Not sure if there's a way to indicate that to argocd via a provisioner or something..

provisioner "local-exec" {
    when    = destroy
    command = "..."
}

It might be that I need to use the helm or kubectl provider directly to install the provisioners.karpenter.sh CRD outside ArgoCD to make this work reliably, but I am trying to keep to the blueprints workflow.

This is possibly emerging as a recommended pattern i.e. deploy all AWS infrastructure via terraform, hand-off to argocd for all k8s resources. We are looking at properly documenting this workflow and it is not in opposition to blueprints workflow.

askulkarni2 avatar Jun 29 '22 23:06 askulkarni2

in the next teration of Bluepritns Addons, this should issue should be less likely. I say less likely because it largely depends on the underlying addon and how they are managing their CRD. In addition, in the new iteration of addons we are no longer creating a namespace directly with the Kubernetes provider and instead offloading that to the helm release resource which has shown better management of resources during cleanup. The new addons can be accessed here https://github.com/aws-ia/terraform-aws-eks-blueprints-addons

And improvements to the integration between Terraform and GitOps controllers can be tracked here https://github.com/aws-ia/terraform-aws-eks-blueprints-addons/issues/114

bryantbiggs avatar Apr 27 '23 19:04 bryantbiggs