Commit b5b8a843 authored by Chris Price's avatar Chris Price

Refactor: create Config class

This commit breaks all of the configuration-related logic out of
the 'util/puppetdb' class and puts it in a new class called `Config`.
This is a better separation of concerns and makes the test files
for the two classes a little easier to read and maintain.
parent bc42e777
require 'puppet/util/puppetdb'
require 'puppet/util/puppetdb/command_names'
Puppet::Face.define(:node, '0.0.1') do
CommandDeactivateNode = Puppet::Util::Puppetdb::CommandDeactivateNode
CommandDeactivateNode = Puppet::Util::Puppetdb::CommandNames::CommandDeactivateNode
action :deactivate do
summary "Deactivate a set of nodes in PuppetDB"
......
......@@ -4,6 +4,7 @@ require 'puppet/util/puppetdb'
class Puppet::Resource::Catalog::Puppetdb < Puppet::Indirector::REST
include Puppet::Util::Puppetdb
include Puppet::Util::Puppetdb::CommandNames
def save(request)
catalog = munge_catalog(request.instance)
......
......@@ -4,6 +4,7 @@ require 'puppet/util/puppetdb'
class Puppet::Node::Facts::Puppetdb < Puppet::Indirector::REST
include Puppet::Util::Puppetdb
include Puppet::Util::Puppetdb::CommandNames
def save(request)
facts = request.instance.dup
......
require 'puppet'
require 'puppet/util/puppetdb'
require 'puppet/util/puppetdb/command_names'
require 'puppet/util/puppetdb/report_helper'
Puppet::Reports.register_report(:puppetdb) do
CommandStoreReport = Puppet::Util::Puppetdb::CommandStoreReport
CommandStoreReport = Puppet::Util::Puppetdb::CommandNames::CommandStoreReport
desc <<-DESC
Send report information to PuppetDB via the REST API. Reports are serialized to
......
......@@ -2,6 +2,7 @@ require 'puppet/util'
require 'puppet/util/logging'
require 'puppet/util/puppetdb/command_names'
require 'puppet/util/puppetdb/command'
require 'puppet/util/puppetdb/config'
require 'digest/sha1'
require 'time'
require 'fileutils'
......@@ -16,15 +17,6 @@ require 'fileutils'
# below, and see also `Puppet::Util::Puppetdb::ReportHelper` for an example of
# how to work around this limitation for the time being.
module Puppet::Util::Puppetdb
include CommandNames
# a map for looking up config file section names that correspond to our
# individual commands
CommandConfigSectionNames = {
CommandReplaceCatalog => :catalogs,
CommandReplaceFacts => :facts,
CommandStoreReport => :reports,
}
## HACK: the existing `http_*` methods and the
# `Puppet::Util::PuppetDb#submit_command` expect their first argument to
......@@ -46,7 +38,7 @@ module Puppet::Util::Puppetdb
end
def self.config
@config ||= load_puppetdb_config
@config ||= Puppet::Util::Puppetdb::Config.load
@config
end
......@@ -81,7 +73,7 @@ module Puppet::Util::Puppetdb
def submit_command(certname, payload, command_name, version)
command = Puppet::Util::Puppetdb::Command.new(command_name, version, certname, payload)
spool = command_spooled?(command_name)
spool = config.command_spooled?(command_name)
if (spool)
command.enqueue
......@@ -94,69 +86,6 @@ module Puppet::Util::Puppetdb
private
# Private class methods
def self.load_puppetdb_config
default_server = "puppetdb"
default_port = 8081
default_spool_settings = {
CommandReplaceCatalog => false,
CommandReplaceFacts => false,
CommandStoreReport => true,
}
config_file = File.join(Puppet[:confdir], "puppetdb.conf")
if File.exists?(config_file)
Puppet.debug("Configuring PuppetDB terminuses with config file #{config_file}")
content = File.read(config_file)
else
Puppet.debug("No puppetdb.conf file found; falling back to default #{default_server}:#{default_port}")
content = ''
end
result = {}
section = nil
content.lines.each_with_index do |line,number|
# Gotta track the line numbers properly
number += 1
case line
when /^\[(\w+)\s*\]$/
section = $1
result[section] ||= {}
when /^\s*(\w+)\s*=\s*(\S+)\s*$/
raise "Setting '#{line}' is illegal outside of section in PuppetDB config #{config_file}:#{number}" unless section
result[section][$1] = $2
when /^\s*[#;]/
# Skip comments
when /^\s*$/
# Skip blank lines
else
raise "Unparseable line '#{line}' in PuppetDB config #{config_file}:#{number}"
end
end
config_hash = {}
main_section = result['main'] || {}
config_hash[:server] = (main_section['server'] || default_server).strip
config_hash[:port] = (main_section['port'] || default_port).to_i
[CommandReplaceCatalog, CommandReplaceFacts, CommandStoreReport].each do |c|
config_hash[c] = {}
command_section = result[CommandConfigSectionNames[c].to_s]
config_hash[c][:spool] = (command_section && command_section.has_key?('spool')) ?
(command_section['spool'] == "true") :
default_spool_settings[c]
end
config_hash
rescue => detail
puts detail.backtrace if Puppet[:trace]
Puppet.warning "Could not configure PuppetDB terminuses: #{detail}"
raise
end
## Private instance methods
def config
......@@ -164,11 +93,6 @@ module Puppet::Util::Puppetdb
end
def command_spooled?(command_name)
config.has_key?(command_name) ? config[command_name][:spool] : false
end
def flush_commands
######################################
# TODO: IMPLEMENT A MAX FAILURE COUNT
......
require 'puppet/util/puppetdb/command_names'
module Puppet::Util::Puppetdb
class Config
include Puppet::Util::Puppetdb::CommandNames
# a map for looking up config file section names that correspond to our
# individual commands
CommandConfigSectionNames = {
CommandReplaceCatalog => :catalogs,
CommandReplaceFacts => :facts,
CommandStoreReport => :reports,
}
# Public class methods
def self.load(config_file = nil)
default_server = "puppetdb"
default_port = 8081
default_spool_settings = {
CommandReplaceCatalog => false,
CommandReplaceFacts => false,
CommandStoreReport => true,
}
config_file ||= File.join(Puppet[:confdir], "puppetdb.conf")
if File.exists?(config_file)
Puppet.debug("Configuring PuppetDB terminuses with config file #{config_file}")
content = File.read(config_file)
else
Puppet.debug("No puppetdb.conf file found; falling back to default #{default_server}:#{default_port}")
content = ''
end
result = {}
section = nil
content.lines.each_with_index do |line,number|
# Gotta track the line numbers properly
number += 1
case line
when /^\[(\w+)\s*\]$/
section = $1
result[section] ||= {}
when /^\s*(\w+)\s*=\s*(\S+)\s*$/
raise "Setting '#{line}' is illegal outside of section in PuppetDB config #{config_file}:#{number}" unless section
result[section][$1] = $2
when /^\s*[#;]/
# Skip comments
when /^\s*$/
# Skip blank lines
else
raise "Unparseable line '#{line}' in PuppetDB config #{config_file}:#{number}"
end
end
config_hash = {}
main_section = result['main'] || {}
config_hash[:server] = (main_section['server'] || default_server).strip
config_hash[:port] = (main_section['port'] || default_port).to_i
[CommandReplaceCatalog, CommandReplaceFacts, CommandStoreReport].each do |c|
config_hash[c] = {}
command_section = result[CommandConfigSectionNames[c].to_s]
config_hash[c][:spool] = (command_section && command_section.has_key?('spool')) ?
(command_section['spool'] == "true") :
default_spool_settings[c]
end
self.new(config_hash)
rescue => detail
puts detail.backtrace if Puppet[:trace]
Puppet.warning "Could not configure PuppetDB terminuses: #{detail}"
raise
end
# Public instance methods
def initialize(config_hash = {})
@config = config_hash
end
def command_spooled?(command_name)
config.has_key?(command_name) ? config[command_name][:spool] : false
end
def [](property)
# TODO: ultimately it might be nice to define a more rigid API, rather than
# allowing access to the internals of the config hash.
config[property]
end
# Private instance methods
private
attr_reader :config
end
end
......@@ -10,6 +10,7 @@ require 'tmpdir'
require 'fileutils'
require 'puppet'
require 'puppet/util/log'
require 'puppet/util/puppetdb/command'
RSpec.configure do |config|
......
......@@ -3,6 +3,7 @@
require 'spec_helper'
require 'puppet/indirector/catalog/puppetdb'
require 'puppet/util/puppetdb/command_names'
describe Puppet::Resource::Catalog::Puppetdb do
before :each do
......@@ -29,7 +30,7 @@ describe Puppet::Resource::Catalog::Puppetdb do
it "should POST the catalog command as a URL-encoded PSON string" do
command_payload = subject.munge_catalog(catalog)
payload = {
:command => Puppet::Util::Puppetdb::CommandReplaceCatalog,
:command => Puppet::Util::Puppetdb::CommandNames::CommandReplaceCatalog,
:version => 2,
:payload => command_payload,
}.to_pson
......
......@@ -2,10 +2,11 @@
require 'spec_helper'
require 'puppet/indirector/facts/puppetdb'
require 'puppet/util/puppetdb/command_names'
describe Puppet::Node::Facts::Puppetdb do
CommandReplaceFacts = Puppet::Util::Puppetdb::CommandReplaceFacts
CommandReplaceFacts = Puppet::Util::Puppetdb::CommandNames::CommandReplaceFacts
before :each do
Puppet::Util::Puppetdb.stubs(:server).returns 'localhost'
......
......@@ -3,10 +3,11 @@
require 'spec_helper'
require 'puppet/indirector/node/puppetdb'
require 'puppet/util/puppetdb/command_names'
describe Puppet::Node::Puppetdb do
CommandDeactivateNode = Puppet::Util::Puppetdb::CommandDeactivateNode
CommandDeactivateNode = Puppet::Util::Puppetdb::CommandNames::CommandDeactivateNode
before :each do
Puppet::Node.indirection.stubs(:terminus).returns(subject)
......
......@@ -4,6 +4,7 @@ require 'spec_helper'
require 'puppet/reports'
require 'net/http'
require 'puppet/network/http_pool'
require 'puppet/util/puppetdb/command_names'
processor = Puppet::Reports.report(:puppetdb)
......@@ -25,7 +26,7 @@ describe processor do
subject.stubs(:run_duration).returns(10)
payload = {
:command => Puppet::Util::Puppetdb::CommandStoreReport,
:command => Puppet::Util::Puppetdb::CommandNames::CommandStoreReport,
:version => 1,
:payload => subject.send(:report_to_hash)
}.to_pson
......
require 'spec_helper'
require 'puppet/util/puppetdb/config'
require 'puppet/util/puppetdb/command_names'
# Create a local copy of these constants so that we don't have to refer to them
# by their full namespaced name
CommandReplaceCatalog = Puppet::Util::Puppetdb::CommandNames::CommandReplaceCatalog
CommandReplaceFacts = Puppet::Util::Puppetdb::CommandNames::CommandReplaceFacts
CommandStoreReport = Puppet::Util::Puppetdb::CommandNames::CommandStoreReport
describe Puppet::Util::Puppetdb::Config do
describe "#load" do
let(:confdir) do
temp = Tempfile.new('confdir')
path = temp.path
temp.close!
Dir.mkdir(path)
path
end
before :each do
Puppet[:confdir] = confdir
end
after :each do
FileUtils.rm_rf(confdir)
end
describe "with no config file" do
it "should use the default server and port" do
config = described_class.load
config[:server].should == 'puppetdb'
config[:port].should == 8081
end
it "should use the default settings for command spooling" do
config = described_class.load
config[CommandReplaceCatalog][:spool].should == false
config[CommandReplaceFacts][:spool].should == false
config[CommandStoreReport][:spool].should == true
end
end
describe "with a config file" do
def write_config(content)
conf = File.join(confdir, 'puppetdb.conf')
File.open(conf, 'w') { |file| file.print(content) }
end
it "should not explode if there are comments in the config file" do
write_config <<CONF
#this is a comment
; so is this
[main]
server = main_server
# yet another comment
port = 1234
CONF
expect { described_class.load }.to_not raise_error
end
it "should use the config value if specified" do
write_config <<CONF
[main]
server = main_server
port = 1234
CONF
config = described_class.load
config[:server].should == 'main_server'
config[:port].should == 1234
end
it "should use the default if no value is specified" do
write_config ''
config = described_class.load
config[:server].should == 'puppetdb'
config[:port].should == 8081
end
it "should be insensitive to whitespace" do
write_config <<CONF
[main]
server = main_server
port = 1234
CONF
config = described_class.load
config[:server].should == 'main_server'
config[:port].should == 1234
end
it "should accept valid hostnames" do
write_config <<CONF
[main]
server = foo.example-thing.com
port = 8081
CONF
config = described_class.load
config[:server].should == 'foo.example-thing.com'
config[:port].should == 8081
end
it "should raise if a setting is outside of a section" do
write_config 'foo = bar'
expect do
described_class.load
end.to raise_error(/Setting 'foo = bar' is illegal outside of section/)
end
it "should raise if an illegal line is encountered" do
write_config 'foo bar baz'
expect do
described_class.load
end.to raise_error(/Unparseable line 'foo bar baz'/)
end
context "command-specific settings" do
it "should use the defaults if no value is specified" do
write_config ''
config = described_class.load
config[CommandReplaceCatalog][:spool].should == false
config[CommandReplaceFacts][:spool].should == false
config[CommandStoreReport][:spool].should == true
end
it "should use the config value if specified" do
write_config <<CONF
[facts]
spool = true
[catalogs]
spool = true
[reports]
spool = false
CONF
config = described_class.load
config[CommandReplaceCatalog][:spool].should == true
config[CommandReplaceFacts][:spool].should == true
config[CommandStoreReport][:spool].should == false
end
end
end
end
end
......@@ -6,158 +6,12 @@ require 'digest/sha1'
require 'puppet/util/puppetdb'
require 'puppet/util/puppetdb/command_names'
# Create a local copy of these constants so that we don't have to refer to them
# by their full namespaced name
CommandReplaceCatalog = Puppet::Util::Puppetdb::CommandNames::CommandReplaceCatalog
CommandReplaceFacts = Puppet::Util::Puppetdb::CommandNames::CommandReplaceFacts
CommandStoreReport = Puppet::Util::Puppetdb::CommandNames::CommandStoreReport
Command = Puppet::Util::Puppetdb::Command
describe Puppet::Util::Puppetdb do
subject { Object.new.extend described_class }
describe "#load_puppetdb_config" do
let(:confdir) do
temp = Tempfile.new('confdir')
path = temp.path
temp.close!
Dir.mkdir(path)
path
end
before :each do
Puppet[:confdir] = confdir
end
after :each do
FileUtils.rm_rf(confdir)
end
describe "with no config file" do
it "should use the default server and port" do
config = described_class.load_puppetdb_config
config[:server].should == 'puppetdb'
config[:port].should == 8081
end
it "should use the default settings for command spooling" do
config = described_class.load_puppetdb_config
config[CommandReplaceCatalog][:spool].should == false
config[CommandReplaceFacts][:spool].should == false
config[CommandStoreReport][:spool].should == true
end
end
describe "with a config file" do
def write_config(content)
conf = File.join(confdir, 'puppetdb.conf')
File.open(conf, 'w') { |file| file.print(content) }
end
it "should not explode if there are comments in the config file" do
write_config <<CONF
#this is a comment
; so is this
[main]
server = main_server
# yet another comment
port = 1234
CONF
expect { described_class.load_puppetdb_config }.to_not raise_error
end
it "should use the config value if specified" do
write_config <<CONF
[main]
server = main_server
port = 1234
CONF
config = described_class.load_puppetdb_config
config[:server].should == 'main_server'
config[:port].should == 1234
end
it "should use the default if no value is specified" do
write_config ''
config = described_class.load_puppetdb_config
config[:server].should == 'puppetdb'
config[:port].should == 8081
end
it "should be insensitive to whitespace" do
write_config <<CONF
[main]
server = main_server
port = 1234
CONF
config = described_class.load_puppetdb_config
config[:server].should == 'main_server'
config[:port].should == 1234
end
it "should accept valid hostnames" do
write_config <<CONF
[main]
server = foo.example-thing.com
port = 8081
CONF
config = described_class.load_puppetdb_config
config[:server].should == 'foo.example-thing.com'
config[:port].should == 8081
end
it "should raise if a setting is outside of a section" do
write_config 'foo = bar'
expect do
described_class.load_puppetdb_config
end.to raise_error(/Setting 'foo = bar' is illegal outside of section/)
end
it "should raise if an illegal line is encountered" do
write_config 'foo bar baz'
expect do
described_class.load_puppetdb_config
end.to raise_error(/Unparseable line 'foo bar baz'/)
end
context "command-specific settings" do
it "should use the defaults if no value is specified" do
write_config ''
config = described_class.load_puppetdb_config
config[CommandReplaceCatalog][:spool].should == false
config[CommandReplaceFacts][:spool].should == false
config[CommandStoreReport][:spool].should == true
end
it "should use the config value if specified" do
write_config <<CONF
[facts]
spool = true
[catalogs]
spool = true
[reports]
spool = false
CONF
config = described_class.load_puppetdb_config
config[CommandReplaceCatalog][:spool].should == true
config[CommandReplaceFacts][:spool].should == true
config[CommandStoreReport][:spool].should == false
end
end
end
end
describe "spooling commands" do
let(:command_dir) { subject.send(:command_dir) }
......@@ -272,7 +126,7 @@ CONF
describe "#submit_command" do
context "when the command is set to spool" do
it "should enqueue the command and then flush" do
subject.expects(:command_spooled?).returns(true)
subject.send(:config).expects(:command_spooled?).returns(true)
# careful here... since we're going to stub Command.new, we need to
# make sure we reference good_command1 first, because it calls Command.new.
good_command1.expects(:enqueue).once
......@@ -287,7 +141,7 @@ CONF
end
context "when the command is set *not* to spool" do
it "should simply submit the command, and not enqueue or flush" do
subject.expects(:command_spooled?).returns(false)
subject.send(:config).expects(:command_spooled?).returns(false)
subject.expects(:enqueue_command).never
subject.expects(:flush_commands).never
subject.expects(:submit_single_command).once
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment