Skip to content
This repository has been archived by the owner on Mar 17, 2024. It is now read-only.

Omit service account generation when not using strimzi (#53) #64

Merged
merged 2 commits into from
Sep 19, 2019

Conversation

khorshuheng
Copy link
Contributor

Addresses #53.

In this PR, i generate the service account dynamically, rather than using a user input as originally discussed in issue 53. If we rely on user input, the helm chart will fail if it is executed twice under different helm release name, as the service account would have already existed.

@seglo seglo self-requested a review September 17, 2019 14:53
Copy link
Owner

@seglo seglo left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Good point regarding the name of a service account. Can you please add a reference to this configuration in the README.md?

@khorshuheng
Copy link
Contributor Author

I have added example usage.

@seglo
Copy link
Owner

seglo commented Sep 18, 2019

Thanks @khorshuheng, but I meant could you add a reference to this configuration option to the configuration table in the README.md (https://github.com/lightbend/kafka-lag-exporter/#configuration-1)

@khorshuheng
Copy link
Contributor Author

Thanks @khorshuheng, but I meant could you add a reference to this configuration option to the configuration table in the README.md (https://github.com/lightbend/kafka-lag-exporter/#configuration-1)

That is for standalone installation configuration. Service account generation seems to be helm specific, and not applicable to standalone installation?

@seglo
Copy link
Owner

seglo commented Sep 19, 2019

@khorshuheng My mistake, you're correct. I would like to expand the configuration on the README.md to include a reference for Helm, but indeed that's not currently the case.

Thanks for the contribution!

@seglo seglo self-requested a review September 19, 2019 14:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants