Merge pull request #214 from rhefner1/ntap-nfs-shares

Bring puppet-cinder up to date to support new NetApp parameter

Puppetfile

@@ -15,7 +15,7 @@ mod 'certmonger',
   :git => 'https://github.com/rcritten/puppet-certmonger.git'
 
 mod 'cinder',
-  :commit => '62823628ca092a2ee45faf3d40622a95f5483097',
+  :commit => '1fc21c688a66d25441e4e2e0067f2126c19bf5c3',
   :git => 'https://github.com/stackforge/puppet-cinder.git'
 
 mod 'common',

cinder/.fixtures.yml

@@ -1,15 +1,12 @@
 fixtures:
   repositories:
     'apt': 'git://github.com/puppetlabs/puppetlabs-apt.git'
+    'concat': 'git://github.com/puppetlabs/puppetlabs-concat.git'
     'inifile':  'git://github.com/puppetlabs/puppetlabs-inifile'
     'keystone': 'git://github.com/stackforge/puppet-keystone.git'
-    'mysql':
-      repo: 'git://github.com/puppetlabs/puppetlabs-mysql.git'
-      ref:  'origin/2.2.x'
+    'mysql': 'git://github.com/puppetlabs/puppetlabs-mysql.git'
     'openstacklib': 'git://github.com/stackforge/puppet-openstacklib.git'
-    'postgresql':
-      repo: 'git://github.com/puppetlabs/puppet-postgresql.git'
-      ref: '2.5.0'
+    'postgresql': 'git://github.com/puppetlabs/puppet-postgresql.git'
     'qpid': 'git://github.com/dprince/puppet-qpid.git'
     'rabbitmq':
       repo: 'git://github.com/puppetlabs/puppetlabs-rabbitmq'

cinder/manifests/api.pp

@@ -201,17 +201,34 @@
     cinder_config {
       'DEFAULT/auth_strategy':     value => 'keystone' ;
     }
+
+    $identity_uri = "${keystone_auth_protocol}://${keystone_auth_host}:${keystone_auth_port}"
+    if $keystone_auth_admin_prefix {
+      validate_re($keystone_auth_admin_prefix, '^(/.+[^/])?$')
+      # a leading slash on keystone_auth_admin_prefix is already required,
+      # don't add it here
+      $identity_uri_real = "${identity_uri}${keystone_auth_admin_prefix}"
+    }
+    else {
+      $identity_uri_real = $identity_uri
+    }
+
     cinder_api_paste_ini {
       'filter:authtoken/service_protocol':  value => $keystone_auth_protocol;
       'filter:authtoken/service_host':      value => $keystone_auth_host;
       'filter:authtoken/service_port':      value => $service_port;
-      'filter:authtoken/auth_protocol':     value => $keystone_auth_protocol;
-      'filter:authtoken/auth_host':         value => $keystone_auth_host;
-      'filter:authtoken/auth_port':         value => $keystone_auth_port;
+      'filter:authtoken/identity_uri':      value => $identity_uri_real;
       'filter:authtoken/admin_tenant_name': value => $keystone_tenant;
       'filter:authtoken/admin_user':        value => $keystone_user;
       'filter:authtoken/admin_password':    value => $keystone_password, secret => true;
+
+      # deprecated parameters - replaced with identity_uri
+      'filter:authtoken/auth_protocol':     ensure => absent;
+      'filter:authtoken/auth_host':         ensure => absent;
+      'filter:authtoken/auth_port':         ensure => absent;
+      'filter:authtoken/auth_admin_prefix': ensure => absent;
     }
+  }
 
   if ($ratelimits != undef) {
     cinder_api_paste_ini {
@@ -220,18 +237,6 @@
     }
   }
 
-    if $keystone_auth_admin_prefix {
-      validate_re($keystone_auth_admin_prefix, '^(/.+[^/])?$')
-      cinder_api_paste_ini {
-        'filter:authtoken/auth_admin_prefix': value => $keystone_auth_admin_prefix;
-      }
-    } else {
-      cinder_api_paste_ini {
-        'filter:authtoken/auth_admin_prefix': ensure => absent;
-      }
-    }
-  }
-
   if $default_volume_type {
     cinder_config {
       'DEFAULT/default_volume_type': value => $default_volume_type;

cinder/manifests/backend/netapp.pp

@@ -94,9 +94,14 @@
 #   last M minutes, where M is the value of the expiry_thres_minutes parameter.
 #   Defaults to 60
 #
+# [*nfs_shares*]
+#   (optional) Array of NFS exports in the form of host:/share; will be written into
+#    file specified in nfs_shares_config
+#    Defaults to undef
+#
 # [*nfs_shares_config*]
 #   (optional) File with the list of available NFS shares
-#   Defaults to ''
+#   Defaults to '/etc/cinder/shares.conf'
 #
 # [*netapp_copyoffload_tool_path*]
 #   (optional) This option specifies the path of the NetApp Copy Offload tool
@@ -166,14 +171,24 @@
   $expiry_thres_minutes         = '720',
   $thres_avl_size_perc_start    = '20',
   $thres_avl_size_perc_stop     = '60',
-  $nfs_shares_config            = '',
+  $nfs_shares                   = undef,
+  $nfs_shares_config            = '/etc/cinder/shares.conf',
   $netapp_copyoffload_tool_path = '',
   $netapp_controller_ips        = '',
   $netapp_sa_password           = '',
   $netapp_storage_pools         = '',
   $netapp_webservice_path       = '/devmgr/v2',
 ) {
 
+  if $nfs_shares {
+    validate_array($nfs_shares)
+    file {$nfs_shares_config:
+      content => join($nfs_shares, "\n"),
+      require => Package['cinder'],
+      notify  => Service['cinder-volume']
+    }
+  }
+
   cinder_config {
     "${volume_backend_name}/volume_backend_name":          value => $volume_backend_name;
     "${volume_backend_name}/volume_driver":                value => 'cinder.volume.drivers.netapp.common.NetAppDriver';

cinder/manifests/db/postgresql.pp

@@ -16,20 +16,30 @@
 #   (Optional) User to connect to the database.
 #   Defaults to 'cinder'.
 #
+#  [*encoding*]
+#    (Optional) The charset to use for the database.
+#    Default to undef.
+#
+#  [*privileges*]
+#    (Optional) Privileges given to the database user.
+#    Default to 'ALL'
+#
 class cinder::db::postgresql(
   $password,
-  $dbname = 'cinder',
-  $user   = 'cinder'
+  $dbname     = 'cinder',
+  $user       = 'cinder',
+  $encoding   = undef,
+  $privileges = 'ALL',
 ) {
 
-  require postgresql::python
-
-  Postgresql::Db[$dbname]    ~> Exec<| title == 'cinder-manage db_sync' |>
-  Package['python-psycopg2'] -> Exec<| title == 'cinder-manage db_sync' |>
-
-  postgresql::db { $dbname:
-    user     => $user,
-    password => $password,
+  ::openstacklib::db::postgresql { 'cinder':
+    password_hash => postgresql_password($user, $password),
+    dbname        => $dbname,
+    user          => $user,
+    encoding      => $encoding,
+    privileges    => $privileges,
   }
 
+  ::Openstacklib::Db::Postgresql['cinder']    ~> Exec<| title == 'cinder-manage db_sync' |>
+
 }

cinder/manifests/volume/netapp.pp

@@ -92,9 +92,14 @@
 #   'minutes, where M is the value of the expiry_thres_minutes parameter.
 #   Defaults to 60
 #
+# [*nfs_shares*]
+#   (optional) Array of NFS exports in the form of host:/share; will be written into
+#    file specified in nfs_shares_config
+#    Defaults to undef
+#
 # [*nfs_shares_config*]
 #   (optional) File with the list of available NFS shares
-#   Defaults to ''
+#   Defaults to '/etc/cinder/shares.conf'
 #
 # [*netapp_copyoffload_tool_path*]
 #   (optional) This option specifies the path of the NetApp Copy Offload tool
@@ -163,7 +168,8 @@
   $expiry_thres_minutes         = '720',
   $thres_avl_size_perc_start    = '20',
   $thres_avl_size_perc_stop     = '60',
-  $nfs_shares_config            = '',
+  $nfs_shares                   = undef,
+  $nfs_shares_config            = '/etc/cinder/shares.conf',
   $netapp_copyoffload_tool_path = '',
   $netapp_controller_ips        = '',
   $netapp_sa_password           = '',
@@ -186,6 +192,7 @@
     expiry_thres_minutes         => $expiry_thres_minutes,
     thres_avl_size_perc_start    => $thres_avl_size_perc_start,
     thres_avl_size_perc_stop     => $thres_avl_size_perc_stop,
+    nfs_shares                   => $nfs_shares,
     nfs_shares_config            => $nfs_shares_config,
     netapp_copyoffload_tool_path => $netapp_copyoffload_tool_path,
     netapp_controller_ips        => $netapp_controller_ips,

cinder/spec/classes/cinder_api_spec.rb

@@ -42,14 +42,17 @@
       should contain_cinder_api_paste_ini('filter:authtoken/service_port').with(
         :value => '5000'
       )
+      should contain_cinder_api_paste_ini('filter:authtoken/identity_uri').with(
+        :value => 'http://localhost:35357'
+      )
       should contain_cinder_api_paste_ini('filter:authtoken/auth_protocol').with(
-        :value => 'http'
+        :ensure => 'absent'
       )
       should contain_cinder_api_paste_ini('filter:authtoken/auth_host').with(
-        :value => 'localhost'
+        :ensure => 'absent'
       )
       should contain_cinder_api_paste_ini('filter:authtoken/auth_port').with(
-        :value => '35357'
+        :ensure => 'absent'
       )
       should contain_cinder_api_paste_ini('filter:authtoken/auth_admin_prefix').with(
         :ensure => 'absent'
@@ -70,6 +73,7 @@
       )
 
       should_not contain_cinder_config('DEFAULT/os_region_name')
+
     end
   end
 
@@ -126,8 +130,8 @@
         }
       end
 
-      it { should contain_cinder_api_paste_ini('filter:authtoken/auth_admin_prefix').with(
-        :value => keystone_auth_admin_prefix
+      it { should contain_cinder_api_paste_ini('filter:authtoken/identity_uri').with(
+        :value => "http://localhost:35357#{keystone_auth_admin_prefix}"
       )}
     end
   end
@@ -148,7 +152,7 @@
         }
       end
 
-      it { expect { should contain_cinder_api_paste_ini('filter:authtoken/auth_admin_prefix') }.to \
+      it { expect { should contain_cinder_api_paste_ini('filter:authtoken/identity_uri') }.to \
         raise_error(Puppet::Error, /validate_re\(\): "#{keystone_auth_admin_prefix}" does not match/) }
     end
   end

cinder/spec/classes/cinder_db_postgresql_spec.rb

@@ -3,24 +3,56 @@
 describe 'cinder::db::postgresql' do
 
   let :req_params do
-    {:password => 'pw'}
+    { :password => 'pw' }
   end
 
-  let :facts do
-    {
-      :postgres_default_version => '8.4',
-      :osfamily => 'RedHat',
-    }
+  let :pre_condition do
+    'include postgresql::server'
   end
 
-  describe 'with only required params' do
-    let :params do
-      req_params
+  context 'on a RedHat osfamily' do
+    let :facts do
+      {
+        :osfamily                 => 'RedHat',
+        :operatingsystemrelease   => '7.0',
+        :concat_basedir => '/var/lib/puppet/concat'
+      }
     end
-    it { should contain_postgresql__db('cinder').with(
-      :user         => 'cinder',
-      :password     => 'pw'
-     ) }
+
+    context 'with only required parameters' do
+      let :params do
+        req_params
+      end
+
+      it { should contain_postgresql__server__db('cinder').with(
+        :user     => 'cinder',
+        :password => 'md506736c3030793e09882cc536063d433f'
+      )}
+    end
+
+  end
+
+  context 'on a Debian osfamily' do
+    let :facts do
+      {
+        :operatingsystemrelease => '7.8',
+        :operatingsystem        => 'Debian',
+        :osfamily               => 'Debian',
+        :concat_basedir => '/var/lib/puppet/concat'
+      }
+    end
+
+    context 'with only required parameters' do
+      let :params do
+        req_params
+      end
+
+      it { should contain_postgresql__server__db('cinder').with(
+        :user     => 'cinder',
+        :password => 'md506736c3030793e09882cc536063d433f'
+      )}
+    end
+
   end
 
 end

cinder/spec/classes/cinder_volume_netapp_spec.rb

@@ -23,7 +23,7 @@
       :expiry_thres_minutes         => '720',
       :thres_avl_size_perc_start    => '20',
       :thres_avl_size_perc_stop     => '60',
-      :nfs_shares_config            => '',
+      :nfs_shares_config            => '/etc/cinder/shares.conf',
       :netapp_copyoffload_tool_path => '',
       :netapp_controller_ips        => '',
       :netapp_sa_password           => '',
@@ -67,4 +67,16 @@
   context 'with provided parameters' do
     it_configures 'netapp volume driver'
   end
+
+  context 'with NFS shares provided' do
+    let (:req_params) { params.merge!({
+        :nfs_shares => ['10.0.0.1:/test1', '10.0.0.2:/test2'],
+        :nfs_shares_config => '/etc/cinder/shares.conf',
+    }) }
+
+    it 'writes NFS shares to file' do
+      should contain_file("#{req_params[:nfs_shares_config]}")
+        .with_content("10.0.0.1:/test1\n10.0.0.2:/test2")
+    end
+  end
 end

cinder/spec/defines/cinder_backend_netapp_spec.rb

@@ -26,7 +26,7 @@
       :expiry_thres_minutes         => '720',
       :thres_avl_size_perc_start    => '20',
       :thres_avl_size_perc_stop     => '60',
-      :nfs_shares_config            => '',
+      :nfs_shares_config            => '/etc/cinder/shares.conf',
       :netapp_copyoffload_tool_path => '',
       :netapp_controller_ips        => '',
       :netapp_sa_password           => '',
@@ -77,4 +77,30 @@
 
     it { should contain_cinder_config("#{req_params[:volume_backend_name]}/use_multipath_for_image_xfer").with_value('true') }
   end
+
+  context 'with NFS shares provided' do
+    let (:req_params) { params.merge!({
+        :nfs_shares => ['10.0.0.1:/test1', '10.0.0.2:/test2'],
+        :nfs_shares_config => '/etc/cinder/shares.conf',
+    }) }
+
+    it 'writes NFS shares to file' do
+      should contain_file("#{req_params[:nfs_shares_config]}") \
+        .with_content("10.0.0.1:/test1\n10.0.0.2:/test2")
+    end
+  end
+
+  context 'with invalid NFS shares provided' do
+    before do
+      params.merge!({
+        :nfs_shares => "not an array",
+        :nfs_shares_config => '/etc/cinder/shares.conf',
+      })
+    end
+
+    it 'throw error' do
+        expect {subject}.to raise_error(Puppet::Error, /"not an array" is not an Array.  It looks to be a String/)
+    end
+  end
+
 end