feat: Add support for addons #1443

tnimni · 2021-06-15T03:15:44Z

PR o'clock

Description

Add support for eks addons recommended by aws

VPC CNI
CoreDns
kube-proxy

https://docs.aws.amazon.com/eks/latest/userguide/eks-add-ons.html

The default behavior will NOT install the addons to the cluster.

Requires irsa to be enabled to deploy the VPC CNI addon

Checklist

[V] README.md has been updated after any changes to variables and outputs. See https://github.com/terraform-aws-modules/terraform-aws-eks/#doc-generation

daroga0002

can we add some example into examples directory which will be using this?

daroga0002 · 2021-06-15T14:47:27Z

modules/addons/variables.tf

+# Hack for a homemade `depends_on` https://discuss.hashicorp.com/t/tips-howto-implement-module-depends-on-emulation/2305/2
+# Will be removed in Terraform 0.13 with the support of module's `depends_on` https://github.com/hashicorp/terraform/issues/10462
+variable "eks_depends_on" {
+  description = "List of references to other resources this submodule depends on."


maybe this should me moved to module level as terraform from some time allowing it?

This pattern is used in modules/fargate and modules/node_groups, I just copied it from there to keep everything consistent.
I'm not sure which example you require, as eks_depends_on is defined in addons.tf in the root of the main module, https://github.com/tnimni/terraform-aws-eks/blob/feature/addons/addons.tf#L11-L14

daroga0002 · 2021-06-15T14:50:10Z

modules/addons/variables.tf

+
+variable "coredns_versions" {
+  # Versions are taken from https://docs.aws.amazon.com/eks/latest/userguide/managing-coredns.html#updating-coredns-add-on
+  type        = map(any)


I think better will be map(string) just to not loose type over time

Changed to map(string)

daroga0002 · 2021-06-15T14:50:11Z

modules/addons/variables.tf

+
+variable "kube_proxy_versions" {
+  # Versions are taken from https://docs.aws.amazon.com/eks/latest/userguide/managing-kube-proxy.html#updating-kube-proxy-add-on
+  type        = map(any)


I think better will be map(string) just to not loose type over time

Changed to map(string)

daroga0002 · 2021-06-15T14:50:16Z

modules/addons/variables.tf

+variable "vpc_cni_versions" {
+  # Versions are taken from https://docs.aws.amazon.com/eks/latest/userguide/managing-vpc-cni.html#updating-vpc-cni-add-on
+  # Latest patch version is taken from https://github.com/aws/amazon-vpc-cni-k8s
+  type        = map(any)


I think better will be map(string) just to not loose type over time

Changed to map(string)

daroga0002 · 2021-06-15T14:50:48Z

modules/addons/variables.tf

+# Will be removed in Terraform 0.13 with the support of module's `depends_on` https://github.com/hashicorp/terraform/issues/10462
+variable "eks_depends_on" {
+  description = "List of references to other resources this submodule depends on."
+  type        = any


probably string will be better

based on https://github.com/tnimni/terraform-aws-eks/blob/feature/addons/modules/fargate/variables.tf#L59-L65 , I copied it to keep everything consistent

yup, this was created before modules were supporting dependncies. Lets maintainers decide does we want to leave this for consistency or we will start using module dependency.

daroga0002 · 2021-06-15T14:52:11Z

modules/addons/outputs.tf

@@ -0,0 +1,14 @@
+output "vpc_cni_id" {
+  description = "The id of the Amazon VPC CNI addon"
+  value       = aws_eks_addon.vpc_cni[0].id


does it will be working when I will not create this addon (there will be no element 0)?

Fixed the count=0 issue

daroga0002 · 2021-06-15T14:53:01Z

modules/addons/coredns.tf

+  cluster_name      = var.cluster_name
+  addon_name        = "coredns"
+  resolve_conflicts = "OVERWRITE"
+  addon_version     = lookup(var.coredns_versions, var.cluster_version, "Not Found")


how AWS API will react when we pass "Not Found", does there is validation on their end?

This is on purpose, it will not even reach aws api. terraform plan will throw an error stating the addon version can not be "Not Found" giving a hint for the user that the cluster version is not supported by the module

Uncomment 1.20

Update README.md

daroga0002

for me it looks good, @barryib can you take a look on it?

stevehipwell · 2021-06-16T12:40:11Z

Please can the defaults not be to enable these.

…into feature/addons

tnimni · 2021-06-16T23:32:12Z

@stevehipwell
changed defaults to false

tnimni · 2021-06-17T14:10:37Z

Can we please get this approved? @barryib

thecosmicfrog · 2021-06-22T12:10:54Z

Good work here @tnimni - hoping for it to be merged soon.

tnimni · 2021-06-22T19:21:54Z

Good work here @tnimni - hoping for it to be merged soon.

Thank you

mmerickel · 2021-06-23T05:48:33Z

So if we enable the vpc cni addon then should we pass attach_worker_cni_policy=false? Would be good to mention that in the docs if so.

…sing the vpc cni addon

tnimni · 2021-06-23T21:09:36Z

attach_worker_cni_policy=false

Hi,
it is not required to do so,

I checked and aws recommend it here https://docs.aws.amazon.com/eks/latest/userguide/cni-iam-role.html

I added to the example and also updated the main module readme to reflect that the variable attach_worker_cni_policy should be set to false if deploying the vpc cni addon

I appreciate your input

nikitacr7 · 2021-06-29T10:48:28Z

Hi

Awesome. We're waiting for it.

thapakazi · 2021-06-30T07:46:49Z

Same Same, nice work @tnimni 👏

tnimni · 2021-06-30T10:32:53Z

@nikitacr7 @thapakazi thank you both

csschwe · 2021-07-09T13:09:28Z

@tnimni This is great!!!

I do have one request though. Can you add support for tags like the node_groups module

https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/node_groups.tf#L9

Thanks

tnimni · 2021-07-15T12:12:30Z

@tnimni This is great!!!

I do have one request though. Can you add support for tags like the node_groups module

https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/node_groups.tf#L9

Thanks

I'll have a look into it

Edit: I have added tags to addons
they do not reflect in was UI, but they are added

use addon_tags = {
Environment = "dev"
}

wellermann · 2021-07-22T17:59:59Z

EKS 1.21 just came out this week so it might be worth adding it now.

Either way this is great work @tnimni Thank you for doing this.

stevehipwell · 2021-08-12T07:39:10Z

Thanks @antonbabenko your design deals with my concerns and will allow anyone who wants to use addons to do so without impacting any of us who choose not to.

stale · 2021-10-03T21:30:11Z

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
To track this PR (even if closed), please open a corresponding issue if one does not already exist.

fubar · 2021-10-21T03:53:09Z

We'd love to have support for this, is this PR still being worked on?

tnimni · 2021-10-21T07:08:02Z

We'd love to have support for this, is this PR still being worked on?

I'm waiting for the provider to be updated

Support aws_eks_addon_version data source hashicorp/terraform-provider-aws#20725
they haven't updated it yet

I'm considering releasing this as a stand alone module, perhaps n 2 weeks time

github-actions · 2021-11-21T00:40:55Z

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

bryantbiggs · 2021-11-21T13:04:50Z

not stale

beingamarnath · 2021-11-29T12:36:11Z

Any updates on this? Now that EKS version 1.21 is out there, can we introduce this to the module directly

github-actions · 2021-12-30T00:42:07Z

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

Lillecarl · 2021-12-30T00:45:07Z

Not stale

antonbabenko · 2022-01-05T19:58:11Z

This issue has been resolved in version 18.0.0 🎉

mmerickel · 2022-01-06T21:15:12Z

I think this was closed in error... I do not see any of the changes from this PR in master or the 18.0.3 release.

bryantbiggs · 2022-01-06T21:17:22Z

@mmerickel

terraform-aws-eks/main.tf

Line 220 in 3c66863

resource "aws_eks_addon" "this" {

mmerickel · 2022-01-06T21:23:48Z

Huh ok so it was refactored and done differently from this PR (addons.tf is not there, or any of the examples, etc). It's also not in the changelog. But thanks yes it looks like something like this PR is in master now. Thanks!

bryantbiggs · 2022-01-06T21:26:03Z

yes, see convo above #1443 (comment)

there are examples of using this features as well

terraform-aws-eks/examples/complete/main.tf

Line 29 in 3c66863

cluster_addons = {

github-actions · 2022-11-11T02:33:42Z

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.

Tsachi Nimni added 4 commits June 15, 2021 04:58

Add support for eks addons using submodule

f745f16

Add README to addons module, update main module docs

54832d6

Fix enable_irsa variable description for module/addons

ac70410

Add outputs to addons module

f530a82

tnimni mentioned this pull request Jun 15, 2021

How to install/manage EKS add-ons for #1385

Closed

daroga0002 reviewed Jun 15, 2021

View reviewed changes

Tsachi Nimni and others added 4 commits June 16, 2021 01:02

Fix comment from PR

b80ab6e

Uncomment 1.20

30ff548

Update variables.tf

ae5110b

Uncomment 1.20

Update README.md

9dd04c8

Update README.md

daroga0002 reviewed Jun 16, 2021

View reviewed changes

Tsachi Nimni added 3 commits June 17, 2021 02:21

Merge branch 'feature/addons' of github.com:tnimni/terraform-aws-eks …

e49cc82

…into feature/addons

Change defults for false

87bed07

Add example for module usage

041d497

Add to docs that attach_worker_cni_policy should be set to false if u…

5cce47c

…sing the vpc cni addon

Add tags to addons, Update addon version for cni plugin

3484299

bryantbiggs mentioned this pull request Aug 25, 2021

docs: eks cluster upgrade instructions #1333

Closed

2 tasks

daroga0002 mentioned this pull request Aug 26, 2021

Implement node groups #1533

Closed

arianvp mentioned this pull request Aug 30, 2021

Support aws_eks_addon_version data source hashicorp/terraform-provider-aws#20725

Closed

stale bot added the stale label Oct 3, 2021

daroga0002 removed the stale label Oct 4, 2021

singhswg mentioned this pull request Nov 10, 2021

vpc-cni 1.10.0 upgrade fails aws/amazon-vpc-cni-k8s#1738

Closed

github-actions bot added the stale label Nov 21, 2021

bryantbiggs removed the stale label Nov 21, 2021

razvan-moj mentioned this pull request Nov 24, 2021

Planning and testing upgrade Live/Manager cluster to Kubernetes 1.20.x ministryofjustice/cloud-platform#3343

Closed

2 tasks

vijay-veeranki mentioned this pull request Nov 24, 2021

Ensure we get higher pod density on EKS nodes ministryofjustice/cloud-platform#3282

Closed

bryantbiggs mentioned this pull request Dec 7, 2021

feat!: Removed support for launch configuration and replace count with for_each #1680

Merged

1 task

github-actions bot added the stale label Dec 30, 2021

github-actions bot removed the stale label Dec 31, 2021

antonbabenko closed this in #1680 Jan 5, 2022

github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2022

feat: Add support for addons #1443

feat: Add support for addons #1443

Conversation

tnimni commented Jun 15, 2021 • edited Loading

PR o'clock

Description

Checklist

daroga0002 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnimni Jun 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daroga0002 left a comment

Choose a reason for hiding this comment

stevehipwell commented Jun 16, 2021

tnimni commented Jun 16, 2021

tnimni commented Jun 17, 2021 • edited Loading

thecosmicfrog commented Jun 22, 2021

tnimni commented Jun 22, 2021 • edited Loading

mmerickel commented Jun 23, 2021

tnimni commented Jun 23, 2021

nikitacr7 commented Jun 29, 2021

thapakazi commented Jun 30, 2021

tnimni commented Jun 30, 2021

csschwe commented Jul 9, 2021

tnimni commented Jul 15, 2021 • edited Loading

wellermann commented Jul 22, 2021 • edited Loading

stevehipwell commented Aug 12, 2021

stale bot commented Oct 3, 2021

fubar commented Oct 21, 2021

tnimni commented Oct 21, 2021

github-actions bot commented Nov 21, 2021

bryantbiggs commented Nov 21, 2021

beingamarnath commented Nov 29, 2021

github-actions bot commented Dec 30, 2021

Lillecarl commented Dec 30, 2021

antonbabenko commented Jan 5, 2022

mmerickel commented Jan 6, 2022

bryantbiggs commented Jan 6, 2022

mmerickel commented Jan 6, 2022

bryantbiggs commented Jan 6, 2022

github-actions bot commented Nov 11, 2022

tnimni commented Jun 15, 2021 •

edited

Loading

tnimni Jun 15, 2021 •

edited

Loading

tnimni commented Jun 17, 2021 •

edited

Loading

tnimni commented Jun 22, 2021 •

edited

Loading

tnimni commented Jul 15, 2021 •

edited

Loading

wellermann commented Jul 22, 2021 •

edited

Loading