From 09f726c3634a3828aafa492e4914cd6da3835217 Mon Sep 17 00:00:00 2001 From: Dan Bode Date: Tue, 5 Jun 2012 16:45:47 -0700 Subject: [PATCH] Allow multiple devices per endpoint. Previously, the Puppet modules only supported a single device per endpoint. This commits allows multiple devices per endpoint by making the device name a part of the namevar for the swift ring devices. it also updates the relevent configs. --- examples/multi.pp | 9 ++-- lib/puppet/provider/swift_ring_builder.rb | 15 ++---- lib/puppet/type/ring_account_device.rb | 6 +-- lib/puppet/type/ring_container_device.rb | 6 +-- lib/puppet/type/ring_object_device.rb | 6 +-- spec/classes/swift_ringbuilder_spec.rb | 52 ++++++++++--------- spec/classes/swift_storage_all_spec.rb | 6 ++- .../provider/swift_ring_builder_spec.rb | 33 ++++++++++++ .../puppet/type/ring_account_device_spec.rb | 15 ++++++ .../puppet/type/ring_container_device_spec.rb | 16 ++++++ .../puppet/type/ring_object_device_spec.rb | 15 ++++++ 11 files changed, 125 insertions(+), 54 deletions(-) create mode 100644 spec/unit/puppet/provider/swift_ring_builder_spec.rb create mode 100644 spec/unit/puppet/type/ring_account_device_spec.rb create mode 100644 spec/unit/puppet/type/ring_container_device_spec.rb create mode 100644 spec/unit/puppet/type/ring_object_device_spec.rb diff --git a/examples/multi.pp b/examples/multi.pp index c5a47ab9..33621c63 100644 --- a/examples/multi.pp +++ b/examples/multi.pp @@ -163,22 +163,19 @@ class role_swift_storage inherits role_swift { # these exported resources write ring config # resources into the database so that they can be # consumed by the ringbuilder role - @@ring_object_device { "${swift_local_net_ip}:6000": + @@ring_object_device { "${swift_local_net_ip}:6000/1": zone => $swift_zone, - device_name => 1, weight => 1, } - @@ring_container_device { "${swift_local_net_ip}:6001": + @@ring_container_device { "${swift_local_net_ip}:6001/1": zone => $swift_zone, - device_name => 1, weight => 1, } # TODO should device be changed to volume - @@ring_account_device { "${swift_local_net_ip}:6002": + @@ring_account_device { "${swift_local_net_ip}:6002/1": zone => $swift_zone, - device_name => 1, weight => 1, } diff --git a/lib/puppet/provider/swift_ring_builder.rb b/lib/puppet/provider/swift_ring_builder.rb index 425c4f5b..235c94ec 100644 --- a/lib/puppet/provider/swift_ring_builder.rb +++ b/lib/puppet/provider/swift_ring_builder.rb @@ -15,10 +15,9 @@ class Puppet::Provider::SwiftRingBuilder < Puppet::Provider if rows = swift_ring_builder(builder_file_path).split("\n")[4..-1] rows.each do |row| if row =~ /^\s+(\d+)\s+(\d+)\s+(\S+)\s+(\d+)\s+(\S+)\s+(\d+\.\d+)\s+(\d+)\s+(-?\d+\.\d+)\s+(\S*)$/ - object_hash["#{$3}:#{$4}"] = { + object_hash["#{$3}:#{$4}/#{$5}"] = { :id => $1, :zone => $2, - :device_name => $5, :weight => $6, :partitions => $7, :balance => $8, @@ -46,13 +45,13 @@ class Puppet::Provider::SwiftRingBuilder < Puppet::Provider end def create - [:zone, :device_name, :weight].each do |param| + [:zone, :weight].each do |param| raise(Puppet::Error, "#{param} is required") unless resource[param] end swift_ring_builder( builder_file_path, 'add', - "z#{resource[:zone]}-#{resource[:name]}/#{resource[:device_name]}", + "z#{resource[:zone]}-#{resource[:name]}", resource[:weight] ) end @@ -74,14 +73,6 @@ class Puppet::Provider::SwiftRingBuilder < Puppet::Provider Puppet.warning('Setting zone is not yet supported, I am not even sure if it is supported') end - def device_name - ring[resource[:name]][:device_name] - end - - def device_name=(name) - Puppet.warning('I think it makes sense to set the name, it is just not yet supported') - end - def weight ring[resource[:name]][:weight] # get the weight diff --git a/lib/puppet/type/ring_account_device.rb b/lib/puppet/type/ring_account_device.rb index 6e3ea375..dc12a4b1 100644 --- a/lib/puppet/type/ring_account_device.rb +++ b/lib/puppet/type/ring_account_device.rb @@ -6,15 +6,15 @@ Puppet::Type.newtype(:ring_account_device) do newparam(:name, :namevar => true) do validate do |value| address = value.split(':') - raise(Puppet::Error, "invalid name #{value}") unless address.size == 2 + raise(Puppet::Error, "invalid name #{value}, should contain address:port/device") unless address.size == 2 + port_device = address[1].split('/') + raise(Puppet::Error, "namevar should contain a device") unless port_device.size == 2 IPAddr.new(address[0]) end end newproperty(:zone) - newproperty(:device_name) - newproperty(:weight) do munge do |value| "%.2f" % value diff --git a/lib/puppet/type/ring_container_device.rb b/lib/puppet/type/ring_container_device.rb index e84b31e4..a8a37dee 100644 --- a/lib/puppet/type/ring_container_device.rb +++ b/lib/puppet/type/ring_container_device.rb @@ -6,15 +6,15 @@ Puppet::Type.newtype(:ring_container_device) do newparam(:name, :namevar => true) do validate do |value| address = value.split(':') - raise(Puppet::Error, "invalid name #{value}") unless address.size == 2 + raise(Puppet::Error, "invalid name #{value}, should contain address:port/device") unless address.size == 2 + port_device = address[1].split('/') + raise(Puppet::Error, "namevar should contain a device") unless port_device.size == 2 IPAddr.new(address[0]) end end newproperty(:zone) - newproperty(:device_name) - newproperty(:weight) do munge do |value| "%.2f" % value diff --git a/lib/puppet/type/ring_object_device.rb b/lib/puppet/type/ring_object_device.rb index afcd6d66..1ee9b108 100644 --- a/lib/puppet/type/ring_object_device.rb +++ b/lib/puppet/type/ring_object_device.rb @@ -6,15 +6,15 @@ Puppet::Type.newtype(:ring_object_device) do newparam(:name, :namevar => true) do validate do |value| address = value.split(':') - raise(Puppet::Error, "invalid name #{value}") unless address.size == 2 + raise(Puppet::Error, "invalid name #{value}, should contain address:port/device") unless address.size == 2 + port_device = address[1].split('/') + raise(Puppet::Error, "namevar should contain a device") unless port_device.size == 2 IPAddr.new(address[0]) end end newproperty(:zone) - newproperty(:device_name) - newproperty(:weight) do munge do |value| "%.2f" % value diff --git a/spec/classes/swift_ringbuilder_spec.rb b/spec/classes/swift_ringbuilder_spec.rb index b7bff658..3edd7f9a 100644 --- a/spec/classes/swift_ringbuilder_spec.rb +++ b/spec/classes/swift_ringbuilder_spec.rb @@ -5,7 +5,8 @@ describe 'swift::ringbuilder' do { :operatingsystem => 'Ubuntu', :osfamily => 'Debian', - :processorcount => 1 + :processorcount => 1, + :concat_basedir => '/tmp/foo' } end describe 'when swift class is not included' do @@ -23,9 +24,11 @@ describe 'swift::ringbuilder' do class { 'ssh::server::install': }" end - it { should contain_swift__ringbuilder__rebalance('object') } - it { should contain_swift__ringbuilder__rebalance('account') } - it { should contain_swift__ringbuilder__rebalance('container') } + it 'should rebalance the ring for all ring types' do + should contain_swift__ringbuilder__rebalance('object') + should contain_swift__ringbuilder__rebalance('account') + should contain_swift__ringbuilder__rebalance('container') + end describe 'with default parameters' do ['object', 'account', 'container'].each do |type| @@ -60,43 +63,42 @@ describe 'swift::ringbuilder' do 'class { memcached: max_memory => 1} class { swift: swift_hash_suffix => string } class { "ssh::server::install": } - ring_object_device { "127.0.0.1:6000": + ring_object_device { "127.0.0.1:6000/1": zone => 1, - device_name => 1, weight => 1, } - ring_container_device { "127.0.0.1:6001": + ring_container_device { "127.0.0.1:6001/1": zone => 2, - device_name => 1, weight => 1, } - ring_account_device { "127.0.0.1:6002": + ring_account_device { "127.0.0.1:6002/1": zone => 3, - device_name => 1, weight => 1, }' end - it { should contain_swift__ringbuilder__create('object').with( - {:before => 'Ring_object_device[127.0.0.1:6000]'} - )} - it { should contain_swift__ringbuilder__create('container').with( - {:before => 'Ring_container_device[127.0.0.1:6001]'} - )} - it { should contain_swift__ringbuilder__create('account').with( - {:before => 'Ring_account_device[127.0.0.1:6002]'} - )} - it { should contain_ring_object_device('127.0.0.1:6000').with( + it 'should set up all of the correct dependencies' do + should contain_swift__ringbuilder__create('object').with( + {:before => 'Ring_object_device[127.0.0.1:6000/1]'} + ) + should contain_swift__ringbuilder__create('container').with( + {:before => 'Ring_container_device[127.0.0.1:6001/1]'} + ) + should contain_swift__ringbuilder__create('account').with( + {:before => 'Ring_account_device[127.0.0.1:6002/1]'} + ) + should contain_ring_object_device('127.0.0.1:6000/1').with( {:notify => 'Swift::Ringbuilder::Rebalance[object]'} - )} - it { should contain_ring_container_device('127.0.0.1:6001').with( + ) + should contain_ring_container_device('127.0.0.1:6001/1').with( {:notify => 'Swift::Ringbuilder::Rebalance[container]'} - )} - it { should contain_ring_account_device('127.0.0.1:6002').with( + ) + should contain_ring_account_device('127.0.0.1:6002/1').with( {:notify => 'Swift::Ringbuilder::Rebalance[account]'} - )} + ) + end end end end diff --git a/spec/classes/swift_storage_all_spec.rb b/spec/classes/swift_storage_all_spec.rb index 3d6e46f1..de781728 100644 --- a/spec/classes/swift_storage_all_spec.rb +++ b/spec/classes/swift_storage_all_spec.rb @@ -7,7 +7,8 @@ describe 'swift::storage::all' do { :concat_basedir => '/tmp/', :operatingsystem => 'Ubuntu', - :osfamily => 'Debian' + :osfamily => 'Debian', + :concat_basedir => '/tmp/foo' } end @@ -113,7 +114,8 @@ describe 'swift::storage::all' do let :facts do { :operatingsystem => 'Debian', - :osfamily => 'Debian' + :osfamily => 'Debian', + :concat_basedir => '/tmp/foo' } end diff --git a/spec/unit/puppet/provider/swift_ring_builder_spec.rb b/spec/unit/puppet/provider/swift_ring_builder_spec.rb new file mode 100644 index 00000000..5f66f3ad --- /dev/null +++ b/spec/unit/puppet/provider/swift_ring_builder_spec.rb @@ -0,0 +1,33 @@ +require 'puppet' +require 'mocha' +require File.join(File.dirname(__FILE__), '..', '..', '..', '..', 'lib', 'puppet', 'provider', 'swift_ring_builder') +RSpec.configure do |config| + config.mock_with :mocha +end +provider_class = Puppet::Provider::SwiftRingBuilder +describe provider_class do + + let :builder_file_path do + '/etc/swift/account.builder' + end + + it 'should be able to lookup the local ring and build an object' do + File.expects(:exists?).with(builder_file_path).returns(true) + provider_class.expects(:builder_file_path).twice.returns(builder_file_path) + provider_class.expects(:swift_ring_builder).returns( +'/etc/swift/account.builder, build version 3 +262144 partitions, 3 replicas, 3 zones, 3 devices, 0.00 balance +The minimum number of hours before a partition can be reassigned is 1 +Devices: id zone ip address port name weight partitions balance meta + 2 2 192.168.101.14 6002 1 1.00 262144 0.00 + 0 3 192.168.101.15 6002 1 1.00 262144 0.00 + 1 1 192.168.101.13 6002 1 1.00 262144 0.00 + +' + ) + resources = provider_class.lookup_ring.inspect + resources['192.168.101.15:6002/1'].should_not be_nil + resources['192.168.101.13:6002/1'].should_not be_nil + resources['192.168.101.14:6002/1'].should_not be_nil + end +end diff --git a/spec/unit/puppet/type/ring_account_device_spec.rb b/spec/unit/puppet/type/ring_account_device_spec.rb new file mode 100644 index 00000000..b890f36c --- /dev/null +++ b/spec/unit/puppet/type/ring_account_device_spec.rb @@ -0,0 +1,15 @@ +require 'puppet' +describe Puppet::Type.type(:ring_account_device) do + + it 'should fail if the name has no ":"' do + expect do + Puppet::Type.type(:ring_account_device).new(:name => 'foo/bar') + end.should raise_error(Puppet::Error, /should contain address:port\/device/) + end + + it 'should fail if the name does not contain a "/"' do + expect do + Puppet::Type.type(:ring_account_device).new(:name => 'foo:80') + end.should raise_error(Puppet::Error, /should contain a device/) + end +end diff --git a/spec/unit/puppet/type/ring_container_device_spec.rb b/spec/unit/puppet/type/ring_container_device_spec.rb new file mode 100644 index 00000000..97971fbe --- /dev/null +++ b/spec/unit/puppet/type/ring_container_device_spec.rb @@ -0,0 +1,16 @@ + +require 'puppet' +describe Puppet::Type.type(:ring_container_device) do + + it 'should fail if the name has no ":"' do + expect do + Puppet::Type.type(:ring_account_device).new(:name => 'foo/bar') + end.should raise_error(Puppet::Error, /should contain address:port\/device/) + end + + it 'should fail if the name does not contain a "/"' do + expect do + Puppet::Type.type(:ring_account_device).new(:name => 'foo:80') + end.should raise_error(Puppet::Error, /should contain a device/) + end +end diff --git a/spec/unit/puppet/type/ring_object_device_spec.rb b/spec/unit/puppet/type/ring_object_device_spec.rb new file mode 100644 index 00000000..6910f01b --- /dev/null +++ b/spec/unit/puppet/type/ring_object_device_spec.rb @@ -0,0 +1,15 @@ +require 'puppet' +describe Puppet::Type.type(:ring_object_device) do + + it 'should fail if the name has no ":"' do + expect do + Puppet::Type.type(:ring_account_device).new(:name => 'foo/bar') + end.should raise_error(Puppet::Error, /should contain address:port\/device/) + end + + it 'should fail if the name does not contain a "/"' do + expect do + Puppet::Type.type(:ring_account_device).new(:name => 'foo:80') + end.should raise_error(Puppet::Error, /should contain a device/) + end +end