Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

feat(metricbeat): annotation support for daemonset and deployment #713

Merged
merged 9 commits into from
Jul 8, 2020

Conversation

kernkonzentrat
Copy link
Contributor

  • Chart version not bumped (the versions are all bumped and released at the same time)
  • README.md updated with any new values or changes
  • Updated template tests in ${CHART}/tests/*.py
  • Updated integration tests in ${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

@elasticmachine
Copy link
Collaborator

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?

@fatmcgav
Copy link
Contributor

fatmcgav commented Jul 7, 2020

Jenkins test this please

@fatmcgav fatmcgav changed the base branch from 7.7 to master July 7, 2020 08:52
@fatmcgav fatmcgav changed the base branch from master to 7.7 July 7, 2020 08:52
@fatmcgav fatmcgav added enhancement New feature or request metricbeat labels Jul 7, 2020
Copy link
Contributor

@fatmcgav fatmcgav left a 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.

"""
r = helm_template(config)
assert "grault" in r["deployment"][name + "-metrics"]["metadata"]["annotations"]
assert r["deployment"][name + "-metrics"]["metadata"]["annotations"]["grault"] == "waldo"
Copy link
Contributor

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:

Suggested change
assert r["deployment"][name + "-metrics"]["metadata"]["annotations"]["grault"] == "waldo"
assert (
r["deployment"][name + "-metrics"]["metadata"]["annotations"]["grault"]
== "waldo"
)

Copy link
Contributor Author

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

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.

@fatmcgav
Copy link
Contributor

fatmcgav commented Jul 7, 2020

Jenkins test this please

@fatmcgav
Copy link
Contributor

fatmcgav commented Jul 7, 2020

@kernkonzentrat Thanks for that.

Jenkins test this please

@kernkonzentrat
Copy link
Contributor Author

kernkonzentrat commented Jul 7, 2020

@fatmcgav black still wanted to reformat...sorry, VS Code didn't remove blank line at the end when auto formatting...locally black --diff --check --exclude='ve/|venv/' . now gives me 1 file would be left unchanged. - seems good now

@fatmcgav
Copy link
Contributor

fatmcgav commented Jul 8, 2020

Jenkins test this please

@fatmcgav
Copy link
Contributor

fatmcgav commented Jul 8, 2020

Thanks for persevering there @kernkonzentrat, CI passed on that run :)

Copy link
Contributor

@fatmcgav fatmcgav left a comment

Choose a reason for hiding this comment

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

LGTM

@kernkonzentrat kernkonzentrat deleted the feat/annotation-support branch July 8, 2020 09:54
@fatmcgav
Copy link
Contributor

fatmcgav commented Jul 8, 2020

@kernkonzentrat Thanks again for the contribution.

I've back-ported the changes onto the 7.8 and 6.8 branches, so these changes will be included in the next 7.8 patch release.

@kernkonzentrat
Copy link
Contributor Author

@fatmcgav Thank you for support and reviewing...the project team is looking forward to getting in the update :)

@jmlrt jmlrt removed the v7.9.0 label Jul 13, 2020
This was referenced Jul 16, 2020
This was referenced Jul 27, 2020
@jmlrt jmlrt mentioned this pull request Oct 28, 2020
This was referenced Nov 17, 2020
@jmlrt jmlrt mentioned this pull request Feb 8, 2021
This was referenced Mar 15, 2021
@jmlrt jmlrt mentioned this pull request May 25, 2021
@jmlrt jmlrt mentioned this pull request Mar 8, 2022
@jmlrt jmlrt mentioned this pull request Apr 21, 2022
This was referenced Sep 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants