From 82a9b003cc039fd8a6949801cd6a110b757f5da9 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Tue, 8 Oct 2024 18:09:04 +0900 Subject: [PATCH] nova_aggregate: Fix match for array hosts The property is not fully applied because of lack of array_matching definition. Change-Id: I1a9bba3e597bc61329c4526b051787723f2a7dc9 --- lib/puppet/type/nova_aggregate.rb | 36 +++++++++++++------ ...nova-aggregate-hosts-fd40d74a46193941.yaml | 10 ++++++ spec/type/nova_aggregate_spec.rb | 20 +++++------ spec/unit/type/nova_aggregate_spec.rb | 4 +-- 4 files changed, 47 insertions(+), 23 deletions(-) create mode 100644 releasenotes/notes/nova-aggregate-hosts-fd40d74a46193941.yaml diff --git a/lib/puppet/type/nova_aggregate.rb b/lib/puppet/type/nova_aggregate.rb index 8cce20b8a..f24a1ca52 100644 --- a/lib/puppet/type/nova_aggregate.rb +++ b/lib/puppet/type/nova_aggregate.rb @@ -37,10 +37,9 @@ # Optional # # [*hosts*] -# A comma separated list with hosts or a single host. ie "host1,host2" +# An array of hosts or a single host. ie "host1,host2" # Optional # - require 'puppet' Puppet::Type.newtype(:nova_aggregate) do @@ -116,16 +115,32 @@ Puppet::Type.newtype(:nova_aggregate) do end end - newproperty(:hosts) do - desc 'Single host or comma separated list of hosts' - #convert DSL/string form to internal form - munge do |value| - if value.is_a?(Array) - return value - elsif value.is_a?(String) + newproperty(:hosts, :array_matching => :all) do + desc 'Single host or array of hosts' + + def should + val = super + val.flatten + end + + def should=(values) + super + @should = @should.flatten + end + + def issync?(is) + return false unless is.is_a? Array + is.sort == should.sort + end + + validate do |value| + if value.is_a?(String) + if value.include?(',') + warning('Using comma separated string for hosts is deprecated. Use an array instead.') + end return value.split(",").map{|el| el.strip()}.sort else - raise ArgumentError, "Invalid hosts #{value}. Requires a String or an Array, not a #{value.class}" + raise ArgumentError, "Invalid hosts #{value}. Requires a String, not a #{value.class}" end end end @@ -133,5 +148,4 @@ Puppet::Type.newtype(:nova_aggregate) do validate do raise ArgumentError, 'Name type must be set' unless self[:name] end - end diff --git a/releasenotes/notes/nova-aggregate-hosts-fd40d74a46193941.yaml b/releasenotes/notes/nova-aggregate-hosts-fd40d74a46193941.yaml new file mode 100644 index 000000000..feebbfa45 --- /dev/null +++ b/releasenotes/notes/nova-aggregate-hosts-fd40d74a46193941.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + The ``nova_aggregate`` resource now requires an array value for its + ``hosts`` property in case multiple hosts should belong to the aggregate. + +fixes: + - | + Fixed ``nova_aggregate`` resource so that the resource ensures all values + given for the ``hosts`` property present. diff --git a/spec/type/nova_aggregate_spec.rb b/spec/type/nova_aggregate_spec.rb index 7d859a6a1..60fe478c6 100644 --- a/spec/type/nova_aggregate_spec.rb +++ b/spec/type/nova_aggregate_spec.rb @@ -30,21 +30,21 @@ describe Puppet::Type.type(:nova_aggregate) do expect(described_class.new(:name => 'agg0', :availability_zone => 'myzone', :metadata => "a=b, c=d", - :hosts => "host1, host2")).to_not be_nil + :hosts => ['host1', 'host2'])).to_not be_nil end it "should be able to create an more complex instance with hash for metadata" do expect(described_class.new(:name => 'agg0', :availability_zone => 'myzone', :metadata => { 'a' => 'b', 'c' => 'd' }, - :hosts => "host1, host2")).to_not be_nil + :hosts => ['host1', 'host2'])).to_not be_nil end it "should be able to create an more complex instance with hash for metadata and values containing commas" do expect(described_class.new(:name => 'agg0', :availability_zone => 'myzone', :metadata => { 'a' => 'b,e,f,g', 'c' => 'd,h,i,j' }, - :hosts => "host1, host2")).to_not be_nil + :hosts => ['host1', 'host2'])).to_not be_nil end it "should be able to create a instance and have the default values" do @@ -59,40 +59,40 @@ describe Puppet::Type.type(:nova_aggregate) do c = described_class.new(:name => 'agg0', :availability_zone => 'myzone', :metadata => " a = b , c= d ", - :hosts => " host1, host2 ") + :hosts => 'host1') expect(c[:name]).to eq("agg0") expect(c[:availability_zone]).to eq("myzone") expect(c[:metadata]).to eq({"a" => "b", "c" => "d"}) - expect(c[:hosts]).to eq(["host1" , "host2"]) + expect(c[:hosts]).to eq(['host1']) end it "should return the given values with hash for metadata" do c = described_class.new(:name => 'agg0', :availability_zone => 'myzone', :metadata => { 'a' => 'b', 'c' => 'd' },, - :hosts => " host1, host2 ") + :hosts => 'host1') expect(c[:name]).to eq("agg0") expect(c[:availability_zone]).to eq("myzone") expect(c[:metadata]).to eq({"a" => "b", "c" => "d"}) - expect(c[:hosts]).to eq(["host1" , "host2"]) + expect(c[:hosts]).to eq(['host1']) end it "should return the given values with hash for metadata and values containing commas" do c = described_class.new(:name => 'agg0', :availability_zone => 'myzone', :metadata => { 'a' => 'b,e,f,g', 'c' => 'd,h,i,j' }, - :hosts => " host1, host2 ") + :hosts => 'host1') expect(c[:name]).to eq("agg0") expect(c[:availability_zone]).to eq("myzone") expect(c[:metadata]).to eq({"a" => "b", "c" => "d"}) - expect(c[:hosts]).to eq(["host1" , "host2"]) + expect(c[:hosts]).to eq(['host1']) end it "should return the given values" do c = described_class.new(:name => 'agg0', :availability_zone => "", :metadata => "", - :hosts => "") + :hosts => []) expect(c[:name]).to eq("agg0") expect(c[:availability_zone]).to eq("") expect(c[:metadata]).to eq({}) diff --git a/spec/unit/type/nova_aggregate_spec.rb b/spec/unit/type/nova_aggregate_spec.rb index b60389d3c..06ea6a992 100644 --- a/spec/unit/type/nova_aggregate_spec.rb +++ b/spec/unit/type/nova_aggregate_spec.rb @@ -25,7 +25,7 @@ describe Puppet::Type.type(:nova_aggregate) do it 'should raise error if wrong type for availability zone' do incorrect_input = { - :name => 'new_aggr', + :name => 'new_aggr', :availability_zone => {'zone'=>'23'}, } expect { Puppet::Type.type(:nova_aggregate).new(incorrect_input) }.to raise_error(Puppet::ResourceError, /availability zone must be a String/) @@ -33,7 +33,7 @@ describe Puppet::Type.type(:nova_aggregate) do it 'should raise error if non-boolean-y input for filter_hosts' do incorrect_input = { - :name => 'new_aggr', + :name => 'new_aggr', :filter_hosts => 'some non boolean-y value', } expect { Puppet::Type.type(:nova_aggregate).new(incorrect_input) }.to raise_error(Puppet::ResourceError, /Valid values are true, false/)