-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[WIP] Etcd integration for configuration #651
Conversation
192aa91
to
9d7acb2
Compare
New features: Agent specific configEach agent start reading the value of key
Configuration folderYou can now set your configuration in a folder like this:
(An example is available here: https://github.com/titilambert/telegraf/tree/etcd/internal/etcd/testdata/test1) Then you can send your configuration folder to etcd:
Then you can start all your telegraf agents like this:
|
@sparrc rebased ! (I don't give up ;) ) |
great! Sorry I haven't had time to get a full review on this one, I've been slammed by some other features |
@@ -19,6 +19,7 @@ import ( | |||
"github.com/influxdata/telegraf/plugins/serializers" | |||
|
|||
"github.com/influxdata/config" | |||
"github.com/naoina/toml" |
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.
Can you remove the naoina/toml dependency here? I believe there is a function in influxdata/config that you can use to load and parse the toml file
d32fab5
to
1d3f26d
Compare
@sparrc changes done ! I add 2 tests to get more coverage. |
7849b11
to
c05fba3
Compare
@sparrc What you think about those features ? BTW, it's still missing:
|
e4f4f63
to
39818d6
Compare
Added option to select the root folder name in etcd (default "/telegraf") |
Rebased with the new toml lib |
Rebased !
We will use
|
why do you want to do that? is there a precedent? |
It's just to separate daemon binary from utils binary. I don't know if it's a good idea, it's just to copy etcd/kubernetes/... |
@sparrc could you just confirm that this PR is in scope of Telegraf ? :) |
yes, it is :) |
@sparrc cool :) |
fd743d8
to
47963a7
Compare
@sparrc Rebased ! |
Why not just use https://github.com/kelseyhightower/confd together with a kill HUP signal? |
@balboah This does add a single point of failure, doesn't it ? |
@titilambert As I've looked through this PR more, and looked into etcd and configuration management options, I feel like this is going to get messy. Yes, it's nice that we could have etcd directly baked into telegraf in some ways. But it's also complicated and it ignores all the other options that there are out there for achieving this (consul, redis, vault, etc) As @balboah suggests, this seems more like a configuration management issue, so why not use configuration management tools to solve it? Why should telegraf become a configuration management tool on top of all the other things it does? |
ps: do you have any examples of a project similar to telegraf that integrates directly with etcd? Or are there any libraries that we could use to generically get conf files? (like a library version of confd?) |
@titilambert not sure what you mean with single point of failure. In my case confd runs inside the container, monitors the etcd cluster for changes and updates the config file + sends the kill signal when there is some change. |
@balboah Single point of failure: What's happen if confd crashed ? We don't have any fallback for confd. I can not see how running confd inside a container can solve this issue. Docker can still restart condf but the single point of failure is now on Docker. The only single point of failure should be Telegraf. @sparrc I'm agree with you, this PR limits Telegraf to Etcd. But, imo, confd seems a single point of failure.
|
@titilambert but what if your etcd server goes down? This could be a problem if etcd is integrated directly into telegraf. If you decouple the two services (telegraf and config management) then Telegraf is completely unaffected by any status or change in etcd. |
@sparrc etcd can run as a cluster (with at least 3 nodes) which means etcd isn't a single point of failure. |
sorry maybe I still don't understand, but I fail to see how integrating basically the same use case as confd into telegraf solves the availability issue differently? The code would be pretty much the same, the number of processes would be the same, how is the "single point of failure" different? Also confd already supports toml templating that you're introducing, has plugins for different statements and supports more sources than etcd. |
@balboah With this PR Telegraf will never get config files. It loads conf directly from etcd in memory. This eliminate the write config step. @sparrc I think you're right ! Maybe we need to use https://github.com/spf13/viper ? We can use it to read config only from remote sources or for both remote and local files. What should be the best choice ? |
when you plan to release a version with etcd integration? |
I'll chime in my 2 cents - I don't think telegraf should go down the road of supporting config backends directly. Once If integrated directly and It also protects |
@johnrengelman what do you think about https://github.com/spf13/viper ?
but I'm sure about using confd+telegraf in docker envs... |
Not sure if this has been mentioned before. Docker's libkv is another potential option for supporting multiple distributed config stores. |
that library looks fantastic, thanks @gunnaraasen |
@sparrc what do you think ? I rewrite the PR with https://github.com/docker/libkv or https://github.com/spf13/viper ? |
closing this because I prefer to have this conversation in #272 |
Rebased PR #465
Hello !
I just started an example of what could be the etcd integration with telegraf
Here an example:
1 . Make a myconf.conf file, which will be stored in etcd, with the following content :
2 . Send this file to etcd using the label mylabel
3 . You can check if data is really written in etcd with
3 . Now any telegraf agent can load this config use the label mylabel
Notes:
Agent config reading order:
/telegraf/main
key/telegraf/hosts/HOSTNAME
key/telegraf/labels/LABEL1
key4.
/telegraf/labels/LABEL2
keyFeatures:
/telegraf/hosts/HOSTNAME
)Missing:
Add an option to select the root folder name in etcd (default "/telegraf")DONEHandle multiple etcd serversDONEHandle update/set/delete in etcdDONEDocumentationDONE