-
Notifications
You must be signed in to change notification settings - Fork 109
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: some fixes for karpenter deploy #358
Conversation
@@ -3,8 +3,8 @@ locals { | |||
karpenter = { | |||
name = try(var.helm.release_name, "karpenter") | |||
enabled = true | |||
chart = try(var.helm.chart_name, "karpenter") | |||
repository = try(var.helm.repository, "oci://public.ecr.aws/karpenter") | |||
chart = try(var.helm.chart_name, "oci://public.ecr.aws/karpenter/karpenter") |
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.
Why did you change this? What was the problem here?
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.
This configuration worked for github-runners that also use oci helm repository. And it's working now for deploy karpenter in the some project.
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.
I've added information about this changes in the description of the PR
@@ -144,8 +148,6 @@ resource "helm_release" "this" { | |||
version = local.karpenter.chart_version | |||
namespace = module.namespace[count.index].name | |||
max_history = 3 | |||
repository_username = data.aws_ecrpublic_authorization_token.token.user_name |
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.
I'm not sure it works. Let's test it together
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.
I tested this yesterday in the sandbox account, and in the in one of the projects. All works fine. More over, this data works only for us-east-1
region. For other regions we need to use additional aws provider with alias.
Quality Gate passedIssues Measures |
Fixes for karpenter deploy
First error:
Solution: set
apiVersion: karpenter.k8s.aws/v1
for resourceskubectl_manifest
:ec2nodeclass_private
andec2nodeclass_public
(fileterraform/modules/k8s-karpenter/main.tf
).Second error:
Solution: Added fielsds
group
andkind
for private and public NodePools (fileterragrunt/ACCOUNT_ID/us-east-1/demo/env.yaml
)Third error:
Solution: Added field
consolidateAfter: 1m
for private and public NodePools (fileterragrunt/ACCOUNT_ID/us-east-1/demo/env.yaml
)Last one:
I removed the unnecessary data block
data "aws_ecrpublic_authorization_token" "token" {}
and the fields from thehelm_release
resource:repository_username = data.aws_ecrpublic_authorization_token.token.user_name
andrepository_password = data.aws_ecrpublic_authorization_token.token.password
. I tested these changes, and everything works perfectly.I also updated the
locals
block for Karpenter. I set thechart
value tooci://public.ecr.aws/karpenter
/karpenter and left therepository
field empty.This configuration has been tested across different Helm releases using OCI repositories, and everything works flawlessly. I successfully tested the Karpenter deployment from a Docker container in our sandbox account, and it was deployed without any issues.
There was another issue related to the
aws_ecrpublic_authorization_token
, related to aws provider. Since the public ECR is located in theus-east-1
region, this data doesn't work in other regions. An additional AWS provider with an alias is required to handle this.