Please review pull request #123: allow extra environment vars for puppet commands opened by (cprice-puppet)
Description:
When working on some tickets relating to setting system locales (Facter #12012, Puppet #11860), I came across a situation where I needed to be able to set some environment variables before running puppet commands in my acceptance tests. This changeset adds the ability to pass in an optional ":environment" hash when calling "apply_manifest_on"; the key/value pairs in this hash will be set as environment variables for the duration of the execution of the "puppet apply" operation.
This implementation isn't ideal, but the implementation details are not very visible in the actual acceptance test syntax. We might want to re-work the implementation later, but for now this was the only reasonably easy way to pull it off.
Also--this logic could very easily be extended to work for other puppet commands besides "apply", but hasn't at this point in time.
- Opened: Wed Jan 25 22:58:52 UTC 2012
- Based on: puppetlabs:master (31e61a1c8c4639a5a8ebebe19985199f19c45744)
- Requested merge: cprice-puppet:allow_extra_env_vars_to_puppet_commands (6c02f8c3b9c2dd10a5f605995887b0502c00e18b)
Diff follows:
diff --git a/lib/command.rb b/lib/command.rb
index 4ebd321..247c234 100644
--- a/lib/command.rb
+++ b/lib/command.rb
@@ -16,12 +16,24 @@ def exec(host, options={})
end
# Determine the appropriate puppet env command for the given host.
- def puppet_env_command(host_info)
+ # parameters:
+ # [host_info] a Hash containing info about the host
+ # [environment] an optional Hash containing key-value pairs to be treated as environment variables that should be
+ # set for the duration of the puppet command.
+ def puppet_env_command(host_info, environment = {})
rubylib = [host_info['pluginlibpath'], host_info['puppetlibdir'], host_info['facterlibdir'],'$RUBYLIB'].compact.join(':')
path = [host_info['puppetbindir'], host_info['facterbindir'],'$PATH' ].compact.join(':')
cmd = host_info['platform'] =~ /windows/ ? 'cmd.exe /c' : ''
- %Q{env RUBYLIB="#{rubylib}" PATH="#{path}" #{cmd}}
+ # if the caller passed in an "environment" hash, we need to build up a string of the form " KEY1=VAL1 KEY2=VAL2"
+ # containing all of the specified environment vars. We prefix it with a space because we will insert it into
+ # the broader command below
+ environment_vars_string = environment.nil? ? "" :
+ " %s" % environment.collect { |key, val| "#{key}=#{val}" } .join(" ")
+
+ # build up the actual command, prefixed with the RUBYLIB and PATH environment vars, plus our string containing
+ # additional environment variables (which may be an empty string)
+ %Q{env RUBYLIB="#{rubylib}" PATH="#{path}"#{environment_vars_string} #{cmd}}
end
end
@@ -29,6 +41,17 @@ class PuppetCommand < Command
def initialize(sub_command, *args)
@sub_command = sub_command
@options = args.last.is_a?(Hash) ? args.pop : {}
+
+ # Not at all happy with this implementation, but it was the path of least resistance for the moment.
+ # This constructor already does a little "magic" by allowing the final value in the *args Array to
+ # be a Hash, which will be treated specially: popped from the regular args list and assigned to @options,
+ # where it will later be used to add extra command line args to the puppet command (--key=value).
+ #
+ # Here we take this one step further--if the @options hash is passed in, we check and see if it has the
+ # special key :environment in it. If it does, then we'll pull that out of the options hash and treat
+ # it specially too; we'll use it to set additional environment variables for the duration of the puppet
+ # command.
+ @environment = @options.has_key?(:environment) ? @options.delete(:environment) : nil
# Dom: commenting these lines addressed bug #6920
# @options[:vardir] ||= '/tmp'
# @options[:confdir] ||= '/tmp'
@@ -40,7 +63,7 @@ def cmd_line(host_info)
puppet_path = host_info[:puppetbinpath] || "/bin/puppet" # TODO: is this right?
args_string = (@args + @options.map { |key, value| "--#{key}=#{value}" }).join(' ')
- "#{puppet_env_command(host_info)} puppet #{@sub_command} #{args_string}"
+ "#{puppet_env_command(host_info, @environment)} puppet #{@sub_command} #{args_string}"
end
end
diff --git a/lib/test_case.rb b/lib/test_case.rb
index 42d4d32..83ce60f 100644
--- a/lib/test_case.rb
+++ b/lib/test_case.rb
@@ -200,11 +200,33 @@ def host_command(command_string)
HostCommand.new(command_string)
end
+ # method apply_manifest_on
+ # runs a 'puppet apply' command on a remote host
+ # parameters:
+ # [host] an instance of Host which contains the info about the host that this command should be run on
+ # [manifest] a string containing a puppet manifest to apply
+ # [options] an optional hash containing options; legal values include:
+ # :acceptable_exit_codes => an array of integer exit codes that should be considered acceptable. an error will be
+ # thrown if the exit code does not match one of the values in this list.
+ # :parseonly => any value. If this key exists in the Hash, the "--parseonly" command line parameter will be
+ # passed to the 'puppet apply' command.
+ # :environment => a Hash containing string->string key value pairs. These will be treated as extra environment
+ # variables that should be set before running the puppet command.
+ # [&block] this method will yield to a block of code passed by the caller; this can be used for additional validation,
+ # etc.
def apply_manifest_on(host,manifest,options={},&block)
_on_options_ = {:stdin => manifest + "\n"}
on_options[:acceptable_exit_codes] = options.delete(:acceptable_exit_codes) if options.keys.include?(:acceptable_exit_codes)
args = ["--verbose"]
args << "--parseonly" if options[:parseonly]
+
+ # Not really thrilled with this implementation, might want to improve it later. Basically, there is a magic
+ # trick in the constructor of PuppetCommand which allows you to pass in a Hash for the last value in the *args
+ # Array; if you do so, it will be treated specially. So, here we check to see if our caller passed us a hash
+ # of environment variables that they want to set for the puppet command. If so, we set the final value of
+ # *args to a new hash with just one entry (the value of which is our environment variables hash)
+ args << { :environment => options[:environment]} if options.has_key?(:environment)
+
on host, puppet_apply(*args), on_options, &block
end
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
