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

Support karpenter-crd Helm Chart and Fix Node Interruption Handling #868

Merged
merged 25 commits into from
Nov 21, 2023

Conversation

milldr
Copy link
Member

@milldr milldr commented Oct 10, 2023

what

  • Support deploying the karpenter-crd helm chart alongside the karpenter chart with the eks/karpenter component
  • Bring back documentation for the Node Interruption Handler with Karpenter
  • Replace iam_policy_statements with iam_policy

why

  • karpenter-crd can be installed alongside the karpenter helm chart to automatically manage the lifecycle of Karpenter CRDs.
  • Node Interruption Handling with eks/karpenter was added with Karpenter Node Interruption Handler #713, but documentation was unintentionally removed with Enable terraform plan access via dynamic Terraform roles #715
  • On initial creation, the helm-release resource will not know the result of the SQS queue resource, and the plan will fail with the following issue. iam_policy solves this issue and is the updated pattern
│ The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the
│ -target argument to first apply only the resources that the count depends on.

references

@milldr milldr requested review from a team as code owners October 10, 2023 23:52
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, a few nitpicks

Copy link
Contributor

@Nuru Nuru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments. Significant changes need to be made to prevent the Kubernetes namespace from being destroyed on configuration changes, and documentation needs to explain how the upgrade path.

modules/eks/karpenter/README.md Outdated Show resolved Hide resolved
modules/eks/karpenter/README.md Outdated Show resolved Hide resolved
modules/eks/karpenter/README.md Outdated Show resolved Hide resolved
modules/eks/karpenter/main.tf Outdated Show resolved Hide resolved
modules/eks/karpenter/README.md Show resolved Hide resolved
@milldr milldr changed the title Support karpenter-crd Helm Chart and Revert Docs on Node Interruption Support karpenter-crd Helm Chart and Fix Node Interruption Handling Oct 12, 2023
modules/eks/karpenter/main.tf Outdated Show resolved Hide resolved

### Upgrading an existing `eks/karpenter` deployment where the `karpenter-crd` chart is already deployed

If you currently have `eks/karpenter` deployed to an EKS cluster, have upgraded to this version of the component, and already have the `karpenter-crd` chart installed, simply set `var.crd_chart_enabled` to `true` and redeploy Terraform to have Terraform manage the helm release for `karpenter-crd`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you don't have to import the CRD helm chart?

modules/eks/karpenter/README.md Outdated Show resolved Hide resolved
modules/eks/karpenter/README.md Outdated Show resolved Hide resolved
modules/eks/karpenter/README.md Outdated Show resolved Hide resolved
modules/eks/karpenter/README.md Show resolved Hide resolved
@Nuru Nuru self-requested a review November 21, 2023 14:23
Nuru
Nuru previously approved these changes Nov 21, 2023
@Nuru Nuru self-requested a review November 21, 2023 14:33
@Nuru Nuru merged commit 6a05227 into main Nov 21, 2023
4 checks passed
@Nuru Nuru deleted the support-karpenter-crd branch November 21, 2023 14:42
mgledi pushed a commit to mfuhrmeisterDM/terraform-aws-components that referenced this pull request Nov 29, 2023
goruha pushed a commit to cloudposse-terraform-components/aws-eks-karpenter-controller that referenced this pull request Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants