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 optional namespace restriction to prometheus input plugin #5697

Merged
merged 2 commits into from
Apr 10, 2019

Conversation

Benjamintf1
Copy link
Contributor

Allow prometheus to operate as a namespaced pod with namespace level permissions. This addresses #5306

Required for all PRs:

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

…permissions

Signed-off-by: David Timm <dtimm@pivotal.io>
@Benjamintf1
Copy link
Contributor Author

We see that there was a previous issue about this. Seeing their solution, it seems like it could be reasonable/convenient to have it be a toggle and ONLY apply to the namespace of the telegraf config, but we also think the usecases that this supports are wider.

@Benjamintf1 Benjamintf1 closed this Apr 8, 2019
@Benjamintf1 Benjamintf1 reopened this Apr 8, 2019
@Benjamintf1
Copy link
Contributor Author

We seem to be having failed tests unrelated to our changes.. I tried to close/open this pr to run the tests again, this does not seem to re-run tests.

@danielnelson danielnelson added this to the 1.11.0 milestone Apr 9, 2019
@danielnelson danielnelson added area/prometheus feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin labels Apr 9, 2019
Signed-off-by: Ben Fuller <bfuller@pivotal.io>
@Benjamintf1
Copy link
Contributor Author

Benjamintf1 commented Apr 9, 2019

Done! Although we notice monitor_kubernetes_pods is set to true here, which I do not believe is the default.

@danielnelson
Copy link
Contributor

Thanks, I'll switch that option after we merge this. I don't see your github name in our records as having signed the CLA, could you sign it? If you already signed, sorry something didn't work, its okay to do it again.

@Benjamintf1
Copy link
Contributor Author

I believe we should be on the pivotal corporate cla. I'm double checking some stuff though as well.

@danielnelson
Copy link
Contributor

I don't see a completed CCLA for Pivotal on my side either, but even so it is also required to do the individual CLA. There is a more info on needing both here: https://www.influxdata.com/legal/ccla/

@Benjamintf1
Copy link
Contributor Author

Ok, I should have just signed the individual cla.

@danielnelson danielnelson merged commit b2baa2f into influxdata:master Apr 10, 2019
dupondje pushed a commit to dupondje/telegraf that referenced this pull request Apr 22, 2019
hwaastad pushed a commit to hwaastad/telegraf that referenced this pull request Jun 13, 2019
bitcharmer pushed a commit to bitcharmer/telegraf that referenced this pull request Oct 18, 2019
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/prometheus feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants