Navigation Menu

Skip to content

Commit

Permalink
(puppetlabs#4487) Fix environment column in hosts table
Browse files Browse the repository at this point in the history
An entire environment object was being stored in a string field,
causing the ZAML form of the environment to be stored. This was
over-ridden to return just the ZAML serialized version of the name.
Since the hosts model didn't know how to interpret a serialized
value, it just returned the ZAML string as the environment. This
patch stringifies the environment before putting it in the hosts
table, which stores it properly.

This patch also introduces a new method of testing using Tableless
ActiveRecord models, which emulate their database schema. This
helps to eliminate some stubbing, but it is still impossible to
fully and accurately test all ActiveRecord interactions without a
real database.

Paired-With: Matt Robinson
  • Loading branch information
nicklewis committed Dec 14, 2010
1 parent 9352671 commit 7f4e058
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 12 deletions.
2 changes: 1 addition & 1 deletion lib/puppet/indirector/catalog/active_record.rb
Expand Up @@ -32,7 +32,7 @@ def save(request)

if node = Puppet::Node.find(catalog.name)
host.ip = node.parameters["ipaddress"]
host.environment = node.environment
host.environment = node.environment.to_s
end

host.save
Expand Down
37 changes: 26 additions & 11 deletions spec/unit/indirector/catalog/active_record_spec.rb
Expand Up @@ -6,6 +6,23 @@
describe "Puppet::Resource::Catalog::ActiveRecord" do
confine "Missing Rails" => Puppet.features.rails?

require 'puppet/rails'
class Tableless < ActiveRecord::Base
def self.columns
@columns ||= []
end
def self.column(name, sql_type=nil, default=nil, null=true)
columns << ActiveRecord::ConnectionAdapters::Column.new(name.to_s, default, sql_type.to_s, null)
end
end

class Host < Tableless
column :name, :string, :null => false
column :ip, :string
column :environment, :string
column :last_compile, :datetime
end

before do
require 'puppet/indirector/catalog/active_record'
Puppet.features.stubs(:rails?).returns true
Expand Down Expand Up @@ -76,15 +93,17 @@

describe "when saving an instance" do
before do
@host = stub 'host', :name => "foo", :save => nil, :merge_resources => nil, :last_compile= => nil, :ip= => nil, :environment= => nil
@host = Host.new(:name => "foo")
@host.stubs(:merge_resources)
@host.stubs(:save)
@host.stubs(:railsmark).yields

@node = stub_everything 'node', :parameters => {}
Puppet::Node.stubs(:find).returns(@node)
@node = Puppet::Node.new("foo", :environment => "environment")
Puppet::Node.indirection.stubs(:find).with("foo").returns(@node)

Puppet::Rails::Host.stubs(:find_by_name).returns @host
@catalog = Puppet::Resource::Catalog.new("foo")
@request = stub 'request', :key => "foo", :instance => @catalog
@request = Puppet::Indirector::Request.new(:active_record, :save, @catalog)
end

it "should find the Rails host with the same name" do
Expand All @@ -111,25 +130,21 @@
it "should set host ip if we could find a matching node" do
@node.stubs(:parameters).returns({"ipaddress" => "192.168.0.1"})

@host.expects(:ip=).with '192.168.0.1'

@terminus.save(@request)
@host.ip.should == '192.168.0.1'
end

it "should set host environment if we could find a matching node" do
@node.stubs(:environment).returns("myenv")

@host.expects(:environment=).with 'myenv'

@terminus.save(@request)
@host.environment.should == "environment"
end

it "should set the last compile time on the host" do
now = Time.now
Time.expects(:now).returns now
@host.expects(:last_compile=).with now

@terminus.save(@request)
@host.last_compile.should == now
end

it "should save the Rails host instance" do
Expand Down

0 comments on commit 7f4e058

Please sign in to comment.