-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(cluster-autoscaler): allow user to upgrade cluster-autoscaler #715
feat(cluster-autoscaler): allow user to upgrade cluster-autoscaler #715
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.
Thanks for the fix @chubchubsancho . I agree with this approach 👍🏼
@@ -19,6 +19,7 @@ module "helm_addon" { | |||
values = [templatefile("${path.module}/values.yaml", { | |||
aws_region = var.addon_context.aws_region_name, | |||
eks_cluster_id = var.addon_context.eks_cluster_id |
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.
Just noticed. You are missing a ,
at end of this line.
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.
nice catch i fix this
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 dont think you need a ,
in values template but the line number 20 has got one. That can be removed instead
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.
you're right ,
is not needed here just removed it
a9c9119
to
e3e840f
Compare
Previously `image.tag` was set through `set_values` Due to this we were unable to upgrade cluster-autoscaler * redifine cluster-autoscaler image.tag on `values.yaml` * remove `set_values`
e3e840f
to
3855efe
Compare
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.
LGTM 👍🏼
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.
LGTM!
Previously
image.tag
was set throughset_values
Due to this we were unable to upgrade cluster-autoscaler
values.yaml
set_values
What does this PR do?
Define default value for
cluster-autoscaler
based on cluster version and let user override this valueMotivation
Resolv #714
More
pre-commit run -a
with this PRNote: Not all the PRs required examples and docs except a new pattern or add-on added.
For Moderators
Additional Notes