Stefan,
This 4th commit didn't get in with your previous change set since it
was added after we initially looked at the code. Also, it really
doesn't have anything to do with ticket #5274 which is about inline
comments, so this commit should really be part of another ticket.
We've opened a new ticket, #5427 which is was unfortunately generated
with a number that is very similar and thus a tad confusing. We've
added you as a watcher for that ticket and have some comments there,
so check that out.
Thanks
Matt
On Thu, Nov 18, 2010 at 2:54 PM, Stefan Schulte
<stefan.schu...@taunusstein.net> wrote:
> This uses the propertyclass Puppet::Property::OrderedList to represent
> the list of host_aliases. This lets us remove the in_sync, shoult_to_s
> etc overrides. The new membership paramater can even be used to just
> specify a minimal list of host_aliases (but default is inclusive).
>
> In the provider class the list is represented by a string (=no array)
> so there were a few changes necessary as well.
>
> Because Puppet::Property::List uses the specified delimiter when
> converting should values to strings, I changed the delimiter to a simple
> space instead a tab. This keeps messages produced by puppet in a nice
> format.
>
> The tests had to be changed to work with the new behaviour of
> host_aliases. There are a few additional tests as well.
>
> Signed-off-by: Stefan Schulte <stefan.schu...@taunusstein.net>
> ---
> lib/puppet/provider/host/parsed.rb | 8 ++---
> lib/puppet/type/host.rb | 41 ++++++++++------------------
> spec/unit/provider/host/parsed_spec.rb | 28 ++++++++++++-------
> spec/unit/type/host_spec.rb | 46
> +++++++++++++++++++++++++++++---
> 4 files changed, 78 insertions(+), 45 deletions(-)
>
> diff --git a/lib/puppet/provider/host/parsed.rb
> b/lib/puppet/provider/host/parsed.rb
> index a303c4b..2ba01a4 100644
> --- a/lib/puppet/provider/host/parsed.rb
> +++ b/lib/puppet/provider/host/parsed.rb
> @@ -22,9 +22,7 @@ Puppet::Type.type(:host).provide(:parsed,:parent =>
> Puppet::Provider::ParsedFile
> # An absent comment should match "comment => ''"
> hash[:comment] = '' if hash[:comment].nil? or hash[:comment] == :absent
> unless hash[:host_aliases].nil? or hash[:host_aliases] == :absent
> - hash[:host_aliases] = hash[:host_aliases].split(/\s+/)
> - else
> - hash[:host_aliases] = []
> + hash[:host_aliases].gsub!(/\s+/,' ') # Change delimiter
> end
> },
> :to_line => proc { |hash|
> @@ -32,8 +30,8 @@ Puppet::Type.type(:host).provide(:parsed,:parent =>
> Puppet::Provider::ParsedFile
> raise ArgumentError, "#{n} is a required attribute for hosts" unless
> hash[n] and hash[n] != :absent
> end
> str = "#{hash[:ip]}\t#{hash[:name]}"
> - if hash.include? :host_aliases and !hash[:host_aliases].empty?
> - str += "\t#{hash[:host_aliases].join("\t")}"
> + if hash.include? :host_aliases and !hash[:host_aliases].nil? and
> hash[:host_aliases] != :absent
> + str += "\t#{hash[:host_aliases]}"
> end
> if hash.include? :comment and !hash[:comment].empty?
> str += "\t# #{hash[:comment]}"
> diff --git a/lib/puppet/type/host.rb b/lib/puppet/type/host.rb
> index 1af74d8..3520ae2 100755
> --- a/lib/puppet/type/host.rb
> +++ b/lib/puppet/type/host.rb
> @@ -1,3 +1,5 @@
> +require 'puppet/property/ordered_list'
> +
> module Puppet
> newtype(:host) do
> ensurable
> @@ -13,41 +15,28 @@ module Puppet
>
> end
>
> - newproperty(:host_aliases) do
> + # for now we use OrderedList to indicate that the order does matter.
> + newproperty(:host_aliases, :parent => Puppet::Property::OrderedList) do
> desc "Any aliases the host might have. Multiple values must be
> specified as an array."
>
> - def insync?(is)
> - is == @should
> + validate do |value|
> + raise Puppet::Error, "Host aliases cannot include whitespace" if
> value =~ /\s/
> + raise Puppet::Error, "Host alias cannot be an empty string. Use an
> empty array to delete all host_aliases " if value =~ /^\s*$/
> end
>
> - def is_to_s(currentvalue = @is)
> - currentvalue = [currentvalue] unless currentvalue.is_a? Array
> - currentvalue.join(" ")
> + def delimiter
> + " "
> end
>
> - # We actually want to return the whole array here, not just the first
> - # value.
> - def should
> - if defined?(@should)
> - if @should == [:absent]
> - return :absent
> - else
> - return @should
> - end
> - else
> - return nil
> - end
> - end
> + end
>
> - def should_to_s(newvalue = @should)
> - newvalue.join(" ")
> - end
> + newparam(:membership) do
> + desc "This paramter decides if the specified host_aliases should be
> the only aliases for the
> + host or if host_aliases should only specify the minimal subset."
>
> - validate do |value|
> - raise Puppet::Error, "Host aliases cannot include whitespace" if
> value =~ /\s/
> - raise Puppet::Error, "Host alias cannot be an empty string. Use an
> empty array to delete all host_aliases " if value =~ /^\s*$/
> - end
> + newvalues :inclusive, :minimum
> + defaultto :inclusive
> end
>
> newproperty(:comment) do
> diff --git a/spec/unit/provider/host/parsed_spec.rb
> b/spec/unit/provider/host/parsed_spec.rb
> index 239e3bd..0bc9651 100644
> --- a/spec/unit/provider/host/parsed_spec.rb
> +++ b/spec/unit/provider/host/parsed_spec.rb
> @@ -28,9 +28,13 @@ describe provider_class do
> hostresource = Puppet::Type::Host.new(:name => args[:name])
> hostresource.stubs(:should).with(:target).returns @hostfile
>
> - # Using setters of provider
> + # Using setters of provider to build our testobject
> + # Note: We already proved, that in case of host_aliases
> + # the provider setter "host_aliases=(value)" will be
> + # called with the joined array, so we just simulate that
> host = @provider.new(hostresource)
> args.each do |property,value|
> + value = value.join(" ") if property == :host_aliases and
> value.is_a?(Array)
> host.send("#{property}=", value)
> end
> host
> @@ -63,6 +67,10 @@ describe provider_class do
> @provider.parse_line("::1 localhost")[:comment].should == ""
> end
>
> + it "should set host_aliases to :absent" do
> + �...@provider.parse_line("::1 localhost")[:host_aliases].should ==
> :absent
> + end
> +
> end
>
> describe "when parsing a line with ip, hostname and comment" do
> @@ -87,13 +95,13 @@ describe provider_class do
> describe "when parsing a line with ip, hostname and aliases" do
>
> it "should parse alias from the third field" do
> - �...@provider.parse_line("127.0.0.1 localhost
> localhost.localdomain")[:host_aliases].should == ["localhost.localdomain"]
> + �...@provider.parse_line("127.0.0.1 localhost
> localhost.localdomain")[:host_aliases].should == "localhost.localdomain"
> end
>
> it "should parse multiple aliases" do
> - �...@provider.parse_line("127.0.0.1 host alias1
> alias2")[:host_aliases].should == ['alias1', 'alias2']
> - �...@provider.parse_line("127.0.0.1 host
> alias1\talias2")[:host_aliases].should == ['alias1', 'alias2']
> - �...@provider.parse_line("127.0.0.1 host alias1\talias2
> alias3")[:host_aliases].should == ['alias1', 'alias2', 'alias3']
> + �...@provider.parse_line("127.0.0.1 host alias1
> alias2")[:host_aliases].should == 'alias1 alias2'
> + �...@provider.parse_line("127.0.0.1 host
> alias1\talias2")[:host_aliases].should == 'alias1 alias2'
> + �...@provider.parse_line("127.0.0.1 host alias1\talias2
> alias3")[:host_aliases].should == 'alias1 alias2 alias3'
> end
>
> end
> @@ -114,7 +122,7 @@ describe provider_class do
> end
>
> it "should parse all host_aliases from the third field" do
> - �...@provider.parse_line(@testline)[:host_aliases].should == ['alias1'
> ,'alias2', 'alias3']
> + �...@provider.parse_line(@testline)[:host_aliases].should == 'alias1
> alias2 alias3'
> end
>
> it "should parse the comment after the first '#' character" do
> @@ -143,7 +151,7 @@ describe provider_class do
> host = mkhost(
> :name => 'localhost.localdomain',
> :ip => '127.0.0.1',
> - :host_aliases => ['localhost'],
> + :host_aliases => 'localhost',
> :ensure => :present
> )
> genhost(host).should == "127.0.0.1\tlocalhost.localdomain\tlocalhost\n"
> @@ -156,7 +164,7 @@ describe provider_class do
> :host_aliases => [ 'a1','a2','a3','a4' ],
> :ensure => :present
> )
> - genhost(host).should == "192.0.0.1\thost\ta1\ta2\ta3\ta4\n"
> + genhost(host).should == "192.0.0.1\thost\ta1 a2 a3 a4\n"
> end
>
> it "should be able to generate a simple hostfile entry with comments" do
> @@ -173,7 +181,7 @@ describe provider_class do
> host = mkhost(
> :name => 'localhost.localdomain',
> :ip => '127.0.0.1',
> - :host_aliases => ['localhost'],
> + :host_aliases => 'localhost',
> :comment => 'Bazinga!',
> :ensure => :present
> )
> @@ -188,7 +196,7 @@ describe provider_class do
> :comment => 'Bazinga!',
> :ensure => :present
> )
> - genhost(host).should == "192.0.0.1\thost\ta1\ta2\ta3\ta4\t# Bazinga!\n"
> + genhost(host).should == "192.0.0.1\thost\ta1 a2 a3 a4\t# Bazinga!\n"
> end
>
> end
> diff --git a/spec/unit/type/host_spec.rb b/spec/unit/type/host_spec.rb
> index 12ae2af..4c15f76 100755
> --- a/spec/unit/type/host_spec.rb
> +++ b/spec/unit/type/host_spec.rb
> @@ -2,12 +2,14 @@
>
> require File.dirname(__FILE__) + '/../../spec_helper'
>
> -ssh_authorized_key = Puppet::Type.type(:ssh_authorized_key)
> +host = Puppet::Type.type(:host)
>
> -describe Puppet::Type.type(:host) do
> +describe host do
> before do
> - �...@class = Puppet::Type.type(:host)
> + �...@class = host
> @catalog = Puppet::Resource::Catalog.new
> + �...@provider = stub 'provider'
> + �...@resource = stub 'resource', :resource => nil, :provider => @provider
> end
>
> it "should have :name be its namevar" do
> @@ -15,7 +17,7 @@ describe Puppet::Type.type(:host) do
> end
>
> describe "when validating attributes" do
> - [:name, :provider ].each do |param|
> + [:name, :provider, :membership ].each do |param|
> it "should have a #{param} parameter" do
> @class.attrtype(param).should == :param
> end
> @@ -26,6 +28,11 @@ describe Puppet::Type.type(:host) do
> @class.attrtype(property).should == :property
> end
> end
> +
> + it "should have a list host_aliases" do
> + �...@class.attrclass(:host_aliases).ancestors.should
> be_include(Puppet::Property::OrderedList)
> + end
> +
> end
>
> describe "when validating values" do
> @@ -80,4 +87,35 @@ describe Puppet::Type.type(:host) do
> proc { @class.new(:name => "foo", :host_aliases => ['alias1',''])
> }.should raise_error
> end
> end
> +
> + describe "when syncing" do
> +
> + it "should send the first value to the provider for ip property" do
> + �...@ip = @class.attrclass(:ip).new(:resource => @resource, :should =>
> %w{192.168.0.1 192.168.0.2})
> + �...@provider.expects(:ip=).with '192.168.0.1'
> + �...@ip.sync
> + end
> +
> + it "should send the first value to the provider for comment property" do
> + �...@comment = @class.attrclass(:comment).new(:resource => @resource,
> :should => %w{Bazinga Notme})
> + �...@provider.expects(:comment=).with 'Bazinga'
> + �...@comment.sync
> + end
> +
> + it "should send the joined array to the provider for host_alias" do
> + �...@host_aliases = @class.attrclass(:host_aliases).new(:resource =>
> @resource, :should => %w{foo bar})
> + �...@resource.stubs(:[]).with(:membership).returns :inclusive
> + �...@provider.expects(:host_aliases=).with 'foo bar'
> + �...@host_aliases.sync
> + end
> +
> + it "should also use the specified delimiter for joining" do
> + �...@host_aliases = @class.attrclass(:host_aliases).new(:resource =>
> @resource, :should => %w{foo bar})
> + �...@resource.stubs(:[]).with(:membership).returns :inclusive
> + �...@host_aliases.stubs(:delimiter).returns "\t"
> + �...@provider.expects(:host_aliases=).with "foo\tbar"
> + �...@host_aliases.sync
> + end
> +
> + end
> end
> --
> 1.7.3.2
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.
> To post to this group, send email to puppet-...@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-dev+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-dev?hl=en.
>
>
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-...@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
