Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[eks/karpenter] Add support for kubelet config, fix IAM support for v1alpha cleanup #1076

Merged
merged 2 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading