-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix: create spot instance service-linked role for Karpenter #14
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create the linked roles out of the module. I had the idea to create a bootstrap module that managed the required settings per account.
I guess you tried to deploy to a new account so you had issues with the instance profile, but it's not required once the service linked roles are created.
cluster_name = module.eks.cluster_name | ||
enable_irsa = true | ||
irsa_oidc_provider_arn = module.eks.oidc_provider_arn | ||
irsa_namespace_service_accounts = ["kube-system:karpenter"] | ||
iam_role_name = "karpenter-${local.id}" | ||
iam_role_use_name_prefix = false | ||
create_instance_profile = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Karpenter creates instance profiles itself so this should be disabled by default
@@ -201,6 +204,15 @@ module "karpenter" { | |||
tags = local.tags | |||
} | |||
|
|||
# Allow creation of SPOT instances | |||
resource "aws_iam_service_linked_role" "spot" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can only have one per account so should not be in this module.
aws_service_name = "spot.amazonaws.com" | ||
} | ||
|
||
resource "aws_iam_service_linked_role" "spot_fleet" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
@swibrow Thanks for the feedback. Yes I tried running this in the ATM Staging account. I will update the PR to explicit the dependencies that this module needs in order to run with no issues. I'm good with an external bootstrap module. |
Closed in favor of #15 |
No description provided.