Skip to content
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

add extraPorts for extraContainers #176

Merged

Conversation

cruwe
Copy link
Contributor

@cruwe cruwe commented Nov 28, 2022

In parallel to extraContainers, extraPorts may be required to scrape additional metrics generated by said extraContainers.

Thanks for your work on the kminion prometheus exporter, it has proven to be extremely useful.

@CLAassistant
Copy link

CLAassistant commented Nov 28, 2022

CLA assistant check
All committers have signed the CLA.

@cruwe
Copy link
Contributor Author

cruwe commented Jan 10, 2023

politely bumping ... would this PR be acceptable for you? Thx again for your work on the kminion exporter

@@ -50,6 +50,7 @@ service:
type: ClusterIP
port: 8080 # This port is also used as exposed container port
annotations: {} # # Annotations to add to the service
extraPorts: []
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you provide a commented sample value here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done ... thanks for your time

@weeco
Copy link
Contributor

weeco commented Jan 10, 2023

Hey @cruwe ,
sorry for the delay.

Generally I recommend to fork the chart at some point if the customization needs are too specific. We can't fit all usecases with a single chart in favour of maintainability and simplicity. I'll be okay with this change, just asked for additional documentation for this parameter in the values.yaml though.

Thanks

@cruwe cruwe force-pushed the add-extraPorts-for-extraContainers branch from b71f80c to 4bdbda2 Compare January 13, 2023 10:37
In parallel to extraContainers, extraPorts may be required to scrape
additional metrics generated by said extraContainers.
@cruwe cruwe force-pushed the add-extraPorts-for-extraContainers branch from 4bdbda2 to ba7d561 Compare January 13, 2023 10:41
@cruwe
Copy link
Contributor Author

cruwe commented Jan 13, 2023

Hi @weeco, I understand why you do not want excessive customization :-) ... just hoped as []extraContainers does exist, you wouldn't mind the []extraPorts.

Thank you for your time and for developing kminion. Cheers!

Copy link
Contributor

@weeco weeco left a comment

Choose a reason for hiding this comment

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

Thank you :)

@weeco weeco merged commit 1be4f8f into redpanda-data:master Jan 13, 2023
@cruwe cruwe deleted the add-extraPorts-for-extraContainers branch January 13, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants