Create the base ovs provider

The commands to query or update ovs db records are similar for
different resources (bridge, port and interface).

This creates the base implementation to avoid maintaining similar
logics for multiple resources.

Change-Id: Ibd75f464af984987c27548a08da26dea622c7e79

lib/puppet/provider/ovs.rb

@@ -0,0 +1,43 @@
+require 'puppet'
+
+class Puppet::Provider::Ovs < Puppet::Provider
+
+  initvars
+  commands :vsctl => 'ovs-vsctl'
+
+  protected
+
+  def self.get_property(type, name, key)
+    return vsctl('get', type, name, key).strip
+  end
+
+  def self.set_property(type, name, key, val=nil)
+    if val.nil? or val.empty?
+      vsctl('clear', type, name, key)
+    else
+      vsctl('set', type, name, "#{key}=#{val}")
+    end
+  rescue
+    set_property(type, name, key, val.to_s)
+  end
+
+  def self.get_other_config(type, name, key)
+    value = vsctl('get', type, name, 'other_config').strip
+    value = parse_hash(value.gsub(/^{|}$/, ''))
+    value[key]
+  end
+
+  def self.set_other_config(type, name, key, val=nil)
+    if val.nil? or val.empty?
+      vsctl('remove', type, name, 'other_config', key)
+    else
+      vsctl('set', type, name, "other_config:#{key}=#{val}")
+    end
+  rescue
+    set_other_config(type, name, key, val.to_s)
+  end
+
+  def self.parse_hash(string, splitter=',')
+    return Hash[string.split(splitter).map{|i| i.strip.split('=')}]
+  end
+end

lib/puppet/provider/vs_bridge/ovs.rb

@@ -1,8 +1,11 @@
-require 'puppet'
+require File.join(File.dirname(__FILE__), '..','..','..', 'puppet/provider/ovs')
 
-Puppet::Type.type(:vs_bridge).provide(:ovs) do
+Puppet::Type.type(:vs_bridge).provide(
-  commands :vsctl => 'ovs-vsctl'
+  :ovs,
+  :parent => Puppet::Provider::Ovs
+) do
   commands :ip    => 'ip'
+  commands :vsctl => 'ovs-vsctl'
 
   def exists?
     vsctl("br-exists", @resource[:name])
@@ -47,8 +50,7 @@ Puppet::Type.type(:vs_bridge).provide(:ovs) do
 
   def self.get_external_ids(br)
     value = vsctl('br-get-external-id', br)
-    value = value.split("\n").map{|i| i.strip}
+    return parse_hash(value, "\n")
-    return Hash[value.map{|i| i.split('=')}]
   end
 
   def self.set_external_ids(br, value)
@@ -69,21 +71,11 @@ Puppet::Type.type(:vs_bridge).provide(:ovs) do
   end
 
   def self.get_mac_table_size(br)
-    value = get_bridge_other_config(br)['mac-table-size']
+    value = get_other_config('Bridge', br, 'mac-table-size')
-    if value
+    if value.nil? then nil else Integer(value.gsub(/^"|"$/, '')) end
-      Integer(value.gsub(/^"|"$/, ''))
-    else
-      nil
-    end
   end
 
   def self.set_mac_table_size(br, value)
-    vsctl('set', 'Bridge', br, "other-config:mac-table-size=#{value}")
+    set_other_config('Bridge', br, 'mac-table-size', value)
-  end
-
-  def self.get_bridge_other_config(br)
-    value = vsctl('get', 'Bridge', br, 'other-config').strip
-    value = value.gsub(/^{|}$/, '').split(',').map{|i| i.strip}
-    return Hash[value.map{|i| i.split('=')}]
   end
 end

lib/puppet/provider/vs_port/ovs.rb

@@ -1,17 +1,18 @@
-require 'puppet'
+require File.join(File.dirname(__FILE__), '..','..','..', 'puppet/provider/ovs')
 
-
+Puppet::Type.type(:vs_port).provide(
-Puppet::Type.type(:vs_port).provide(:ovs) do
+  :ovs,
-  desc 'Openvswitch port manipulation'
+  :parent => Puppet::Provider::Ovs
+) do
 
   UUID_RE ||= /[a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89aAbB][a-f0-9]{3}-[a-f0-9]{12}/
 
+  commands :vsctl => 'ovs-vsctl'
+
   has_feature :bonding
   has_feature :vlan
   has_feature :interface_type
 
-  commands :vsctl => 'ovs-vsctl'
-
   def exists?
     vsctl('list-ports', @resource[:bridge]).split("\n").include? @resource[:port]
   rescue Puppet::ExecutionFailure => e
@@ -59,7 +60,7 @@ Puppet::Type.type(:vs_port).provide(:ovs) do
   end
 
   def interface
-    get_port_interface_column('name')
+    get_port_interface_property('name')
   end
 
   def interface=(value)
@@ -74,7 +75,7 @@ Puppet::Type.type(:vs_port).provide(:ovs) do
   end
 
   def interface_type
-    types = get_port_interface_column('type').uniq
+    types = get_port_interface_property('type').uniq
     types != nil ? types.join(' ') : :system
   end
 
@@ -85,51 +86,52 @@ Puppet::Type.type(:vs_port).provide(:ovs) do
   end
 
   def bond_mode
-    get_port_column('bond_mode')
+    get_port_property('bond_mode')
   end
 
   def bond_mode=(value)
-    set_port_column('bond_mode', value)
+    set_port_property('bond_mode', value)
   end
 
   def lacp
-    get_port_column('lacp')
+    get_port_property('lacp')
   end
 
   def lacp=(value)
-    set_port_column('lacp', value)
+    set_port_property('lacp', value)
   end
 
   def lacp_time
-    get_port_column('other_config:lacp-time')
+    val = self.class.get_other_config('Port', @resource[:port], 'lacp-time')
+    if val.nil? then '' else val.gsub(/^"|"$/, '') end
   end
 
   def lacp_time=(value)
-    set_port_column('other_config:lacp-time', value)
+    self.class.set_other_config('Port', @resource[:port], 'lacp-time', value)
   end
 
   def vlan_mode
-    get_port_column('vlan_mode')
+    get_port_property('vlan_mode')
   end
 
   def vlan_mode=(value)
-    set_port_column('vlan_mode', value)
+    set_port_property('vlan_mode', value)
   end
 
   def vlan_tag
-    get_port_column('tag')
+    get_port_property('tag')
   end
 
   def vlan_tag=(value)
-    set_port_column('tag', value)
+    set_port_property('tag', value)
   end
 
   def vlan_trunks
-    get_port_column('trunks').scan(/\d+/)
+    get_port_property('trunks').scan(/\d+/)
   end
 
   def vlan_trunks=(value)
-    set_port_column('trunks', value.join(' '))
+    set_port_property('trunks', value.join(' '))
   end
 
   protected
@@ -143,36 +145,17 @@ Puppet::Type.type(:vs_port).provide(:ovs) do
 
   private
 
-  def port_column_command(command, column, value=nil)
+  def get_port_property(key)
-    if value
+    value = self.class.get_property('Port', @resource[:port], key)
-      vsctl(command, 'Port', @resource[:port], column, value)
-    else
-      vsctl('--if-exists', command, 'Port', @resource[:port], column)
-    end
-  end
-
-  def get_port_column(column)
-    value = port_column_command('get', column).strip
     if value == '[]' then '' else value end
   end
 
-  def set_port_column(column, value)
+  def set_port_property(key, value)
-    if ! value or value.empty?
+    self.class.set_property('Port', @resource[:port], key, value)
-      # columns with maps need special handling, single map entries
-      # can be removed with the remove command
-      column, key = column.split(':')
-      if ! key
-        port_column_command('clear', column)
-      else
-        port_column_command('remove', [column, key])
-      end
-    else
-      port_column_command('set', "#{column}=#{value}")
-    end
   end
 
-  def get_port_interface_column(column)
+  def get_port_interface_property(key)
-    uuids = get_port_column('interfaces').scan(UUID_RE)
+    uuids = get_port_property('interfaces').scan(UUID_RE)
-    uuids.map!{|id| vsctl('get', 'Interface', id, column).strip.tr('"', '')}
+    uuids.map!{|id| self.class.get_property('Interface', id, key).gsub(/^"|"$/, '')}
   end
 end

lib/puppet/provider/vs_port/ovs_redhat.rb

@@ -6,8 +6,6 @@ Puppet::Type.type(:vs_port).provide(
   :parent => Puppet::Type.type(:vs_port).provider(:ovs)
 ) do
 
-  desc 'Openvswitch port manipulation for RedHat OSes family'
-
   BASE ||= '/etc/sysconfig/network-scripts/ifcfg-'
 
   # When not seedling from interface file

spec/unit/provider/ovs_spec.rb

@@ -0,0 +1,111 @@
+require 'puppet'
+require 'spec_helper'
+require 'puppet/provider/ovs'
+
+describe Puppet::Provider::Ovs do
+  describe '#get_property' do
+    it 'returns the property' do
+      expect(described_class).to receive(:vsctl)
+        .with('get', 'Port', 'testport', 'key')
+        .and_return('value')
+      expect(described_class.get_property('Port', 'testport', 'key')).to eq('value')
+    end
+    it 'returns the property without surrounding spaces' do
+      expect(described_class).to receive(:vsctl)
+        .with('get', 'Port', 'testport', 'key')
+        .and_return('  value  ')
+      expect(described_class.get_property('Port', 'testport', 'key')).to eq('value')
+    end
+  end
+
+  describe '#set_property' do
+    it 'sets the property' do
+      expect(described_class).to receive(:vsctl)
+        .with('set', 'Port', 'testport', 'key=value')
+      described_class.set_property('Port', 'testport', 'key', 'value')
+    end
+    it 'sets the property (integer)' do
+      expect(described_class).to receive(:vsctl)
+        .with('set', 'Port', 'testport', 'key=1')
+      described_class.set_property('Port', 'testport', 'key', 1)
+    end
+    it 'sets the property (boolean)' do
+      expect(described_class).to receive(:vsctl)
+        .with('set', 'Port', 'testport', 'key=true')
+      described_class.set_property('Port', 'testport', 'key', true)
+    end
+    it 'clears the property when nil' do
+      expect(described_class).to receive(:vsctl)
+        .with('clear', 'Port', 'testport', 'key')
+      described_class.set_property('Port', 'testport', 'key')
+    end
+    it 'clears the property when empty (string)' do
+      expect(described_class).to receive(:vsctl)
+        .with('clear', 'Port', 'testport', 'key')
+      described_class.set_property('Port', 'testport', 'key', '')
+    end
+    it 'clears the property when empty (array)' do
+      expect(described_class).to receive(:vsctl)
+        .with('clear', 'Port', 'testport', 'key')
+      described_class.set_property('Port', 'testport', 'key', [])
+    end
+  end
+
+  describe '#get_other_config' do
+    it 'returns the configs' do
+      expect(described_class).to receive(:vsctl)
+        .with('get', 'Port', 'testport', 'other_config')
+        .and_return('{key1=value1,key2=value2}')
+      expect(described_class.get_other_config('Port', 'testport', 'key1')).to eq('value1')
+    end
+    it 'returns the configs when not exist' do
+      expect(described_class).to receive(:vsctl)
+        .with('get', 'Port', 'testport', 'other_config')
+        .and_return('{key1=value1,key2=value2}')
+      expect(described_class.get_other_config('Port', 'testport', 'key3')).to eq(nil)
+    end
+  end
+
+  describe '#set_other_config' do
+    it 'sets the configs' do
+      expect(described_class).to receive(:vsctl)
+        .with('set', 'Port', 'testport', 'other_config:key=value')
+      described_class.set_other_config('Port', 'testport', 'key', 'value')
+    end
+    it 'sets the configs (integer)' do
+      expect(described_class).to receive(:vsctl)
+        .with('set', 'Port', 'testport', 'other_config:key=1')
+      described_class.set_other_config('Port', 'testport', 'key', 1)
+    end
+    it 'sets the configs (boolean)' do
+      expect(described_class).to receive(:vsctl)
+        .with('set', 'Port', 'testport', 'other_config:key=true')
+      described_class.set_other_config('Port', 'testport', 'key', true)
+    end
+    it 'clears the configs when nil' do
+      expect(described_class).to receive(:vsctl)
+        .with('remove', 'Port', 'testport', 'other_config', 'key')
+      described_class.set_other_config('Port', 'testport', 'key')
+    end
+    it 'clears the configs when empty (string)' do
+      expect(described_class).to receive(:vsctl)
+        .with('remove', 'Port', 'testport', 'other_config', 'key')
+      described_class.set_other_config('Port', 'testport', 'key', '')
+    end
+    it 'clears the configs when empty (array)' do
+      expect(described_class).to receive(:vsctl)
+        .with('remove', 'Port', 'testport', 'other_config', 'key')
+      described_class.set_other_config('Port', 'testport', 'key', [])
+    end
+  end
+
+  describe '#parse_hash' do
+    it 'parse hash value with default splitter' do
+      expect(described_class.parse_hash('a=b,c=d')).to eq({'a' => 'b', 'c' => 'd'})
+    end
+    it 'parse hash value with custom splitter' do
+      expect(described_class.parse_hash('a=b
+c=d', "\n")).to eq({'a' => 'b', 'c' => 'd'})
+    end
+  end
+end

spec/unit/provider/vs_bridge/ovs_spec.rb

@@ -66,7 +66,7 @@ describe Puppet::Type.type(:vs_bridge).provider(:ovs) do
         )
 
         expect(described_class).to receive(:vsctl).with(
-          'set', 'Bridge', 'testbr', 'other-config:mac-table-size=60000'
+          'set', 'Bridge', 'testbr', 'other_config:mac-table-size=60000'
         )
 
         provider.create
@@ -125,7 +125,7 @@ k3=v3')
   describe '#mac_table_size' do
     it 'returns mac table size' do
       expect(described_class).to receive(:vsctl).with(
-        'get', 'Bridge', 'testbr', 'other-config'
+        'get', 'Bridge', 'testbr', 'other_config'
       ).and_return(
         '{disable-in-band="true", mac-table-size="50000"}'
       )
@@ -136,7 +136,7 @@ k3=v3')
   describe '#mac_table_size=' do
     it 'sets mac table size' do
       expect(described_class).to receive(:vsctl).with(
-        'set', 'Bridge', 'testbr', 'other-config:mac-table-size=60000'
+        'set', 'Bridge', 'testbr', 'other_config:mac-table-size=60000'
       )
       provider.mac_table_size = 60000
     end

spec/unit/provider/vs_port/ovs_spec.rb

@@ -51,22 +51,33 @@ yetanothertestport')
   end
 
   describe '#interface' do
-    it 'returns interface' do
+    it 'returns interface if empty' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'get', 'Port', 'testport', 'interfaces'
+        'get', 'Port', 'testport', 'interfaces'
+      ).and_return('[]')
+      expect(provider.interface).to eq([])
+    end
+    it 'returns interfaceg' do
+      expect(described_class).to receive(:vsctl).with(
+        'get', 'Port', 'testport', 'interfaces'
       ).and_return('[c9b76714-e353-4b02-ae0f-36b9e6fce5af]')
       expect(described_class).to receive(:vsctl).with(
         'get', 'Interface', 'c9b76714-e353-4b02-ae0f-36b9e6fce5af', 'name'
       ).and_return('testif')
-
       expect(provider.interface).to eq(['testif'])
     end
   end
 
   describe '#bond_mode' do
-    it 'returns bond mode' do
+    it 'returns bond mode if empty' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'get', 'Port', 'testport', 'bond_mode'
+        'get', 'Port', 'testport', 'bond_mode'
+      ).and_return('[]')
+      expect(provider.bond_mode).to eq('')
+    end
+    it 'returns bond modeg' do
+      expect(described_class).to receive(:vsctl).with(
+        'get', 'Port', 'testport', 'bond_mode'
       ).and_return('balance-slb')
       expect(provider.bond_mode).to eq('balance-slb')
     end
@@ -75,20 +86,26 @@ yetanothertestport')
   describe '#bond_mode=' do
     it 'configures bond mode' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'set', 'Port', 'testport', 'bond_mode=balance-slb')
+        'set', 'Port', 'testport', 'bond_mode=balance-slb')
       provider.bond_mode = 'balance-slb'
     end
     it 'clears bond mode' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'clear', 'Port', 'testport', 'bond_mode')
+        'clear', 'Port', 'testport', 'bond_mode')
       provider.bond_mode = ''
     end
   end
 
   describe '#lacp' do
+    it 'returns lacp if empty' do
+      expect(described_class).to receive(:vsctl).with(
+        'get', 'Port', 'testport', 'lacp'
+      ).and_return('[]')
+      expect(provider.lacp).to eq('')
+    end
     it 'returns lacp' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'get', 'Port', 'testport', 'lacp'
+        'get', 'Port', 'testport', 'lacp'
       ).and_return('active')
       expect(provider.lacp).to eq('active')
     end
@@ -97,16 +114,22 @@ yetanothertestport')
   describe '#lacp=' do
     it 'configures lacp' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'set', 'Port', 'testport', 'lacp=active')
+        'set', 'Port', 'testport', 'lacp=active')
       provider.lacp = 'active'
     end
   end
 
   describe '#lacp_time' do
+    it 'returns lacp_time if empty' do
+      expect(described_class).to receive(:vsctl).with(
+        'get', 'Port', 'testport', 'other_config'
+      ).and_return('{}')
+      expect(provider.lacp_time).to eq('')
+    end
     it 'returns lacp_time' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'get', 'Port', 'testport', 'other_config:lacp-time'
+        'get', 'Port', 'testport', 'other_config'
-      ).and_return('fast')
+      ).and_return('{lacp-time="fast"}')
       expect(provider.lacp_time).to eq('fast')
     end
   end
@@ -114,20 +137,26 @@ yetanothertestport')
   describe '#lacp_time=' do
     it 'configures lacp_time' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'set', 'Port', 'testport', 'other_config:lacp-time=fast')
+        'set', 'Port', 'testport', 'other_config:lacp-time=fast')
       provider.lacp_time = 'fast'
     end
     it 'clears lacp_time' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'remove', 'Port', 'testport', ['other_config', 'lacp-time'])
+        'remove', 'Port', 'testport', 'other_config', 'lacp-time')
       provider.lacp_time = ''
     end
   end
 
   describe '#vlan_mode' do
+    it 'returns vlan_mode if empty' do
+      expect(described_class).to receive(:vsctl).with(
+        'get', 'Port', 'testport', 'vlan_mode'
+      ).and_return('[]')
+      expect(provider.vlan_mode).to eq('')
+    end
     it 'returns vlan_mode' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'get', 'Port', 'testport', 'vlan_mode'
+        'get', 'Port', 'testport', 'vlan_mode'
       ).and_return('native-tagged')
       expect(provider.vlan_mode).to eq('native-tagged')
     end
@@ -136,20 +165,26 @@ yetanothertestport')
   describe '#vlan_mode=' do
     it 'configures vlan_mode' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'set', 'Port', 'testport', 'vlan_mode=native-tagged')
+        'set', 'Port', 'testport', 'vlan_mode=native-tagged')
       provider.vlan_mode = 'native-tagged'
     end
     it 'clears vlan_mode' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'clear', 'Port', 'testport', 'vlan_mode')
+        'clear', 'Port', 'testport', 'vlan_mode')
       provider.vlan_mode = ''
     end
   end
 
   describe '#vlan_tag' do
+    it 'returns vlan_tag if empty' do
+      expect(described_class).to receive(:vsctl).with(
+        'get', 'Port', 'testport', 'tag'
+      ).and_return('[]')
+      expect(provider.vlan_tag).to eq('')
+    end
     it 'returns vlan_tag' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'get', 'Port', 'testport', 'tag'
+        'get', 'Port', 'testport', 'tag'
       ).and_return('100')
       expect(provider.vlan_tag).to eq('100')
     end
@@ -158,20 +193,26 @@ yetanothertestport')
   describe '#vlan_tag=' do
     it 'configures vlan_tag' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'set', 'Port', 'testport', 'tag=100')
+        'set', 'Port', 'testport', 'tag=100')
       provider.vlan_tag = '100'
     end
     it 'clears vlan_tag' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'clear', 'Port', 'testport', 'tag')
+        'clear', 'Port', 'testport', 'tag')
       provider.vlan_tag = ''
     end
   end
 
   describe '#vlan_trunks' do
+    it 'returns vlan_trunks if empty' do
+      expect(described_class).to receive(:vsctl).with(
+        'get', 'Port', 'testport', 'trunks'
+      ).and_return('[]')
+      expect(provider.vlan_trunks).to eq([])
+    end
     it 'returns vlan_trunks' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'get', 'Port', 'testport', 'trunks'
+        'get', 'Port', 'testport', 'trunks'
       ).and_return('[0 1 2]')
       expect(provider.vlan_trunks).to eq(['0', '1', '2'])
     end
@@ -180,12 +221,12 @@ yetanothertestport')
   describe '#vlan_trunks=' do
     it 'configures vlan_trunks' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'set', 'Port', 'testport', 'trunks=0 1 2')
+        'set', 'Port', 'testport', 'trunks=0 1 2')
       provider.vlan_trunks = ['0', '1', '2']
     end
     it 'clears vlan_trunks' do
       expect(described_class).to receive(:vsctl).with(
-        '--if-exists', 'clear', 'Port', 'testport', 'trunks')
+        'clear', 'Port', 'testport', 'trunks')
       provider.vlan_trunks = []
     end
   end