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

[inputs.vsphere] Resolved issue 4790 (Resource whitelisting) #5165

Merged
merged 39 commits into from
Feb 12, 2019
Merged

[inputs.vsphere] Resolved issue 4790 (Resource whitelisting) #5165

merged 39 commits into from
Feb 12, 2019

Conversation

prydin
Copy link
Contributor

@prydin prydin commented Dec 19, 2018

Required for all PRs:

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

Addresses issues #4789 and #4989 and allows users to select resources using inventory paths. Each resource type is given a *_include clause in the config file that looks like this:

vm_include = [ "*/vm/a_folder/a_subfolder/foobar*" ]

@glinton and @danielnelson This PR also has the changes provided in #5113. I'd love to get this in for 1.10 and it would probably make sense to merge this one and retire #5113 in that case.

@prydin prydin changed the title Prydin issue 4790 [inputs.vsphere] Resolved issue 4790 (Resource whitelisting) Dec 19, 2018
@danielnelson danielnelson added this to the 1.10.0 milestone Dec 20, 2018
@danielnelson
Copy link
Contributor

Let's do #5113 separately and then rebase this once it is merged.

@prydin
Copy link
Contributor Author

prydin commented Feb 4, 2019

@danielnelson @glinton Is the plan still to pull this in by 1.10? I haven't seen much movement on it.

@danielnelson
Copy link
Contributor

I'd like to include it, can you rebase?

Gopkg.lock Outdated Show resolved Hide resolved
plugins/inputs/vsphere/vsphere_test.go Outdated Show resolved Hide resolved
@@ -209,7 +290,11 @@ func TestParseConfig(t *testing.T) {
require.NotNil(t, tab)
}

<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Merge

plugins/inputs/vsphere/vsphere_test.go Outdated Show resolved Hide resolved
plugins/inputs/vsphere/README.md Show resolved Hide resolved
plugins/inputs/vsphere/panic_handler.go Outdated Show resolved Hide resolved
@prydin prydin closed this Feb 12, 2019
@prydin prydin reopened this Feb 12, 2019
@prydin
Copy link
Contributor Author

prydin commented Feb 12, 2019

Fixed the merge issues, but circleci is failing at a step where it's trying to download some package. Don't think that's related to this PR. Maybe something intermittent in the test harness?

Gopkg.lock Outdated Show resolved Hide resolved
return r.finder.FindAll(ctx, r.resType, r.paths, dst)
}

func init() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: I don't think there is a reason to use the init function, I'd just set these up top with the variable.

@danielnelson danielnelson added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin area/vsphere labels Feb 12, 2019
@danielnelson danielnelson merged commit c0bb862 into influxdata:master Feb 12, 2019
trevorwhitney pushed a commit to trevorwhitney/telegraf that referenced this pull request Feb 14, 2019
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
dupondje pushed a commit to dupondje/telegraf that referenced this pull request Apr 22, 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
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vsphere 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.

2 participants