-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(metricbeat): annotation support for daemonset and deployment #713
Conversation
…entrat/helm-charts into feat/annotation-support
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? |
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.
Changes look sane to me.
One CI failure that needs fixing though.
metricbeat/tests/metricbeat_test.py
Outdated
""" | ||
r = helm_template(config) | ||
assert "grault" in r["deployment"][name + "-metrics"]["metadata"]["annotations"] | ||
assert r["deployment"][name + "-metrics"]["metadata"]["annotations"]["grault"] == "waldo" |
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.
Looks like there's one Python lint issue here:
assert r["deployment"][name + "-metrics"]["metadata"]["annotations"]["grault"] == "waldo" | |
assert ( | |
r["deployment"][name + "-metrics"]["metadata"]["annotations"]["grault"] | |
== "waldo" | |
) |
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.
@fatmcgav ...added the suggested change to daemonset
and deployment
test
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.
@kernkonzentrat It looks like the lint step is still not liking this format:
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.
I formatted the tests with black
now. I guess it will work eventually.
Jenkins test this please |
@kernkonzentrat Thanks for that. Jenkins test this please |
@fatmcgav |
Jenkins test this please |
Thanks for persevering there @kernkonzentrat, CI passed on that run :) |
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
@kernkonzentrat Thanks again for the contribution. I've back-ported the changes onto the |
@fatmcgav Thank you for support and reviewing...the project team is looking forward to getting in the update :) |
${CHART}/tests/*.py
${CHART}/examples/*/test/goss.yaml
Add annotation support for daemonset and deployment. Annotations are only added to the k8s objects, if values are set for annotations. PR for issue #684