From 32ca5213d5efe0042b9e7f7fe1f0b874a4107155 Mon Sep 17 00:00:00 2001 From: Cody Herriges Date: Mon, 22 Jun 2015 13:53:42 +0200 Subject: [PATCH] Include tests that test the use of create. This commit will add new tests for neutron providers that have the create method. Includes a fix to the Puppet::Provider::Neutron_subnet which was parsing host_routes backwards and replaces the use of the model method with @resource. Without this we are not successfully testing for methods removed in the Puppet 3 to 4 transition. Co-Authored-By: Lukas Bezdicka Change-Id: I82382b9f9da7db6910a3d9b5c7060c3f03473585 Close-Bug: #1466555 --- .../provider/neutron_network/neutron.rb | 2 +- lib/puppet/provider/neutron_port/neutron.rb | 2 +- lib/puppet/provider/neutron_router/neutron.rb | 2 +- lib/puppet/provider/neutron_subnet/neutron.rb | 10 +- .../provider/neutron_network/neutron_spec.rb | 123 ++++++++++++------ .../provider/neutron_port/neutron_spec.rb | 57 ++++++++ .../provider/neutron_router/neutron_spec.rb | 36 ++++- .../new_neutron_spec.rb | 49 +++++++ .../provider/neutron_subnet/neutron_spec.rb | 54 ++++++-- 9 files changed, 271 insertions(+), 64 deletions(-) create mode 100644 spec/unit/provider/neutron_port/neutron_spec.rb create mode 100644 spec/unit/provider/neutron_router_interface/new_neutron_spec.rb diff --git a/lib/puppet/provider/neutron_network/neutron.rb b/lib/puppet/provider/neutron_network/neutron.rb index e5a0c39bf..b1ef75331 100644 --- a/lib/puppet/provider/neutron_network/neutron.rb +++ b/lib/puppet/provider/neutron_network/neutron.rb @@ -58,7 +58,7 @@ Puppet::Type.type(:neutron_network).provide( end if @resource[:tenant_name] - tenant_id = self.class.get_tenant_id(model.catalog, + tenant_id = self.class.get_tenant_id(@resource.catalog, @resource[:tenant_name]) network_opts << "--tenant_id=#{tenant_id}" elsif @resource[:tenant_id] diff --git a/lib/puppet/provider/neutron_port/neutron.rb b/lib/puppet/provider/neutron_port/neutron.rb index 609bc63a8..14da7c5e8 100644 --- a/lib/puppet/provider/neutron_port/neutron.rb +++ b/lib/puppet/provider/neutron_port/neutron.rb @@ -72,7 +72,7 @@ Puppet::Type.type(:neutron_port).provide( if @resource[:tenant_name] tenant_id = self.class.get_tenant_id( - model.catalog, + @resource.catalog, @resource[:tenant_name] ) opts << "--tenant_id=#{tenant_id}" diff --git a/lib/puppet/provider/neutron_router/neutron.rb b/lib/puppet/provider/neutron_router/neutron.rb index fa5f7f02a..7cf36e54d 100644 --- a/lib/puppet/provider/neutron_router/neutron.rb +++ b/lib/puppet/provider/neutron_router/neutron.rb @@ -51,7 +51,7 @@ Puppet::Type.type(:neutron_router).provide( end if @resource[:tenant_name] - tenant_id = self.class.get_tenant_id(model.catalog, + tenant_id = self.class.get_tenant_id(@resource.catalog, @resource[:tenant_name]) opts << "--tenant_id=#{tenant_id}" elsif @resource[:tenant_id] diff --git a/lib/puppet/provider/neutron_subnet/neutron.rb b/lib/puppet/provider/neutron_subnet/neutron.rb index 0da21804d..0503928d0 100644 --- a/lib/puppet/provider/neutron_subnet/neutron.rb +++ b/lib/puppet/provider/neutron_subnet/neutron.rb @@ -69,10 +69,10 @@ Puppet::Type.type(:neutron_subnet).provide( host_routes = [] return [] if values.empty? for value in Array(values) - matchdata = /\{\s*"destination"\s*:\s*"(.*)"\s*,\s*"nexthop"\s*:\s*"(.*)"\s*\}/.match(value.gsub(/\\"/,'"')) - destination = matchdata[1] - nexthop = matchdata[2] - host_routes << "destination=#{destination},nexthop=#{nexthop}" + matchdata = /\{\s*"nexthop"\s*:\s*"(.*)"\s*,\s*"destination"\s*:\s*"(.*)"\s*\}/.match(value.gsub(/\\"/,'"')) + nexthop = matchdata[1] + destination = matchdata[2] + host_routes << "nexthop=#{nexthop},destination=#{destination}" end return host_routes end @@ -126,7 +126,7 @@ Puppet::Type.type(:neutron_subnet).provide( end if @resource[:tenant_name] - tenant_id = self.class.get_tenant_id(model.catalog, + tenant_id = self.class.get_tenant_id(@resource.catalog, @resource[:tenant_name]) opts << "--tenant_id=#{tenant_id}" elsif @resource[:tenant_id] diff --git a/spec/unit/provider/neutron_network/neutron_spec.rb b/spec/unit/provider/neutron_network/neutron_spec.rb index 974a7057b..d01a009bc 100644 --- a/spec/unit/provider/neutron_network/neutron_spec.rb +++ b/spec/unit/provider/neutron_network/neutron_spec.rb @@ -17,55 +17,100 @@ describe provider_class do :admin_state_up => 'True', :router_external => 'False', :shared => 'False', - :tenant_id => '', + :tenant_id => '60f9544eb94c42a6b7e8e98c2be981b1', } end - describe 'when updating a network' do + let :resource do + Puppet::Type::Neutron_network.new(net_attrs) + end + + let :provider do + provider_class.new(resource) + end + + shared_examples 'neutron_network' do + + describe 'when creating a network' do + + it 'should call net-create with appropriate command line options' do + provider.class.stubs(:get_tenant_id).returns(net_attrs[:tenant_id]) + + output = 'Created a new network: +admin_state_up="True" +id="d9ac3494-20ea-406c-a4ba-145923dfadc9" +name="net1" +shared="True" +status="ACTIVE" +subnets="" +tenant_id="60f9544eb94c42a6b7e8e98c2be981b1"' + + provider.expects(:auth_neutron).with('net-create', + '--format=shell', ['--shared', "--tenant_id=#{net_attrs[:tenant_id]}"], + net_name).returns(output) + + provider.create + end + end + + describe 'when updating a network' do + + it 'should call net-update to change admin_state_up' do + provider.expects(:auth_neutron).with('net-update', + '--admin_state_up=False', + net_name) + provider.admin_state_up=('False') + end + + it 'should call net-update to change shared' do + provider.expects(:auth_neutron).with('net-update', + '--shared=True', + net_name) + provider.shared=('True') + end + + it 'should call net-update to change router_external' do + provider.expects(:auth_neutron).with('net-update', + '--router:external=False', + net_name) + provider.router_external=('False') + end + + it 'should call net-update to change router_external' do + provider.expects(:auth_neutron).with('net-update', + '--router:external', + net_name) + provider.router_external=('True') + end + + [:provider_network_type, :provider_physical_network, :provider_segmentation_id].each do |attr| + it "should fail when #{attr.to_s} is update " do + expect do + provider.send("#{attr}=", 'foo') + end.to raise_error(Puppet::Error, /does not support being updated/) + end + end + + end + end + + describe "with tenant_name set" do + + let :local_attrs do + attrs = net_attrs.merge({:tenant_name => 'a_person'}) + attrs.delete(:tenant_id) + attrs + end + let :resource do - Puppet::Type::Neutron_network.new(net_attrs) + Puppet::Type::Neutron_network.new(local_attrs) end let :provider do provider_class.new(resource) end - it 'should call net-update to change admin_state_up' do - provider.expects(:auth_neutron).with('net-update', - '--admin_state_up=False', - net_name) - provider.admin_state_up=('False') - end - - it 'should call net-update to change shared' do - provider.expects(:auth_neutron).with('net-update', - '--shared=True', - net_name) - provider.shared=('True') - end - - it 'should call net-update to change router_external' do - provider.expects(:auth_neutron).with('net-update', - '--router:external=False', - net_name) - provider.router_external=('False') - end - - it 'should call net-update to change router_external' do - provider.expects(:auth_neutron).with('net-update', - '--router:external', - net_name) - provider.router_external=('True') - end - - [:provider_network_type, :provider_physical_network, :provider_segmentation_id].each do |attr| - it "should fail when #{attr.to_s} is update " do - expect do - provider.send("#{attr}=", 'foo') - end.to raise_error(Puppet::Error, /does not support being updated/) - end - end - + it_behaves_like('neutron_network') end end diff --git a/spec/unit/provider/neutron_port/neutron_spec.rb b/spec/unit/provider/neutron_port/neutron_spec.rb new file mode 100644 index 000000000..1dfbd06d3 --- /dev/null +++ b/spec/unit/provider/neutron_port/neutron_spec.rb @@ -0,0 +1,57 @@ +require 'puppet' +require 'spec_helper' +require 'puppet/provider/neutron_port/neutron' + +provider_class = Puppet::Type.type(:neutron_port).provider(:neutron) + +describe provider_class do + + let :port_name do + 'port1' + end + + let :port_attrs do + { + :name => port_name, + :ensure => 'present', + :admin_state_up => 'True', + :tenant_id => '60f9544eb94c42a6b7e8e98c2be981b1', + :network_name => 'net1' + } + end + + let :resource do + Puppet::Type::Neutron_port.new(port_attrs) + end + + let :provider do + provider_class.new(resource) + end + + describe 'when creating a port' do + + it 'should call port-create with appropriate command line options' do + provider.class.stubs(:get_tenant_id).returns(port_attrs[:tenant_id]) + + output = 'Created a new port: +admin_state_up="True" +device_id="" +device_owner="" +fixed_ips="{\"subnet_id\": \"40af01ac-52c7-4235-bbcf-d9c02325ab5e\", \"ip_address\": \"192.168.0.39\"}" +id="5222573b-314d-45f9-b6bd-299288ba667a" +mac_address="fa:16:3e:45:3c:10" +name="port1" +network_id="98873773-aa34-4b87-af05-70903659246f" +security_groups="f1f0c3a3-9f2c-46b9-b2a5-b97d9a87bd7e" +status="ACTIVE" +tenant_id="60f9544eb94c42a6b7e8e98c2be981b1"' + + provider.expects(:auth_neutron).with('port-create', + '--format=shell', "--name=#{port_attrs[:name]}", + ["--tenant_id=#{port_attrs[:tenant_id]}"], + port_attrs[:network_name]).returns(output) + + provider.create + end + end +end diff --git a/spec/unit/provider/neutron_router/neutron_spec.rb b/spec/unit/provider/neutron_router/neutron_spec.rb index 8a2ba4a63..0e2c166a7 100644 --- a/spec/unit/provider/neutron_router/neutron_spec.rb +++ b/spec/unit/provider/neutron_router/neutron_spec.rb @@ -15,18 +15,40 @@ describe provider_class do :name => router_name, :ensure => 'present', :admin_state_up => 'True', - :tenant_id => '', + :tenant_id => '60f9544eb94c42a6b7e8e98c2be981b1', } end - describe 'when updating a router' do - let :resource do - Puppet::Type::Neutron_router.new(router_attrs) - end + let :resource do + Puppet::Type::Neutron_router.new(router_attrs) + end - let :provider do - provider_class.new(resource) + let :provider do + provider_class.new(resource) + end + + describe 'when creating a router' do + + it 'should call router-create with appropriate command line options' do + provider.class.stubs(:get_tenant_id).returns(router_attrs[:tenant_id]) + + output = 'Created a new router: +admin_state_up="True" +external_gateway_info="" +id="c5f799fa-b3e0-47ca-bdb7-abeff209b816" +name="router1" +status="ACTIVE" +tenant_id="60f9544eb94c42a6b7e8e98c2be981b1"' + + provider.expects(:auth_neutron).with('router-create', + '--format=shell', ["--tenant_id=#{router_attrs[:tenant_id]}"], + router_name).returns(output) + + provider.create end + end + + describe 'when updating a router' do it 'should call router-update to change admin_state_up' do provider.expects(:auth_neutron).with('router-update', diff --git a/spec/unit/provider/neutron_router_interface/new_neutron_spec.rb b/spec/unit/provider/neutron_router_interface/new_neutron_spec.rb new file mode 100644 index 000000000..0e8ce814c --- /dev/null +++ b/spec/unit/provider/neutron_router_interface/new_neutron_spec.rb @@ -0,0 +1,49 @@ +require 'puppet' +require 'spec_helper' +require 'puppet/provider/neutron_router_interface/neutron' + +provider_class = Puppet::Type.type(:neutron_router_interface).provider(:neutron) + +describe provider_class do + + let :interface_attrs do + { + :name => 'router:subnet', + :ensure => 'present', + } + end + + let :resource do + Puppet::Type::Neutron_router_interface.new(interface_attrs) + end + + let :provider do + provider_class.new(resource) + end + + describe 'when creating a router interface' do + + it 'should call port-create with appropriate command line options' do + provider.class.stubs(:get_tenant_id).returns(interface_attrs[:tenant_id]) + + output = 'Added interface b03610fd-ac31-4521-ad06-2ac74af959ad to router router' + + provider.expects(:auth_neutron).with(['router-interface-add', + '--format=shell', 'router', 'subnet=subnet']).returns(output) + + provider.create + end + end + + describe 'when accessing attributes of an interface' do + it 'should return the correct router name' do + expect(provider.router_name).to eql('router') + end + + it 'should return the correct subnet name' do + expect(provider.subnet_name).to eql('subnet') + end + + end + +end diff --git a/spec/unit/provider/neutron_subnet/neutron_spec.rb b/spec/unit/provider/neutron_subnet/neutron_spec.rb index ed49cd664..0b3f473a2 100644 --- a/spec/unit/provider/neutron_subnet/neutron_spec.rb +++ b/spec/unit/provider/neutron_subnet/neutron_spec.rb @@ -19,22 +19,56 @@ describe provider_class do :gateway_ip => '10.0.0.1', :enable_dhcp => 'False', :network_name => 'net1', - :tenant_id => '', - :allocation_pools => 'start=7.0.0.1,end=7.0.0.10', + :tenant_id => '60f9544eb94c42a6b7e8e98c2be981b1', + :allocation_pools => 'start=10.0.0.2,end=10.0.0.10', :dns_nameservers => '8.8.8.8', :host_routes => 'destination=12.0.0.0/24,nexthop=10.0.0.1', } end + let :resource do + Puppet::Type::Neutron_subnet.new(subnet_attrs) + end + + let :provider do + provider_class.new(resource) + end + + describe 'when creating a subnet' do + + it 'should call subnet-create with appropriate command line options' do + provider.class.stubs(:get_tenant_id).returns(subnet_attrs[:tenant_id]) + + output = 'Created a new subnet: +allocation_pools="{\"start\": \"10.0.0.2\", \"end\": \"10.0.0.10\"}" +cidr="10.0.0.0/24" +dns_nameservers="8.8.8.8" +enable_dhcp="False" +gateway_ip="10.0.0.1" +host_routes="{\"nexthop\": \"10.0.0.1\", \"destination\": \"12.0.0.0/24\"}" +id="dd5e0ef1-2c88-4b0b-ba08-7df65be87963" +ip_version="4" +name="net1" +network_id="98873773-aa34-4b87-af05-70903659246f" +tenant_id="60f9544eb94c42a6b7e8e98c2be981b1"' + + provider.expects(:auth_neutron).with('subnet-create', '--format=shell', + ["--name=#{subnet_attrs[:name]}", + "--ip-version=#{subnet_attrs[:ip_version]}", + "--gateway-ip=#{subnet_attrs[:gateway_ip]}", + "--disable-dhcp", + "--allocation-pool=#{subnet_attrs[:allocation_pools]}", + "--dns-nameserver=#{subnet_attrs[:dns_nameservers]}", + "--host-route=#{subnet_attrs[:host_routes]}", + "--tenant_id=#{subnet_attrs[:tenant_id]}", + subnet_name], + subnet_attrs[:cidr]).returns(output) + + provider.create + end + end + describe 'when updating a subnet' do - let :resource do - Puppet::Type::Neutron_subnet.new(subnet_attrs) - end - - let :provider do - provider_class.new(resource) - end - it 'should call subnet-update to change gateway_ip' do provider.expects(:auth_neutron).with('subnet-update', '--gateway-ip=10.0.0.2',