-
Notifications
You must be signed in to change notification settings - Fork 814
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
[integration][vsphere] Add optional vm include parameter #2459
Conversation
I've tested this. I had the agent running with Note: |
@@ -633,9 +649,10 @@ def _cache_morlist_raw(self, instance): | |||
'host_include': instance.get('host_include_only_regex'), | |||
'vm_include': instance.get('vm_include_only_regex') | |||
} | |||
include_only_marked = instance.get('include_only_marked') |
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.
Should probably default to False
:
include_only_marked = instance.get('include_only_marked', False)
we should also probably also use is_affirmative()
so the user can be a little more loose in the way they specify if the flag is enabled.
Looks good other than the nitpick. |
974735a
to
2962980
Compare
@truthbk thanks for the note! I took your suggestions, and made the changes. How's it look now? |
@@ -602,6 +607,18 @@ def _cache_morlist_raw_atomic(self, i_key, obj_type, obj, tags, regexes=None): | |||
if not match: | |||
self.log.debug(u"Filtered out VM {0} because of vm_include_only_regex".format(obj.name)) | |||
return | |||
# Also, if include_only_marked is true, then check if there exists a | |||
# custom field with the value DatadogMonitored | |||
if _is_affirmative(include_only_marked): |
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.
nitpick, but feel free to ignore: I'd probably put is_affirmative()
here:
include_only_marked = _is_affrmative(instance.get('include_only_marked', False))
on line 653
👍 squash and merge as soon as CI passes! Thanks! |
This adds the ability to set a custom field from the vSphere PowerCLI which acts as a marker for which VMs you would like to have monitored by the agent. In order to add this custom field, a user can use this command:
Get-VM <MyVMName> | Set-CustomField -Name "DatadogMonitored" -Value "DatadogMonitored"
The ideal would have been to use the new tagging feature brought in vSphere 6.0 (usable also through the vSphere Web Client), but the vSphere API (at least in vCenter 5.5) does not expose these tags. There is an attribute on the managed object for VirtualMachine, called tag. Unfortunately, this is actually not the same tag set as the ones set in the vSphere Web Client (at least in vCenter 5.5). The tags set in the web client are instead stored in the Inventory Service.
Additionally, Custom Attributes could have been an option, which can be set via PowerCLI or Web Client (I think), but Custom Attributes were deprecated in exchange for Tags in vSphere 6.0.
Alas, this fix gives the ability for all. When vSphere releases another update that makes it easy to access the tags set on VirtualMachine managed objects, then we can add a feature to use tags.