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

Deploy telegraf configuration as a "non config" file #7250

Merged
merged 2 commits into from
Apr 9, 2020

Conversation

kir4h
Copy link
Contributor

@kir4h kir4h commented Mar 29, 2020

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

Description

This PR fixes #4966, by changing the way telegraf.conf is packaged and deployed:

  • telegraf.conf is now packaged as telegraf.conf.sample. This will make package managers compare over the .sample file instead of the .conf one, making it more user friendly since /etc/telegraf.conf is usually configured by the user
  • If no /etc/telegraf/telegraf.conf is present on the target system (typical case of a fresh install), the post installation step will copy /etc/telegraf/telegraf.conf.sample to /etc/telegraf/telegraf.conf to have an initial working configuration

Notes:

  • There is a non related first commit with some changes to build.py file, based on PEP-8 recommendations
  • I have tested this with linux/amd64 rpm/deb packages, I take it it should be similar for the rest since there is a lot of shared code but if I'm missing some scenario please let me know
  • I have done this also for Windows for consistency, but haven't looked into how Windows is actually deployed (I guess just a zip file)
  • Not sure if I need to edit the CHANGELOG myself, if so please let me know

Feel free to add any suggestion, I was not really familiar with deb rpm packaging internals, nor with fpm.

@kir4h
Copy link
Contributor Author

kir4h commented Apr 1, 2020

Anything missing from my side?

@kir4h
Copy link
Contributor Author

kir4h commented Apr 6, 2020

@danielnelson is there any procedure for triggering a review? (other than the CLA)

@kir4h
Copy link
Contributor Author

kir4h commented Apr 8, 2020

@ssoroka could I request a reviewer for this PR? (not sure if you're organized by topics or it's best effort). Thanks!

Copy link
Contributor

@ssoroka ssoroka left a comment

Choose a reason for hiding this comment

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

Looks pretty reasonable

@ssoroka ssoroka merged commit cc6c77f into influxdata:master Apr 9, 2020
@kir4h kir4h deleted the deploy_telegraf_conf_as_non_config branch April 9, 2020 18:57
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
veorlo pushed a commit to GlobalNOC/telegraf that referenced this pull request May 4, 2020
@danielnelson danielnelson added this to the 1.15.0 milestone Jun 24, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
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.

Try to update "telegraf.conf" every time (at least) debian package is upgraded
3 participants