From 179600217256166aad072c25349e1a656e715ad4 Mon Sep 17 00:00:00 2001 From: Michael Polenchuk Date: Tue, 8 Nov 2016 12:34:41 +0300 Subject: [PATCH] Bring l2_bridge/dpdkovs provider in Get all dpdk/ovs bridge related code into single provider of l2_bridge/dpdkovs. Closes-Bug: #1638036 Change-Id: I922c4a0770216bb6c797984e495af78be54f54f6 --- .../l23_stored_config/dpdkovs_ubuntu.rb | 84 +++++++++++++----- .../provider/l23_stored_config/ovs_ubuntu.rb | 17 ---- .../lib/puppet/provider/l2_bridge/dpdkovs.rb | 36 ++++++++ .../lib/puppet/provider/l2_bridge/ovs.rb | 16 +--- .../dpdkovs_ubuntu__spec/ifcfg-bridge | 11 +++ .../ovs_ubuntu__spec/ifcfg-bridge | 1 - .../dpdkovs_ubuntu__bridge__spec.rb | 85 +++++++++++++++++++ .../ovs_ubuntu__bridge__spec.rb | 5 +- .../provider/l2_patch/ovs_patch__spec.rb | 6 +- .../puppet/provider/l2_port/dpdkovs__spec.rb | 7 +- .../unit/puppet/provider/l2_port/ovs__spec.rb | 2 +- 11 files changed, 203 insertions(+), 67 deletions(-) create mode 100644 deployment/puppet/l23network/lib/puppet/provider/l2_bridge/dpdkovs.rb create mode 100644 deployment/puppet/l23network/spec/fixtures/provider/l23_stored_config/dpdkovs_ubuntu__spec/ifcfg-bridge create mode 100644 deployment/puppet/l23network/spec/unit/puppet/provider/l23_stored_config/dpdkovs_ubuntu__bridge__spec.rb diff --git a/deployment/puppet/l23network/lib/puppet/provider/l23_stored_config/dpdkovs_ubuntu.rb b/deployment/puppet/l23network/lib/puppet/provider/l23_stored_config/dpdkovs_ubuntu.rb index 173502253b..e44d26db6a 100644 --- a/deployment/puppet/l23network/lib/puppet/provider/l23_stored_config/dpdkovs_ubuntu.rb +++ b/deployment/puppet/l23network/lib/puppet/provider/l23_stored_config/dpdkovs_ubuntu.rb @@ -23,6 +23,9 @@ Puppet::Type.type(:l23_stored_config).provide(:dpdkovs_ubuntu, :parent => Puppet if_data[:if_type] = "bond" if_data[:if_provider] = :dpdkovs true + elsif if_data[:if_type] == :bridge + if_data[:if_provider] = :dpdkovs + true else if_data[:if_provider] = nil false @@ -30,23 +33,22 @@ Puppet::Type.type(:l23_stored_config).provide(:dpdkovs_ubuntu, :parent => Puppet end def self.property_mappings - rv = super - rv.merge!({ - :if_type => 'ovs_type', - :bridge => 'ovs_bridge', - :dpdk_port => 'dpdk_port', - :bond_slaves => 'ovs_bonds', - :bond_mode => 'ovs_options', - :bond_miimon => 'ovs_options', - :bond_use_carrier => 'ovs_options', - :bond_lacp_rate => 'ovs_options', - :bond_lacp => 'ovs_options', + super.merge( + :if_type => 'ovs_type', + :bridge => 'ovs_bridge', + :bridge_ports => 'ovs_ports', + :dpdk_port => 'dpdk_port', + :bond_slaves => 'ovs_bonds', + :bond_mode => 'ovs_options', + :bond_miimon => 'ovs_options', + :bond_use_carrier => 'ovs_options', + :bond_lacp_rate => 'ovs_options', + :bond_lacp => 'ovs_options', :bond_xmit_hash_policy => '', # unused - :bond_ad_select => '', - :bond_updelay => 'ovs_options', - :bond_downdelay => 'ovs_options', - }) - return rv + :bond_ad_select => '', + :bond_updelay => 'ovs_options', + :bond_downdelay => 'ovs_options', + ) end def self.oneline_properties @@ -84,15 +86,35 @@ Puppet::Type.type(:l23_stored_config).provide(:dpdkovs_ubuntu, :parent => Puppet def oneline_properties self.class.collected_properties end + + def self.collected_properties + super.merge( + :datapath_type => { + :detect_re => /(ovs_)?extra\s+set\s+Bridge\s+([a-z][0-9a-z\-]*[0-9a-z])\s+datapath_type=([a-z]+)/, + :detect_shift => 3, + }, + ) + end + def self.iface_file_header(provider) header = [] props = {} - bridge = provider.bridge[0] - props[:bridge] = bridge + header << self.puppet_header - header << "allow-#{bridge} #{provider.name}" + bridge = provider.bridge[0] + + if provider.if_type == :bridge + header << "auto #{provider.name}" if provider.onboot + header << "allow-ovs #{provider.name}" + props[:bridge] = nil + else + header << "allow-#{bridge} #{provider.name}" + props[:bridge] = bridge + end + header << "iface #{provider.name} inet #{provider.method}" - return header, props + + [header, props] end def dpdk_port @@ -121,10 +143,26 @@ Puppet::Type.type(:l23_stored_config).provide(:dpdkovs_ubuntu, :parent => Puppet end def self.unmangle__if_type(provider, val) - val = 'DPDKOVSPort' if val.to_s == 'ethernet' - val = 'DPDKOVSBond' if val.to_s == 'bond' - val + case val + when :ethernet; 'DPDKOVSPort' + when :bond; 'DPDKOVSBond' + else "OVS#{val.to_s.capitalize}" + end end + + def self.mangle__if_type(val) + val.sub(/^OVS/, '').downcase.to_sym + end + + def self.unmangle__datapath_type(provider, val) + ["ovs_extra set Bridge #{provider.name} datapath_type=#{provider.datapath_type}"] \ + if provider.if_type == :bridge && provider.datapath_type + end + + def self.mangle__datapath_type(data) + data.join + end + end # vim: set ts=2 sw=2 et : diff --git a/deployment/puppet/l23network/lib/puppet/provider/l23_stored_config/ovs_ubuntu.rb b/deployment/puppet/l23network/lib/puppet/provider/l23_stored_config/ovs_ubuntu.rb index 49a56723b4..aa4318df4a 100644 --- a/deployment/puppet/l23network/lib/puppet/provider/l23_stored_config/ovs_ubuntu.rb +++ b/deployment/puppet/l23network/lib/puppet/provider/l23_stored_config/ovs_ubuntu.rb @@ -144,10 +144,6 @@ Puppet::Type.type(:l23_stored_config).provide(:ovs_ubuntu, :parent => Puppet::Pr :detect_re => /(ovs_)?extra\s+--\s+set\s+Port\s+(.*[\d+])\s+tag=(\d+)/, :detect_shift => 3, }, - :datapath_type => { - :detect_re => /(ovs_)?extra\s+set\s+Bridge\s+([a-z][0-9a-z\-]*[0-9a-z])\s+datapath_type=([a-z]+)/, - :detect_shift => 3, - }, }) return rv end @@ -176,18 +172,5 @@ Puppet::Type.type(:l23_stored_config).provide(:ovs_ubuntu, :parent => Puppet::Pr data.join() end - def self.unmangle__datapath_type(provider, val) - if provider.if_type.to_s == 'bridge' - if provider.datapath_type - rv = [] - rv << "ovs_extra set Bridge #{provider.name} datapath_type=#{provider.datapath_type}" - end - end - end - - def self.mangle__datapath_type(data) - data.join() - end - end # vim: set ts=2 sw=2 et : diff --git a/deployment/puppet/l23network/lib/puppet/provider/l2_bridge/dpdkovs.rb b/deployment/puppet/l23network/lib/puppet/provider/l2_bridge/dpdkovs.rb new file mode 100644 index 0000000000..eea3040804 --- /dev/null +++ b/deployment/puppet/l23network/lib/puppet/provider/l2_bridge/dpdkovs.rb @@ -0,0 +1,36 @@ +Puppet::Type.type(:l2_bridge).provide(:dpdkovs, :parent => :ovs, :source => :ovs) do + + commands :vsctl => 'ovs-vsctl' + + def create + debug("CREATE resource: #{@resource}") + @old_property_hash = {} + @property_flush = {}.merge! @resource + + vendor_specific = @resource[:vendor_specific] || {} + + datapath_type = vendor_specific['datapath_type'] + cmd = ['add-br', @resource[:bridge]] + cmd += ['--', 'set', 'Bridge', @resource[:bridge], "datapath_type=#{datapath_type}"] if datapath_type + vsctl(cmd) + + # set vxlan id + vlan_id = vendor_specific['vlan_id'] + vsctl('set', 'Port', @resource[:bridge], "tag=#{vlan_id}") if vlan_id + + self.class.interface_up(@resource[:bridge]) + notice("bridge '#{@resource[:bridge]}' created.") + end + + def flush + unless @property_flush.empty? + super + # handle vxlan id changes + if @property_flush[:vendor_specific] && @property_flush[:vendor_specific] != :absent + vlan_id = @property_flush[:vendor_specific]['vlan_id'] || [] + vsctl('set', 'Port', @resource[:bridge], "tag=#{vlan_id}") + end + end + end + +end diff --git a/deployment/puppet/l23network/lib/puppet/provider/l2_bridge/ovs.rb b/deployment/puppet/l23network/lib/puppet/provider/l2_bridge/ovs.rb index 8f8979db92..16a848c0b0 100644 --- a/deployment/puppet/l23network/lib/puppet/provider/l2_bridge/ovs.rb +++ b/deployment/puppet/l23network/lib/puppet/provider/l2_bridge/ovs.rb @@ -23,16 +23,8 @@ Puppet::Type.type(:l2_bridge).provide(:ovs, :parent => Puppet::Provider::Ovs_bas debug("CREATE resource: #{@resource}") @old_property_hash = {} @property_flush = {}.merge! @resource - vendor_specific = @resource[:vendor_specific] || {} - datapath_type = vendor_specific["datapath_type"] - cmd = ['add-br', @resource[:bridge]] - cmd += ['--', 'set', 'Bridge', @resource[:bridge], "datapath_type=#{datapath_type}"] if datapath_type - vsctl(cmd) - - # set vxlan id - vlan_id = vendor_specific['vlan_id'] - vsctl('set', 'Port', @resource[:bridge], "tag=#{vlan_id}") if vlan_id + vsctl('add-br', @resource[:bridge]) self.class.interface_up(@resource[:bridge]) notice("bridge '#{@resource[:bridge]}' created.") end @@ -61,12 +53,6 @@ Puppet::Type.type(:l2_bridge).provide(:ovs, :parent => Puppet::Provider::Ovs_bas end end # - # handle vxlan tag changes - if @property_flush[:vendor_specific] and @property_flush[:vendor_specific] != :absent - vlan_id = @property_flush[:vendor_specific]['vlan_id'] || [] - vsctl('set', 'Port', @resource[:bridge], "tag=#{vlan_id}") - end - @property_hash = resource.to_hash end end diff --git a/deployment/puppet/l23network/spec/fixtures/provider/l23_stored_config/dpdkovs_ubuntu__spec/ifcfg-bridge b/deployment/puppet/l23network/spec/fixtures/provider/l23_stored_config/dpdkovs_ubuntu__spec/ifcfg-bridge new file mode 100644 index 0000000000..066cd77048 --- /dev/null +++ b/deployment/puppet/l23network/spec/fixtures/provider/l23_stored_config/dpdkovs_ubuntu__spec/ifcfg-bridge @@ -0,0 +1,11 @@ +# ********************************************************************* +# This file is being managed by Puppet. Changes to interfaces +# that are not being managed by Puppet will persist; +# however changes to interfaces that are being managed by Puppet will +# be overwritten. +# ********************************************************************* +auto br0 +allow-ovs br0 +iface br0 inet manual + ovs_type OVSBridge + ovs_extra set Bridge br0 datapath_type=netdev diff --git a/deployment/puppet/l23network/spec/fixtures/provider/l23_stored_config/ovs_ubuntu__spec/ifcfg-bridge b/deployment/puppet/l23network/spec/fixtures/provider/l23_stored_config/ovs_ubuntu__spec/ifcfg-bridge index 03ab3be309..30beaed2bb 100644 --- a/deployment/puppet/l23network/spec/fixtures/provider/l23_stored_config/ovs_ubuntu__spec/ifcfg-bridge +++ b/deployment/puppet/l23network/spec/fixtures/provider/l23_stored_config/ovs_ubuntu__spec/ifcfg-bridge @@ -8,4 +8,3 @@ allow-ovs br9 iface br9 inet manual ovs_type OVSBridge mtu 9000 - ovs_extra set Bridge br9 datapath_type=netdev \ No newline at end of file diff --git a/deployment/puppet/l23network/spec/unit/puppet/provider/l23_stored_config/dpdkovs_ubuntu__bridge__spec.rb b/deployment/puppet/l23network/spec/unit/puppet/provider/l23_stored_config/dpdkovs_ubuntu__bridge__spec.rb new file mode 100644 index 0000000000..f01a0e2719 --- /dev/null +++ b/deployment/puppet/l23network/spec/unit/puppet/provider/l23_stored_config/dpdkovs_ubuntu__bridge__spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' +require 'yaml' + +describe Puppet::Type.type(:l23_stored_config).provider(:dpdkovs_ubuntu) do + + let(:input_data) do + { + :br0 => { + :name => 'br0', + :ensure => 'present', + :if_type => 'bridge', + :onboot => true, + :method => 'manual', + :provider => 'dpdkovs_ubuntu', + :datapath_type => 'netdev', + }, + } + end + + let(:resources) do + resources = {} + input_data.each do |name, res| + resources.store name, Puppet::Type.type(:l23_stored_config).new(res) + end + return resources + end + + let(:providers) do + providers = {} + resources.each do |name, resource| + provider = resource.provider + if ENV['SPEC_PUPPET_DEBUG'] + class << provider + def debug(msg) + puts msg + end + end + end + provider.create + providers.store name, provider + end + providers + end + + before(:each) do + puppet_debug_override() + end + + def fixture_path + File.join(PROJECT_ROOT, 'spec', 'fixtures', 'provider', 'l23_stored_config', 'dpdkovs_ubuntu__spec') + end + + def fixture_file(file) + File.join(fixture_path, file) + end + + def fixture_data(file) + File.read(fixture_file(file)) + end + + context "standalone DPDKOVS bridge" do + + context 'format file' do + subject { providers[:br0] } + let(:cfg_file) { subject.class.format_file('filepath', [subject]) } + it { expect(cfg_file).to match(/auto\s+br0/) } + it { expect(cfg_file).to match(/allow-ovs\s+br0/) } + it { expect(cfg_file).to match(/iface\s+br0\s+inet\s+manual/) } + it { expect(cfg_file).to match(/ovs_type\s+OVSBridge/) } + it { expect(cfg_file).to match(/ovs_extra\s+set\s+Bridge\s+br0\s+datapath_type=netdev/) } + it { expect(cfg_file.split(/\n/).reject{|x| x=~/(^\s*$)|(^#.*$)/}.length). to eq(5) } # no more lines in the interface file + end + + context "parse data from fixture" do + let(:res) { subject.class.parse_file('br0', fixture_data('ifcfg-bridge'))[0] } + it { expect(res[:method]).to eq :manual } + it { expect(res[:name]).to eq 'br0' } + it { expect(res[:if_type].to_s).to eq 'bridge' } + it { expect(res[:if_provider].to_s).to eq 'dpdkovs' } + it { expect(res[:datapath_type].to_s).to eq 'netdev' } + end + + end + +end diff --git a/deployment/puppet/l23network/spec/unit/puppet/provider/l23_stored_config/ovs_ubuntu__bridge__spec.rb b/deployment/puppet/l23network/spec/unit/puppet/provider/l23_stored_config/ovs_ubuntu__bridge__spec.rb index 7f47004a9e..bd839727fa 100644 --- a/deployment/puppet/l23network/spec/unit/puppet/provider/l23_stored_config/ovs_ubuntu__bridge__spec.rb +++ b/deployment/puppet/l23network/spec/unit/puppet/provider/l23_stored_config/ovs_ubuntu__bridge__spec.rb @@ -13,7 +13,6 @@ describe Puppet::Type.type(:l23_stored_config).provider(:ovs_ubuntu) do :onboot => true, :method => 'manual', :provider => "ovs_ubuntu", - :datapath_type => "netdev", }, } end @@ -76,8 +75,7 @@ describe Puppet::Type.type(:l23_stored_config).provider(:ovs_ubuntu) do it { expect(cfg_file).to match(/iface\s+br9\s+inet\s+manual/) } it { expect(cfg_file).to match(/ovs_type\s+OVSBridge/) } it { expect(cfg_file).to match(/mtu\s+9000/) } - it { expect(cfg_file).to match(/ovs_extra\s+set\s+Bridge\s+br9\s+datapath_type=netdev/) } - it { expect(cfg_file.split(/\n/).reject{|x| x=~/(^\s*$)|(^#.*$)/}.length). to eq(6) } # no more lines in the interface file + it { expect(cfg_file.split(/\n/).reject{|x| x=~/(^\s*$)|(^#.*$)/}.length). to eq(5) } # no more lines in the interface file end context "parse data from fixture" do @@ -87,7 +85,6 @@ describe Puppet::Type.type(:l23_stored_config).provider(:ovs_ubuntu) do it { expect(res[:mtu]).to eq '9000' } it { expect(res[:if_type].to_s).to eq 'bridge' } it { expect(res[:if_provider].to_s).to eq 'ovs' } - it { expect(res[:datapath_type].to_s).to eq 'netdev' } end end diff --git a/deployment/puppet/l23network/spec/unit/puppet/provider/l2_patch/ovs_patch__spec.rb b/deployment/puppet/l23network/spec/unit/puppet/provider/l2_patch/ovs_patch__spec.rb index 4b6b7c6daf..feaa24eac2 100644 --- a/deployment/puppet/l23network/spec/unit/puppet/provider/l2_patch/ovs_patch__spec.rb +++ b/deployment/puppet/l23network/spec/unit/puppet/provider/l2_patch/ovs_patch__spec.rb @@ -43,8 +43,8 @@ describe Puppet::Type.type(:l2_patch).provider(:ovs) do puppet_debug_override() provider_br1.class.stubs(:iproute) provider_br2.class.stubs(:iproute) - provider_br1.class.stubs(:vsctl).with(['add-br', 'br1']).returns(true) - provider_br2.class.stubs(:vsctl).with(['add-br', 'br2']).returns(true) + provider_br1.class.stubs(:vsctl).with('add-br', 'br1').returns(true) + provider_br2.class.stubs(:vsctl).with('add-br', 'br2').returns(true) provider_patch.class.stubs(:vsctl).with([ '--may-exist', 'add-port', 'br1', 'p_39a440c1-0', '--', 'set', 'Interface', 'p_39a440c1-0', 'type=patch', 'option:peer=p_39a440c1-1' ]).returns(true) @@ -80,7 +80,7 @@ describe Puppet::Type.type(:l2_patch).provider(:ovs) do Puppet::Util::Log.newdestination(:console) end provider_br1.class.stubs(:iproute) - provider_br1.class.stubs(:vsctl).with(['add-br', 'br1']).returns(true) + provider_br1.class.stubs(:vsctl).with('add-br', 'br1').returns(true) provider_br2.class.stubs(:iproute).with().returns(true) provider_br2.class.stubs(:iproute).with('link', 'set', 'up', 'dev', 'br2').returns(true) provider_br2.stubs(:brctl).with(['addbr', 'br2']).returns(true) diff --git a/deployment/puppet/l23network/spec/unit/puppet/provider/l2_port/dpdkovs__spec.rb b/deployment/puppet/l23network/spec/unit/puppet/provider/l2_port/dpdkovs__spec.rb index bd645d4121..6edd1f1a30 100644 --- a/deployment/puppet/l23network/spec/unit/puppet/provider/l2_port/dpdkovs__spec.rb +++ b/deployment/puppet/l23network/spec/unit/puppet/provider/l2_port/dpdkovs__spec.rb @@ -3,11 +3,12 @@ require 'spec_helper' describe Puppet::Type.type(:l2_port).provider(:dpdkovs) do let(:resource_br1) { Puppet::Type.type(:l2_bridge).new( - :provider => 'ovs', + :provider => 'dpdkovs', :name => 'br1', :bridge => 'br1', :vendor_specific => { - :datapath_type => 'netdev', + :datapath_type => 'netdev', + :vlan_id => '10', } ) } @@ -33,7 +34,7 @@ describe Puppet::Type.type(:l2_port).provider(:dpdkovs) do puppet_debug_override() provider_br1.class.stubs(:vsctl).with(['add-br', 'br1', '--', 'set', 'Bridge', 'br1', 'datapath_type=netdev']).returns(true) provider_br1.class.stubs(:vsctl).with('set', 'Bridge', 'br1', 'stp_enable=false').returns(true) - provider_br1.class.stubs(:vsctl).with('set', 'Port', 'br1', 'tag=[]').returns(true) + provider_br1.class.stubs(:vsctl).with('set', 'Port', 'br1', 'tag=10').returns(true) provider_br1.class.stubs(:interface_up).with('br1').returns(true) end diff --git a/deployment/puppet/l23network/spec/unit/puppet/provider/l2_port/ovs__spec.rb b/deployment/puppet/l23network/spec/unit/puppet/provider/l2_port/ovs__spec.rb index d6e95e5a96..ae50ce6a4a 100644 --- a/deployment/puppet/l23network/spec/unit/puppet/provider/l2_port/ovs__spec.rb +++ b/deployment/puppet/l23network/spec/unit/puppet/provider/l2_port/ovs__spec.rb @@ -33,7 +33,7 @@ describe Puppet::Type.type(:l2_port).provider(:ovs) do before(:each) do puppet_debug_override() - provider_br1.class.stubs(:vsctl).with(['add-br', 'br1']).returns(true) + provider_br1.class.stubs(:vsctl).with('add-br', 'br1').returns(true) provider_br1.class.stubs(:interface_up).with('br1').returns(true) end