From 74b7d7edfc7eee5bcb7ff5f1b6955c999aed279e Mon Sep 17 00:00:00 2001
From: ZhongShengping <chdzsp@163.com>
Date: Tue, 29 Nov 2016 17:28:20 +0800
Subject: [PATCH] Add support for Debian system

1) Add server.pp for congress
2) Add hooks for external install & svc management
3) Add the acceptance test to deploy the service now

Change-Id: I0c3adac2a35a729c15a8abfbc000b40063ff34c7
---
 manifests/client.pp                    |  1 +
 manifests/config.pp                    |  2 +
 manifests/db.pp                        |  2 +
 manifests/db/mysql.pp                  |  7 ++-
 manifests/db/postgresql.pp             |  6 ++-
 manifests/db/sync.pp                   | 24 +++++----
 manifests/deps.pp                      | 35 +++++++++++++
 manifests/init.pp                      | 17 ++++++
 manifests/keystone/auth.pp             |  9 +++-
 manifests/keystone/authtoken.pp        |  2 +
 manifests/logging.pp                   |  2 +
 manifests/params.pp                    | 23 ++++----
 manifests/policy.pp                    |  2 +
 manifests/server.pp                    | 63 ++++++++++++++++++++++
 spec/acceptance/basic_congress_spec.rb | 72 ++++++++++++++++++++++++++
 spec/classes/congress_client_spec.rb   | 12 +----
 spec/classes/congress_db_sync_spec.rb  |  8 ++-
 spec/classes/congress_init_spec.rb     |  9 +++-
 spec/classes/congress_server_spec.rb   | 61 ++++++++++++++++++++++
 19 files changed, 318 insertions(+), 39 deletions(-)
 create mode 100644 manifests/deps.pp
 create mode 100644 manifests/server.pp
 create mode 100644 spec/acceptance/basic_congress_spec.rb
 create mode 100644 spec/classes/congress_server_spec.rb

diff --git a/manifests/client.pp b/manifests/client.pp
index ad3c7f9..1a33e06 100644
--- a/manifests/client.pp
+++ b/manifests/client.pp
@@ -10,6 +10,7 @@ class congress::client (
   $ensure = 'present'
 ) {
 
+  include ::congress::deps
   include ::congress::params
 
   package { 'python-congressclient':
diff --git a/manifests/config.pp b/manifests/config.pp
index 0a59d07..255e8bb 100644
--- a/manifests/config.pp
+++ b/manifests/config.pp
@@ -24,6 +24,8 @@ class congress::config (
   $congress_config = {},
 ) {
 
+  include ::congress::deps
+
   validate_hash($congress_config)
 
   create_resources('congress_config', $congress_config)
diff --git a/manifests/db.pp b/manifests/db.pp
index 0562464..1dfb6c0 100644
--- a/manifests/db.pp
+++ b/manifests/db.pp
@@ -49,6 +49,8 @@ class congress::db (
   $database_max_overflow   = $::os_service_default,
 ) {
 
+  include ::congress::deps
+
   validate_re($database_connection,
     '^(sqlite|mysql(\+pymysql)?|postgresql):\/\/(\S+:\S+@\S+\/\S+)?')
 
diff --git a/manifests/db/mysql.pp b/manifests/db/mysql.pp
index f9fcd9d..0072993 100644
--- a/manifests/db/mysql.pp
+++ b/manifests/db/mysql.pp
@@ -53,6 +53,8 @@ class congress::db::mysql(
   $allowed_hosts = undef
 ) {
 
+  include ::congress::deps
+
   validate_string($password)
 
   ::openstacklib::db::mysql { 'congress':
@@ -65,5 +67,8 @@ class congress::db::mysql(
     allowed_hosts => $allowed_hosts,
   }
 
-  ::Openstacklib::Db::Mysql['congress'] ~> Exec<| title == 'congress-db-sync' |>
+  Anchor['congress::db::begin']
+  ~> Class['congress::db::mysql']
+  ~> Anchor['congress::db::end']
+
 }
diff --git a/manifests/db/postgresql.pp b/manifests/db/postgresql.pp
index 6bade5e..9953851 100644
--- a/manifests/db/postgresql.pp
+++ b/manifests/db/postgresql.pp
@@ -40,7 +40,7 @@ class congress::db::postgresql(
   $privileges = 'ALL',
 ) {
 
-  Class['congress::db::postgresql'] -> Service<| title == 'congress' |>
+  include ::congress::deps
 
   ::openstacklib::db::postgresql { 'congress':
     password_hash => postgresql_password($user, $password),
@@ -50,6 +50,8 @@ class congress::db::postgresql(
     privileges    => $privileges,
   }
 
-  ::Openstacklib::Db::Postgresql['congress'] ~> Exec<| title == 'congress-db-sync' |>
+  Anchor['congress::db::begin']
+  ~> Class['congress::db::postgresql']
+  ~> Anchor['congress::db::end']
 
 }
diff --git a/manifests/db/sync.pp b/manifests/db/sync.pp
index 04de4f1..8ed9357 100644
--- a/manifests/db/sync.pp
+++ b/manifests/db/sync.pp
@@ -3,22 +3,28 @@
 #
 # == Parameters
 #
-# [*extra_params*]
-#   (optional) String of extra command line parameters to append
-#   to the congress-dbsync command.
-#   Defaults to undef
+# [*user*]
+#   (optional) User to run dbsync command.
+#   Defaults to 'congress'
 #
 class congress::db::sync(
-  $extra_params  = undef,
+  $user = 'congress',
 ) {
+
+  include ::congress::deps
+
   exec { 'congress-db-sync':
-    command     => "congress-db-manage upgrade head ${extra_params}",
+    command     => 'congress-db-manage --config-file /etc/congress/congress.conf upgrade head',
     path        => ['/bin', '/usr/bin'],
-    user        => 'congress',
+    user        => $user,
     refreshonly => true,
     logoutput   => 'on_failure',
-    subscribe   => [Package['congress']],
+    subscribe   => [
+      Anchor['congress::install::end'],
+      Anchor['congress::config::end'],
+      Anchor['congress::dbsync::begin']
+    ],
+    notify      => Anchor['congress::dbsync::end'],
   }
 
-  Exec['congress-db-sync'] ~> Service<| title == 'congress' |>
 }
diff --git a/manifests/deps.pp b/manifests/deps.pp
new file mode 100644
index 0000000..c04cb94
--- /dev/null
+++ b/manifests/deps.pp
@@ -0,0 +1,35 @@
+# == Class: congress::deps
+#
+#  Congress anchors and dependency management
+#
+class congress::deps {
+  # Setup anchors for install, config and service phases of the module.  These
+  # anchors allow external modules to hook the begin and end of any of these
+  # phases.  Package or service management can also be replaced by ensuring the
+  # package is absent or turning off service management and having the
+  # replacement depend on the appropriate anchors.  When applicable, end tags
+  # should be notified so that subscribers can determine if installation,
+  # config or service state changed and act on that if needed.
+  anchor { 'congress::install::begin': }
+  -> Package<| tag == 'congress-package'|>
+  ~> anchor { 'congress::install::end': }
+  -> anchor { 'congress::config::begin': }
+  -> Congress_config<||>
+  ~> anchor { 'congress::config::end': }
+  -> anchor { 'congress::db::begin': }
+  -> anchor { 'congress::db::end': }
+  ~> anchor { 'congress::dbsync::begin': }
+  -> anchor { 'congress::dbsync::end': }
+  ~> anchor { 'congress::service::begin': }
+  ~> Service<| tag == 'congress-service' |>
+  ~> anchor { 'congress::service::end': }
+
+  # policy config should occur in the config block also.
+  Anchor['congress::config::begin']
+  -> Openstacklib::Policy::Base<||>
+  ~> Anchor['congress::config::end']
+
+  # Installation or config changes will always restart services.
+  Anchor['congress::install::end'] ~> Anchor['congress::service::begin']
+  Anchor['congress::config::end']  ~> Anchor['congress::service::begin']
+}
diff --git a/manifests/init.pp b/manifests/init.pp
index 9610f02..7271973 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -173,6 +173,14 @@
 #   (Optional) Virtual_host to use.
 #   Defaults to $::os_service_default
 #
+# [*package_name*]
+#   (Optional) Package name to install for congress.
+#   Defaults to $::congress::params::package_name
+#
+# [*package_ensure*]
+#   (Optional) Ensure state for package.
+#   Defaults to present.
+#
 # == Authors
 #
 #   Dan Radez <dradez@redhat.com>
@@ -214,6 +222,8 @@ class congress(
   $drivers                            = $::congress::params::drivers,
   $policy_path                        = $::congress::params::policy_path,
   $sync_db                            = true,
+  $package_name                       = $::congress::params::package_name,
+  $package_ensure                     = 'present',
   # DEPRECATED PARAMETERS
   $rabbit_host                        = $::os_service_default,
   $rabbit_port                        = $::os_service_default,
@@ -223,6 +233,7 @@ class congress(
   $rabbit_password                    = $::os_service_default,
 ) inherits congress::params {
 
+  include ::congress::deps
   include ::congress::logging
 
   if !is_service_default($rabbit_host) or
@@ -236,6 +247,12 @@ congress::rabbit_port, congress::rabbit_userid and congress::rabbit_virtual_host
 deprecated. Please use congress::default_transport_url instead.")
   }
 
+  package { 'congress-common':
+    ensure => $package_ensure,
+    name   => $package_name,
+    tag    => ['openstack', 'congress-package'],
+  }
+
   congress_config {
     'DEFAULT/drivers'     : value => join(any2array($drivers), ',');
     'DEFAULT/policy_path' : value => $policy_path;
diff --git a/manifests/keystone/auth.pp b/manifests/keystone/auth.pp
index fd43127..62c7725 100644
--- a/manifests/keystone/auth.pp
+++ b/manifests/keystone/auth.pp
@@ -69,10 +69,15 @@ class congress::keystone::auth (
   $internal_url        = 'http://127.0.0.1:1789',
 ) {
 
+  include ::congress::deps
+
   if $configure_user_role {
-    Keystone_user_role["${auth_name}@${tenant}"] ~> Service <| name == 'congress-server' |>
+    Keystone_user_role["${auth_name}@${tenant}"] ~> Anchor['congress::service::end']
+  }
+
+  if $configure_endpoint {
+    Keystone_endpoint["${region}/${service_name}::${service_type}"]  ~> Anchor['congress::service::end']
   }
-  Keystone_endpoint["${region}/${service_name}::${service_type}"]  ~> Service <| name == 'congress-server' |>
 
   keystone::resource::service_identity { 'congress':
     configure_user      => $configure_user,
diff --git a/manifests/keystone/authtoken.pp b/manifests/keystone/authtoken.pp
index 2362631..cdfe421 100644
--- a/manifests/keystone/authtoken.pp
+++ b/manifests/keystone/authtoken.pp
@@ -222,6 +222,8 @@ class congress::keystone::authtoken(
   $token_cache_time               = $::os_service_default,
 ) {
 
+  include ::congress::deps
+
   keystone::resource::authtoken { 'congress_config':
     username                       => $username,
     password                       => $password,
diff --git a/manifests/logging.pp b/manifests/logging.pp
index b7b8041..c87f6d2 100644
--- a/manifests/logging.pp
+++ b/manifests/logging.pp
@@ -113,6 +113,8 @@ class congress::logging(
   $log_date_format               = $::os_service_default,
 ) {
 
+  include ::congress::deps
+
   oslo::log { 'congress_config':
     use_stderr                    => $use_stderr,
     use_syslog                    => $use_syslog,
diff --git a/manifests/params.pp b/manifests/params.pp
index f4d1593..40a5547 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -5,23 +5,22 @@
 class congress::params {
   include ::openstacklib::defaults
 
-  $drivers     = ['congress.datasources.neutronv2_driver.NeutronV2Driver,congress.datasources.glancev2_driver.GlanceV2Driver',
-                  'congress.datasources.nova_driver.NovaDriver',
-                  'congress.datasources.keystone_driver.KeystoneDriver',
-                  'congress.datasources.ceilometer_driver.CeilometerDriver',
-                  'congress.datasources.cinder_driver.CinderDriver']
-  $policy_path = '/etc/congress/snapshot/'
+  $drivers             = ['congress.datasources.neutronv2_driver.NeutronV2Driver,congress.datasources.glancev2_driver.GlanceV2Driver',
+                          'congress.datasources.nova_driver.NovaDriver',
+                          'congress.datasources.keystone_driver.KeystoneDriver',
+                          'congress.datasources.ceilometer_driver.CeilometerDriver',
+                          'congress.datasources.cinder_driver.CinderDriver']
+  $policy_path         = '/etc/congress/snapshot/'
+  $client_package_name = 'python-congressclient'
 
   case $::osfamily {
     'RedHat': {
-      $package_name        = 'openstack-congress'
-      $client_package_name = 'python-congressclient'
-      $service_name        = 'congress-server'
+      $package_name = 'openstack-congress'
+      $service_name = 'congress-server'
     }
     'Debian': {
-      $package_name        = 'congress'
-      $client_package_name = 'congressclient'
-      $service_name        = 'congress'
+      $package_name = 'congress-server'
+      $service_name = 'congress-server'
     }
     default: {
       fail("Unsupported osfamily: ${::osfamily} operatingsystem")
diff --git a/manifests/policy.pp b/manifests/policy.pp
index 8ec8ae9..77ff777 100644
--- a/manifests/policy.pp
+++ b/manifests/policy.pp
@@ -28,6 +28,8 @@ class congress::policy (
   $policy_path = '/etc/congress/policy.json',
 ) {
 
+  include ::congress::deps
+
   validate_hash($policies)
 
   Openstacklib::Policy::Base {
diff --git a/manifests/server.pp b/manifests/server.pp
new file mode 100644
index 0000000..910816c
--- /dev/null
+++ b/manifests/server.pp
@@ -0,0 +1,63 @@
+# = Class: congress::server
+#
+# This class manages the Congress server.
+#
+# [*enabled*]
+#   (Optional) Service enable state for congress-server.
+#   Defaults to true.
+#
+# [*manage_service*]
+#   (Optional) Whether the service is managed by this puppet class.
+#   Defaults to true.
+#
+# [*auth_strategy*]
+#   (optional) Type of authentication to be used.
+#   Defaults to 'keystone'
+#
+# [*bind_host*]
+#   (optional) The host IP to bind to.
+#   Defaults to $::os_service_default
+#
+# [*bind_port*]
+#   (optional) The port to bind to.
+#   Defaults to $::os_service_default
+#
+class congress::server(
+  $manage_service = true,
+  $enabled        = true,
+  $auth_strategy  = 'keystone',
+  $bind_host      = $::os_service_default,
+  $bind_port      = $::os_service_default,
+) {
+
+  include ::congress::deps
+  include ::congress::params
+  include ::congress::policy
+
+  if $auth_strategy == 'keystone' {
+    include ::congress::keystone::authtoken
+  }
+
+  congress_config {
+    'DEFAULT/bind_host' : value => $bind_host;
+    'DEFAULT/bind_port' : value => $bind_port;
+  }
+
+  if $manage_service {
+    if $enabled {
+      $service_ensure = 'running'
+    } else {
+      $service_ensure = 'stopped'
+    }
+  }
+
+  if $manage_service {
+    service { 'congress-server':
+      ensure => $service_ensure,
+      name   => $::congress::params::service_name,
+      enable => $enabled,
+      tag    => 'congress-service'
+    }
+  }
+
+}
diff --git a/spec/acceptance/basic_congress_spec.rb b/spec/acceptance/basic_congress_spec.rb
new file mode 100644
index 0000000..f9b3317
--- /dev/null
+++ b/spec/acceptance/basic_congress_spec.rb
@@ -0,0 +1,72 @@
+require 'spec_helper_acceptance'
+
+describe 'basic congress' do
+
+  context 'default parameters' do
+
+    it 'should work with no errors' do
+      pp= <<-EOS
+      include ::openstack_integration
+      include ::openstack_integration::repos
+      include ::openstack_integration::rabbitmq
+      include ::openstack_integration::mysql
+      include ::openstack_integration::keystone
+
+      rabbitmq_user { 'congress':
+        admin    => true,
+        password => 'an_even_bigger_secret',
+        provider => 'rabbitmqctl',
+        require  => Class['rabbitmq'],
+      }
+      rabbitmq_user_permissions { 'congress@/':
+        configure_permission => '.*',
+        write_permission     => '.*',
+        read_permission      => '.*',
+        provider             => 'rabbitmqctl',
+        require              => Class['rabbitmq'],
+      }
+      # Congress resources
+      case $::osfamily {
+        'RedHat': {
+          warning('Congress is not yet packaged on RedHat systems.')
+        }
+        'Debian': {
+          # TODO(zhongshengping): temporarily added this package
+          package { 'python-antlr3':
+            ensure => present,
+          }
+          class { '::congress':
+            default_transport_url => 'rabbit://congress:an_even_bigger_secret@127.0.0.1/',
+          }
+          class { '::congress::keystone::auth':
+            password => 'a_big_secret',
+          }
+          class { '::congress::db::mysql':
+            password => 'a_big_secret',
+          }
+          class { '::congress::db':
+            database_connection => 'mysql+pymysql://congress:a_big_secret@127.0.0.1/congress?charset=utf8',
+          }
+          class { '::congress::keystone::authtoken':
+            password => 'a_big_secret',
+          }
+          class { '::congress::server': }
+          class { '::congress::client': }
+        }
+      }
+      EOS
+
+
+      # Run it twice to test for idempotency
+      apply_manifest(pp, :catch_failures => true)
+      apply_manifest(pp, :catch_changes => true)
+    end
+
+    if os[:family].casecmp('Debian') == 0
+      describe port(1789) do
+        it { is_expected.to be_listening.with('tcp') }
+      end
+    end
+
+  end
+end
diff --git a/spec/classes/congress_client_spec.rb b/spec/classes/congress_client_spec.rb
index 6893694..2faf810 100644
--- a/spec/classes/congress_client_spec.rb
+++ b/spec/classes/congress_client_spec.rb
@@ -6,12 +6,13 @@ describe 'congress::client' do
 
     context 'with default parameters' do
       it 'contains congress::params' do
+          is_expected.to contain_class('congress::deps')
           is_expected.to contain_class('congress::params')
       end
       it 'contains congressclient' do
           is_expected.to contain_package('python-congressclient').with(
               :ensure => 'present',
-              :name   => platform_params[:congressclient_package]
+              :name   => 'python-congressclient',
           )
       end
     end
@@ -26,16 +27,7 @@ describe 'congress::client' do
         facts.merge!(OSDefaults.get_facts())
       end
 
-      let(:platform_params) do
-        case facts[:osfamily]
-        when 'Debian'
-          { :congressclient_package => 'congressclient' }
-        when 'RedHat'
-          { :congressclient_package => 'python-congressclient' }
-        end
-      end
       it_behaves_like 'congress::client'
-
     end
   end
 
diff --git a/spec/classes/congress_db_sync_spec.rb b/spec/classes/congress_db_sync_spec.rb
index 6572cbb..3c896fa 100644
--- a/spec/classes/congress_db_sync_spec.rb
+++ b/spec/classes/congress_db_sync_spec.rb
@@ -6,11 +6,15 @@ describe 'congress::db::sync' do
 
     it 'runs congress-db-sync' do
       is_expected.to contain_exec('congress-db-sync').with(
-        :command     => 'congress-db-manage upgrade head ',
+        :command     => 'congress-db-manage --config-file /etc/congress/congress.conf upgrade head',
         :path        => ["/bin", "/usr/bin"],
         :refreshonly => true,
         :user        => 'congress',
-        :logoutput   => 'on_failure'
+        :logoutput   => 'on_failure',
+        :subscribe   => ['Anchor[congress::install::end]',
+                         'Anchor[congress::config::end]',
+                         'Anchor[congress::dbsync::begin]'],
+        :notify      => 'Anchor[congress::dbsync::end]',
       )
     end
 
diff --git a/spec/classes/congress_init_spec.rb b/spec/classes/congress_init_spec.rb
index 15f9c38..6494775 100644
--- a/spec/classes/congress_init_spec.rb
+++ b/spec/classes/congress_init_spec.rb
@@ -12,9 +12,16 @@ describe 'congress' do
       end
 
       it 'contains the logging class' do
+        is_expected.to contain_class('congress::deps')
         is_expected.to contain_class('congress::logging')
       end
 
+      it { is_expected.to contain_package('congress-common').with(
+        :ensure => 'present',
+        :name   => platform_params[:congress_package],
+        :tag    => ['openstack', 'congress-package']
+      )}
+
       it 'configures rabbit' do
         is_expected.to contain_congress_config('DEFAULT/rpc_backend').with_value('rabbit')
         is_expected.to contain_congress_config('oslo_messaging_rabbit/rabbit_host').with_value('<SERVICE DEFAULT>')
@@ -268,7 +275,7 @@ describe 'congress' do
       let(:platform_params) do
         case facts[:osfamily]
         when 'Debian'
-          { :congress_package => 'congress' }
+          { :congress_package => 'congress-server' }
         when 'RedHat'
           { :congress_package => 'openstack-congress' }
         end
diff --git a/spec/classes/congress_server_spec.rb b/spec/classes/congress_server_spec.rb
new file mode 100644
index 0000000..c9926fd
--- /dev/null
+++ b/spec/classes/congress_server_spec.rb
@@ -0,0 +1,61 @@
+require 'spec_helper'
+
+describe 'congress::server' do
+
+  let :pre_condition do
+    "class { '::congress::keystone::authtoken':
+       password =>'foo',
+     }
+     class {'::congress': }"
+  end
+
+  let :params do
+    { :enabled        => true,
+      :manage_service => true,
+      :bind_host      => '0.0.0.0',
+      :bind_port      => '1789'
+    }
+  end
+
+  shared_examples_for 'congress::server' do
+
+    it { is_expected.to contain_class('congress::deps') }
+    it { is_expected.to contain_class('congress::params') }
+    it { is_expected.to contain_class('congress::policy') }
+
+    it 'configures api' do
+      is_expected.to contain_congress_config('DEFAULT/bind_host').with_value( params[:bind_host] )
+      is_expected.to contain_congress_config('DEFAULT/bind_port').with_value( params[:bind_port] )
+    end
+
+    [{:enabled => true}, {:enabled => false}].each do |param_hash|
+      context "when service should be #{param_hash[:enabled] ? 'enabled' : 'disabled'}" do
+        before do
+          params.merge!(param_hash)
+        end
+
+        it 'configures congress-server service' do
+          is_expected.to contain_service('congress-server').with(
+            :ensure => (params[:manage_service] && params[:enabled]) ? 'running' : 'stopped',
+            :name   => 'congress-server',
+            :enable => params[:enabled],
+            :tag    => 'congress-service',
+          )
+        end
+      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
+
+      it_configures 'congress::server'
+    end
+  end
+end