From d4922bee39b6ebf33eafe170154943a37a9aba0f Mon Sep 17 00:00:00 2001
From: Marcus Furlong <furlongm@gmail.com>
Date: Tue, 27 Sep 2016 17:50:21 +1000
Subject: [PATCH] memcache_servers support in object-expirer and
 container-reconciler

This commit adds support for the memcache_servers option in
object-expirer and container-reconciler. Without this, both
currently default to 127.0.0.1:11211.

Closes-Bug: #1627927
Change-Id: Ie139f018c4a742b014dd4d682970e154d66a8c6d
---
 .../ini_setting.rb                            |  10 ++
 .../type/swift_container_reconciler_config.rb |  55 ++++++
 manifests/containerreconciler.pp              | 103 +++++++++++
 manifests/objectexpirer.pp                    |  25 ++-
 manifests/params.pp                           |   3 +
 ...ache-servers-support-092ed535a0c26ae1.yaml |   3 +
 .../classes/swift_containerreconciler_spec.rb | 108 ++++++++++++
 spec/classes/swift_objectexpirer_spec.rb      | 161 ++++++++----------
 8 files changed, 376 insertions(+), 92 deletions(-)
 create mode 100644 lib/puppet/provider/swift_container_reconciler_config/ini_setting.rb
 create mode 100644 lib/puppet/type/swift_container_reconciler_config.rb
 create mode 100644 manifests/containerreconciler.pp
 create mode 100644 releasenotes/notes/memcache-servers-support-092ed535a0c26ae1.yaml
 create mode 100644 spec/classes/swift_containerreconciler_spec.rb

diff --git a/lib/puppet/provider/swift_container_reconciler_config/ini_setting.rb b/lib/puppet/provider/swift_container_reconciler_config/ini_setting.rb
new file mode 100644
index 00000000..188fb7fc
--- /dev/null
+++ b/lib/puppet/provider/swift_container_reconciler_config/ini_setting.rb
@@ -0,0 +1,10 @@
+Puppet::Type.type(:swift_container_reconciler_config).provide(
+  :ini_setting,
+  :parent => Puppet::Type.type(:openstack_config).provider(:ini_setting)
+) do
+
+  def self.file_path
+    '/etc/swift/container-reconciler.conf'
+  end
+
+end
diff --git a/lib/puppet/type/swift_container_reconciler_config.rb b/lib/puppet/type/swift_container_reconciler_config.rb
new file mode 100644
index 00000000..6ff9ad99
--- /dev/null
+++ b/lib/puppet/type/swift_container_reconciler_config.rb
@@ -0,0 +1,55 @@
+Puppet::Type.newtype(:swift_container_reconciler_config) do
+
+  ensurable
+
+  newparam(:name, :namevar => true) do
+    desc 'Section/setting name to manage from /etc/swift/container-reconciler.conf'
+    newvalues(/\S+\/\S+/)
+  end
+
+  newproperty(:value) do
+    desc 'The value of the setting to be defined.'
+    munge do |value|
+      value = value.to_s.strip
+      value.capitalize! if value =~ /^(true|false)$/i
+      value
+    end
+
+    def is_to_s( currentvalue )
+      if resource.secret?
+        return '[old secret redacted]'
+      else
+        return currentvalue
+      end
+    end
+
+    def should_to_s( newvalue )
+      if resource.secret?
+        return '[new secret redacted]'
+      else
+        return newvalue
+      end
+    end
+  end
+
+  newparam(:secret, :boolean => true) do
+    desc 'Whether to hide the value from Puppet logs. Defaults to `false`.'
+    newvalues(:true, :false)
+    defaultto false
+  end
+
+  newparam(:ensure_absent_val) do
+    desc 'A value that is specified as the value property will behave as if ensure => absent was specified'
+    defaultto('<SERVICE DEFAULT>')
+  end
+
+  # Require the package providing container-reconciler to be present
+  autorequire(:package) do
+    if Facter.value(:osfamily) == 'Debian'
+      'swift-container-reconciler'
+    elsif Facter.value(:osfamily) == 'RedHat'
+      'openstack-swift-container'
+    end
+  end
+
+end
diff --git a/manifests/containerreconciler.pp b/manifests/containerreconciler.pp
new file mode 100644
index 00000000..2e23ec9f
--- /dev/null
+++ b/manifests/containerreconciler.pp
@@ -0,0 +1,103 @@
+# Class swift::containerreconciler
+#
+# == Parameters
+#
+#  [*enabled*]
+#    (optional) Should the service be enabled.
+#    Defaults to true
+#
+#  [*manage_service*]
+#    (optional) Whether the service should be managed by Puppet.
+#    Defaults to true.
+#
+#  [*package_ensure*]
+#    (optional) Value of package resource parameter 'ensure'.
+#    Defaults to 'present'.
+#
+#  [*pipeline*]
+#    (optional) The list of elements of the container reconciler pipeline.
+#    Defaults to ['catch_errors', 'proxy-logging', 'cache', 'proxy-server']
+#
+#  [*interval*]
+#    (optional) Minimum time for a pass to take, in seconds.
+#    Defaults to 300.
+#
+#  [*reclaim_age*]
+#    (optional) The reconciler will re-attempt reconciliation if the source
+#    object is not available up to reclaim_age seconds before it gives up and
+#    deletes the entry in the queue.
+#    Defaults to 604800 (1 week).
+#
+#  [*request_tries*]
+#    (optional) Server errors from requests will be retried by default
+#    Defaults to 3.
+#
+#  [*service_provider*]
+#    (optional)
+#    To use the swiftinit service provider to manage swift services, set
+#    service_provider to "swiftinit".  When enable is true the provider
+#    will populate boot files that start swift using swift-init at boot.
+#    See README for more details.
+#    Defaults to $::swift::params::service_provider.
+#
+#  [*memcache_servers*]
+#    (optional)
+#    A list of the memcache servers to be used. Entries should be in the
+#    form host:port. This value is only used if 'cache' is added to the
+#    pipeline,
+#    e.g. ['catch_errors', 'proxy-logging', 'cache', 'proxy-server']
+#    Defaults to ['127.0.0.1:11211']
+#
+class swift::containerreconciler(
+  $manage_service   = true,
+  $enabled          = true,
+  $package_ensure   = 'present',
+  $pipeline         = ['catch_errors', 'proxy-logging', 'proxy-server'],
+  $interval         = 300,
+  $reclaim_age      = 604800,
+  $request_tries    = 3,
+  $service_provider = $::swift::params::service_provider,
+  $memcache_servers = ['127.0.0.1:11211'],
+) inherits ::swift::params {
+
+  include ::swift::deps
+  Swift_config<| |> ~> Service['swift-container-reconciler']
+  Swift_container_reconciler_config<||> ~> Service['swift-container-reconciler']
+
+  # only add memcache servers if 'cache' is included in the pipeline
+  if !empty(grep(any2array($pipeline), 'cache')) {
+
+    swift_container_reconciler_config {
+      'filter:cache/memcache_servers': value => join(any2array($memcache_servers), ',');
+    }
+
+    # require the memcached class if it is on the same machine
+    if !empty(grep(any2array($memcache_servers), '127.0.0.1')) {
+      Class['::memcached'] -> Class['::swift::containerreconciler']
+    }
+  }
+
+  swift_container_reconciler_config {
+    'pipeline:main/pipeline':             value => join($pipeline, ' ');
+    'container-reconciler/interval':      value => $interval;
+    'container-reconciler/reclaim_age':   value => $reclaim_age;
+    'container-reconciler/request_tries': value => $request_tries;
+  }
+
+  if $manage_service {
+    if $enabled {
+      $service_ensure = 'running'
+    } else {
+      $service_ensure = 'stopped'
+    }
+  }
+
+  swift::service { 'swift-container-reconciler':
+    os_family_service_name => $::swift::params::container_reconciler_service_name,
+    service_ensure         => $service_ensure,
+    enabled                => $enabled,
+    config_file_name       => 'container-reconciler.conf',
+    service_provider       => $service_provider,
+  }
+
+}
diff --git a/manifests/objectexpirer.pp b/manifests/objectexpirer.pp
index 27017256..ca71014a 100644
--- a/manifests/objectexpirer.pp
+++ b/manifests/objectexpirer.pp
@@ -64,11 +64,18 @@
 #    See README for more details.
 #    Defaults to $::swift::params::service_provider.
 #
+#  [*memcache_servers*]
+#    (optional)
+#    A list of the memcache servers to be used. Entries should be in the
+#    form host:port. This value is only used if 'cache' is added to the
+#    pipeline, e.g. ['catch_errors', 'cache', 'proxy-server']
+#    Defaults to ['127.0.0.1:11211']
+#
 class swift::objectexpirer(
   $manage_service                = true,
   $enabled                       = true,
   $package_ensure                = 'present',
-  $pipeline                      = ['catch_errors', 'cache', 'proxy-server'],
+  $pipeline                      = ['catch_errors', 'proxy-server'],
   $auto_create_account_prefix    = '.',
   $concurrency                   = 1,
   $expiring_objects_account_name = 'expiring_objects',
@@ -78,7 +85,8 @@ class swift::objectexpirer(
   $reclaim_age                   = 604800,
   $recon_cache_path              = '/var/cache/swift',
   $report_interval               = 300,
-  $service_provider              = $::swift::params::service_provider
+  $service_provider              = $::swift::params::service_provider,
+  $memcache_servers              = ['127.0.0.1:11211'],
 ) inherits ::swift::params {
 
   include ::swift::deps
@@ -95,6 +103,19 @@ class swift::objectexpirer(
     }
   }
 
+  # only add memcache servers if 'cache' is included in the pipeline
+  if !empty(grep(any2array($pipeline), 'cache')) {
+
+    swift_object_expirer_config {
+      'filter:cache/memcache_servers': value => join(any2array($memcache_servers), ',');
+    }
+
+    # require the memcached class if it is on the same machine
+    if !empty(grep(any2array($memcache_servers), '127.0.0.1')) {
+      Class['::memcached'] -> Class['::swift::objectexpirer']
+    }
+  }
+
   swift_object_expirer_config {
     'pipeline:main/pipeline':                       value => join($pipeline, ' ');
     'object-expirer/auto_create_account_prefix':    value => $auto_create_account_prefix;
diff --git a/manifests/params.pp b/manifests/params.pp
index 84d04607..527c00b0 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -26,6 +26,7 @@ class swift::params {
       $container_replicator_service_name = 'swift-container-replicator'
       $container_updater_service_name    = 'swift-container-updater'
       $container_sync_service_name       = 'swift-container-sync'
+      $container_reconciler_service_name = 'swift-container-reconciler'
       $account_package_name              = 'swift-account'
       $account_server_service_name       = 'swift-account'
       $account_auditor_service_name      = 'swift-account-auditor'
@@ -50,6 +51,7 @@ class swift::params {
       $container_replicator_service_name = 'openstack-swift-container-replicator'
       $container_updater_service_name    = 'openstack-swift-container-updater'
       $container_sync_service_name       = 'openstack-swift-container-sync'
+      $container_reconciler_service_name = 'openstack-swift-container-reconciler'
       $account_package_name              = 'openstack-swift-account'
       $account_server_service_name       = 'openstack-swift-account'
       $account_auditor_service_name      = 'openstack-swift-account-auditor'
@@ -77,5 +79,6 @@ class swift::params {
     'swift-container-server',
     'swift-container-sync',
     'swift-container-updater',
+    'swift-container-reconciler',
   ]
 }
diff --git a/releasenotes/notes/memcache-servers-support-092ed535a0c26ae1.yaml b/releasenotes/notes/memcache-servers-support-092ed535a0c26ae1.yaml
new file mode 100644
index 00000000..a3601a27
--- /dev/null
+++ b/releasenotes/notes/memcache-servers-support-092ed535a0c26ae1.yaml
@@ -0,0 +1,3 @@
+---
+features:
+  - memcache_servers support in object-expirer and container-reconciler
diff --git a/spec/classes/swift_containerreconciler_spec.rb b/spec/classes/swift_containerreconciler_spec.rb
new file mode 100644
index 00000000..c44298d4
--- /dev/null
+++ b/spec/classes/swift_containerreconciler_spec.rb
@@ -0,0 +1,108 @@
+require 'spec_helper'
+
+describe 'swift::containerreconciler' do
+
+  let :default_params do
+    { :manage_service   => true,
+      :enabled          => true,
+      :package_ensure   => 'present',
+      :pipeline         => ['catch_errors', 'proxy-logging', 'proxy-server'],
+      :interval         => 300,
+      :reclaim_age      => 604800,
+      :request_tries    => 3,
+      :memcache_servers => ['127.0.0.1:11211'] }
+  end
+
+  let :params do
+    {}
+  end
+
+  let :pre_condition do
+    'class { "memcached": max_memory => 1 }'
+  end
+
+  shared_examples_for 'swift-container-reconciler' do
+    let (:p) { default_params.merge!(params) }
+
+    context 'with defaults' do
+      it 'configures container-reconciler.conf' do
+        is_expected.to contain_swift_container_reconciler_config(
+          'pipeline:main/pipeline').with_value(p[:pipeline].join(' '))
+        is_expected.to contain_swift_container_reconciler_config(
+          'container-reconciler/interval').with_value(p[:interval])
+        is_expected.to contain_swift_container_reconciler_config(
+          'container-reconciler/reclaim_age').with_value(p[:reclaim_age])
+        is_expected.to contain_swift_container_reconciler_config(
+          'container-reconciler/request_tries').with_value(p[:request_tries])
+      end
+
+      it 'configures container-reconciler service' do
+        is_expected.to contain_service('swift-container-reconciler').with(
+          :ensure   => (p[:manage_service] && p[:enabled]) ? 'running' : 'stopped',
+          :name     => platform_params[:service_name],
+          :provider => platform_params[:service_provider],
+          :enable   => p[:enabled]
+        )
+      end
+    end
+
+    context 'when overridding parameters' do
+      before do
+        params.merge!(
+          :interval      => '600',
+          :reclaim_age   => '10000',
+          :request_tries => '3',
+        )
+      end
+    end
+
+    context 'when including cache in pipeline' do
+      before do
+        params.merge!(
+          :pipeline         => ['catch_errors', 'proxy-logging', 'cache', 'proxy-server'],
+          :memcache_servers => ['127.0.0.1:11211'],
+        )
+      end
+
+      it 'configures memcache servers' do
+        is_expected.to contain_swift_container_reconciler_config(
+          'filter:cache/memcache_servers').with_value(p[:memcache_servers])
+      end
+    end
+
+    context 'when using swiftinit service provider' do
+      before do
+        params.merge!({ :service_provider => 'swiftinit' })
+      end
+
+      before do
+        platform_params.merge!({ :service_provider => 'swiftinit' })
+      end
+    end
+
+  end
+
+  on_supported_os({
+    :supported_os => OSDefaults.get_supported_os
+  }).each do |os,facts|
+    context "on #{os}" do
+      let (:facts) do
+        facts.merge!(OSDefaults.get_facts())
+      end
+
+      let(:platform_params) do
+        case facts[:osfamily]
+        when 'Debian'
+          { :service_name     => 'swift-container-reconciler',
+            :service_provider => nil }
+        when 'RedHat'
+          { :service_name     => 'openstack-swift-container-reconciler',
+            :service_provider => nil }
+        end
+      end
+
+      it_configures 'swift-container-reconciler'
+    end
+
+  end
+end
diff --git a/spec/classes/swift_objectexpirer_spec.rb b/spec/classes/swift_objectexpirer_spec.rb
index fefa9a39..01eba4b0 100644
--- a/spec/classes/swift_objectexpirer_spec.rb
+++ b/spec/classes/swift_objectexpirer_spec.rb
@@ -6,7 +6,7 @@ describe 'swift::objectexpirer' do
     { :manage_service                => true,
       :enabled                       => true,
       :package_ensure                => 'present',
-      :pipeline                      => ['catch_errors', 'cache', 'proxy-server'],
+      :pipeline                      => ['catch_errors', 'proxy-server'],
       :auto_create_account_prefix    => '.',
       :concurrency                   => 1,
       :expiring_objects_account_name => 'expiring_objects',
@@ -15,80 +15,80 @@ describe 'swift::objectexpirer' do
       :processes                     => 0,
       :reclaim_age                   => 604800,
       :recon_cache_path              => '/var/cache/swift',
-      :report_interval               => 300 }
+      :report_interval               => 300,
+      :memcache_servers              => ['127.0.0.1:11211'] }
   end
 
   let :params do
     {}
   end
 
+  let :pre_condition do
+    'class { "memcached": max_memory => 1 }'
+  end
 
   shared_examples_for 'swift-object-expirer' do
     let (:p) { default_params.merge!(params) }
 
-    it 'configures object-expirer.conf' do
-      is_expected.to contain_swift_object_expirer_config(
-        'pipeline:main/pipeline').with_value(p[:pipeline].join(' '))
-      is_expected.to contain_swift_object_expirer_config(
-        'object-expirer/auto_create_account_prefix').with_value(p[:auto_create_account_prefix])
-      is_expected.to contain_swift_object_expirer_config(
-        'object-expirer/concurrency').with_value(p[:concurrency])
-      is_expected.to contain_swift_object_expirer_config(
-        'object-expirer/expiring_objects_account_name').with_value(p[:expiring_objects_account_name])
-      is_expected.to contain_swift_object_expirer_config(
-        'object-expirer/interval').with_value(p[:interval])
-      is_expected.to contain_swift_object_expirer_config(
-        'object-expirer/process').with_value(p[:process])
-      is_expected.to contain_swift_object_expirer_config(
-        'object-expirer/processes').with_value(p[:processes])
-      is_expected.to contain_swift_object_expirer_config(
-        'object-expirer/reclaim_age').with_value(p[:reclaim_age])
-      is_expected.to contain_swift_object_expirer_config(
-       'object-expirer/recon_cache_path').with_value(p[:recon_cache_path])
-      is_expected.to contain_swift_object_expirer_config(
-        'object-expirer/report_interval').with_value(p[:report_interval])
+    context 'with defaults' do
+      it 'configures object-expirer.conf' do
+        is_expected.to contain_swift_object_expirer_config(
+          'pipeline:main/pipeline').with_value(p[:pipeline].join(' '))
+        is_expected.to contain_swift_object_expirer_config(
+          'object-expirer/auto_create_account_prefix').with_value(p[:auto_create_account_prefix])
+        is_expected.to contain_swift_object_expirer_config(
+          'object-expirer/concurrency').with_value(p[:concurrency])
+        is_expected.to contain_swift_object_expirer_config(
+          'object-expirer/expiring_objects_account_name').with_value(p[:expiring_objects_account_name])
+        is_expected.to contain_swift_object_expirer_config(
+          'object-expirer/interval').with_value(p[:interval])
+        is_expected.to contain_swift_object_expirer_config(
+          'object-expirer/process').with_value(p[:process])
+        is_expected.to contain_swift_object_expirer_config(
+          'object-expirer/processes').with_value(p[:processes])
+        is_expected.to contain_swift_object_expirer_config(
+          'object-expirer/reclaim_age').with_value(p[:reclaim_age])
+        is_expected.to contain_swift_object_expirer_config(
+          'object-expirer/recon_cache_path').with_value(p[:recon_cache_path])
+        is_expected.to contain_swift_object_expirer_config(
+          'object-expirer/report_interval').with_value(p[:report_interval])
+      end
+
+      it 'configures object-expirer service' do
+        is_expected.to contain_service('swift-object-expirer').with(
+          :ensure    => (p[:manage_service] && p[:enabled]) ? 'running' : 'stopped',
+          :name      => platform_params[:service_name],
+          :provider  => platform_params[:service_provider],
+          :enable    => p[:enabled]
+        )
+      end
     end
 
-    it 'configures object-expirer service' do
-      is_expected.to contain_service('swift-object-expirer').with(
-        :ensure    => (p[:manage_service] && p[:enabled]) ? 'running' : 'stopped',
-        :name      => platform_params[:service_name],
-        :provider  => platform_params[:service_provider],
-        :enable    => p[:enabled]
-      )
-    end
-
-  end
-
-  context 'on Debian platforms' do
-    let :facts do
-      OSDefaults.get_facts({
-        :operatingsystem  => 'Ubuntu',
-        :osfamily         => 'Debian',
-      })
-    end
-
-    let :platform_params do
-      { :object_expirer_package_name => 'swift-object-expirer',
-        :service_name                => 'swift-object-expirer',
-        :service_provider            => nil }
-    end
-
-    it_configures 'swift-object-expirer'
-
     context 'when overridding parameters' do
       before do
         params.merge!(
-          :interval        => '600',
-          :reclaim_age     => '10000',
-          :concurrency     => '3',
+          :interval    => '600',
+          :reclaim_age => '10000',
+          :concurrency => '3',
+        )
+      end
+    end
+
+    context 'when including cache in pipeline' do
+      before do
+        params.merge!(
+          :pipeline         => ['catch_errors', 'cache', 'proxy-server'],
+          :memcache_servers => ['127.0.0.1:11211'],
         )
       end
 
-      it_configures 'swift-object-expirer'
+      it 'configures memcache servers' do
+        is_expected.to contain_swift_object_expirer_config(
+          'filter:cache/memcache_servers').with_value(p[:memcache_servers])
+      end
     end
 
-    context 'on debian using swiftinit service provider' do
+    context 'when using swiftinit service provider' do
       before do
         params.merge!({ :service_provider => 'swiftinit' })
       end
@@ -96,50 +96,31 @@ describe 'swift::objectexpirer' do
       before do
         platform_params.merge!({ :service_provider => 'swiftinit' })
       end
-
-      it_configures 'swift-object-expirer'
     end
+
   end
 
-  context 'on RedHat platforms' do
-        let :facts do
-          OSDefaults.get_facts({
-            :osfamily        => 'RedHat',
-            :operatingsystem => 'RedHat',
-          })
-        end
+  on_supported_os({
+    :supported_os => OSDefaults.get_supported_os
+  }).each do |os,facts|
+    context "on #{os}" do
+      let (:facts) do
+        facts.merge!(OSDefaults.get_facts())
+      end
 
-        let :platform_params do
-          { :object_expirer_package_name => 'openstack-swift-proxy',
-            :service_name                => 'openstack-swift-object-expirer',
-            :service_provider            => nil }
+      let(:platform_params) do
+        case facts[:osfamily]
+        when 'Debian'
+          { :service_name     => 'swift-object-expirer',
+            :service_provider => nil }
+        when 'RedHat'
+          { :service_name     => 'openstack-swift-object-expirer',
+            :service_provider => nil }
         end
-
-    it_configures 'swift-object-expirer'
-    context 'when overridding parameters' do
-      before do
-        params.merge!(
-          :interval        => '600',
-          :reclaim_age     => '10000',
-          :concurrency     => '3',
-        )
       end
 
       it_configures 'swift-object-expirer'
     end
 
-    context 'on redhat using swiftinit service provider' do
-      before do
-        params.merge!({ :service_provider => 'swiftinit' })
-      end
-
-      let :platform_params do
-        { :object_expirer_package_name => 'openstack-swift-proxy',
-          :service_name                => 'swift-object-expirer',
-          :service_provider            => 'swiftinit' }
-      end
-
-      it_configures 'swift-object-expirer'
-    end
   end
 end