From 3b9931e98550d49b5dd8c31921e6181a4b07cb27 Mon Sep 17 00:00:00 2001 From: Aleksandr Didenko Date: Tue, 7 Jul 2015 11:59:48 +0300 Subject: [PATCH] Adapt synced swift module - Add upstream patches: * rebalance improvements I6bb1a144d1a5189ca32d407c6a7497aa397fafc5 * DLO support Ie209e494d7affd72c3fea3f2cc3459b91d7c6765 - Adapt our manifests for synced swift module - Fix noop tests for swift - Add some new noop tests to cover resource ordering Partially implements: blueprint upgrade-openstack-puppet-modules Change-Id: Ib8fd54ed71613ae88fd87da3d538967712a87357 --- .../puppet/openstack/manifests/swift/proxy.pp | 9 ++-- .../osnailyfacter/modular/swift/swift.pp | 6 +++ deployment/puppet/swift/manifests/init.pp | 4 +- .../puppet/swift/manifests/proxy/dlo.pp | 43 +++++++++++++++ .../swift/manifests/ringbuilder/rebalance.pp | 7 +++ .../spec/classes/swift_proxy_dlo_spec.rb | 53 +++++++++++++++++++ .../puppet/swift/spec/classes/swift_spec.rb | 3 ++ .../swift_ringbuilder_rebalance_spec.rb | 16 +++++- .../puppet/swift/templates/proxy/dlo.conf.erb | 5 ++ tests/noop/spec/hosts/swift/swift_spec.rb | 16 +++++- 10 files changed, 154 insertions(+), 8 deletions(-) create mode 100644 deployment/puppet/swift/manifests/proxy/dlo.pp create mode 100644 deployment/puppet/swift/spec/classes/swift_proxy_dlo_spec.rb create mode 100644 deployment/puppet/swift/templates/proxy/dlo.conf.erb diff --git a/deployment/puppet/openstack/manifests/swift/proxy.pp b/deployment/puppet/openstack/manifests/swift/proxy.pp index 7627cc638d..dfeb693277 100644 --- a/deployment/puppet/openstack/manifests/swift/proxy.pp +++ b/deployment/puppet/openstack/manifests/swift/proxy.pp @@ -70,8 +70,6 @@ class openstack::swift::proxy ( $log_level = 'WARNING' } - #FIXME(bogdando) the memcached class must be included in catalog if swift node is a standalone! - if $ceilometer { $new_proxy_pipeline = split( inline_template( @@ -135,6 +133,9 @@ class openstack::swift::proxy ( } if $primary_proxy { + # we need to exec swift ringrebuilder commands under swift user + Exec { user => 'swift' } + # collect all of the resources that are needed # to balance the ring if $collect_exported { @@ -159,8 +160,8 @@ class openstack::swift::proxy ( } # resource ordering - Anchor <| title == 'rebalance_end' |> -> Service['swift-proxy'] - Anchor <| title == 'rebalance_end' |> -> Swift::Storage::Generic <| |> + Swift::Ringbuilder::Rebalance <||> -> Service['swift-proxy'] + Swift::Ringbuilder::Rebalance <||> -> Swift::Storage::Generic <| |> Swift::Ringbuilder::Create<||> -> Ring_devices<||> ~> Swift::Ringbuilder::Rebalance <||> diff --git a/deployment/puppet/osnailyfacter/modular/swift/swift.pp b/deployment/puppet/osnailyfacter/modular/swift/swift.pp index 79a25466e3..d737b8fefb 100644 --- a/deployment/puppet/osnailyfacter/modular/swift/swift.pp +++ b/deployment/puppet/osnailyfacter/modular/swift/swift.pp @@ -111,3 +111,9 @@ if !($storage_hash['images_ceph'] and $storage_hash['objects_ceph']) and !$stora class ceilometer {} include ceilometer +# Class[Swift::Proxy::Cache] requires Class[Memcached] if memcache_servers +# contains 127.0.0.1. But we're deploying memcached in another task. So we +# need to add this stub here. +class memcached {} +include memcached + diff --git a/deployment/puppet/swift/manifests/init.pp b/deployment/puppet/swift/manifests/init.pp index 7416c58a5b..f1cc554287 100644 --- a/deployment/puppet/swift/manifests/init.pp +++ b/deployment/puppet/swift/manifests/init.pp @@ -64,7 +64,9 @@ class swift( file { '/var/run/swift': ensure => directory, } - + file { '/etc/swift/backups': + ensure => directory, + } file { '/etc/swift/swift.conf': ensure => file, mode => '0660', diff --git a/deployment/puppet/swift/manifests/proxy/dlo.pp b/deployment/puppet/swift/manifests/proxy/dlo.pp new file mode 100644 index 0000000000..ca14547ab6 --- /dev/null +++ b/deployment/puppet/swift/manifests/proxy/dlo.pp @@ -0,0 +1,43 @@ +# +# Configure swift dlo. +# +# == Examples +# +# include ::swift::proxy::dlo +# +# == Parameters +# +# [*rate_limit_after_segment*] +# Start rate-limiting DLO segment serving after the Nth segment of a segmented object. +# Default to 10. +# +# [*rate_limit_segments_per_sec*] +# Once segment rate-limiting kicks in for an object, limit segments served to N per second. +# 0 means no rate-limiting. +# Default to 1. +# +# [*max_get_time*] +# Time limit on GET requests (seconds). +# Default to 86400. +# +# == Authors +# +# Aleksandr Didenko adidenko@mirantis.com +# +# == Copyright +# +# Copyright 2015 Mirantis Inc, unless otherwise noted. +# +class swift::proxy::dlo ( + $rate_limit_after_segment = '10', + $rate_limit_segments_per_sec = '1', + $max_get_time = '86400' +) { + + concat::fragment { 'swift_dlo': + target => '/etc/swift/proxy-server.conf', + content => template('swift/proxy/dlo.conf.erb'), + order => '36', + } + +} diff --git a/deployment/puppet/swift/manifests/ringbuilder/rebalance.pp b/deployment/puppet/swift/manifests/ringbuilder/rebalance.pp index c028bd4862..7d407ffe70 100644 --- a/deployment/puppet/swift/manifests/ringbuilder/rebalance.pp +++ b/deployment/puppet/swift/manifests/ringbuilder/rebalance.pp @@ -17,9 +17,16 @@ define swift::ringbuilder::rebalance( validate_re($seed, '^\d+$') } + exec { "hours_passed_${name}": + command => "swift-ring-builder /etc/swift/${name}.builder pretend_min_part_hours_passed", + path => ['/usr/bin'], + refreshonly => true, + returns => [0,1], + } -> exec { "rebalance_${name}": command => strip("swift-ring-builder /etc/swift/${name}.builder rebalance ${seed}"), path => ['/usr/bin'], refreshonly => true, + returns => [0,1], } } diff --git a/deployment/puppet/swift/spec/classes/swift_proxy_dlo_spec.rb b/deployment/puppet/swift/spec/classes/swift_proxy_dlo_spec.rb new file mode 100644 index 0000000000..2fefd93206 --- /dev/null +++ b/deployment/puppet/swift/spec/classes/swift_proxy_dlo_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe 'swift::proxy::dlo' do + + let :facts do + {} + end + + let :pre_condition do + 'class { "concat::setup": } + concat { "/etc/swift/proxy-server.conf": }' + end + + let :fragment_file do + "/var/lib/puppet/concat/_etc_swift_proxy-server.conf/fragments/36_swift_dlo" + end + + describe "when using default parameters" do + it 'should build the fragment with correct parameters' do + verify_contents(catalogue, fragment_file, + [ + '[filter:dlo]', + 'use = egg:swift#dlo', + 'rate_limit_after_segment = 10', + 'rate_limit_segments_per_sec = 1', + 'max_get_time = 86400', + ] + ) + end + end + + describe "when overriding default parameters" do + let :params do + { + :rate_limit_after_segment => '30', + :rate_limit_segments_per_sec => '5', + :max_get_time => '6400', + } + end + it 'should build the fragment with correct parameters' do + verify_contents(catalogue, fragment_file, + [ + '[filter:dlo]', + 'use = egg:swift#dlo', + 'rate_limit_after_segment = 30', + 'rate_limit_segments_per_sec = 5', + 'max_get_time = 6400', + ] + ) + end + end + +end diff --git a/deployment/puppet/swift/spec/classes/swift_spec.rb b/deployment/puppet/swift/spec/classes/swift_spec.rb index 33324f7a55..d595862ef2 100644 --- a/deployment/puppet/swift/spec/classes/swift_spec.rb +++ b/deployment/puppet/swift/spec/classes/swift_spec.rb @@ -45,6 +45,9 @@ describe 'swift' do it {is_expected.to contain_file('/var/lib/swift').with( {:ensure => 'directory'}.merge(file_defaults) )} + it {is_expected.to contain_file('/etc/swift/backups').with( + {:ensure => 'directory'}.merge(file_defaults) + )} it {is_expected.to contain_file('/etc/swift/swift.conf').with( { :ensure => 'file', :mode => '0660' diff --git a/deployment/puppet/swift/spec/defines/swift_ringbuilder_rebalance_spec.rb b/deployment/puppet/swift/spec/defines/swift_ringbuilder_rebalance_spec.rb index ed275ea711..a4ab290238 100644 --- a/deployment/puppet/swift/spec/defines/swift_ringbuilder_rebalance_spec.rb +++ b/deployment/puppet/swift/spec/defines/swift_ringbuilder_rebalance_spec.rb @@ -9,8 +9,14 @@ describe 'swift::ringbuilder::rebalance' do it { is_expected.to contain_exec("rebalance_#{type}").with( {:command => "swift-ring-builder /etc/swift/#{type}.builder rebalance", :path => ['/usr/bin'], - :refreshonly => true} + :refreshonly => true, + :returns => [0,1]} )} + it { is_expected.to contain_exec("hours_passed_#{type}").with( + {:command => "swift-ring-builder /etc/swift/#{type}.builder pretend_min_part_hours_passed", + :path => ['/usr/bin'], + :refreshonly => true} + ).that_comes_before("Exec[rebalance_#{type}]")} end end end @@ -24,8 +30,14 @@ describe 'swift::ringbuilder::rebalance' do it { is_expected.to contain_exec("rebalance_object").with( {:command => "swift-ring-builder /etc/swift/object.builder rebalance 999", :path => ['/usr/bin'], - :refreshonly => true} + :refreshonly => true, + :returns => [0,1]} )} + it { is_expected.to contain_exec("hours_passed_object").with( + {:command => "swift-ring-builder /etc/swift/object.builder pretend_min_part_hours_passed", + :path => ['/usr/bin'], + :refreshonly => true} + ).that_comes_before("Exec[rebalance_object]")} end describe 'with an invalid seed' do let :title do diff --git a/deployment/puppet/swift/templates/proxy/dlo.conf.erb b/deployment/puppet/swift/templates/proxy/dlo.conf.erb new file mode 100644 index 0000000000..7ae2ff4372 --- /dev/null +++ b/deployment/puppet/swift/templates/proxy/dlo.conf.erb @@ -0,0 +1,5 @@ +[filter:dlo] +use = egg:swift#dlo +rate_limit_after_segment = <%= @rate_limit_after_segment %> +rate_limit_segments_per_sec = <%= @rate_limit_segments_per_sec %> +max_get_time = <%= @max_get_time %> diff --git a/tests/noop/spec/hosts/swift/swift_spec.rb b/tests/noop/spec/hosts/swift/swift_spec.rb index bf9af2fe7d..0ad362f66a 100644 --- a/tests/noop/spec/hosts/swift/swift_spec.rb +++ b/tests/noop/spec/hosts/swift/swift_spec.rb @@ -5,7 +5,7 @@ manifest = 'swift/swift.pp' describe manifest do shared_examples 'catalog' do role = Noop.hiera 'role' - storage_hash = Noop.hiera['storage'] + storage_hash = Noop.hiera 'storage' nodes = Noop.hiera 'nodes' primary_controller_nodes = Noop::Utils.filter_nodes(nodes,'role','primary-controller') controllers = primary_controller_nodes + Noop::Utils.filter_nodes(nodes,'role','controller') @@ -25,13 +25,27 @@ describe manifest do should contain_exec("rebalance_#{ring}").with( 'command' => "swift-ring-builder /etc/swift/#{ring}.builder rebalance", 'user' => 'swift', + 'returns' => [0,1], ).that_requires("Exec[hours_passed_#{ring}]") should contain_exec("create_#{ring}").with( 'user' => 'swift', ) end end + ['account', 'object', 'container'].each do | ring | + it "should define swift::ringbuilder::rebalance[#{ring}] before Service[swift-proxy]" do + should contain_swift__ringbuilder__rebalance(ring).that_comes_before('Service[swift-proxy]') + end + end + ['account', 'object', 'container'].each do | ring | + ['account', 'object', 'container'].each do | storage | + it "should define swift::ringbuilder::rebalance[#{ring}] before swift::storage::generic[#{storage}]" do + should contain_swift__ringbuilder__rebalance(ring).that_comes_before("Swift::Storage::Generic[#{storage}]") + end + end + end end + it 'should create /etc/swift/backups directory with correct ownership' do should contain_file('/etc/swift/backups').with( 'ensure' => 'directory',