From 9ddd277e9369690527b0dd07fde0c3bc04b8fd5e Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Sun, 26 Nov 2023 01:37:48 +0900 Subject: [PATCH] vs_bridge: Use hash for external_ids ... so that data type is consistent with actual data type in ovs. Closes-Bug: #2044136 Change-Id: I7b861d21d35d1886d64e47ea3c1f0fb15fa1c3af --- lib/puppet/provider/vs_bridge/ovs.rb | 13 ++++------ lib/puppet/type/vs_bridge.rb | 24 +++++++++++++++---- .../external-id-hash-cc61f1a3ab7f93ce.yaml | 5 ++++ spec/acceptance/basic_vswitch_spec.rb | 14 ++++++++++- spec/unit/provider/vs_bridge/ovs_spec.rb | 8 +++---- spec/unit/type/vs_bridge_spec.rb | 11 +++++++++ 6 files changed, 57 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/external-id-hash-cc61f1a3ab7f93ce.yaml diff --git a/lib/puppet/provider/vs_bridge/ovs.rb b/lib/puppet/provider/vs_bridge/ovs.rb index aa9f5ffd..070284ce 100644 --- a/lib/puppet/provider/vs_bridge/ovs.rb +++ b/lib/puppet/provider/vs_bridge/ovs.rb @@ -46,13 +46,14 @@ Puppet::Type.type(:vs_bridge).provide(:ovs) do private def self.get_external_ids(br) - result = vsctl('br-get-external-id', br) - return result.split("\n").join(',') + value = vsctl('br-get-external-id', br) + value = value.split("\n").map{|i| i.strip} + return Hash[value.map{|i| i.split('=')}] end def self.set_external_ids(br, value) - old_ids = _split(get_external_ids(br)) - new_ids = _split(value) + old_ids = get_external_ids(br) + new_ids = value new_ids.each do |k,v| if !old_ids.has_key?(k) or old_ids[k] != v @@ -80,10 +81,6 @@ Puppet::Type.type(:vs_bridge).provide(:ovs) do vsctl('set', 'Bridge', br, "other-config:mac-table-size=#{value}") end - def self._split(string, splitter=',') - return Hash[string.split(splitter).map{|i| i.split('=')}] - end - def self.get_bridge_other_config(br) value = vsctl('get', 'Bridge', br, 'other-config').strip value = value.gsub(/^{|}$/, '').split(',').map{|i| i.strip} diff --git a/lib/puppet/type/vs_bridge.rb b/lib/puppet/type/vs_bridge.rb index db578f36..0772f309 100644 --- a/lib/puppet/type/vs_bridge.rb +++ b/lib/puppet/type/vs_bridge.rb @@ -18,12 +18,26 @@ Puppet::Type.newtype(:vs_bridge) do newproperty(:external_ids) do desc 'External IDs for the bridge: "key1=value2,key2=value2"' - validate do |value| - if !value.is_a?(String) - raise ArgumentError, "Invalid external_ids #{value}. Requires a String, not a #{value.class}" + munge do |value| + return value if value.is_a? Hash + + internal = Hash.new + value.split(",").map{|el| el.strip()}.each do |pair| + key, value = pair.split("=", 2) + internal[key.strip()] = value.strip() end - if value !~ /^(?>[a-zA-Z]\S*=\S*){1}(?>[,][a-zA-Z]\S*=\S*)*$/ - raise ArgumentError, "Invalid external_ids #{value}. Must a list of key1=value2,key2=value2" + return internal + end + + validate do |value| + if value.is_a?(Hash) + true + elsif value.is_a?(String) + if value !~ /^(?>[a-zA-Z]\S*=\S*){1}(?>[,][a-zA-Z]\S*=\S*)*$/ + raise ArgumentError, "Invalid external_ids #{value}. Must a list of key1=value2,key2=value2" + end + else + raise ArgumentError, "Invalid external_ids #{value}. Requires a String or a Hash, not a #{value.class}" end end end diff --git a/releasenotes/notes/external-id-hash-cc61f1a3ab7f93ce.yaml b/releasenotes/notes/external-id-hash-cc61f1a3ab7f93ce.yaml new file mode 100644 index 00000000..59d56580 --- /dev/null +++ b/releasenotes/notes/external-id-hash-cc61f1a3ab7f93ce.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + The ``external_ids`` property of the ``vs_bridge`` resource now supports + a hash value. diff --git a/spec/acceptance/basic_vswitch_spec.rb b/spec/acceptance/basic_vswitch_spec.rb index 3636e665..0e488cc7 100644 --- a/spec/acceptance/basic_vswitch_spec.rb +++ b/spec/acceptance/basic_vswitch_spec.rb @@ -27,7 +27,7 @@ describe 'basic vswitch' do vs_bridge { 'br-ci4': ensure => present, - external_ids => 'bridge-id=br-ci4', + external_ids => {'bridge-id' => 'br-ci4'}, mac_table_size => 50000, } ~> exec { 'create_loop1_port': @@ -42,6 +42,12 @@ describe 'basic vswitch' do bridge => 'br-ci4', } + vs_bridge { 'br-ci5': + ensure => present, + external_ids => 'bridge-id=br-ci5', + mac_table_size => 50000, + } + vs_config { 'external_ids:ovn-remote': ensure => present, value => 'tcp:127.0.0.1:2300', @@ -113,6 +119,12 @@ describe 'basic vswitch' do end end + it 'should have external_ids on br-ci5 bridge' do + command('ovs-vsctl br-get-external-id br-ci5') do |r| + expect(r.stdout).to match(/bridge-id=br-ci5/) + end + end + it 'should have mac-table-size on br-ci4 bridge' do command('ovs-vsctl get Bridge br-ci4 other-config:mac-table-size') do |r| expect(r.stdout).to match(/\"50000\"/) diff --git a/spec/unit/provider/vs_bridge/ovs_spec.rb b/spec/unit/provider/vs_bridge/ovs_spec.rb index 66b1266b..c5af9227 100644 --- a/spec/unit/provider/vs_bridge/ovs_spec.rb +++ b/spec/unit/provider/vs_bridge/ovs_spec.rb @@ -48,7 +48,7 @@ describe Puppet::Type.type(:vs_bridge).provider(:ovs) do context 'with parameters' do before :each do resource_attrs.merge!( - :external_ids => 'k=v', + :external_ids => {'k' => 'v'}, :mac_table_size => 60000, ) end @@ -91,7 +91,7 @@ describe Puppet::Type.type(:vs_bridge).provider(:ovs) do ).and_return( 'k=v' ) - expect(provider.external_ids).to eq('k=v') + expect(provider.external_ids).to eq({'k' => 'v'}) end end @@ -103,7 +103,7 @@ describe Puppet::Type.type(:vs_bridge).provider(:ovs) do expect(described_class).to receive(:vsctl).with( 'br-set-external-id', 'testbr', 'k', 'v' ) - provider.external_ids = 'k=v' + provider.external_ids = {'k' => 'v'} end it 'configures external ids when ids already exist' do @@ -118,7 +118,7 @@ k3=v3') expect(described_class).to receive(:vsctl).with( 'br-set-external-id', 'testbr', 'k3' ) - provider.external_ids = 'k1=v1,k2=v2new' + provider.external_ids = {'k1' => 'v1', 'k2' => 'v2new'} end end diff --git a/spec/unit/type/vs_bridge_spec.rb b/spec/unit/type/vs_bridge_spec.rb index 4b88b317..55b0d587 100644 --- a/spec/unit/type/vs_bridge_spec.rb +++ b/spec/unit/type/vs_bridge_spec.rb @@ -3,9 +3,20 @@ require 'spec_helper' describe Puppet::Type.type(:vs_bridge) do it "should support present as a value for ensure" do + expect do + described_class.new(:name => 'foo', :ensure => :present) + end.to_not raise_error + end + + it "should support a string value for external_ids" do expect do described_class.new(:name => 'foo', :ensure => :present, :external_ids => 'foo=br-ex,blah-id=bar)') end.to_not raise_error end + it "should support a hash value for external_ids" do + expect do + described_class.new(:name => 'foo', :ensure => :present, :external_ids => {'foo' => 'br-ex'}) + end.to_not raise_error + end end