From a3b324dba249f5db38a4ff71b9bb5bc0f7d68147 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Thu, 29 Aug 2013 12:56:10 -0400 Subject: [PATCH] Inline a custom file_line provider to fix agent. Fixes an issue in the ceilometer::agent::compute manifest where nova.conf config values for the notification_driver would get added to the wrong section (always to the end of the file). As part of the fix a custom file_line 'after' provider which supports a new after option has been added. This allows us to have some control over which section *new* file lines go into. If there are any pre-existing matching lines in the file the assumption is that they are already in the correct section and can be edited in place. NOTE: I've submitted a pull request to the upstream stdlib repo here to add the new 'after' option: https://github.com/puppetlabs/puppetlabs-stdlib/pull/174 Once this (or something better) lands in stdlib we can update puppet-ceilometer to use it. Fixes LP Bug #1217867 Change-Id: Ic09f5232b322cde687d663d1ef38ef0fd12f32ff --- lib/puppet/provider/file_line_after/ruby.rb | 83 +++++++++++++++++++ lib/puppet/type/file_line_after.rb | 79 ++++++++++++++++++ manifests/agent/compute.pp | 9 +- spec/classes/ceilometer_agent_compute_spec.rb | 4 +- 4 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 lib/puppet/provider/file_line_after/ruby.rb create mode 100644 lib/puppet/type/file_line_after.rb diff --git a/lib/puppet/provider/file_line_after/ruby.rb b/lib/puppet/provider/file_line_after/ruby.rb new file mode 100644 index 00000000..0d7f287c --- /dev/null +++ b/lib/puppet/provider/file_line_after/ruby.rb @@ -0,0 +1,83 @@ +Puppet::Type.type(:file_line_after).provide(:ruby) do + def exists? + lines.find do |line| + line.chomp == resource[:line].chomp + end + end + + def create + if resource[:match] + handle_create_with_match + elsif resource[:after] + handle_create_with_after + else + append_line + end + end + + def destroy + local_lines = lines + File.open(resource[:path],'w') do |fh| + fh.write(local_lines.reject{|l| l.chomp == resource[:line] }.join('')) + end + end + + private + def lines + # If this type is ever used with very large files, we should + # write this in a different way, using a temp + # file; for now assuming that this type is only used on + # small-ish config files that can fit into memory without + # too much trouble. + @lines ||= File.readlines(resource[:path]) + end + + def handle_create_with_match() + regex = resource[:match] ? Regexp.new(resource[:match]) : nil + match_count = lines.select { |l| regex.match(l) }.size + if match_count > 1 && resource[:multiple].to_s != 'true' + raise Puppet::Error, "More than one line in file '#{resource[:path]}' matches pattern '#{resource[:match]}'" + end + File.open(resource[:path], 'w') do |fh| + lines.each do |l| + fh.puts(regex.match(l) ? resource[:line] : l) + end + + if (match_count == 0) + fh.puts(resource[:line]) + end + end + end + + def handle_create_with_after + regex = Regexp.new(resource[:after]) + + count = lines.count {|l| l.match(regex)} + + case count + when 1 # find the line to put our line after + File.open(resource[:path], 'w') do |fh| + lines.each do |l| + fh.puts(l) + if regex.match(l) then + fh.puts(resource[:line]) + end + end + end + when 0 # append the line to the end of the file + append_line + else + raise Puppet::Error, "#{count} lines match pattern '#{resource[:after]}' in file '#{resource[:path]}'. One or no line must match the pattern." + end + end + + ## + # append the line to the file. + # + # @api private + def append_line + File.open(resource[:path], 'a') do |fh| + fh.puts resource[:line] + end + end +end diff --git a/lib/puppet/type/file_line_after.rb b/lib/puppet/type/file_line_after.rb new file mode 100644 index 00000000..d71d7c29 --- /dev/null +++ b/lib/puppet/type/file_line_after.rb @@ -0,0 +1,79 @@ +Puppet::Type.newtype(:file_line_after) do + + desc <<-EOT + Ensures that a given line is contained within a file. The implementation + matches the full line, including whitespace at the beginning and end. If + the line is not contained in the given file, Puppet will add the line to + ensure the desired state. Multiple resources may be declared to manage + multiple lines in the same file. + + Example: + + file_line_after { 'sudo_rule': + path => '/etc/sudoers', + line => '%sudo ALL=(ALL) ALL', + } + file_line_after { 'sudo_rule_nopw': + path => '/etc/sudoers', + line => '%sudonopw ALL=(ALL) NOPASSWD: ALL', + } + + In this example, Puppet will ensure both of the specified lines are + contained in the file /etc/sudoers. + + EOT + + ensurable do + defaultvalues + defaultto :present + end + + newparam(:name, :namevar => true) do + desc 'An arbitrary name used as the identity of the resource.' + end + + newparam(:match) do + desc 'An optional regular expression to run against existing lines in the file;\n' + + 'if a match is found, we replace that line rather than adding a new line.' + end + + newparam(:multiple) do + desc 'An optional value to determine if match can change multiple lines.' + newvalues(true, false) + end + + newparam(:after) do + desc 'An optional value used to specify the line after which we will add any new lines. (Existing lines are added in place)' + end + + newparam(:line) do + desc 'The line to be appended to the file located by the path parameter.' + end + + newparam(:path) do + desc 'The file Puppet will ensure contains the line specified by the line parameter.' + validate do |value| + unless (Puppet.features.posix? and value =~ /^\//) or (Puppet.features.microsoft_windows? and (value =~ /^.:\// or value =~ /^\/\/[^\/]+\/[^\/]+/)) + raise(Puppet::Error, "File paths must be fully qualified, not '#{value}'") + end + end + end + + # Autorequire the file resource if it's being managed + autorequire(:file) do + self[:path] + end + + validate do + unless self[:line] and self[:path] + raise(Puppet::Error, "Both line and path are required attributes") + end + + if (self[:match]) + unless Regexp.new(self[:match]).match(self[:line]) + raise(Puppet::Error, "When providing a 'match' parameter, the value must be a regex that matches against the value of your 'line' parameter") + end + end + + end +end diff --git a/manifests/agent/compute.pp b/manifests/agent/compute.pp index 9ce4dace..e5cc3327 100644 --- a/manifests/agent/compute.pp +++ b/manifests/agent/compute.pp @@ -102,22 +102,27 @@ class ceilometer::agent::compute ( 'DEFAULT/instance_usage_audit_period' : value => 'hour'; } + #NOTE(dprince): This is using a custom (inline) file_line provider + # until this lands upstream: + # https://github.com/puppetlabs/puppetlabs-stdlib/pull/174 Nova_config<| |> { - before +> File_line[ + before +> File_line_after[ 'nova-notification-driver-common', 'nova-notification-driver-ceilometer' ], } - file_line { + file_line_after { 'nova-notification-driver-common': line => 'notification_driver=nova.openstack.common.notifier.rpc_notifier', path => '/etc/nova/nova.conf', + after => '\[DEFAULT\]', notify => Service['nova-compute']; 'nova-notification-driver-ceilometer': line => 'notification_driver=ceilometer.compute.nova_notifier', path => '/etc/nova/nova.conf', + after => '\[DEFAULT\]', notify => Service['nova-compute']; } diff --git a/spec/classes/ceilometer_agent_compute_spec.rb b/spec/classes/ceilometer_agent_compute_spec.rb index c09b4798..a6092fab 100644 --- a/spec/classes/ceilometer_agent_compute_spec.rb +++ b/spec/classes/ceilometer_agent_compute_spec.rb @@ -74,12 +74,12 @@ describe 'ceilometer::agent::compute' do end it 'configures nova notification driver' do - should contain_file_line('nova-notification-driver-common').with( + should contain_file_line_after('nova-notification-driver-common').with( :line => 'notification_driver=nova.openstack.common.notifier.rpc_notifier', :path => '/etc/nova/nova.conf', :notify => 'Service[nova-compute]' ) - should contain_file_line('nova-notification-driver-ceilometer').with( + should contain_file_line_after('nova-notification-driver-ceilometer').with( :line => 'notification_driver=ceilometer.compute.nova_notifier', :path => '/etc/nova/nova.conf', :notify => 'Service[nova-compute]'