Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes #14447 - add cache around parser using serialised YAML file #135

Merged
merged 1 commit into from Apr 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 = c.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
47 changes: 47 additions & 0 deletions lib/kafo/parser_cache_reader.rb
@@ -0,0 +1,47 @@
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

logger.debug "Using #{cache_path} cache with parsed modules"
new(parsed)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you mind adding logger.debug "Using #{cache_path} cache with parsed modules" in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, added.

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