influxdata / tick-charts

A repository for Helm Charts for the full TICK Stack
Apache License 2.0
90 stars 72 forks source link

[Telegraf] Allow full use of duplicate input/ouputs in telegraf configuration #28

Closed amoskyler closed 6 years ago

amoskyler commented 6 years ago

What this PR does / why we need it: This change converts the inputs and outputs fields from an object with keys associated to configs, to an array of objects which contain keys associated to objects.

Before:

config:
  outputs:
     influxdb:
      ...config...

After:

config:
  outputs:
    - influxdb:
       ...config...

Telegraf supports multiples of same type input and output configurations. In the current chart implementation, each input and output is configured with keys in the objects inputs and outputs respectively. YAML overrides the first key in the object however, and therefore the templating process from YAMl to TOML drops valid TOML configurations, which incorrectly limits valid configurations.

timhallinflux commented 6 years ago

@amoskyler -- I need you to complete the CLA before we can accept your submission. https://www.influxdata.com/legal/cla/

Apologies and thank you for your interest and contribution!

amoskyler commented 6 years ago

@timhallinflux CLA has been signed!

amoskyler commented 6 years ago

@timhallinflux makes sense regarding Chart version. My reasoning behind changing the version is this change will in fact break previous chart value configurations. But, as it is pinned to the GA of Telegraf I will modify.

amoskyler commented 6 years ago

@timhallinflux Any chance we can get this merged in? Or of course if you have any further issues/comments I am willing to address them.

amoskyler commented 6 years ago

I intended that to be an example of using duplicate configuration keys.

timhallinflux commented 6 years ago

also in telegraf-s/templates/NOTES.txt -- the -s is for the standalone Telegraf and the -ds is the daemonset. It looks like you made changes to the telegraf-s/templates/NOTES.txt which reference the daemonset --- but I don't think that's appropriate in this case.

dtshepherd commented 6 years ago

@amoskyler @timhallinflux Any updates on getting this merged in? I just came across the limitation and need to define two outputs to two different influxdbs with different configs (database names, password vs ssl, etc). This PR also makes it possible to define other config keys such as ssl_key, ssl_cert, and ssl_ca.

I'm trying to avoid forking this repo and maintaining my own chart repo if possible :)

amoskyler commented 6 years ago

@timhallinflux pinging you to follow up on this. I just merged in upstream's GA upgrade from 1.7.3 to 1.7.4 as well as removed the last ds reference in the telegraf-s chart.

Is there anything else we need to address before this is ready to merge?

timhallinflux commented 6 years ago

fixes #11