Merge pull request #6 from bodepd/master

Refactor

.gitignore

@@ -0,0 +1,5 @@
+spec/fixtures/modules/apt
+spec/fixtures/modules/memcached
+spec/fixtures/modules/rsync
+spec/fixtures/modules/ssh
+spec/fixtures/modules/stdlib

.travis.yml

@@ -0,0 +1,22 @@
+language: ruby
+rvm:
+  - 1.8.7
+  - 1.9.2
+before_script:
+  - 'git clone git://github.com/saz/puppet-ssh spec/fixtures/modules/ssh'
+  - 'git clone git://github.com/puppetlabs/puppetlabs-rsync spec/fixtures/modules/rsync'
+  - 'git clone git://github.com/saz/puppet-memcached spec/fixtures/modules/memcached'
+  - 'git clone git://github.com/puppetlabs/puppetlabs-stdlib spec/fixtures/modules/stdlib'
+  - 'git clone git://github.com/puppetlabs/puppet-apt spec/fixtures/modules/apt'
+script: "rake spec"
+branches:
+  only:
+    - master
+env:
+  - PUPPET_VERSION=2.6.12
+  - PUPPET_VERSION=2.7.6
+  - PUPPET_VERSION=2.7.11
+matrix:
+  exclude:
+    - rvm: 1.9.2
+      env: PUPPET_VERSION=2.6.12

Gemfile

@@ -0,0 +1,11 @@
+source :rubygems
+
+puppetversion = ENV.key?('PUPPET_VERSION') ? "= #{ENV['PUPPET_VERSION']}" : ['>= 2.7']
+
+gem 'puppet', puppetversion
+
+group :test do
+  gem 'rake', '>= 0.9.0'
+  gem 'rspec', '>= 2.8.0'
+  gem 'rspec-puppet', '>= 0.1.1'
+end

QUESTIONS

@@ -0,0 +1 @@
+do we need ssh keys for anything?

Rakefile

@@ -1,14 +1,21 @@
+require 'rubygems'
 require 'rake'
+require 'rspec/core/rake_task'
 
-task :default => [:spec]
+task :default do
+  system("rake -T")
+end
 
-desc "Run all module spec tests (Requires rspec-puppet gem)"
+task :specs => [:spec]
-task :spec do
+
-  system("rspec spec/**/*_spec.rb")
+desc "Run all rspec-puppet tests"
+RSpec::Core::RakeTask.new(:spec) do |t|
+  t.rspec_opts = ['--color']
+  # ignores fixtures directory.
+  t.pattern = 'spec/{classes,defines,unit}/**/*_spec.rb'
 end
 
 desc "Build package"
 task :build do
   system("puppet-module build")
 end
-

examples/all.pp

@@ -3,7 +3,8 @@
 # same as what I have been using
 # to build swift in my environment (which is based on vagrant)
 #
-$proxy_local_net_ip='127.0.0.1'
+$swift_local_net_ip='127.0.0.1'
+
 $swift_shared_secret='changeme'
 
 Exec { logoutput => true }
@@ -13,7 +14,7 @@ package { 'curl': ensure => present }
 class { 'ssh::server::install': }
 
 class { 'memcached':
-  listen_ip => $proxy_local_net_ip,
+  listen_ip => $swift_local_net_ip,
 }
 
 class { 'swift':
@@ -22,6 +23,12 @@ class { 'swift':
   package_ensure => latest,
 }
 
+# === Configure Storage
+
+class { 'swift::storage':
+  storage_local_net_ip => $swift_local_net_ip
+}
+
 # create xfs partitions on a loopback device and mounts them
 swift::storage::loopback { ['1', '2', '3']:
   require => Class['swift'],
@@ -34,22 +41,22 @@ Swift::Storage::Node {
   mnt_base_dir         => '/srv/node',
   weight               => 1,
   manage_ring          => true,
-  storage_local_net_ip => '127.0.0.1',
+  storage_local_net_ip => $swift_local_net_ip,
 }
 
 swift::storage::node { '1':
-  zone                 => 1,
+  zone    => 1,
-  require              => Swift::Storage::Loopback[1],
+  require => Swift::Storage::Loopback[1],
 }
 
 swift::storage::node { '2':
-  zone                 => 2,
+  zone    => 2,
-  require              => Swift::Storage::Loopback[2],
+  require => Swift::Storage::Loopback[2],
 }
 
 swift::storage::node { '3':
-  zone                 => 3,
+  zone    => 3,
-  require              => Swift::Storage::Loopback[3],
+  require => Swift::Storage::Loopback[3],
 }
 
 class { 'swift::ringbuilder':
@@ -59,7 +66,6 @@ class { 'swift::ringbuilder':
   require        => Class['swift'],
 }
 
-class { 'swift::storage': }
 
 # TODO should I enable swath in the default config?
 class { 'swift::proxy':

examples/multi.pp

@@ -65,6 +65,7 @@ node 'swift_storage_3' {
 #
 node 'swift_proxy' {
 
+  # TODO this should not be recommended
   class { 'role_swift_ringbuilder': }
   class { 'role_swift_proxy':
     require => Class['role_swift_ringbuilder'],
@@ -128,11 +129,6 @@ class role_swift_proxy inherits role_swift {
 
 class role_swift_storage inherits role_swift {
 
-  class { 'swift::storage':
-    storage_local_net_ip => $swift_local_net_ip,
-    devices              => '/srv/node',
-  }
-
   # create xfs partitions on a loopback device and mount them
   swift::storage::loopback { '1':
     base_dir     => '/srv/loopback-device',
@@ -140,6 +136,15 @@ class role_swift_storage inherits role_swift {
     require      => Class['swift'],
   }
 
+  # install all swift storage servers together
+  class { 'swift::storage::all':
+    storage_local_net_ip => $swift_local_net_ip,
+  }
+
+
+  # TODO I need to wrap these in a define so that
+  # mcollective can collect that define
+
   # these exported resources write ring config
   # resources into the database so that they can be
   # consumed by the ringbuilder role
@@ -155,6 +160,7 @@ class role_swift_storage inherits role_swift {
     weight      => 1,
   }
 
+  # TODO should device be changed to volume
   @@ring_account_device { "${swift_local_net_ip}:6002":
     zone        => $swift_zone,
     device_name => 1,

manifests/init.pp

@@ -7,9 +7,6 @@
 # [*package_ensure*] The ensure state for the swift package.
 #   Optional. Defaults to present.
 #
-# [*swift_ssh_key*] NOT YET IMPLEMENTED. I am not entirely sure what
-#  this key is intended to be used for.
-#
 # == Dependencies
 #
 #   Class['ssh::server::install']
@@ -24,11 +21,9 @@
 #
 class swift(
   $swift_hash_suffix,
-#  $swift_ssh_key,
   $package_ensure = 'present'
 ) {
 
-  # maybe I should just install ssh?
   Class['ssh::server::install'] -> Class['swift']
 
   package { 'swift':
@@ -56,31 +51,4 @@ class swift(
     mode    => 0660,
     content => template('swift/swift.conf.erb'),
   }
-
-#  if ($swift_ssh_key) {
-#    if $swift_ssh_key !~ /^(ssh-...) +([^ ]*) *([^ \n]*)/ {
-#      err("Can't parse swift_ssh_key")
-#      notify { "Can't parse public key file $name on the keymaster: skipping ensure => $e
-#nsure": }
-#    } else {
-#      $keytype = $1
-#      $modulus = $2
-#      $comment = $3
-#      ssh_authorized_key { $comment:
-#        ensure  => "present",
-#        user    => "swift",
-#        type    => $keytype,
-#        key     => $modulus,
-#        options => $options ? { "" => undef, default => $options },
-#        require => File["/home/swift"]
-#      }
-#    }
-#  }
-# does swift need an ssh key?
-# they are adding one in the openstack modules
-
-#
-# I do not understand how to configure the rings
-# or why rings would be configured on the proxy?
-
 }

manifests/proxy.pp

@@ -1,21 +1,33 @@
+#
+# TODO - assumes that proxy server is always a memcached server
+#
+# TODO - the full list of all things that can be configured is here
+#  https://github.com/openstack/swift/tree/master/swift/common/middleware
+#
 # Installs and configures the swift proxy node.
 #
 # [*Parameters*]
 #
-# [*allow_account_management*]
-# [*account_autocreate*] Rather accounts should automatically be created.
-#  I think this may be tempauth specific
 # [*proxy_local_net_ip*] The address that the proxy will bind to.
-#   Optional. Defaults to 127.0.0.1
+#   Required.
-#   TODO - this default is probably not ideal
+# [*port*] The port to which the proxy server will bind.
-# [*proxy_port*] Port that the swift proxy service will bind to.
+#   Optional. Defaults to 8080.
-#   Optional. Defaults to 11211
+# [*workers*] Number of threads to process requests.
+#  Optional. Defaults to the number of processors.
 # [*auth_type*] - Type of authorization to use.
 #  valid values are tempauth, swauth, and keystone.
 #  Optional. Defaults to tempauth.
+# [*allow_account_management*]
+#   Rather or not requests through this proxy can create and
+#   delete accounts. Optional. Defaults to true.
+# [*account_autocreate*] Rather accounts should automatically be created.
+#  Has to be set to true for tempauth. Optional. Defaults to true.
+# [*proxy_port*] Port that the swift proxy service will bind to.
+#   Optional. Defaults to 11211
 # [*package_ensure*] Ensure state of the swift proxy package.
 #   Optional. Defaults to present.
-#
+# [*cache_servers*] A list of the memcache servers to be used. Entries
+#  should be in the form host:port.
 # == sw auth specific configuration
 # [*swauth_endpoint*]
 # [*swauth_super_admin_user*]
@@ -35,21 +47,30 @@
 # Copyright 2011 Puppetlabs Inc, unless otherwise noted.
 #
 class swift::proxy(
-  # why did cloudbuilders default this to false?
+  $proxy_local_net_ip,
+  $port = '8080',
+  $workers = $::processorcount,
+  $cache_servers = ['127.0.0.1:11211'],
   $allow_account_management = true,
-  $account_autocreate = false,
-  $proxy_local_net_ip = '127.0.0.1',
-  $proxy_port = '11211',
   $auth_type = 'tempauth',
+  $account_autocreate = true,
   $swauth_endpoint = '127.0.0.1',
   $swauth_super_admin_key = 'swauthkey',
   $package_ensure = 'present'
 ) inherits swift {
 
-  Class['memcached'] -> Class['swift::proxy']
+  validate_bool($account_autocreate)
-
+  validate_bool($allow_account_management)
   validate_re($auth_type, 'tempauth|swauth|keystone')
 
+  if($auth_type == 'tempauth' and ! $account_autocreate ){
+    fail("\$account_autocreate must be set to true when auth type is tempauth")
+  }
+
+  if $cache_server_ips =~ /^127\.0\.0\.1/ {
+    Class['memcached'] -> Class['swift::proxy']
+  }
+
   if(auth_type == 'keystone') {
     fail('Keystone is currently not supported, it should be supported soon :)')
   }

manifests/ringbuilder.pp

@@ -2,10 +2,9 @@
 # Rings are used to make decicions about how to map objects in the cluster
 #
 # Specifies the following relationship:
-#  Rings shoudl be created before any devices are added to them
+#  Rings should be created before any devices are added to them
 #  Rings should be rebalanced if anything changes
 # == Parameters
-#  # TODO - I need to review the ringbuilder docs
 #  [*part_power*] The total number of partitions that should exist in the ring.
 #    This is expressed as a power of 2.
 #  [*replicas*] Numer of replicas that should be maintained of each stored object.

manifests/ringbuilder/create.pp

@@ -8,18 +8,16 @@
 #  [*part_power*] Number of partitions in the ring. (specified as the power of 2)
 #    Optional. Defaults to 18 (2^18)
 #  [*replicas] Number of replicas to store.
-#    Optional. Defaults to 5
+#    Optional. Defaults to 3
-#      TODO should it default to 3?
 #  [*min_part_hours*] Time before a partition can be moved.
-#    Optional. Defaults to 1.
+#    Optional. Defaults to 24.
-#      TODO should it be 24?
 #
 # == Examples
 #
 #   swift::ringbuilder::create { 'account':
 #     part_power     => 19,
-#     replicas       => 3,
+#     replicas       => 5,
-#     min_part_hours => 24,
+#     min_part_hours => 1,
 #   }
 #
 # == Authors
@@ -32,8 +30,8 @@
 #
 define swift::ringbuilder::create(
   $part_power = 18,
-  $replicas = 5,
+  $replicas = 3,
-  $min_part_hours = 1
+  $min_part_hours = 24
 ) {
 
   validate_re($name, '^object|container|account$')

manifests/storage.pp

@@ -1,25 +1,12 @@
 #
-#  Configures a swift storage node to host servers for object,
+# Configures dependencies that are common for all storage
-#  container, and accounts.
+# types.
+#   - installs an rsync server
+#   - installs required packages
 #
-#  Includes:
-#    installing an rsync server
-#    installs storeage packages (object,account,containers)
 # == Parameters
 #  [*storeage_local_net_ip*] ip address that the swift servers should
-#    bind to. Optional. Defaults to 127.0.0.1 .
+#    bind to. Required.
-#    TODO - should this default to 0.0.0.0 ?
-#  [*package_ensure*] The desired ensure state of the swift storage packages.
-#    Optional. Defaults to present.
-#  [*devices*] The path where the managed volumes can be found.
-#    This assumes that all servers use the same path.
-#    Optional. Defaults to /srv/node/
-#  [*object_port*] Port where object storage server should be hosted.
-#    Optional. Defaults to 6000.
-#  [*container_port*] Port where the container storage server should be hosted.
-#    Optional. Defaults to 6001.
-#  [*account_port*] Port where the account storage server should be hosted.
-#    Optional. Defaults to 6002.
 # == Dependencies
 #
 # == Examples
@@ -33,107 +20,16 @@
 # Copyright 2011 Puppetlabs Inc, unless otherwise noted.
 #
 class swift::storage(
-  $package_ensure = 'present',
+  $storage_local_net_ip
-  # TODO - should this default to 0.0.0.0?
-  $storage_local_net_ip = '127.0.0.1',
-  $devices = '/srv/node',
-  $object_port = '6000',
-  $container_port = '6001',
-  $account_port = '6002'
 ) inherits swift {
 
-
   class{ 'rsync::server':
     use_xinetd => false,
-    address => $storage_local_net_ip,
+    address    => $storage_local_net_ip,
-  }
-
-  Service {
-    ensure    => running,
-    enable    => true,
-    hasstatus => true,
-    subscribe => Service['rsync'],
-  }
-
-  File {
-    owner => 'swift',
-    group => 'swift',
-  }
-
-  Swift::Storage::Server {
-    devices              => $devices,
-    storage_local_net_ip => $storage_local_net_ip,
   }
 
   # package dependencies
   package { ['xfsprogs', 'parted']:
     ensure => 'present'
   }
-
-  package { 'swift-account':
-    ensure => $package_ensure,
-  }
-
-  swift::storage::server { $account_port:
-    type             => 'account',
-    config_file_path => 'account-server.conf',
-  }
-
-  file { '/etc/swift/account-server/':
-    ensure => directory,
-  }
-
-  service { 'swift-account':
-    provider  => 'upstart',
-  }
-
-  # container server configuration
-  package { 'swift-container':
-    ensure => $package_ensure,
-  }
-
-  swift::storage::server { $container_port:
-    type             => 'container',
-    config_file_path => 'container-server.conf',
-  }
-
-  file { '/etc/swift/container-server/':
-    ensure => directory,
-  }
-
-  service { 'swift-container':
-    provider  => 'upstart',
-  }
-
-  # object server configuration
-  package { 'swift-object':
-    ensure => $package_ensure,
-  }
-
-  swift::storage::server { $object_port:
-    type             => 'object',
-    config_file_path => 'object-server.conf',
-  }
-
-  file { '/etc/swift/object-server/':
-    ensure => directory,
-  }
-
-  service { 'swift-object':
-    provider  => 'upstart',
-  }
-
-  # TODO this should be removed when the upstart packages are fixed.
-  define upstart() {
-    file { "/etc/init/swift-${name}.conf":
-      mode   => '0644',
-      owner  => 'root',
-      group  => 'root',
-      source => "puppet:///modules/swift/swift-${name}.conf.upstart",
-      before => Service["swift-${name}"],
-    }
-  }
-
-  swift::storage::upstart { ['object', 'container', 'account']: }
-
 }

manifests/storage/account.pp

@@ -0,0 +1,7 @@
+class swift::storage::account(
+  $package_ensure = 'present'
+) {
+  swift::storage::generic { 'account':
+    package_ensure => $package_ensure,
+  }
+}

manifests/storage/all.pp

@@ -0,0 +1,49 @@
+#
+# configures all storage types
+# on the same node
+#
+#  [*storeage_local_net_ip*] ip address that the swift servers should
+#    bind to. Required
+#  [*devices*] The path where the managed volumes can be found.
+#    This assumes that all servers use the same path.
+#    Optional. Defaults to /srv/node/
+#  [*object_port*] Port where object storage server should be hosted.
+#    Optional. Defaults to 6000.
+#  [*container_port*] Port where the container storage server should be hosted.
+#    Optional. Defaults to 6001.
+#  [*account_port*] Port where the account storage server should be hosted.
+#    Optional. Defaults to 6002.
+#
+#
+class swift::storage::all(
+  $storage_local_net_ip,
+  $devices        = '/srv/node',
+  $object_port    = '6000',
+  $container_port = '6001',
+  $account_port   = '6002'
+) {
+
+  class { 'swift::storage':
+    storage_local_net_ip => $storage_local_net_ip,
+  }
+
+  Swift::Storage::Server {
+    devices              => $devices,
+    storage_local_net_ip => $storage_local_net_ip,
+  }
+
+  swift::storage::server { $account_port:
+    type             => 'account',
+    config_file_path => 'account-server.conf',
+  }
+
+  swift::storage::server { $container_port:
+    type             => 'container',
+    config_file_path => 'container-server.conf',
+  }
+
+  swift::storage::server { $object_port:
+    type             => 'object',
+    config_file_path => 'object-server.conf',
+  }
+}

manifests/storage/container.pp

@@ -0,0 +1,7 @@
+class swift::storage::container(
+  $package_ensure = 'present'
+) {
+  swift::storage::generic { 'container':
+    package_ensure => $package_ensure
+  }
+}

manifests/storage/generic.pp

@@ -0,0 +1,59 @@
+# Creates the files packages and services that are
+# needed to deploy each type of storage server.
+#
+# == Parameters
+#  [*package_ensure*] The desired ensure state of the swift storage packages.
+#    Optional. Defaults to present.
+#  [*service_provider*] The provider to use for the service
+#
+# == Dependencies
+#  Requires Class[swift::storage]
+# == Examples
+#
+# == Authors
+#
+#   Dan Bode dan@puppetlabs.com
+#
+# == Copyright
+#
+# Copyright 2011 Puppetlabs Inc, unless otherwise noted.
+define swift::storage::generic(
+  $package_ensure   = 'present',
+  $service_provider = 'upstart'
+) {
+
+  Class['swift::storage'] -> Swift::Storage::Generic[$name]
+
+  validate_re($name, '^object|container|account$')
+
+  package { "swift-${name}":
+    ensure => $package_ensure,
+  }
+
+  file { "/etc/swift/${name}-server/":
+    ensure => directory,
+    owner  => 'swift',
+    group  => 'swift',
+  }
+
+  service { "swift-${name}":
+    ensure    => running,
+    enable    => true,
+    hasstatus => true,
+    provider  => $service_provider,
+    subscribe => Service['rsync'],
+  }
+
+  swift::storage::generic::upstart { $name: }
+
+}
+# TODO this should be removed when the upstart packages are fixed.
+define swift::storage::generic::upstart() {
+  file { "/etc/init/swift-${name}.conf":
+    mode   => '0644',
+    owner  => 'root',
+    group  => 'root',
+    source => "puppet:///modules/swift/swift-${name}.conf.upstart",
+    before => Service["swift-${name}"],
+  }
+}

manifests/storage/object.pp

@@ -0,0 +1,7 @@
+class swift::storage::object(
+  $package_ensure = 'present'
+) {
+  swift::storage::generic { 'object':
+    package_ensure => $package_ensure
+  }
+}

manifests/storage/server.pp

@@ -5,15 +5,17 @@
 #  name - is going to be port
 define swift::storage::server(
   $type,
+  $storage_local_net_ip,
   $devices = '/srv/node',
   $owner = 'swift',
   $group  = 'swift',
   $max_connections = 25,
-  $storage_local_net_ip = '127.0.0.1',
   # this parameters needs to be specified after type and name
   $config_file_path = "${type}-server/${name}.conf"
 ) {
 
+  include "swift::storage::$type"
+
   validate_re($name, '^\d+$')
   validate_re($type, '^object|container|account$')
   # TODO - validate that name is an integer

spec/classes/swift_proxy_spec.rb

@@ -29,14 +29,20 @@ describe 'swift::proxy' do
        class { 'ssh::server::install': }"
     end
 
-    describe 'with default parameters' do
+    describe 'without the proxy local network ip address being specified' do
+      it "should fail" do
+        expect do
+          subject
+        end.should raise_error(Puppet::Error, /Must pass proxy_local_net_ip/)
+      end
+    end
 
-      let :config_file do
+    describe 'when proxy_local_net_ip is set' do
-        File.join(fixture_dir, 'default_proxy_server')
+
+      let :params do
+        {:proxy_local_net_ip => '127.0.0.1'}
       end
 
-      it { should contain_package('swift-proxy').with_ensure('present') }
-      it { should_not contain_package('python-swauth') }
       it { should contain_service('swift-proxy').with(
         {:ensure    => 'running',
          :provider  => 'upstart',
@@ -49,58 +55,167 @@ describe 'swift::proxy' do
          :owner   => 'swift',
          :group   => 'swift',
          :mode    => '0660',
-         :content => File.read(config_file),
          :require => 'Package[swift-proxy]'
         }
       )}
+
+      it 'should contain default config file' do
+        content = param_value(
+          subject,
+          'file', '/etc/swift/proxy-server.conf',
+          'content'
+        )
+        expected_lines =
+        [
+          '[DEFAULT]',
+          'bind_port = 8080',
+          "workers = #{facts[:processorcount]}",
+          'user = swift',
+          '[pipeline:main]',
+          'pipeline = healthcheck cache tempauth proxy-server',
+          '[app:proxy-server]',
+          'use = egg:swift#proxy',
+          'allow_account_management = true',
+          'account_autocreate = true',
+          '[filter:healthcheck]',
+          'use = egg:swift#healthcheck',
+          '[filter:cache]',
+          'use = egg:swift#memcache',
+          'memcache_servers = 127.0.0.1:11211'
+        ]
+        (content.split("\n") & expected_lines).should =~ expected_lines
+      end
+
+      describe 'when more parameters are set' do
+        let :params do
+          {
+           :proxy_local_net_ip => '10.0.0.2',
+           :port => '80',
+           :workers => 3,
+           :cache_servers => ['foo:1', 'bar:2'],
+           :allow_account_management => true
+          }
+        end
+        it 'should contain default config file' do
+          content = param_value(
+            subject,
+            'file', '/etc/swift/proxy-server.conf',
+            'content'
+          )
+          expected_lines =
+            [
+              'bind_port = 80',
+              "workers = 3",
+              'allow_account_management = true',
+              'memcache_servers = foo:1,bar:2'
+            ]
+          (content.split("\n") & expected_lines).should =~ expected_lines
+        end
+      end
       # TODO this resource should just be here temporarily until packaging
       # is fixed
       it { should contain_file('/etc/init/swift-proxy.conf') }
 
+      describe 'when using tempauth' do
+
+        it { should_not contain_package('python-swauth') }
+        it 'should fail when setting account_autocreate to false' do
+          params[:auth_type] = 'tempauth'
+          params[:account_autocreate] = false
+          expect do
+            subject
+          end.should raise_error(Puppet::Error, /account_autocreate must be set to true when auth type is tempauth/)
+        end
+        it 'should contain tempauth configuration' do
+          content = param_value(
+            subject,
+            'file', '/etc/swift/proxy-server.conf',
+            'content'
+          )
+          expected_lines =
+          [
+          'pipeline = healthcheck cache tempauth proxy-server',
+          '[filter:tempauth]',
+          'use = egg:swift#tempauth',
+          'user_admin_admin = admin .admin .reseller_admin',
+          'user_test_tester = testing .admin',
+          'user_test2_tester2 = testing2 .admin',
+          'user_test_tester3 = testing3'
+          ]
+          (content.split("\n") & expected_lines).should =~ expected_lines
+        end
+      end
+
+      describe 'when supplying bad values for parameters' do
+        [:account_autocreate, :allow_account_management].each do |param|
+          it "should fail when #{param} is not passed a boolean" do
+            params[param] = 'false'
+            expect do
+              subject
+            end.should raise_error(Puppet::Error, /is not a boolean/)
+          end
+        end
+      end
     end
 
     describe 'when using swauth' do
 
       let :params do
-        {:auth_type => 'swauth' }
+        {:proxy_local_net_ip => '127.0.0.1',
+         :auth_type => 'swauth' }
       end
 
       describe 'with defaults' do
 
-        let :config_file do
-          File.join(fixture_dir, 'swauth_default_proxy_server')
-        end
-
         it { should contain_package('python-swauth').with(
           {:ensure => 'present',
            :before => 'Package[swift-proxy]'
           }
         )}
-        it { should contain_file('/etc/swift/proxy-server.conf').with(
+        it 'should create a config file with default swauth config' do
-          {:content => File.read(config_file)}
+          content = param_value(
-        )}
+            subject,
+            'file', '/etc/swift/proxy-server.conf',
+            'content'
+          )
+          expected_lines =
+          [
+            '[filter:swauth]',
+            'use = egg:swauth#swauth',
+            'default_swift_cluster = local#127.0.0.1',
+            'super_admin_key = swauthkey'
+          ]
+          (content.split("\n") & expected_lines).should =~ expected_lines
+
+        end
       end
 
       describe 'with parameter overrides' do
 
         let :params do
-          {:auth_type => 'swauth',
+          {:proxy_local_net_ip => '127.0.0.1',
+           :auth_type => 'swauth',
            :swauth_endpoint => '10.0.0.1',
            :swauth_super_admin_key => 'key'
           }
         end
-        let :config_file do
+
-          File.join(fixture_dir, 'swauth_overrides_proxy_server')
+        it 'should create a config file with default swauth config' do
+          content = param_value(
+            subject,
+            'file', '/etc/swift/proxy-server.conf',
+            'content'
+          )
+          expected_lines =
+          [
+            '[filter:swauth]',
+            'use = egg:swauth#swauth',
+            'default_swift_cluster = local#10.0.0.1',
+            'super_admin_key = key'
+          ]
+          (content.split("\n") & expected_lines).should =~ expected_lines
         end
-
-        it { should contain_file('/etc/swift/proxy-server.conf').with(
-          {:content => File.read(config_file)}
-        )}
-
       end
-
     end
-
   end
-
 end

spec/classes/swift_ringbuilder_spec.rb

@@ -29,8 +29,8 @@ describe 'swift::ringbuilder' do
       ['object', 'account', 'container'].each do |type|
         it { should contain_swift__ringbuilder__create(type).with(
           :part_power     => '18',
-          :replicas       => '5',
+          :replicas       => '3',
-          :min_part_hours => '1'
+          :min_part_hours => '24'
         )}
       end
     end

spec/classes/swift_storage_account_spec.rb

@@ -0,0 +1,27 @@
+require 'spec_helper'
+describe 'swift::storage::account' do
+
+  let :pre_condition do
+    "class { 'ssh::server::install': }
+     class { 'swift': swift_hash_suffix => 'foo' }
+     class { 'swift::storage': storage_local_net_ip => '10.0.0.1' }"
+  end
+
+  let :default_params do
+    {:package_ensure => 'present'}
+  end
+
+  [{},
+   {:package_ensure => 'latest'}
+  ].each do |param_set|
+    describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do
+      let :param_hash do
+        default_params.merge(param_set)
+      end
+      let :params do
+        param_set
+      end
+      it { should contain_swift__storage__generic('account').with_package_ensure(param_hash[:package_ensure]) }
+    end
+  end
+end

spec/classes/swift_storage_all_spec.rb

@@ -0,0 +1,95 @@
+require 'spec_helper'
+
+describe 'swift::storage::all' do
+  # TODO I am not testing the upstart code b/c it should be temporary
+
+  let :pre_condition do
+    "class { 'swift': swift_hash_suffix => 'changeme' }
+     include ssh::server::install
+    "
+  end
+
+  let :default_params do
+    {
+      :devices => '/srv/node',
+      :object_port => '6000',
+      :container_port => '6001',
+      :account_port => '6002'
+    }
+  end
+
+  describe 'when an internal network ip is not specified' do
+    it 'should fail' do
+      expect do
+        subject
+      end.should raise_error(Puppet::Error, /Must pass storage_local_net_ip/)
+    end
+  end
+
+  [{  :storage_local_net_ip => '127.0.0.1' },
+   {
+      :devices => '/tmp/node',
+      :storage_local_net_ip => '10.0.0.1',
+      :object_port => '7000',
+      :container_port => '7001',
+      :account_port => '7002'
+    }
+  ].each do |param_set|
+
+    describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do
+      let :param_hash do
+        default_params.merge(param_set)
+      end
+
+      let :params do
+        param_set
+      end
+
+      ['xfsprogs', 'parted'].each do |present_package|
+        it { should contain_package(present_package).with_ensure('present') }
+      end
+
+      ['object', 'container', 'account'].each do |type|
+        it { should contain_package("swift-#{type}").with_ensure('present') }
+        it { should contain_service("swift-#{type}").with(
+          {:provider => 'upstart',
+           :ensure   => 'running',
+           :enable    => true,
+           :hasstatus => true,
+           :subscribe => 'Service[rsync]'}
+        )}
+        it { should contain_file("/etc/swift/#{type}-server/").with(
+          {:ensure => 'directory',
+           :owner  => 'swift',
+           :group  => 'swift'}
+        )}
+      end
+
+      let :storage_server_defaults do
+        {:devices              => param_hash[:devices],
+         :storage_local_net_ip => param_hash[:storage_local_net_ip]
+        }
+      end
+
+      it { should contain_swift__storage__server(param_hash[:account_port]).with(
+        {:type => 'account',
+         :config_file_path => 'account-server.conf'}.merge(storage_server_defaults)
+      )}
+      it { should contain_swift__storage__server(param_hash[:object_port]).with(
+        {:type => 'object',
+         :config_file_path => 'object-server.conf'}.merge(storage_server_defaults)
+      )}
+      it { should contain_swift__storage__server(param_hash[:container_port]).with(
+        {:type => 'container',
+         :config_file_path => 'container-server.conf'}.merge(storage_server_defaults)
+      )}
+
+      it { should contain_class('rsync::server').with(
+        {:use_xinetd => false,
+         :address    => param_hash[:storage_local_net_ip]
+        }
+      )}
+
+    end
+  end
+end

spec/classes/swift_storage_container_spec.rb

@@ -0,0 +1,27 @@
+require 'spec_helper'
+describe 'swift::storage::container' do
+
+  let :pre_condition do
+    "class { 'ssh::server::install': }
+     class { 'swift': swift_hash_suffix => 'foo' }
+     class { 'swift::storage': storage_local_net_ip => '10.0.0.1' }"
+  end
+
+  let :default_params do
+    {:package_ensure => 'present'}
+  end
+
+  [{},
+   {:package_ensure => 'latest'}
+  ].each do |param_set|
+    describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do
+      let :param_hash do
+        default_params.merge(param_set)
+      end
+      let :params do
+        param_set
+      end
+      it { should contain_swift__storage__generic('container').with_package_ensure(param_hash[:package_ensure]) }
+    end
+  end
+end

spec/classes/swift_storage_object_spec.rb

@@ -0,0 +1,27 @@
+require 'spec_helper'
+describe 'swift::storage::object' do
+
+  let :pre_condition do
+    "class { 'ssh::server::install': }
+     class { 'swift': swift_hash_suffix => 'foo' }
+     class { 'swift::storage': storage_local_net_ip => '10.0.0.1' }"
+  end
+
+  let :default_params do
+    {:package_ensure => 'present'}
+  end
+
+  [{},
+   {:package_ensure => 'latest'}
+  ].each do |param_set|
+    describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do
+      let :param_hash do
+        default_params.merge(param_set)
+      end
+      let :params do
+        param_set
+      end
+      it { should contain_swift__storage__generic('object').with_package_ensure(param_hash[:package_ensure]) }
+    end
+  end
+end

spec/classes/swift_storage_spec.rb

@@ -3,88 +3,41 @@ require 'spec_helper'
 describe 'swift::storage' do
   # TODO I am not testing the upstart code b/c it should be temporary
 
-  let :pre_condition do
+  describe 'when required classes are specified' do
-    "class { 'swift': swift_hash_suffix => 'changeme' }
+    let :pre_condition do
-     include ssh::server::install
+      "class { 'swift': swift_hash_suffix => 'changeme' }
-    "
+       include ssh::server::install
-  end
+      "
-
+    end
-  let :default_params do
-    {
-      :package_ensure => 'present',
-      :storage_local_net_ip => '127.0.0.1',
-      :devices => '/srv/node',
-      :object_port => '6000',
-      :container_port => '6001',
-      :account_port => '6002'
-    }
-  end
-
-  [{},
-   {
-      :package_ensure => 'latest',
-      :devices => '/tmp/node',
-      :storage_local_net_ip => '10.0.0.1',
-      :object_port => '7000',
-      :container_port => '7001',
-      :account_port => '7002'
-    }
-  ].each do |param_set|
-
-    describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do
-      let :param_hash do
-        default_params.merge(param_set)
-      end
 
+    describe 'when the local net ip is specified' do
       let :params do
-        param_set
+        {
+          :storage_local_net_ip => '127.0.0.1',
+        }
       end
 
       ['xfsprogs', 'parted'].each do |present_package|
         it { should contain_package(present_package).with_ensure('present') }
       end
 
-      ['object', 'container', 'account'].each do |type|
-        it { should contain_package("swift-#{type}").with_ensure(param_hash[:package_ensure]) }
-        it { should contain_service("swift-#{type}").with(
-          {:provider => 'upstart',
-           :ensure   => 'running',
-           :enable    => true,
-           :hasstatus => true,
-           :subscribe => 'Service[rsync]'}
-        )}
-        it { should contain_file("/etc/swift/#{type}-server/").with(
-          {:ensure => 'directory',
-           :owner  => 'swift',
-           :group  => 'swift'}
-        )}
-      end
-
-      let :storage_server_defaults do
-        {:devices              => param_hash[:devices],
-         :storage_local_net_ip => param_hash[:storage_local_net_ip]
-        }
-      end
-
-      it { should contain_swift__storage__server(param_hash[:account_port]).with(
-        {:type => 'account',
-         :config_file_path => 'account-server.conf'}.merge(storage_server_defaults)
-      )}
-      it { should contain_swift__storage__server(param_hash[:object_port]).with(
-        {:type => 'object',
-         :config_file_path => 'object-server.conf'}.merge(storage_server_defaults)
-      )}
-      it { should contain_swift__storage__server(param_hash[:container_port]).with(
-        {:type => 'container',
-         :config_file_path => 'container-server.conf'}.merge(storage_server_defaults)
-      )}
-
       it { should contain_class('rsync::server').with(
         {:use_xinetd => false,
-         :address    => param_hash[:storage_local_net_ip]
+         :address    => params[:storage_local_net_ip]
         }
       )}
-
+    end
+    describe 'when local net ip is not specified' do
+      it 'should fail' do
+        expect do
+          subject
+        end.should raise_error(Puppet::Error, /Must pass storage_local_net_ip/)
+      end
+    end
+  end
+  describe 'when the dependencies are not specified' do
+    it 'should fail' do
+      expect { subject }.should raise_error(Puppet::Error)
     end
   end
 end

spec/defines/swift_ringbuilder_create_spec.rb

@@ -3,8 +3,8 @@ describe 'swift::ringbuilder::create' do
 
   let :default_params do
     {:part_power => 18,
-    :replicas => 5,
+    :replicas => 3,
-    :min_part_hours => 1}
+    :min_part_hours => 24}
   end
 
   describe 'with allowed titles' do

spec/defines/swift_storage_generic_spec.rb

@@ -0,0 +1,62 @@
+require 'spec_helper'
+describe 'swift::storage::generic' do
+
+  let :title do
+    'account'
+  end
+
+  let :pre_condition do
+    "class { 'ssh::server::install': }
+     class { 'swift': swift_hash_suffix => 'foo' }
+     class { 'swift::storage': storage_local_net_ip => '10.0.0.1' }"
+  end
+  let :default_params do
+    {:package_ensure => 'present',
+     :service_provider => 'upstart'}
+  end
+
+  describe 'with an invalid title' do
+    let :title do
+      'foo'
+    end
+    it 'should fail' do
+      expect do
+        subject
+      end.should raise_error(Puppet::Error, /does not match/)
+    end
+  end
+
+  ['account', 'object', 'container'].each do |t|
+    [{},
+     {:package_ensure => 'latest',
+      :service_provider => 'init'}
+    ].each do |param_set|
+      describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do
+        let :title do
+          t
+        end
+        let :param_hash do
+          default_params.merge(param_set)
+        end
+        let :params do
+          param_set
+        end
+        it { should contain_package("swift-#{t}").with_ensure(param_hash[:package_ensure]) }
+        it { should contain_service("swift-#{t}").with(
+          :ensure    => 'running',
+          :enable    => true,
+          :hasstatus => true,
+          :provider  => param_hash[:service_provider],
+          :subscribe => 'Service[rsync]'
+        )}
+        it { should contain_file("/etc/swift/#{t}-server/").with(
+          :ensure => 'directory',
+          :owner  => 'swift',
+          :group  => 'swift'
+        )}
+      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
+    end
+  end
+end

spec/defines/swift_storage_loopback_spec.rb

@@ -1,3 +1,4 @@
 describe 'swift::storage::loopback' do
-
+  # TODO add unit tests
+  # this is not the highest priority b/c it is really for testing
 end

spec/defines/swift_storage_mount_spec.rb

@@ -1,3 +1,4 @@
 describe 'swift::storage::mount' do
-
+  # TODO add unit tests
+  # not a super high priority b/c this is just for testing
 end

spec/defines/swift_storage_node_spec.rb

@@ -1,2 +1,4 @@
 describe 'swift::storage::node' do
+  # this is just for the SAOI
+  # add tests
 end

spec/defines/swift_storage_server_spec.rb

@@ -1,3 +1,97 @@
+require 'spec_helper'
 describe 'swift::storage::server' do
 
+  let :pre_condition do
+    "class { 'ssh::server::install': }
+     class { 'swift': swift_hash_suffix => 'foo' }
+     class { 'swift::storage': storage_local_net_ip => '10.0.0.1' }"
+  end
+  let :default_params do
+    {:devices => '/srv/node',
+     :owner => 'swift',
+     :group  => 'swift',
+     :max_connections => '25'}
+  end
+
+
+
+  describe 'with an invalid title' do
+    let :params do
+      {:storage_local_net_ip => '127.0.0.1',
+      :type => 'object'}
+    end
+    let :title do
+      'foo'
+    end
+    it 'should fail' do
+      expect do
+        subject
+      end.should raise_error(Puppet::Error, /does not match/)
+    end
+  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)
+        end
+        let :params do
+          param_set
+        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,
+          :subscribe => 'Service[rsync]'
+        )}
+        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],
+          :lock_file       => "/var/lock/#{t}#{title}.lock",
+          :uid             => param_hash[:owner],
+          :gid             => param_hash[:group],
+          :max_connections => param_hash[:max_connections],
+          :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
+    end
+  end
 end

spec/fixtures/default_proxy_server

@@ -1,32 +0,0 @@
-# This file is managed by puppet.  Do not edit
-#
-[DEFAULT]
-#cert_file = /etc/swift/cert.crt
-#key_file = /etc/swift/cert.key
-bind_port = 8080
-workers = 8
-user = swift
-
-[pipeline:main]
-# ratelimit?
-pipeline = healthcheck cache tempauth proxy-server
-
-[app:proxy-server]
-use = egg:swift#proxy
-allow_account_management = true
-account_autocreate = false
-
-[filter:tempauth]
-use = egg:swift#tempauth
-user_admin_admin = admin .admin .reseller_admin
-user_test_tester = testing .admin
-user_test2_tester2 = testing2 .admin
-user_test_tester3 = testing3
-
-[filter:healthcheck]
-use = egg:swift#healthcheck
-
-[filter:cache]
-use = egg:swift#memcache
-# multi-proxy config not supported
-memcache_servers = 127.0.0.1:11211

spec/fixtures/modules/swift

@@ -0,0 +1 @@
+../../../

spec/fixtures/swauth_default_proxy_server

@@ -1,31 +0,0 @@
-# This file is managed by puppet.  Do not edit
-#
-[DEFAULT]
-#cert_file = /etc/swift/cert.crt
-#key_file = /etc/swift/cert.key
-bind_port = 8080
-workers = 8
-user = swift
-
-[pipeline:main]
-# ratelimit?
-pipeline = healthcheck cache swauth proxy-server
-
-[app:proxy-server]
-use = egg:swift#proxy
-allow_account_management = true
-account_autocreate = false
-
-[filter:swauth]
-use = egg:swauth#swauth
-# this line is not in the install docs?
-default_swift_cluster = local#127.0.0.1
-super_admin_key = swauthkey
-
-[filter:healthcheck]
-use = egg:swift#healthcheck
-
-[filter:cache]
-use = egg:swift#memcache
-# multi-proxy config not supported
-memcache_servers = 127.0.0.1:11211

spec/fixtures/swauth_overrides_proxy_server

@@ -1,31 +0,0 @@
-# This file is managed by puppet.  Do not edit
-#
-[DEFAULT]
-#cert_file = /etc/swift/cert.crt
-#key_file = /etc/swift/cert.key
-bind_port = 8080
-workers = 8
-user = swift
-
-[pipeline:main]
-# ratelimit?
-pipeline = healthcheck cache swauth proxy-server
-
-[app:proxy-server]
-use = egg:swift#proxy
-allow_account_management = true
-account_autocreate = false
-
-[filter:swauth]
-use = egg:swauth#swauth
-# this line is not in the install docs?
-default_swift_cluster = local#10.0.0.1
-super_admin_key = key
-
-[filter:healthcheck]
-use = egg:swift#healthcheck
-
-[filter:cache]
-use = egg:swift#memcache
-# multi-proxy config not supported
-memcache_servers = 127.0.0.1:11211

spec/spec_helper.rb

@@ -7,5 +7,7 @@ def param_value(subject, type, title, param)
 end
 
 RSpec.configure do |c|
-  c.module_path = File.join(File.dirname(__FILE__), '../../')
+  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
+  c.manifest_dir = File.expand_path(File.join(File.dirname(__FILE__), 'fixtures/manifests'))
 end

templates/proxy-server.conf.erb

@@ -1,10 +1,8 @@
 # This file is managed by puppet.  Do not edit
 #
 [DEFAULT]
-#cert_file = /etc/swift/cert.crt
+bind_port = <%= port %>
-#key_file = /etc/swift/cert.key
+workers = <%= workers %>
-bind_port = 8080
-workers = 8
 user = swift
 
 [pipeline:main]
@@ -37,4 +35,4 @@ use = egg:swift#healthcheck
 [filter:cache]
 use = egg:swift#memcache
 # multi-proxy config not supported
-memcache_servers = <%= proxy_local_net_ip %>:<%= proxy_port %>
+memcache_servers = <%= cache_servers.to_a.join(',') %>