Hi,

I am using puppet-swift to deploy a swift multi node cluster (Icehouse), following the setup in supplied tests/site.pp. I am running into two issues that seem to be related to the subject above:

1/ Errors when the storage replication services try to start before the ring files exist. e.g:

Error: Could not start Service[swift-object-replicator]: Execution of '/sbin/start swift-object-replicator' returned 1: start: Job failed to start
Wrapped exception:
Execution of '/sbin/start swift-object-replicator' returned 1: start: Job failed to start Error: /Stage[main]/Swift::Storage::Object/Swift::Storage::Generic[object]/Service[swift-object-replicator]/ensure: change from stopped to running failed: Could not start Service[swift-object-replicator]: Execution of '/sbin/start swift-object-replicator' returned 1: start: Job failed to start

Now these will be fixed the *next* time I do a puppet run (provided I've performed a run on the appropriate proxy/ringmaster). However the failing services make scripted testing difficult as we have to put in logic to the effect "don't worry about errors the 1st time".

2/ Container and object stats not updated without full restart of services

This one is a bit more subtle - everything works but 'swift stat' always shows zero objects and bytes for every container. The only way to fix this is to stop and start all services on each storage node.

Again this complicates scripted builds as there is the need to go and stop + start all the swift storage services! Not to mention an extra little quirk for ops to remember at zero dark 30 oclock...

I've made a patch that prevents these services starting until the ring files exist (actually for now it just checks the object ring) - see attached.

Now while I'm not entirely sure that this is the best way to solve the issue (custom fact that changes the service start flag)...I *do* think that making the storage services await the ring existence *is* a needed change, so any thoughts on better ways to do this are appreciated.

Also note that this change *does* require one more puppet run on the storage nodes:
- one to create the storage servers config and drives
- one to get the ring from the proxy/ringmaster
- one to start the services

Regards

Mark
>From 1583d68eedbeaecbacb5a29258343b9e980ce4a4 Mon Sep 17 00:00:00 2001
From: Mark Kirkwood <mark.kirkw...@catalyst.net.nz>
Date: Fri, 10 Jul 2015 11:18:16 +1200
Subject: [PATCH] Make storage service startup wait for the ring

This solves two problems:

1/ Errors when the replication services fail to start when the
ring files are absent.

2/ Container and object stats not updated without full restart of services

While neither of these issues prevent a working deployment, they play
havoc with fully automated deployment for ci etc (and make arcane
knowledge necessary for ops).

This change *does* require one more puppet run on the storage nodes:
- one to create the storage servers config and drives
- one to get the ring from the proxy/ringmaster
- one to start the services

(amend: make sure the custom fact goes in the correct dir)
Change-Id: I168c26aed5f6dfc337ea8bc5f863e15f6e86d4a2
---
 modules/swift/lib/facter/ringfact.rb         |  5 +++++
 modules/swift/manifests/storage/account.pp   | 14 ++++++++++----
 modules/swift/manifests/storage/container.pp | 17 ++++++++++++-----
 modules/swift/manifests/storage/object.pp    | 15 +++++++++++----
 4 files changed, 38 insertions(+), 13 deletions(-)
 create mode 100644 modules/swift/lib/facter/ringfact.rb

diff --git a/modules/swift/lib/facter/ringfact.rb b/modules/swift/lib/facter/ringfact.rb
new file mode 100644
index 0000000..4816cb8
--- /dev/null
+++ b/modules/swift/lib/facter/ringfact.rb
@@ -0,0 +1,5 @@
+Facter.add("ringexists") do
+  setcode do
+    File.exists?("/etc/swift/object.ring.gz") ? "true" : "false"
+  end
+end
diff --git a/modules/swift/manifests/storage/account.pp b/modules/swift/manifests/storage/account.pp
index a4398c3..1719776 100644
--- a/modules/swift/manifests/storage/account.pp
+++ b/modules/swift/manifests/storage/account.pp
@@ -18,16 +18,22 @@ class swift::storage::account(
   $enabled        = true,
   $package_ensure = 'present'
 ) {
+  if $::ringexists == "false" {
+    $service_enabled = false
+  } else {
+    $service_enabled = $enabled
+  }
+
   swift::storage::generic { 'account':
     manage_service => $manage_service,
-    enabled        => $enabled,
+    enabled        => $service_enabled,
     package_ensure => $package_ensure,
   }
 
   include swift::params
 
   if $manage_service {
-    if $enabled {
+    if $service_enabled {
       $service_ensure = 'running'
     } else {
       $service_ensure = 'stopped'
@@ -37,7 +43,7 @@ class swift::storage::account(
   service { 'swift-account-reaper':
     ensure    => $service_ensure,
     name      => $::swift::params::account_reaper_service_name,
-    enable    => $enabled,
+    enable    => $service_enabled,
     provider  => $::swift::params::service_provider,
     require   => Package['swift-account'],
   }
@@ -45,7 +51,7 @@ class swift::storage::account(
   service { 'swift-account-auditor':
     ensure    => $service_ensure,
     name      => $::swift::params::account_auditor_service_name,
-    enable    => $enabled,
+    enable    => $service_enabled,
     provider  => $::swift::params::service_provider,
     require   => Package['swift-account'],
   }
diff --git a/modules/swift/manifests/storage/container.pp b/modules/swift/manifests/storage/container.pp
index 741dcba..d400d24 100644
--- a/modules/swift/manifests/storage/container.pp
+++ b/modules/swift/manifests/storage/container.pp
@@ -22,16 +22,23 @@ class swift::storage::container(
   $package_ensure     = 'present',
   $allowed_sync_hosts = ['127.0.0.1'],
 ) {
+  if $::ringexists == "false" {
+    $service_enabled = false
+  } else {
+    $service_enabled = $enabled
+  }
+
+
   swift::storage::generic { 'container':
     manage_service => $manage_service,
-    enabled        => $enabled,
+    enabled        => $service_enabled,
     package_ensure => $package_ensure
   }
 
   include swift::params
 
   if $manage_service {
-    if $enabled {
+    if $service_enabled {
       $service_ensure = 'running'
     } else {
       $service_ensure = 'stopped'
@@ -41,7 +48,7 @@ class swift::storage::container(
   service { 'swift-container-updater':
     ensure    => $service_ensure,
     name      => $::swift::params::container_updater_service_name,
-    enable    => $enabled,
+    enable    => $service_enabled,
     provider  => $::swift::params::service_provider,
     require   => Package['swift-container'],
   }
@@ -49,7 +56,7 @@ class swift::storage::container(
   service { 'swift-container-auditor':
     ensure    => $service_ensure,
     name      => $::swift::params::container_auditor_service_name,
-    enable    => $enabled,
+    enable    => $service_enabled,
     provider  => $::swift::params::service_provider,
     require   => Package['swift-container'],
   }
@@ -66,7 +73,7 @@ class swift::storage::container(
     }
     service { 'swift-container-sync':
       ensure    => $service_ensure,
-      enable    => $enabled,
+      enable    => $service_enabled,
       provider  => $::swift::params::service_provider,
       require   => File['/etc/init/swift-container-sync.conf', '/etc/init.d/swift-container-sync']
     }
diff --git a/modules/swift/manifests/storage/object.pp b/modules/swift/manifests/storage/object.pp
index 587c60a..d70a289 100644
--- a/modules/swift/manifests/storage/object.pp
+++ b/modules/swift/manifests/storage/object.pp
@@ -18,16 +18,23 @@ class swift::storage::object(
   $enabled        = true,
   $package_ensure = 'present'
 ) {
+  if $::ringexists == "false" {
+    $service_enabled = false
+  } else {
+    $service_enabled = $enabled
+  }
+
+
   swift::storage::generic { 'object':
     manage_service => $manage_service,
-    enabled        => $enabled,
+    enabled        => $service_enabled,
     package_ensure => $package_ensure
   }
 
   include swift::params
 
   if $manage_service {
-    if $enabled {
+    if $service_enabled {
       $service_ensure = 'running'
     } else {
       $service_ensure = 'stopped'
@@ -37,7 +44,7 @@ class swift::storage::object(
   service { 'swift-object-updater':
     ensure    => $service_ensure,
     name      => $::swift::params::object_updater_service_name,
-    enable    => $enabled,
+    enable    => $service_enabled,
     provider  => $::swift::params::service_provider,
     require   => Package['swift-object'],
   }
@@ -45,7 +52,7 @@ class swift::storage::object(
   service { 'swift-object-auditor':
     ensure    => $service_ensure,
     name      => $::swift::params::object_auditor_service_name,
-    enable    => $enabled,
+    enable    => $service_enabled,
     provider  => $::swift::params::service_provider,
     require   => Package['swift-object'],
   }
-- 
2.1.4

__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to