-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Metricbeat daemonset deployment optional #716
Metricbeat daemonset deployment optional #716
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 |
FYI I have signed the CLA. |
Hi @erihanse Thank you for opening this PR. 2 options:
|
Defaults to enabled for both daemonset and deployment
We use dict.get method in case there are no daemonsets or deployments included to avoid KeyError.
@fatmcgav ah, of course! :D Should be okay now. |
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 this PR 👍
That's a really valuable feature you're adding.
We have a test that daemonset exists with default values:
assert name in r["daemonset"] |
Could you add the same tests for deployment?
adding assert name + "-metrics" in r["deployment"]
should be enough.
The rest is OK for me.
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⛴
backported to |
@jmlrt I'm sorry if I don't understand the release process, but when will these changes, including those in #387 be released?
|
Charts are released in the same time as the rest of Elastic Stack. |
${CHART}/tests/*.py
${CHART}/examples/*/test/goss.yaml
Closes #702.