craigwatson / puppet-vmwaretools

Puppet module for non-OSP VMware Tools Installation
http://forge.puppetlabs.com/CraigWatson1987/vmwaretools
Apache License 2.0
27 stars 40 forks source link

checking for virtual vs. physical #15

Closed snobear closed 11 years ago

snobear commented 11 years ago

Thanks for the module, it works nicely for me. One issue though: I use Foreman as an ENC, and would like to apply the vmwaretools class to an entire hostgroup. I was hoping this module would check and see if this was a VMware VM first, since I have a mix of physical and virtual servers.

One solution is for me to add this to my gobal site.pp:

if($is_virtual) {
    include vmwaretools
}

but I prefer to manage the assignment of classes to hosts via Foreman as much as possible. Would it make sense to wrap the main vmwaretools class in that if statement to be safe? And also probably check that its a VMware VM, in case others have a mix of virtual machine types in their environment. Thoughts? I can do this and make a pull request, but would like your advice on that. Thanks.

craigwatson commented 11 years ago

Thanks for the suggestion, unfortunately I only have an iPad for the next 10 days so I'm unable to make the modification - though please by all means submit a pull request and I'll merge when I can :+1:

My suggestion would be to enclose lines 107-110 in manifests/init.pp in an if statement. :)

Aethylred commented 11 years ago

It's already possible:

if $::virtual == 'vmware' {
  include vmwaretools
  # or class{'vmwaretools': <parameters> }
}

$::virtual is a fact provided by facter

You don't want it installed if it's another kind of virtual, like QEMU, Virtualbox or Xen.

snobear commented 11 years ago

@Aethylred adding that chunk of code to my main site.pp is probably the best bet. I use Foreman, and was thinking it'd be nice to apply the class to my top level hostgroup. That means all hosts would have the vmwaretools module applied to them, but only the hosts that are vmware VMs would actually do anything with it. This method is probably inefficient; no sense applying resources to hosts that don't need it.

I'll go with @Aethylred's suggestion. Feel free to open this back up @craigwatson if you think its worth adding.