Skip to content

Commit

Permalink
fixes #14447 - add cache around parser using YAML file
Browse files Browse the repository at this point in the history
A cache of parsed modules can be configured with `:parser_cache_path` to
skip calls out to kafo_parsers when loading Puppet modules. This helps
when no parser is available for the current Puppet installation (e.g.
under Puppet 4 with AIO at the time of writing), and may provide a small
peformance benefit.

The cache can be generated with `kafo-export-params -f parsercache
--no-parser-cache`.

The cache is skipped if the mtime of the manifest is greater than the
mtime when the cache was generated, causing Kafo to use kafo_parsers as
normal.
  • Loading branch information
domcleal committed Apr 5, 2016
1 parent a5317ea commit 78337c2
Show file tree
Hide file tree
Showing 14 changed files with 269 additions and 31 deletions.
16 changes: 16 additions & 0 deletions README.md
Expand Up @@ -908,6 +908,22 @@ parameter `--skip-checks-i-know-better` (or `-s`). This will completely
disable running all system check scripts. Note that this option is
not persisted between runs.
## Parser cache
A cache of parsed Puppet modules and manifests can be created to skip the use
of kafo_parsers at runtime. This is useful when kafo_parsers doesn't support the
version of Puppet in use, and may also provide a small performance benefit.
Create the cache with `kafo-export-params -f parsercache --no-parser-cache` and
configure it in config/kafo.yaml with:
```yaml
:parser_cache_path: ./parser_cache.yaml
```
The cache will be skipped if the file modification time of the manifest is
greater than the mtime recorded in the cache.
## Exit code
Kafo can terminate either before or after puppet is run. If it is run with
Expand Down
56 changes: 39 additions & 17 deletions bin/kafo-export-params
Expand Up @@ -5,8 +5,10 @@ require 'clamp'
require 'logging'
require 'kafo/configuration'
require 'kafo/exceptions'
require 'kafo/parser_cache_writer'
require 'kafo/string_helper'
require 'logger'
require 'yaml'

KafoConfigure = OpenStruct.new
def KafoConfigure.exit(code)
Expand All @@ -15,36 +17,54 @@ end

module Kafo
class KafoExportParams < Clamp::Command
TYPES = %w(md html asciidoc)
TYPES = %w(md html asciidoc parsercache)

option ['-c', '--config'], 'FILE', 'Config file for which should we generate params',
:required => true

option ['-f', '--format'], 'FORMAT',
'Config file for which should we generate params', :default => 'md' do |format|
"Format parameters will be written in, valid options: #{TYPES.join(',')}", :default => 'md' do |format|
format = format.downcase
raise ArgumentError unless TYPES.include?(format)
format
end

option ['-o', '--output'], 'FILE', 'Output file to write parameters into', :default => '-'

option '--[no-]parser-cache', :flag, 'Enable or disable the parser cache, disable for fresh results', :default => true

def execute
c = Configuration.new(config, false)
c.app[:parser_cache_path] = nil unless parser_cache?
KafoConfigure.config = c
KafoConfigure.root_dir = File.expand_path(c.app[:installer_dir])
KafoConfigure.module_dirs = File.expand_path(c.app[:module_dirs])
KafoConfigure.logger = Logger.new(STDOUT)
KafoConfigure.logger = Logger.new(STDERR)

exporter = self.class.const_get(format.capitalize).new(c)
if output == '-'
file = STDOUT
else
file = File.open(output, 'w')
end

exporter = self.class.const_get(format.capitalize).new(c, file)
exporter.print_out
end

class Html
include StringHelper

def initialize(config)
class Writer
def initialize(config, file)
@config = config
@file = file
end

def puts(*args)
@file.puts(*args)
end
end

class Html < Writer
include StringHelper

def print_out
puts '<div id="installer-options">'
puts ' <table class="table table-bordered table-condensed">'
Expand Down Expand Up @@ -77,13 +97,9 @@ module Kafo
end
end

class Asciidoc
class Asciidoc < Writer
include StringHelper

def initialize(config)
@config = config
end

def print_out
@config.modules.sort.each do |mod|
puts "Parameters for '#{mod.name}':\n\n"
Expand All @@ -97,12 +113,12 @@ module Kafo
end
end

class Md
class Md < Writer
include StringHelper

def initialize(config)
@config = config
@max = max_description_length
def initialize(*args)
super
@max = max_description_length
end

def print_out
Expand All @@ -127,6 +143,12 @@ module Kafo
doc_lengths.max
end
end

class Parsercache < Writer
def print_out
puts Kafo::ParserCacheWriter.write(@config.modules).to_yaml
end
end
end
end

Expand Down
2 changes: 2 additions & 0 deletions config/kafo.yaml.example
Expand Up @@ -16,6 +16,8 @@
# :module_dirs: /usr/share/kafo/modules
# Similar as modules_dir but for kafo internal modules, leave nil if you don't need to change it
# :kafo_modules_dir:
# Location of an optional cache of parsed module data, generate with kafo-export-params -f parsercache
# :parser_cache_path: /usr/share/kafo/parser_cache.yaml
# Enable colors? If you don't touch this, we'll autodetect terminal capabilities
# :colors: true
# Color scheme, we support :bright and :dark (first is better for white background, dark for black background)
Expand Down
9 changes: 8 additions & 1 deletion lib/kafo/configuration.rb
Expand Up @@ -27,7 +27,8 @@ class Configuration
:hook_dirs => [],
:custom => {},
:low_priority_modules => [],
:verbose_log_level => 'info'
:verbose_log_level => 'info',
:parser_cache_path => './config/parser_cache.json'
}

def initialize(file, persist = true)
Expand Down Expand Up @@ -257,6 +258,12 @@ def migrations_dir
@config_file.gsub(/\.yaml$/, '.migrations')
end

def parser_cache
if app[:parser_cache_path]
@parser_cache ||= Kafo::ParserCacheReader.new_from_file(File.expand_path(app[:parser_cache_path]))
end
end

private

def custom_storage
Expand Down
2 changes: 1 addition & 1 deletion lib/kafo/param_builder.rb
Expand Up @@ -79,7 +79,7 @@ def find_or_build_group(name)
end

def get_type(type)
type = type.capitalize
type = (type || 'string').capitalize
Params.const_defined?(type) ? Params.const_get(type) : raise(TypeError, "undefined parameter type '#{type}'")
end
end
Expand Down
46 changes: 46 additions & 0 deletions lib/kafo/parser_cache_reader.rb
@@ -0,0 +1,46 @@
module Kafo
class ParserCacheReader
def self.new_from_file(cache_path)
if cache_path.nil? || cache_path.empty?
logger.debug "No parser cache configured in :parser_cache_path, skipping setup"
return nil
end

unless File.exist?(cache_path)
logger.warn "Parser cache configured at #{cache_path} is missing, skipping setup"
return nil
end

parsed = YAML.load(File.read(cache_path))
if !parsed.is_a?(Hash) || parsed[:version] != 1 || !parsed[:files].is_a?(Hash)
logger.warn "Parser cache is from a different version of Kafo, skipping setup"
return nil
end

new(parsed)
end

def self.logger
KafoConfigure.logger
end

def initialize(cache)
@cache = cache
end

def logger
KafoConfigure.logger
end

def get(key, manifest_path)
return nil unless @cache[:files].has_key?(key)

if @cache[:files][key][:mtime] && File.mtime(manifest_path).to_i > @cache[:files][key][:mtime]
logger.debug "Parser cache for #{manifest_path} is outdated, ignoring cache entry"
return nil
end

@cache[:files][key][:data]
end
end
end
14 changes: 14 additions & 0 deletions lib/kafo/parser_cache_writer.rb
@@ -0,0 +1,14 @@
module Kafo
class ParserCacheWriter
def self.write(modules)
{
:version => 1,
:files => Hash[modules.sort.map { |m| write_module(m) }]
}
end

def self.write_module(mod)
[mod.identifier, {:data => mod.raw_data, :mtime => File.mtime(mod.manifest_path).to_i}]
end
end
end
11 changes: 7 additions & 4 deletions lib/kafo/puppet_module.rb
@@ -1,6 +1,7 @@
# encoding: UTF-8
require 'kafo/param'
require 'kafo/param_builder'
require 'kafo/parser_cache_reader'
require 'kafo_parsers/puppet_module_parser'
require 'kafo/validator'

Expand All @@ -9,7 +10,7 @@ class PuppetModule
PRIMARY_GROUP_NAME = 'Parameters'

attr_reader :name, :identifier, :params, :dir_name, :class_name, :manifest_name, :manifest_path,
:groups, :params_path, :params_class_name, :configuration
:groups, :params_path, :params_class_name, :configuration, :raw_data

def initialize(identifier, parser = KafoParsers::PuppetModuleParser, configuration = KafoConfigure.config)
@identifier = identifier
Expand All @@ -27,6 +28,7 @@ def initialize(identifier, parser = KafoParsers::PuppetModuleParser, configurati
end
@manifest_path = File.join(module_dir, module_manifest_path)
@parser = parser
@parser_cache = @configuration.parser_cache
@validations = []
@logger = KafoConfigure.logger
@groups = {}
Expand All @@ -48,9 +50,10 @@ def enable

def parse(builder_klass = ParamBuilder)
@params = []
raw_data = @parser.parse(manifest_path)
builder = builder_klass.new(self, raw_data)
@validations = raw_data[:validations]
@raw_data = @parser_cache.get(identifier, manifest_path) if @parser_cache
@raw_data ||= @parser.parse(manifest_path)
builder = builder_klass.new(self, @raw_data)
@validations = @raw_data[:validations]

builder.validate
@params = builder.build_params
Expand Down
58 changes: 58 additions & 0 deletions test/kafo/parser_cache_reader_test.rb
@@ -0,0 +1,58 @@
require 'test_helper'
require 'kafo/parser_cache_writer'

module Kafo
describe ParserCacheReader do
subject { ParserCacheReader }

describe ".new_from_file" do
let(:valid_cache) do
{:version => 1, :files => {}}
end

specify { subject.new_from_file(nil).must_be_nil }
specify { subject.new_from_file('').must_be_nil }
specify { subject.new_from_file('/non/existent/file').must_be_nil }

describe "with empty file" do
let(:cache) { ParserCacheFactory.build('') }
specify { subject.new_from_file(cache.path).must_be_nil }
end

describe "with version other than 1" do
let(:cache) { ParserCacheFactory.build(valid_cache.update(:version => 2)) }
specify { subject.new_from_file(cache.path).must_be_nil }
end

describe "with missing files section" do
let(:cache) { ParserCacheFactory.build(valid_cache.reject { |k,v| k == :files }) }
specify { subject.new_from_file(cache.path).must_be_nil }
end

describe "with correct format" do
let(:cache) { ParserCacheFactory.build(valid_cache) }
specify { subject.new_from_file(cache.path).must_be_instance_of subject }
end
end

describe "#get" do
specify { subject.new({:files => {}}).get('test', '/test/file.pp').must_be_nil }
specify { subject.new({:files => {'test' => {:data => {:parameters => []}}}}).get('test', '/test/file.pp').must_equal(:parameters => []) }
specify { File.stub(:mtime, 1) { subject.new({:files => {'test' => {:mtime => 1, :data => :test}}}).get('test', '/test/file.pp').must_equal(:test) } }
specify { File.stub(:mtime, 2) { subject.new({:files => {'test' => {:mtime => 1, :data => :test}}}).get('test', '/test/file.pp').must_be_nil } }
end

describe "compatibility with writer" do
before do
KafoConfigure.config = Configuration.new(ConfigFileFactory.build('basic', BASIC_CONFIGURATION).path)
end

let(:parser) { TestParser.new(BASIC_MANIFEST) }
let(:mod) { PuppetModule.new('puppet', parser) }
let(:writer) { ParserCacheWriter.write([mod]) }
let(:cache) { ParserCacheFactory.build(writer) }

specify { File.stub(:mtime, 1) { subject.new_from_file(cache.path).get('puppet', parser.manifest_file).must_equal mod.raw_data } }
end
end
end
32 changes: 32 additions & 0 deletions test/kafo/parser_cache_writer_test.rb
@@ -0,0 +1,32 @@
require 'test_helper'
require 'kafo/parser_cache_writer'
require 'tempfile'

module Kafo
describe ParserCacheWriter do
subject { ParserCacheWriter }

describe ".write" do
specify { subject.write([])[:version].must_equal 1 }
specify { subject.write([])[:files].must_equal({}) }

describe "with a module" do
let(:manifest) { Tempfile.new("#{subject}_manifest") }
let(:mod) do
mod = MiniTest::Mock.new
mod.expect(:identifier, 'module')
mod.expect(:manifest_path, manifest.path)
mod.expect(:raw_data, {:parameters => [], :groups => []})
mod
end
let(:output) { subject.write([mod]) }

specify { output[:files].keys.must_equal ['module'] }
specify { output[:files]['module'].keys.sort.must_equal [:data, :mtime] }
specify { output[:files]['module'][:mtime].must_equal File.mtime(manifest.path).to_i }
specify { output[:files]['module'][:data].has_key?(:parameters).must_equal true }
specify { output[:files]['module'][:data][:parameters].must_equal([]) }
end
end
end
end

0 comments on commit 78337c2

Please sign in to comment.