vmware-archive / vmware-vcenter

VMware vCenter Module
Other
87 stars 102 forks source link

Provide a more centralized way of requiring vmware_lib #177

Closed crayfishx closed 8 years ago

crayfishx commented 8 years ago

Summary

Classes provided by vmware_lib are shipped in a separate module, any library in this module that needs to use resources from vmware_lib do so by invoking a call on Puppet::Module.find to try and determine where the module is in order to require it.

Problem

This becomes a problem for things like rspec, where you are not inside an instance of Puppet and therefore Puppet::Module.find fails since there is no environment to look up against. While there is no standard/reliable way of looking up libraries from other modules (the practice is actually discouraged by Puppet) I think using Puppet::Module.find should be a fall back if the library does not exist in Ruby's load path. It's also more optimal to just require the class from the load path than to call Puppet::Module.find. This approach was already taken lib/puppet/provider/vcenter.rb, but in some types and providers this fall back approach was not used and calls to Puppet::Module.find were always done, which breaks rspec, amoungst other things:

begin
  require 'puppet_x/puppetlabs/transport'
rescue LoadError => e
  require 'pathname' # WORK_AROUND #14073 and #7788
  vmware_module = Puppet::Module.find('vmware_lib', Puppet[:environment].to_s)
  require File.join vmware_module.path, 'lib/puppet_x/puppetlabs/transport'
end

Implementation

Rather than go about putting this logic in 20 or more places in the code base, I've added what might be called pseudo wrapper libraries under lib/puppet_x/vmware/vmware_lib - these provide a central place to determine how libraries are loaded outside of the module. This means within the code base we require vmware_lib by first requiring a local library where we always know the relative path, eg:

require File.join module_lib, 'puppet_x/vmware/vmware_lib/puppet_x/puppetlabs/transport'

This in turn has the logic to try to load puppet_x/puppetlabs/transport from the load path, and fall back on Puppet::Module.find if that fails. This also means if vmware_lib has any namespace changes...etc we only have one central place to manage how it gets loaded.

crayfishx commented 8 years ago

@maniacmurphy I think I've covered all your points in the last three commits - it would be nice to get this PR and #178 merged