Skip to content

helm: make aws service annotations configurable#7450

Closed
tghaas wants to merge 1 commit intogravitational:masterfrom
tghaas:move-aws-annotations
Closed

helm: make aws service annotations configurable#7450
tghaas wants to merge 1 commit intogravitational:masterfrom
tghaas:move-aws-annotations

Conversation

@tghaas
Copy link
Copy Markdown
Contributor

@tghaas tghaas commented Jul 1, 2021

This would allow a user to keep the default configuration for the AWS K8s service object but also allow modification of that within values.yaml. Main use case is if you are using the AWS Load Balancer Controller, this allows you to change the annotations so it can control the service instead of the built-in controller.

Copy link
Copy Markdown
Contributor

@webvictim webvictim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a way to conditionally disable the default AWS service annotations without needing to set defaultServiceAnnotations to true for every single AWS install that doesn't need/want to change them. I'm trying to keep the install commands to be essentials only to avoid people having to set lots of extra parameters that they don't need to understand.

Maybe something like this:

annotations:
  serviceSetAWSDefaults: false
  service:
    aws-special-param: blah
    another-aws-param: something
  • If serviceSetAWSDefaults isn't set, it should default to true.
  • If serviceSetAWSDefaults is set to true but the chart isn't running in AWS mode, it should do nothing.

@russjones
Copy link
Copy Markdown
Contributor

@webvictim Can you take a look? If this is something we want, can you work with @tghaas to fix the conflicts. Then we can get this merged and backported to a release branch.

@russjones
Copy link
Copy Markdown
Contributor

@tghaas Looks like this PR has not had movement in a few months. I am going to close this for now, if you still want to add this functionality, please re-open or create another PR.

@russjones russjones closed this Dec 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants