Add example for IRSA and cluster-autoscaler #710

max-rocket-internet · 2020-01-24T13:45:05Z

PR o'clock

Description

Many people have requested an example using IRSA (IAM Roles for Service Accounts). So here I am adding an example for the cluster-autoscaler.

Checklist

Change added to CHANGELOG.md. All changes must be added and breaking changes and highlighted
CI tests are passing
README.md has been updated after any changes to variables and outputs. See https://github.com/terraform-aws-modules/terraform-aws-eks/#doc-generation

dpiddockcmp

👏 ❤️
Comments suggesting things to delete to remove distractions 😄

dpiddockcmp · 2020-01-24T13:50:21Z

examples/irsa/README.md

+kubectl -n kube-system create serviceaccount tiller
+kubectl create clusterrolebinding tiller --clusterrole cluster-admin --serviceaccount=kube-system:tiller
+helm init --service-account=tiller


Please please can fresh examples use Helm 3? No more tiller!

Maybe we can omit tiller installation. And add note that we must have a working helm installation. Because, it's not relevant here.

I personally use terraform with helm provider to perform cluster-autoscaler installation during my cluster setup. But it doen't matter how we install it. What we need for the serviceaccount is here https://github.com/terraform-aws-modules/terraform-aws-eks/pull/710/files#diff-5d3e96c50e09c013f8553243cb0be4d6R3-R6

I agree 100%. I just haven't used Helm 3 yet 😢

I'll redo next week with Helm 3. I need to it this anyway!

Maybe we can omit tiller installation.

OK good input. I'll remove that part. But I'll keep the helm install command for the chart. Is it same for Helm 3?

examples/irsa/main.tf

max-rocket-internet · 2020-01-24T14:37:31Z

@barryib do you know why the terraform_docs check is failing? I don't get it...

barryib · 2020-01-24T14:46:10Z

@barryib do you know why the terraform_docs check is failing? I don't get it...

Nope. But even the terraform fmt is failing.

barryib · 2020-01-24T14:52:04Z

@max-rocket-internet terraform-doc 0.8.0 is now in stable. We should probably update terraform pre-commit antonbabenko/pre-commit-terraform#85.

pre-commit autoupdate would probably help.

max-rocket-internet · 2020-01-24T15:14:18Z

We should probably update terraform pre-commit antonbabenko/pre-commit-terraform#85.

OK cool. Next week I'll have a look.

I think we should also remove the autoscaling_enabled = true option from workers and remove the autoscaling policies from the module. In a a separate PR. People can do this outside the module now and IRSA is one way to do it.

TBeijen · 2020-01-24T19:25:42Z

examples/irsa/main.tf

+  cluster_name = local.cluster_name
+  subnets      = module.vpc.private_subnets
+  vpc_id       = module.vpc.vpc_id
+  enable_irsa  = true


If I'm not mistaken, using IRSA on cluster-autoscaler, it would be good to add these flags to the cluster variables:

# module no longer needs to manage autoscaling policy since this is moved to the service-account IAM role manage_worker_autoscaling_policy = false attach_worker_autoscaling_policy = false

You are not mistake but I think we just remove the autoscaling policy stuff completely and let users manage that themselves.

May I suggest: Leave the policy stuff in but change default enabled to false in next release (and ofc. mention in release notes). Then remove in subsequent release.

This would allow users to somewhat seamlessly transition to IRSA, without needing to temporary bolt on additional policies.

@TBeijen actually we already have var.attach_worker_autoscaling_policy/var.manage_worker_autoscaling_policy. I'll just make a PR to set these to default of false.

barryib · 2020-01-27T12:32:25Z

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
 - repo: git://github.com/antonbabenko/pre-commit-terraform
-  rev: v1.22.0
+  rev: v1.24.0


@max-rocket-internet please change args into args: ["--args=--with-aggregate-type-defaults", "--args=--no-escape"]

max-rocket-internet · 2020-01-28T15:17:19Z

OK I have:

Switched to use the public module terraform-aws-modules/iam/aws//modules/iam-assumable-role-with-oidc for the IAM part, it's a little bit simpler and also pushes the focus away from this module a bit 😃
Removed helm init stuff
Removed autoscaling_enabled = true and just added the required tag directly to the node group. This way it's clear how it works and the example won't change when we remove the policy.
Simplified the example further by removing NAT gateways and private subnets

Please re-review @barryib @dpiddockcmp @TBeijen 🚀

barryib · 2020-01-28T18:21:34Z

Can you please update your branch and upgrade to terraform-docs 0.8.1 ?

dpiddockcmp

This looks good to me

dpiddockcmp · 2020-01-29T09:37:12Z

examples/irsa/main.tf

+
+data "aws_caller_identity" "current" {}
+
+resource "random_string" "suffix" {


Is this resource used anywhere?

Thanks. I've removed it.

max-rocket-internet · 2020-01-29T14:18:58Z

@barryib done rebase. Thanks for fixing the pre-commit stuff

TBeijen

Looks fine. As mentioned in the other PR: Makes sense also to not have 2 releases with breaking changes, so unlike what I suggested, ditching all references to manage autoscaling looks good.

ankushagarwal · 2020-01-31T17:43:08Z

@max-rocket-internet : Curious as to what is the reason to use worker_groups instead of node_groups in module "eks". I am not sure what the difference between them is. The managed_node_groups/main.tf example uses node_groups in module "eks"

morganchristiansson · 2020-01-31T18:48:57Z

worker_groups is the classic unmanaged way of doing things and allows you to use spot instances and provide kubelet args.

I believe either will work with irsa.

github-actions · 2022-11-20T02:31:25Z

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.

max-rocket-internet requested a review from barryib January 24, 2020 13:45

max-rocket-internet mentioned this pull request Jan 24, 2020

Support for creating service accounts with IAM policy #525

Closed

4 tasks

dpiddockcmp reviewed Jan 24, 2020

View reviewed changes

TBeijen reviewed Jan 24, 2020

View reviewed changes

max-rocket-internet added the wip label Jan 27, 2020

barryib reviewed Jan 27, 2020

View reviewed changes

barryib changed the title ~~Add example for IRSA and cluster-autoscaler~~ WIP: Add example for IRSA and cluster-autoscaler Jan 27, 2020

max-rocket-internet removed the wip label Jan 28, 2020

max-rocket-internet changed the title ~~WIP: Add example for IRSA and cluster-autoscaler~~ Add example for IRSA and cluster-autoscaler Jan 28, 2020

barryib mentioned this pull request Jan 28, 2020

Remove of autoscaling IAM policy related stuff #716

Merged

3 tasks

dpiddockcmp approved these changes Jan 29, 2020

View reviewed changes

max-rocket-internet added 6 commits January 29, 2020 15:08

Add example for IRSA

63f9939

remove unused SGs

e16dda6

remove helm setup part

597d288

simplification and use module

ee840c4

fix fmt error

083a1a0

remove unused resource

f0fd34d

TBeijen approved these changes Jan 29, 2020

View reviewed changes

Merge branch 'master' into irsa_example

df1ef07

max-rocket-internet merged commit 9032dce into terraform-aws-modules:master Jan 30, 2020

max-rocket-internet deleted the irsa_example branch January 30, 2020 14:21

barryib mentioned this pull request Feb 4, 2020

Add autoscaling_enabled to example files #578

Closed

4 tasks

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add example for IRSA and cluster-autoscaler #710

Add example for IRSA and cluster-autoscaler #710

max-rocket-internet commented Jan 24, 2020 •

edited

Loading

dpiddockcmp left a comment

dpiddockcmp Jan 24, 2020

barryib Jan 24, 2020 •

edited

Loading

max-rocket-internet Jan 24, 2020

max-rocket-internet Jan 24, 2020

barryib Jan 24, 2020

max-rocket-internet commented Jan 24, 2020

barryib commented Jan 24, 2020

barryib commented Jan 24, 2020

max-rocket-internet commented Jan 24, 2020

TBeijen Jan 24, 2020

max-rocket-internet Jan 27, 2020

TBeijen Jan 27, 2020

max-rocket-internet Jan 28, 2020

barryib Jan 27, 2020

max-rocket-internet commented Jan 28, 2020

barryib commented Jan 28, 2020

dpiddockcmp left a comment

dpiddockcmp Jan 29, 2020

max-rocket-internet Jan 29, 2020

max-rocket-internet commented Jan 29, 2020

TBeijen left a comment

ankushagarwal commented Jan 31, 2020

morganchristiansson commented Jan 31, 2020 •

edited

Loading

github-actions bot commented Nov 20, 2022


		data "aws_caller_identity" "current" {}

		resource "random_string" "suffix" {

Add example for IRSA and cluster-autoscaler #710

Add example for IRSA and cluster-autoscaler #710

Conversation

max-rocket-internet commented Jan 24, 2020 • edited Loading

PR o'clock

Description

Checklist

dpiddockcmp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barryib Jan 24, 2020 • edited Loading

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

max-rocket-internet commented Jan 24, 2020

barryib commented Jan 24, 2020

barryib commented Jan 24, 2020

max-rocket-internet commented Jan 24, 2020

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

max-rocket-internet commented Jan 28, 2020

barryib commented Jan 28, 2020

dpiddockcmp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

max-rocket-internet commented Jan 29, 2020

TBeijen left a comment

Choose a reason for hiding this comment

ankushagarwal commented Jan 31, 2020

morganchristiansson commented Jan 31, 2020 • edited Loading

github-actions bot commented Nov 20, 2022

max-rocket-internet commented Jan 24, 2020 •

edited

Loading

barryib Jan 24, 2020 •

edited

Loading

morganchristiansson commented Jan 31, 2020 •

edited

Loading