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

Add telegraf-ds (imported and updated from old tick-stack charts) #16

Merged
merged 11 commits into from
Feb 28, 2020
Merged

Add telegraf-ds (imported and updated from old tick-stack charts) #16

merged 11 commits into from
Feb 28, 2020

Conversation

nsteinmetz
Copy link
Contributor

@nsteinmetz nsteinmetz commented Feb 26, 2020

Add telegraf as daemonset - closes #1

  • Influx CLA signed

@nsteinmetz nsteinmetz changed the title Fix #1: Add telegraf-ds (imported and updated from old tick-stack charts) Add telegraf-ds (imported and updated from old tick-stack charts) Feb 26, 2020
@naseemkullah
Copy link
Collaborator

naseemkullah commented Feb 27, 2020

@nsteinmetz @rawkode Would it be too involved to configure the current telegraf chart such that it could be deployed as either a deployment or daemonset?

@nsteinmetz
Copy link
Contributor Author

It should be doable, the issue I see is more about readability and expectability. And maybe there could be some deployment issue if you try to deploy the DS and and the Deployment in the same namespace.

Having a single chart for telegraf would mean that you must explicitely set what kind of Deployment or DS you expect and related plugin are not the same too. So the targeted chart may be hard to read and use.

DaemonSet use case is more on how to monitor nodes and what they contain whereas deployment is more to monitor pods by themselves.

So as use cases are not the same, I think it's better to have duplicated code and two different charts to ease readibility and understanding instead of a single one.

@rawkode
Copy link
Contributor

rawkode commented Feb 27, 2020

100% agree with @nsteinmetz

Lets not merge these, although they're both Telegraf's; they very different use-cases.

I'm testing this chart now 👍

Copy link
Contributor

@gitirabassi gitirabassi left a comment

Choose a reason for hiding this comment

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

I believe in more opinionated installations, and then customize them in case of need

charts/telegraf-ds/Chart.yaml Outdated Show resolved Hide resolved
charts/telegraf-ds/templates/_helpers.tpl Show resolved Hide resolved
charts/telegraf-ds/templates/configmap.yaml Show resolved Hide resolved
charts/telegraf-ds/templates/role.yaml Outdated Show resolved Hide resolved
charts/telegraf-ds/values.yaml Outdated Show resolved Hide resolved
charts/telegraf-ds/values.yaml Outdated Show resolved Hide resolved
@rawkode rawkode requested a review from gitirabassi February 28, 2020 20:51
charts/telegraf-ds/Chart.yaml Outdated Show resolved Hide resolved
charts/telegraf-ds/README.md Outdated Show resolved Hide resolved
charts/telegraf-ds/values.yaml Show resolved Hide resolved
@rawkode rawkode merged commit 6816333 into influxdata:master Feb 28, 2020
@nsteinmetz nsteinmetz deleted the add_telegraf_ds branch February 28, 2020 22:26
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.

Support Node Monitoring Through DaemonSet
4 participants