Refactor of swift server configs

Dan Bode · Dan Bode · commit 59afc07d3bfd · 2012-04-21T12:54:17.000-07:00
This commit performs a refactor of the
swift::storage::config to use fragments.

Updates server templates
  - makes workers,user, and mount_checks configurable
  - adds a default for concurrency
  - makes the pipeline configurable
  - remove vm_test_mode flag

Updates swift::storage::server to use fragments for
the config file.

This has been refactored to allow the end user a
greater level of flexibility over how they can
configure custom plugins for swift.

Also adds additional class params: pipeline,
mount_check, user, workers, concurrency.

Update the unit tests for swift::storage:server
diff --git a/.gitignore b/.gitignore
@@ -4,3 +4,4 @@ spec/fixtures/modules/rsync
 spec/fixtures/modules/ssh
 spec/fixtures/modules/stdlib
 spec/fixtures/modules/xinetd
+spec/fixtures/modules/*
diff --git a/manifests/storage/server.pp b/manifests/storage/server.pp
@@ -6,15 +6,29 @@
 define swift::storage::server(
   $type,
   $storage_local_net_ip,
-  $devices = '/srv/node',
-  $owner = 'swift',
-  $group  = 'swift',
-  $max_connections = 25,
+  $devices          = '/srv/node',
+  $owner            = 'swift',
+  $group            = 'swift',
+  $max_connections  = 25,
+  $pipeline         = ["${type}-server"],
+  $mount_check      = 'false',
+  $user             = 'swift',
+  $workers          = '1',
+  $concurrency      = $::processorcount,
   # this parameters needs to be specified after type and name
   $config_file_path = "${type}-server/${name}.conf"
 ) {
 
+  # TODO if array does not include type-server, warn
+  if(
+    (is_array($pipeline) and ! member($pipeline, "${type}-server")) or
+    $pipline != "${type}-server"
+  ) {
+      warning("swift storage server ${type} must specify ${type}-server")
+  }
+
   include "swift::storage::$type"
+  include 'concat::setup'
 
   validate_re($name, '^\d+$')
   validate_re($type, '^object|container|account$')
@@ -31,11 +45,17 @@
     read_only => false,
   }
 
-  file { "/etc/swift/${config_file_path}":
-    content => template("swift/${type}-server.conf.erb"),
+  concat { "/etc/swift/${config_file_path}":
     owner   => $owner,
     group   => $group,
     notify  => Service["swift-${type}"],
+    mode    => 640,
   }
 
+  # you can now add your custom fragments at the user level
+  concat::fragment { "swift-${type}-${name}":
+    target  => "/etc/swift/${config_file_path}",
+    content => template("swift/${type}-server.conf.erb"),
+    order   => '00',
+  }
 }
diff --git a/spec/defines/swift_storage_server_spec.rb b/spec/defines/swift_storage_server_spec.rb
@@ -4,7 +4,8 @@
   let :facts do
     {
       :operatingsystem => 'Ubuntu',
-      :osfamily        => 'Debian'
+      :osfamily        => 'Debian',
+      :processorcount  => 1
     }
 
   end
@@ -21,8 +22,6 @@ class { 'swift::storage': storage_local_net_ip => '10.0.0.1' }"
      :max_connections => '25'}
   end
 
-
-
   describe 'with an invalid title' do
     let :params do
       {:storage_local_net_ip => '127.0.0.1',
@@ -39,66 +38,97 @@ class { 'swift::storage': storage_local_net_ip => '10.0.0.1' }"
   end
 
   ['account', 'object', 'container'].each do |t|
-    [{:storage_local_net_ip => '10.0.0.1',
-      :type => t},
-     {:storage_local_net_ip => '127.0.0.1',
-      :type => t}
-    ].each do |param_set|
-      describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do
-        let :title do
-          '8000'
-        end
-        let :param_hash do
-          default_params.merge(param_set)
+
+    describe "for type #{t}" do
+
+      let :title do
+        '8000'
+      end
+
+      let :req_params do
+        {:storage_local_net_ip => '10.0.0.1', :type => t}
+      end
+      let :params do
+        req_params
+      end
+
+      it { should contain_package("swift-#{t}").with_ensure('present') }
+      it { should contain_service("swift-#{t}").with(
+        :ensure    => 'running',
+        :enable    => true,
+        :hasstatus => true
+      )}
+      let :fragment_file do
+        "/var/lib/puppet/concat/_etc_swift_#{t}-server_#{title}.conf/fragments/00_swift-#{t}-#{title}"
+      end
+
+      describe 'when parameters are overridden' do
+        {
+          :devices     => '/tmp/foo',
+          :user        => 'dan',
+          :mount_check => true,
+          :concurrency => 5,
+          :workers     => 7,
+          :pipeline    => 'foo'
+        }.each do |k,v|
+          describe "when #{k} is set" do
+            let :params do req_params.merge({k => v}) end
+            it { should contain_file(fragment_file) \
+              .with_content(/^#{k.to_s}\s*=\s*#{v}\s*$/)
+            }
+          end
+          describe "when pipline is passed an array" do
+            let :params do req_params.merge({:pipeline => [1,2,3]})  end
+            it { should contain_file(fragment_file) \
+              .with_content(/^pipeline\s*=\s*1 2 3\s*$/)
+            }
+          end
         end
+      end
+
+      describe 'with all allowed defaults' do
         let :params do
-          param_set
+          req_params
         end
-        let :config_file_path do
-          "#{t}-server/#{title}.conf"
-        end
-        it { should contain_package("swift-#{t}").with_ensure('present') }
-        it { should contain_service("swift-#{t}").with(
-          :ensure    => 'running',
-          :enable    => true,
-          :hasstatus => true
-        )}
-        it { should contain_file("/etc/swift/#{t}-server/").with(
-          :ensure => 'directory',
-          :owner  => 'swift',
-          :group  => 'swift'
-        )}
+
         it { should contain_rsync__server__module("#{t}#{title}").with(
-          :path            => param_hash[:devices],
+          :path            => '/srv/node',
           :lock_file       => "/var/lock/#{t}#{title}.lock",
-          :uid             => param_hash[:owner],
-          :gid             => param_hash[:group],
-          :max_connections => param_hash[:max_connections],
+          :uid             => 'swift',
+          :gid             => 'swift',
+          :max_connections => 25,
           :read_only       => false
         )}
-        it { should contain_file("/etc/swift/#{config_file_path}").with(
-          :owner => param_hash[:owner],
-          :group => param_hash[:group]
-        )}
-        it 'should have some contents' do
-          content = param_value(
-            subject,
-            'file', "/etc/swift/#{config_file_path}",
-            'content'
-          )
-          expected_lines =
-            [
-              '[DEFAULT]',
-              "devices = #{param_hash[:devices]}",
-              "bind_ip = #{param_hash[:storage_local_net_ip]}",
-              "bind_port = #{title}"
-            ]
-          (content.split("\n") & expected_lines).should =~ expected_lines
-        end
-      end
 
-      # TODO - I do not want to add tests for the upstart stuff
-      # I need to check the tickets and see if this stuff is fixed
+        # verify template lines
+        it { should contain_file(fragment_file) \
+          .with_content(/^devices\s*=\s*\/srv\/node\s*$/)
+        }
+        it { should contain_file(fragment_file) \
+          .with_content(/^bind_ip\s*=\s*10\.0\.0\.1\s*$/)
+        }
+        it { should contain_file(fragment_file) \
+          .with_content(/^bind_port\s*=\s*#{title}\s*$/)
+        }
+        it { should contain_file(fragment_file) \
+          .with_content(/^mount_check\s*=\s*false\s*$/)
+        }
+        it { should contain_file(fragment_file) \
+          .with_content(/^user\s*=\s*swift\s*$/)
+        }
+        it { should contain_file(fragment_file) \
+          .with_content(/^log_facility\s*=\s*LOG_LOCAL2\s*$/)
+        }
+        it { should contain_file(fragment_file) \
+          .with_content(/^workers\s*=\s*1\s*$/)
+        }
+        it { should contain_file(fragment_file) \
+          .with_content(/^concurrency\s*=\s*1\s*$/)
+        }
+        it { should contain_file(fragment_file) \
+          .with_content(/^pipeline\s*=\s*#{t}-server\s*$/)
+        }
+      end
     end
   end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -6,6 +6,11 @@ def param_value(subject, type, title, param)
   subject.resource(type, title).send(:parameters)[param.to_sym]
 end
 
+def verify_contents(subject, title, expected_lines)
+  content = subject.resource('file', title).send(:parameters)[:content]
+  (content.split("\n") & expected_lines).should == expected_lines
+end
+
 RSpec.configure do |c|
   c.module_path  = File.expand_path(File.join(File.dirname(__FILE__), 'fixtures/modules'))
   # Using an empty site.pp file to avoid: https://github.com/rodjek/rspec-puppet/issues/15
diff --git a/templates/account-server.conf.erb b/templates/account-server.conf.erb
@@ -2,19 +2,19 @@
 devices = <%= devices %>
 bind_ip = <%= storage_local_net_ip %>
 bind_port = <%= bind_port %>
-mount_check = false
-user = swift
+mount_check = <%= mount_check %>
+user = <%= user %>
 log_facility = LOG_LOCAL2
-workers = 2
+workers = <%= workers %>
+concurrency = <%= concurrency %>
 
 [pipeline:main]
-pipeline = account-server
+pipeline = <%= pipeline.to_a.join(' ') %>
 
 [app:account-server]
 use = egg:swift#account
 
 [account-replicator]
-vm_test_mode = yes
 
 [account-auditor]
 
diff --git a/templates/container-server.conf.erb b/templates/container-server.conf.erb
@@ -2,19 +2,19 @@
 devices = <%= devices %>
 bind_ip = <%= storage_local_net_ip %>
 bind_port = <%= bind_port %>
-mount_check = false
-user = swift
+mount_check = <%= mount_check %>
+user = <%= user %>
 log_facility = LOG_LOCAL2
-workers = 2
+workers = <%= workers %>
+concurrency = <%= concurrency %>
 
 [pipeline:main]
-pipeline = container-server
+pipeline = <%= pipeline.to_a.join(' ') %>
 
 [app:container-server]
 use = egg:swift#container
 
 [container-replicator]
-vm_test_mode = yes
 
 [container-updater]
 
diff --git a/templates/object-server.conf.erb b/templates/object-server.conf.erb
@@ -2,19 +2,19 @@
 devices = <%= devices %>
 bind_ip = <%= storage_local_net_ip %>
 bind_port = <%= bind_port %>
-mount_check = false
-user = swift
+mount_check = <%= mount_check %>
+user = <%= user %>
 log_facility = LOG_LOCAL2
-workers = 2
+workers = <%= workers %>
+concurrency = <%= concurrency %>
 
 [pipeline:main]
-pipeline = object-server
+pipeline = <%= pipeline.to_a.join(' ') %>
 
 [app:object-server]
 use = egg:swift#object
 
 [object-replicator]
-vm_test_mode = yes
 
 [object-updater]
 
