Skip to content

Commit

Permalink
[eks/karpenter] Add support for kubelet config, fix IAM support for…
Browse files Browse the repository at this point in the history
… `v1alpha` cleanup (#1076)
  • Loading branch information
Nuru authored Jul 8, 2024
1 parent 2c7ac72 commit 25e9a3d
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 11 deletions.
15 changes: 15 additions & 0 deletions modules/eks/karpenter-node-pool/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
## Components [PR #1076](https://github.com/cloudposse/terraform-aws-components/pull/1076)

- Allow specifying elements of `spec.template.spec.kubelet`
- Make taint values optional

The `var.node_pools` map now includes a `kubelet` field that allows specifying elements of `spec.template.spec.kubelet`.
This is useful for configuring the kubelet to use custom settings, such as reserving resources for system daemons.

For more information, see:

- [Karpenter documentation](https://karpenter.sh/docs/concepts/nodepools/#spectemplatespeckubelet)
- [Kubernetes documentation](https://kubernetes.io/docs/reference/config-api/kubelet-config.v1beta1/)

The `value` fields of the `taints` and `startup_taints` lists in the `var.node_pools` map are now optional. This is in
alignment with the Kubernetes API, where `key` and `effect` are required, but the `value` field is optional.
2 changes: 1 addition & 1 deletion modules/eks/karpenter-node-pool/README.md

Large diffs are not rendered by default.

12 changes: 10 additions & 2 deletions modules/eks/karpenter-node-pool/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ locals {
public_subnet_ids = module.vpc.outputs.public_subnet_ids

node_pools = { for k, v in var.node_pools : k => v if local.enabled }
kubelets_specs_filtered = { for k, v in local.node_pools : k => {
for kk, vv in v.kubelet : kk => vv if vv != null
}
}
kubelet_specs = { for k, v in local.kubelets_specs_filtered : k => v if length(v) > 0 }
}

# https://karpenter.sh/docs/concepts/nodepools/
Expand Down Expand Up @@ -40,8 +45,8 @@ resource "kubernetes_manifest" "node_pool" {
)
template = {
metadata = {
labels = each.value.labels
annotations = each.value.annotations
labels = coalesce(each.value.labels, {})
annotations = coalesce(each.value.annotations, {})
}
spec = merge({
nodeClassRef = {
Expand All @@ -64,6 +69,9 @@ resource "kubernetes_manifest" "node_pool" {
},
try(length(each.value.startup_taints), 0) == 0 ? {} : {
startupTaints = each.value.startup_taints
},
try(local.kubelet_specs[each.key], null) == null ? {} : {
kubelet = local.kubelet_specs[each.key]
}
)
}
Expand Down
10 changes: 8 additions & 2 deletions modules/eks/karpenter-node-pool/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ variable "node_pools" {
taints = optional(list(object({
key = string
effect = string
value = string
value = optional(string)
})))
startup_taints = optional(list(object({
key = string
effect = string
value = string
value = optional(string)
})))
# Karpenter node metadata options. See https://karpenter.sh/docs/concepts/nodeclasses/#specmetadataoptions for more details
metadata_options = optional(object({
Expand Down Expand Up @@ -120,6 +120,12 @@ variable "node_pools" {
# Operators like "Exists" and "DoesNotExist" do not require a value
values = optional(list(string))
}))
# Any values for spec.template.spec.kubelet allowed by Karpenter.
# Not fully specified, because they are subject to change.
# See:
# https://karpenter.sh/docs/concepts/nodepools/#spectemplatespeckubelet
# https://kubernetes.io/docs/reference/config-api/kubelet-config.v1beta1/
kubelet = optional(any, {})
}))
description = "Configuration for node pools. See code for details."
nullable = false
Expand Down
32 changes: 31 additions & 1 deletion modules/eks/karpenter/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,36 @@
## Components [PR #1076](https://github.com/cloudposse/terraform-aws-components/pull/1076)

#### Bugfix

- Fixed issues with IAM Policy support for cleaning up `v1alpha` resources.

With the previous release of this component, we encouraged users to delete their `v1alpha` Karpenter resources before
upgrading to `v1beta`. However, certain things, such as EC2 Instance Profiles, would not be deleted by Terraform because
they were created or modified by the Karpenter controller.

To enable the `v1beta` Karpenter controller to clean up these resources, we added a second IAM Policy to the official
Karpenter IAM Policy document. This second policy allows the Karpenter controller to delete the `v1alpha` resources.
However, there were 2 problems with that.

First, the policy was subtly incorrect, and did not, in fact, allow the Karpenter controller to delete all the
resources. This has been fixed.

Second, a long EKS cluster name could cause the Karpenter IRSA's policy to exceed the maximum character limit for an IAM
Policy. This has also been fixed by making the `v1alpha` policy a separate managed policy attached to the Karpenter
controller's role, rather than merging the statements into the `v1beta` policy. This change also avoids potential
conflicts with policy SIDs.

:::note Innocuous Changes

Terraform will show IAM Policy changes, including deletion of statements from the existing policy and creation of a new
policy. This is expected and innocuous. The IAM Policy has been split into 2 to avoid exceeding length limits, but the
current (`v1beta`) policy remains the same and the now separate (`v1alpha`) policy has been corrected.

:::

## Version 1.445.0

Components PR #1039
Components [PR #1039](https://github.com/cloudposse/terraform-aws-components/pull/1039)

:::warning Major Breaking Changes

Expand Down
2 changes: 2 additions & 0 deletions modules/eks/karpenter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,8 @@ For more details on the CRDs, see:
|------|------|
| [aws_cloudwatch_event_rule.interruption_handler](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_event_rule) | resource |
| [aws_cloudwatch_event_target.interruption_handler](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_event_target) | resource |
| [aws_iam_policy.v1alpha](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource |
| [aws_iam_role_policy_attachment.v1alpha](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
| [aws_sqs_queue.interruption_handler](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue) | resource |
| [aws_sqs_queue_policy.interruption_handler](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue_policy) | resource |
| [aws_eks_cluster_auth.eks](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/eks_cluster_auth) | data source |
Expand Down
30 changes: 26 additions & 4 deletions modules/eks/karpenter/controller-policy-v1alpha.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
# v1alpha API tag "karpenter.sh/provisioner-name" and to manage the EC2 Instance Profile
# created by the EKS cluster component.
#
# WARNING: it is important that the SID values do not conflict with the SID values in the
# controller-policy.tf file, otherwise they will be overwritten.
# We create a separate policy and attach it separately to the Karpenter controller role
# because the main policy is near the 6,144 character limit for an IAM policy, and
# adding this to it can push it over. See:
# https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_iam-quotas.html#reference_iam-quotas-entities
#

locals {
Expand All @@ -35,10 +37,10 @@ locals {
],
"Condition": {
"StringEquals": {
"aws:ResourceTag/kubernetes.io/cluster/${local.eks_cluster_id}": "owned"
"ec2:ResourceTag/karpenter.k8s.aws/cluster": "${local.eks_cluster_id}"
},
"StringLike": {
"aws:ResourceTag/karpenter.sh/provisioner-name": "*"
"ec2:ResourceTag/karpenter.sh/provisioner-name": "*"
}
}
},
Expand All @@ -65,3 +67,23 @@ locals {
}
EndOfPolicy
}

# We create a separate policy and attach it separately to the Karpenter controller role
# because the main policy is near the 6,144 character limit for an IAM policy, and
# adding this to it can push it over. See:
# https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_iam-quotas.html#reference_iam-quotas-entities
resource "aws_iam_policy" "v1alpha" {
count = local.enabled ? 1 : 0

name = "${module.this.id}-v1alpha"
description = "Legacy Karpenter controller policy for v1alpha workloads"
policy = local.controller_policy_v1alpha_json
tags = module.this.tags
}

resource "aws_iam_role_policy_attachment" "v1alpha" {
count = local.enabled ? 1 : 0

role = module.karpenter.service_account_role_name
policy_arn = one(aws_iam_policy.v1alpha[*].arn)
}
2 changes: 1 addition & 1 deletion modules/eks/karpenter/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ module "karpenter" {
service_account_role_arn_annotation_enabled = true

iam_role_enabled = true
iam_source_policy_documents = [local.controller_policy_v1alpha_json, local.controller_policy_json]
iam_source_policy_documents = [local.controller_policy_json]

values = compact([
yamlencode({
Expand Down

0 comments on commit 25e9a3d

Please sign in to comment.