From bb3007615f2ad7231405dca367d3c8b0ff15a475 Mon Sep 17 00:00:00 2001
From: David Moreau Simard <dmsimard@iweb.com>
Date: Fri, 9 May 2014 13:54:54 -0400
Subject: [PATCH] Add swift-ring-builder multi region support

Since Swift 1.8.0, there is the region layer of topology above zones.
swift-ring-builder supports this natively and this commit allows a user to
specify a region when creating devices.
We maintain backwards compatibility by defaulting to region '1'.
The coverage of the ring builder spec tests were also improved as part of
this commit.

Change-Id: I67cbe6b87c84778f71df59cf00f2c5175342bc1b
---
 README.md                                     |  4 +
 lib/puppet/provider/swift_ring_builder.rb     | 65 +++++++++-----
 lib/puppet/type/ring_account_device.rb        |  2 +
 lib/puppet/type/ring_container_device.rb      |  2 +
 lib/puppet/type/ring_object_device.rb         |  2 +
 .../provider/swift_ring_builder_spec.rb       | 89 ++++++++++++++++---
 6 files changed, 131 insertions(+), 33 deletions(-)

diff --git a/README.md b/README.md
index cf8ea583..d09d50c7 100644
--- a/README.md
+++ b/README.md
@@ -60,6 +60,7 @@ class { 'swift::storage::all':
 }
 
 @@ring_object_device { "${ipaddress_eth0}:6000/1":
+  region => 1, # optional, defaults to 1
   zone   => 1,
   weight => 1,
 }
@@ -73,14 +74,17 @@ class { 'swift::storage::all':
 }
 
 @@ring_object_device { "${ipaddress_eth0}:6000/2":
+  region => 2,
   zone   => 1,
   weight => 1,
 }
 @@ring_container_device { "${ipaddress_eth0}:6001/2":
+  region => 2,
   zone   => 1,
   weight => 1,
 }
 @@ring_account_device { "${ipaddress_eth0}:6002/2":
+  region => 2,
   zone   => 1,
   weight => 1,
 }
diff --git a/lib/puppet/provider/swift_ring_builder.rb b/lib/puppet/provider/swift_ring_builder.rb
index f4dbadb2..7a1bf45b 100644
--- a/lib/puppet/provider/swift_ring_builder.rb
+++ b/lib/puppet/provider/swift_ring_builder.rb
@@ -28,33 +28,36 @@ class Puppet::Provider::SwiftRingBuilder < Puppet::Provider
            # Devices:    id  region  zone      ip address  port  replication ip  replication port      name weight partitions balance meta
            #              0       1     2       127.0.0.1  6021       127.0.0.1              6021         2   1.00     262144    0.00
           # Swift 1.8+ output example:
-          if row =~ /^\s*(\d+)\s+\d+\s+(\d+)\s+(\S+)\s+(\d+)\s+\S+\s+\d+\s+(\S+)\s+(\d+\.\d+)\s+(\d+)\s*((-|\s-?)?\d+\.\d+)\s*(\S*)/
+          if row =~ /^\s*(\d+)\s+(\d+)\s+(\d+)\s+(\S+)\s+(\d+)\s+\S+\s+\d+\s+(\S+)\s+(\d+\.\d+)\s+(\d+)\s*((-|\s-?)?\d+\.\d+)\s*(\S*)/
 
-            object_hash["#{$3}:#{$4}/#{$5}"] = {
+            object_hash["#{$4}:#{$5}/#{$6}"] = {
               :id          => $1,
-              :zone        => $2,
-              :weight      => $6,
-              :partitions  => $7,
-              :balance     => $8,
-              :meta        => $10
+              :region      => $2,
+              :zone        => $3,
+              :weight      => $7,
+              :partitions  => $8,
+              :balance     => $9,
+              :meta        => $11
             }
 
           # Swift 1.8.0 output example:
-          elsif row =~ /^\s*(\d+)\s+\d+\s+(\d+)\s+(\S+)\s+(\d+)\s+(\S+)\s+(\d+\.\d+)\s+(\d+)\s*((-|\s-?)?\d+\.\d+)\s*(\S*)/
+          elsif row =~ /^\s*(\d+)\s+(\d+)\s+(\d+)\s+(\S+)\s+(\d+)\s+(\S+)\s+(\d+\.\d+)\s+(\d+)\s*((-|\s-?)?\d+\.\d+)\s*(\S*)/
 
-            object_hash["#{$3}:#{$4}/#{$5}"] = {
+            object_hash["#{$4}:#{$5}/#{$6}"] = {
               :id          => $1,
-              :zone        => $2,
-              :weight      => $6,
-              :partitions  => $7,
-              :balance     => $8,
-              :meta        => $10
+              :region      => $2,
+              :zone        => $3,
+              :weight      => $7,
+              :partitions  => $8,
+              :balance     => $9,
+              :meta        => $11
             }
            # This regex is for older swift versions
           elsif row =~ /^\s+(\d+)\s+(\d+)\s+(\S+)\s+(\d+)\s+(\S+)\s+(\d+\.\d+)\s+(\d+)\s+(-?\d+\.\d+)\s+(\S*)$/
 
             object_hash["#{$3}:#{$4}/#{$5}"] = {
               :id          => $1,
+              :region      => 'none',
               :zone        => $2,
               :weight      => $6,
               :partitions  => $7,
@@ -87,12 +90,26 @@ class Puppet::Provider::SwiftRingBuilder < Puppet::Provider
     [:zone, :weight].each do |param|
       raise(Puppet::Error, "#{param} is required") unless resource[param]
     end
-    swift_ring_builder(
-      builder_file_path,
-      'add',
-      "z#{resource[:zone]}-#{resource[:name]}",
-      resource[:weight]
-    )
+
+    if :region == 'none'
+      # Prior to Swift 1.8.0, regions did not exist.
+      swift_ring_builder(
+        builder_file_path,
+        'add',
+        "z#{resource[:zone]}-#{resource[:name]}",
+        resource[:weight]
+      )
+    else
+      # Swift 1.8+
+      # Region defaults to 1 if unspecified
+      resource[:region] ||= 1
+      swift_ring_builder(
+        builder_file_path,
+        'add',
+        "r#{resource[:region]}z#{resource[:zone]}-#{resource[:name]}",
+        resource[:weight]
+      )
+    end
   end
 
   def id
@@ -103,6 +120,14 @@ class Puppet::Provider::SwiftRingBuilder < Puppet::Provider
     raise(Puppet::Error, "Cannot assign id, it is immutable")
   end
 
+  def region
+    ring[resource[:name]][:region]
+  end
+
+  def region=(region)
+    raise(Puppet::Error, "Changing the region of a device is not possible.")
+  end
+
   def zone
     ring[resource[:name]][:zone]
   end
diff --git a/lib/puppet/type/ring_account_device.rb b/lib/puppet/type/ring_account_device.rb
index dc12a4b1..5174f23d 100644
--- a/lib/puppet/type/ring_account_device.rb
+++ b/lib/puppet/type/ring_account_device.rb
@@ -13,6 +13,8 @@ Puppet::Type.newtype(:ring_account_device) do
     end
   end
 
+  newproperty(:region)
+
   newproperty(:zone)
 
   newproperty(:weight) do
diff --git a/lib/puppet/type/ring_container_device.rb b/lib/puppet/type/ring_container_device.rb
index a8a37dee..60144b06 100644
--- a/lib/puppet/type/ring_container_device.rb
+++ b/lib/puppet/type/ring_container_device.rb
@@ -13,6 +13,8 @@ Puppet::Type.newtype(:ring_container_device) do
     end
   end
 
+  newproperty(:region)
+
   newproperty(:zone)
 
   newproperty(:weight) do
diff --git a/lib/puppet/type/ring_object_device.rb b/lib/puppet/type/ring_object_device.rb
index 1ee9b108..2de2d806 100644
--- a/lib/puppet/type/ring_object_device.rb
+++ b/lib/puppet/type/ring_object_device.rb
@@ -13,6 +13,8 @@ Puppet::Type.newtype(:ring_object_device) do
     end
   end
 
+  newproperty(:region)
+
   newproperty(:zone)
 
   newproperty(:weight) do
diff --git a/spec/unit/puppet/provider/swift_ring_builder_spec.rb b/spec/unit/puppet/provider/swift_ring_builder_spec.rb
index ecb51592..542adbcd 100644
--- a/spec/unit/puppet/provider/swift_ring_builder_spec.rb
+++ b/spec/unit/puppet/provider/swift_ring_builder_spec.rb
@@ -20,17 +20,33 @@ describe provider_class do
 262144 partitions, 3 replicas, 3 zones, 3 devices, 0.00 balance
 The minimum number of hours before a partition can be reassigned is 1
 Devices:    id  region  zone      ip address  port      replication ip  replication port name weight partitions balance meta
+             1     1     1  192.168.101.13  6002         192.168.101.13  6002            1   1.00     262144 0.00
              2     1     2  192.168.101.14  6002         192.168.101.14  6002            1   1.00     262144 200.00  m2
              0     1     3  192.168.101.15  6002         192.168.101.15  6002            1   1.00     262144-100.00  m2
              3     1     1  192.168.101.16  6002         192.168.101.16  6002            1   1.00     262144-100.00
-             1     1     1  192.168.101.13  6002         192.168.101.13  6002            1   1.00     262144 0.00
 '
     )
-    resources = provider_class.lookup_ring.inspect
-    resources['192.168.101.15:6002/1'].should_not be_nil
+    resources = provider_class.lookup_ring
     resources['192.168.101.13:6002/1'].should_not be_nil
     resources['192.168.101.14:6002/1'].should_not be_nil
+    resources['192.168.101.15:6002/1'].should_not be_nil
     resources['192.168.101.16:6002/1'].should_not be_nil
+
+    resources['192.168.101.13:6002/1'][:id].should eql '1'
+    resources['192.168.101.13:6002/1'][:region].should eql '1'
+    resources['192.168.101.13:6002/1'][:zone].should eql '1'
+    resources['192.168.101.13:6002/1'][:weight].should eql '1.00'
+    resources['192.168.101.13:6002/1'][:partitions].should eql '262144'
+    resources['192.168.101.13:6002/1'][:balance].should eql '0.00'
+    resources['192.168.101.13:6002/1'][:meta].should eql ''
+
+    resources['192.168.101.14:6002/1'][:id].should eql '2'
+    resources['192.168.101.14:6002/1'][:region].should eql '1'
+    resources['192.168.101.14:6002/1'][:zone].should eql '2'
+    resources['192.168.101.14:6002/1'][:weight].should eql '1.00'
+    resources['192.168.101.14:6002/1'][:partitions].should eql '262144'
+    resources['192.168.101.14:6002/1'][:balance].should eql '200.00'
+    resources['192.168.101.14:6002/1'][:meta].should eql 'm2'
   end
 
   it 'should be able to lookup the local ring and build an object 1.8.0' do
@@ -42,17 +58,33 @@ Devices:    id  region  zone      ip address  port      replication ip  replicat
 262144 partitions, 3 replicas, 3 zones, 3 devices, 0.00 balance
 The minimum number of hours before a partition can be reassigned is 1
 Devices:    id  region  zone      ip address  port      name weight partitions balance meta
+             1     1     1  192.168.101.13  6002         1   1.00     262144 0.00
              2     1     2  192.168.101.14  6002         1   1.00     262144 200.00  m2
              0     1     3  192.168.101.15  6002         1   1.00     262144-100.00  m2
              3     1     1  192.168.101.16  6002         1   1.00     262144-100.00
-             1     1     1  192.168.101.13  6002         1   1.00     262144 0.00
 '
     )
-    resources = provider_class.lookup_ring.inspect
-    resources['192.168.101.15:6002/1'].should_not be_nil
+    resources = provider_class.lookup_ring
     resources['192.168.101.13:6002/1'].should_not be_nil
     resources['192.168.101.14:6002/1'].should_not be_nil
+    resources['192.168.101.15:6002/1'].should_not be_nil
     resources['192.168.101.16:6002/1'].should_not be_nil
+
+    resources['192.168.101.13:6002/1'][:id].should eql '1'
+    resources['192.168.101.13:6002/1'][:region].should eql '1'
+    resources['192.168.101.13:6002/1'][:zone].should eql '1'
+    resources['192.168.101.13:6002/1'][:weight].should eql '1.00'
+    resources['192.168.101.13:6002/1'][:partitions].should eql '262144'
+    resources['192.168.101.13:6002/1'][:balance].should eql '0.00'
+    resources['192.168.101.13:6002/1'][:meta].should eql ''
+
+    resources['192.168.101.14:6002/1'][:id].should eql '2'
+    resources['192.168.101.14:6002/1'][:region].should eql '1'
+    resources['192.168.101.14:6002/1'][:zone].should eql '2'
+    resources['192.168.101.14:6002/1'][:weight].should eql '1.00'
+    resources['192.168.101.14:6002/1'][:partitions].should eql '262144'
+    resources['192.168.101.14:6002/1'][:balance].should eql '200.00'
+    resources['192.168.101.14:6002/1'][:meta].should eql 'm2'
   end
 
   it 'should be able to lookup the local ring and build an object 1.7' do
@@ -64,15 +96,31 @@ Devices:    id  region  zone      ip address  port      name weight partitions b
 262144 partitions, 3 replicas, 3 zones, 3 devices, 0.00 balance
 The minimum number of hours before a partition can be reassigned is 1
 Devices:    id  region  zone      ip address  port      name weight partitions balance meta
-             2     1     2  192.168.101.14  6002         1   1.00     262144    0.00 
-             0     1     3  192.168.101.15  6002         1   1.00     262144    0.00 
-             1     1     1  192.168.101.13  6002         1   1.00     262144    0.00 
+             1     1     1  192.168.101.13  6002         1   1.00     262144    0.00
+             2     1     2  192.168.101.14  6002         1   1.00     262144    0.00
+             0     1     3  192.168.101.15  6002         1   1.00     262144    0.00
 '
     )
-    resources = provider_class.lookup_ring.inspect
-    resources['192.168.101.15:6002/1'].should_not be_nil
+    resources = provider_class.lookup_ring
     resources['192.168.101.13:6002/1'].should_not be_nil
     resources['192.168.101.14:6002/1'].should_not be_nil
+    resources['192.168.101.15:6002/1'].should_not be_nil
+
+    resources['192.168.101.13:6002/1'][:id].should eql '1'
+    resources['192.168.101.13:6002/1'][:region].should eql '1'
+    resources['192.168.101.13:6002/1'][:zone].should eql '1'
+    resources['192.168.101.13:6002/1'][:weight].should eql '1.00'
+    resources['192.168.101.13:6002/1'][:partitions].should eql '262144'
+    resources['192.168.101.13:6002/1'][:balance].should eql '0.00'
+    resources['192.168.101.13:6002/1'][:meta].should eql ''
+
+    resources['192.168.101.14:6002/1'][:id].should eql '2'
+    resources['192.168.101.14:6002/1'][:region].should eql '1'
+    resources['192.168.101.14:6002/1'][:zone].should eql '2'
+    resources['192.168.101.14:6002/1'][:weight].should eql '1.00'
+    resources['192.168.101.14:6002/1'][:partitions].should eql '262144'
+    resources['192.168.101.14:6002/1'][:balance].should eql '0.00'
+    resources['192.168.101.14:6002/1'][:meta].should eql ''
   end
 
   it 'should be able to lookup the local ring and build an object legacy' do
@@ -88,10 +136,25 @@ Devices:    id  zone      ip address  port      name weight partitions balance m
              1     1  192.168.101.13  6002         1   1.00     262144    0.00 
 '
     )
-    resources = provider_class.lookup_ring.inspect
+    resources = provider_class.lookup_ring
     resources['192.168.101.15:6002/1'].should_not be_nil
     resources['192.168.101.13:6002/1'].should_not be_nil
     resources['192.168.101.14:6002/1'].should_not be_nil
-  end
 
+    resources['192.168.101.13:6002/1'][:id].should eql '1'
+    resources['192.168.101.13:6002/1'][:region].should eql 'none'
+    resources['192.168.101.13:6002/1'][:zone].should eql '1'
+    resources['192.168.101.13:6002/1'][:weight].should eql '1.00'
+    resources['192.168.101.13:6002/1'][:partitions].should eql '262144'
+    resources['192.168.101.13:6002/1'][:balance].should eql '0.00'
+    resources['192.168.101.13:6002/1'][:meta].should eql ''
+
+    resources['192.168.101.14:6002/1'][:id].should eql '2'
+    resources['192.168.101.14:6002/1'][:region].should eql 'none'
+    resources['192.168.101.14:6002/1'][:zone].should eql '2'
+    resources['192.168.101.14:6002/1'][:weight].should eql '1.00'
+    resources['192.168.101.14:6002/1'][:partitions].should eql '262144'
+    resources['192.168.101.14:6002/1'][:balance].should eql '0.00'
+    resources['192.168.101.14:6002/1'][:meta].should eql ''
+  end
 end