-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Update logstash chart to support custom ports #505
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
💚 CLA has been signed |
I've signed the CLA. Not sure how to rerun the check... |
jenkins test this please |
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.
Hi @ChiefAlexander,
Thanks for this PR.
LGTM, however I would prefer naming this values extraPorts
to keep consistent with the others values prefixed by extra
. WDYT?
Also, it would be great to add a python test for this feature. You can find an example for extra container feature here, tests can then be run with make test
(see Testing).
I can create the test in another PR if you're not confortable with it.
@jmlrt Thanks for the review! I have made your suggested changes. |
jenkins test this please |
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 adding the test.
Some change to do to make Black formatter happy in elastic+helm-charts+pull-request+lint-python/22.
Co-Authored-By: Julien Mailleret <julien.mailleret@elastic.co>
jenkins test this please |
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
${CHART}/tests/*.py
${CHART}/examples/*/test/goss.yaml