From ccdba23158f852e215f399b8bb08fc6f1348f090 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89ric=20Lemoine?= <elemoine@mirantis.com>
Date: Thu, 24 Dec 2015 17:36:39 +0100
Subject: [PATCH] Move Pacemaker/Corosync code out of lma_collector

This commit moves the Pacemaker/Corosync Puppet code from the
lma_collector module to the Fuel-specific base.pp manifest.

This involves the following changes:

* Fuel's "pacemaker_wrappers::service" define is now used in base.pp
  to configure the LMA service resource to using the "pacemaker"
  provider.

* To configure "pacemaker_wrappers::service" we need to know the Heka
  user. So to avoid hacks where we'd use private variables from the
  lma_collector and heka modules to determine the Heka user the
  lma_collector and heka modules are changed to make the Heka user
  configurable. For this the "heka" class "run_as_root" parameter is
  removed in favor of a "user" parameter.

* In other manifests we use a resource collector to make sure that
  the LMA service resource is not re-configured with the default
  provider. This part is a bit hackish, but we haven't been able to
  come up with a better way to address the issue.

Change-Id: I0ed0bddb245dc3a65b034e5caec14a65cfa908cb
Implements: blueprint lma-without-fuel
---
 .../puppet/manifests/aggregator.pp            |  9 ++
 deployment_scripts/puppet/manifests/base.pp   | 96 ++++++++++++++++--
 deployment_scripts/puppet/manifests/cinder.pp | 11 +++
 .../puppet/manifests/compute.pp               | 11 +++
 .../puppet/manifests/configure_afd_filters.pp | 11 +++
 .../puppet/manifests/controller.pp            |  6 ++
 .../puppet/modules/heka/manifests/init.pp     | 11 +--
 .../puppet/modules/heka/manifests/params.pp   |  1 -
 .../modules/heka/spec/classes/heka_spec.rb    | 37 +++++++
 .../puppet/modules/lma_collector/README.md    |  3 +-
 .../modules/lma_collector/manifests/init.pp   | 98 +++----------------
 .../modules/lma_collector/manifests/params.pp |  4 -
 .../lma_collector/manifests/service.pp        | 10 +-
 pre_build_hook                                |  4 +-
 14 files changed, 195 insertions(+), 117 deletions(-)
 create mode 100644 deployment_scripts/puppet/modules/heka/spec/classes/heka_spec.rb

diff --git a/deployment_scripts/puppet/manifests/aggregator.pp b/deployment_scripts/puppet/manifests/aggregator.pp
index 62ae44bde..558ee5da7 100644
--- a/deployment_scripts/puppet/manifests/aggregator.pp
+++ b/deployment_scripts/puppet/manifests/aggregator.pp
@@ -23,6 +23,15 @@ $management_network = hiera('management_network_range')
 $aggregator_port    = 5565
 $check_port         = 5566
 
+if $is_controller {
+  # On controllers make sure the LMA service is configured
+  # with the "pacemaker" provider
+  include lma_collector::params
+  Service<| title == $lma_collector::params::service_name |> {
+    provider => 'pacemaker'
+  }
+}
+
 class { 'lma_collector::aggregator::client':
   address => $aggregator_address,
   port    => $aggregator_port,
diff --git a/deployment_scripts/puppet/manifests/base.pp b/deployment_scripts/puppet/manifests/base.pp
index f6678bd2c..f082365b2 100644
--- a/deployment_scripts/puppet/manifests/base.pp
+++ b/deployment_scripts/puppet/manifests/base.pp
@@ -47,12 +47,8 @@ if $is_controller {
   # "keystone" group required for lma_collector::logs::openstack to be able
   # to read log files located in /var/log/keystone
   $additional_groups = ['haclient', 'keystone']
-  $pacemaker_managed = true
-  $rabbitmq_resource = 'master_p_rabbitmq-server'
-}else{
+} else {
   $additional_groups = []
-  $pacemaker_managed = false
-  $rabbitmq_resource = undef
 }
 
 $elasticsearch_mode = $lma_collector['elasticsearch_mode']
@@ -101,11 +97,93 @@ if $is_controller{
   $pre_script = undef
 }
 
+case $::osfamily {
+  'Debian': {
+    $heka_user = 'heka'
+  }
+  'RedHat': {
+    # For CentOS, the LMA collector needs to run as root because the files
+    # created by RSyslog aren't created with the correct mode for now.
+    $heka_user = 'root'
+  }
+  default: {
+    fail("${::osfamily} not supported")
+  }
+}
+
 class { 'lma_collector':
-  tags              => $tags,
-  groups            => $additional_groups,
-  pacemaker_managed => $pacemaker_managed,
-  rabbitmq_resource => $rabbitmq_resource,
+  tags   => $tags,
+  user   => $heka_user,
+  groups => $additional_groups,
+}
+
+# On controller nodes the LMA collector service is managed by Pacemaker, so we
+# use pacemaker_wrappers::service to reconfigure the service resource to use
+# the "pacemaker" service provider
+if $is_controller {
+
+  include lma_collector::params
+
+  $service_name = $lma_collector::params::service_name
+  $config_dir = $lma_collector::params::config_dir
+  $rabbitmq_resource = 'master_p_rabbitmq-server'
+
+  pacemaker_wrappers::service { $service_name:
+    ensure          => present,
+    prefix          => false,
+    primitive_class => 'ocf',
+    primitive_type  => 'ocf-lma_collector',
+    complex_type    => 'clone',
+    use_handler     => false,
+    ms_metadata     => {
+      # the resource should start as soon as the dependent resources (eg RabbitMQ)
+      # are running *locally*
+      'interleave'          => true,
+      'migration-threshold' => '3',
+      'failure-timeout'     => '120',
+    },
+    parameters      => {
+      'config'   => $config_dir,
+      'log_file' => "/var/log/${service_name}.log",
+      'user'     => $heka_user,
+    },
+    operations      => {
+      'monitor' => {
+        'interval' => '20',
+        'timeout'  => '10',
+      },
+      'start'   => {
+        'timeout' => '30',
+      },
+      'stop'    => {
+        'timeout' => '30',
+      },
+    },
+    ocf_script_file => 'lma_collector/ocf-lma_collector',
+  }
+
+  cs_rsc_colocation { "${service_name}-with-rabbitmq":
+    ensure     => present,
+    alias      => $service_name,
+    primitives => ["clone_${service_name}", $rabbitmq_resource],
+    score      => 0,
+    require    => Pacemaker_wrappers::Service[$service_name],
+  }
+
+  cs_rsc_order { "${service_name}-after-rabbitmq":
+    ensure  => present,
+    alias   => $service_name,
+    first   => $rabbitmq_resource,
+    second  => "clone_${service_name}",
+    # Heka cannot start if RabbitMQ isn't ready to accept connections. But
+    # once it is initialized, it can recover from a RabbitMQ outage. This is
+    # why we set score to 0 (interleave) meaning that the collector should
+    # start once RabbitMQ is active but a restart of RabbitMQ
+    # won't trigger a restart of the LMA collector.
+    score   => 0,
+    require => Cs_rsc_colocation[$service_name],
+    before  => Class['lma_collector'],
+  }
 }
 
 if $elasticsearch_mode != 'disabled' {
diff --git a/deployment_scripts/puppet/manifests/cinder.pp b/deployment_scripts/puppet/manifests/cinder.pp
index 127206538..143e2e592 100644
--- a/deployment_scripts/puppet/manifests/cinder.pp
+++ b/deployment_scripts/puppet/manifests/cinder.pp
@@ -16,6 +16,17 @@ include lma_collector::params
 
 $ceilometer    = hiera_hash('ceilometer', {})
 $lma_collector = hiera_hash('lma_collector')
+$roles         = node_roles(hiera('nodes'), hiera('uid'))
+$is_controller = member($roles, 'controller') or member($roles, 'primary-controller')
+
+if $is_controller {
+  # On controllers make sure the LMA service is configured
+  # with the "pacemaker" provider
+  include lma_collector::params
+  Service<| title == $lma_collector::params::service_name |> {
+    provider => 'pacemaker'
+  }
+}
 
 if $lma_collector['influxdb_mode'] != 'disabled' {
   class { 'lma_collector::logs::counter':
diff --git a/deployment_scripts/puppet/manifests/compute.pp b/deployment_scripts/puppet/manifests/compute.pp
index daf33a60f..19f9cab7e 100644
--- a/deployment_scripts/puppet/manifests/compute.pp
+++ b/deployment_scripts/puppet/manifests/compute.pp
@@ -16,6 +16,17 @@ include lma_collector::params
 
 $ceilometer    = hiera_hash('ceilometer', {})
 $lma_collector = hiera_hash('lma_collector')
+$roles          = node_roles(hiera('nodes'), hiera('uid'))
+$is_controller  = member($roles, 'controller') or member($roles, 'primary-controller')
+
+if $is_controller {
+  # On controllers make sure the LMA service is configured
+  # with the "pacemaker" provider
+  include lma_collector::params
+  Service<| title == $lma_collector::params::service_name |> {
+    provider => 'pacemaker'
+  }
+}
 
 if $lma_collector['elasticsearch_mode'] != 'disabled' {
   lma_collector::logs::openstack { 'nova': }
diff --git a/deployment_scripts/puppet/manifests/configure_afd_filters.pp b/deployment_scripts/puppet/manifests/configure_afd_filters.pp
index 9157375af..c791b224a 100644
--- a/deployment_scripts/puppet/manifests/configure_afd_filters.pp
+++ b/deployment_scripts/puppet/manifests/configure_afd_filters.pp
@@ -15,12 +15,23 @@
 include lma_collector::params
 
 $lma = hiera_hash('lma_collector', {})
+$roles          = node_roles(hiera('nodes'), hiera('uid'))
+$is_controller  = member($roles, 'controller') or member($roles, 'primary-controller')
 
 $alarms_definitions = $lma['alarms']
 if $alarms_definitions == undef {
     fail('Alarms definitions not found. Check files in /etc/hiera/override.')
 }
 
+if $is_controller {
+  # On controllers make sure the LMA service is configured
+  # with the "pacemaker" provider
+  include lma_collector::params
+  Service<| title == $lma_collector::params::service_name |> {
+    provider => 'pacemaker'
+  }
+}
+
 class { 'lma_collector::afds':
     roles                  => hiera('roles'),
     node_cluster_roles     => $lma['node_cluster_roles'],
diff --git a/deployment_scripts/puppet/manifests/controller.pp b/deployment_scripts/puppet/manifests/controller.pp
index b237460f6..67e6bf028 100644
--- a/deployment_scripts/puppet/manifests/controller.pp
+++ b/deployment_scripts/puppet/manifests/controller.pp
@@ -39,6 +39,12 @@ else {
   $rabbitmq_user = 'nova'
 }
 
+# Make sure the LMA service is configured with the "pacemaker" provider
+include lma_collector::params
+Service<| title == $lma_collector::params::service_name |> {
+  provider => 'pacemaker'
+}
+
 # OpenStack notifications are always useful for indexation and metrics collection
 class { 'lma_collector::notifications::controller':
   host     => $messaging_address,
diff --git a/deployment_scripts/puppet/modules/heka/manifests/init.pp b/deployment_scripts/puppet/modules/heka/manifests/init.pp
index 1b87538c0..001e99b02 100644
--- a/deployment_scripts/puppet/modules/heka/manifests/init.pp
+++ b/deployment_scripts/puppet/modules/heka/manifests/init.pp
@@ -24,10 +24,8 @@
 # [*config_dir*]
 #   The directory where to store the configuration (default: '/etc/hekad').
 #
-# [*run_as_root*]
-#   Whether or not to run the Heka service as root (default: false).
-#   You may have to set this parameter to true on some systems to access log
-#   files, run additional commands, ...
+# [*user*]
+#   The user to run the Heka service as (default: 'heka'). You may have to use 'root' on some systems for the Heka service to be able to access log files, run additional commands, ...
 #
 # [*additional_groups*]
 #   Additional groups to which the heka user should be added.
@@ -75,7 +73,7 @@
 class heka (
   $service_name = $heka::params::service_name,
   $config_dir = $heka::params::config_dir,
-  $run_as_root = $heka::params::run_as_root,
+  $user = $heka::params::user,
   $additional_groups = $heka::params::additional_groups,
   $hostname = $heka::params::hostname,
   $maxprocs = $heka::params::maxprocs,
@@ -91,7 +89,8 @@ class heka (
   $hekad_wrapper = "/usr/local/bin/${service_name}_wrapper"
   $base_dir      = "/var/cache/${service_name}"
   $log_file      = "/var/log/${service_name}.log"
-  $heka_user     = $heka::params::user
+  $heka_user     = $user
+  $run_as_root   = $heka_user == 'root'
 
   package { $heka::params::package_name:
     ensure => present,
diff --git a/deployment_scripts/puppet/modules/heka/manifests/params.pp b/deployment_scripts/puppet/modules/heka/manifests/params.pp
index f07c4d479..b9a389738 100644
--- a/deployment_scripts/puppet/modules/heka/manifests/params.pp
+++ b/deployment_scripts/puppet/modules/heka/manifests/params.pp
@@ -16,7 +16,6 @@ class heka::params {
   $package_name = 'heka'
   $service_name = 'hekad'
   $user = 'heka'
-  $run_as_root = false
   $additional_groups = []
 
   $hostname = undef
diff --git a/deployment_scripts/puppet/modules/heka/spec/classes/heka_spec.rb b/deployment_scripts/puppet/modules/heka/spec/classes/heka_spec.rb
new file mode 100644
index 000000000..125040157
--- /dev/null
+++ b/deployment_scripts/puppet/modules/heka/spec/classes/heka_spec.rb
@@ -0,0 +1,37 @@
+#    Copyright 2015 Mirantis, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+require 'spec_helper'
+
+describe 'heka' do
+    let(:facts) do
+        {:kernel => 'Linux', :operatingsystem => 'Ubuntu',
+         :osfamily => 'Debian'}
+    end
+
+    describe 'with defaults' do
+        it { is_expected.to contain_user('heka') }
+        it { is_expected.to contain_file('/etc/init/hekad.conf') \
+             .with_content(/sudo -u heka/) }
+    end
+
+    describe 'with user => "root"' do
+        let(:params) do
+            {:user => 'root'}
+        end
+        it { is_expected.to contain_user('root') }
+        it { is_expected.to contain_file('/etc/init/hekad.conf') }
+        it { is_expected.not_to contain_file('/etc/init/hekad.conf') \
+             .with_content(/sudo -u/) }
+    end
+end
diff --git a/deployment_scripts/puppet/modules/lma_collector/README.md b/deployment_scripts/puppet/modules/lma_collector/README.md
index d381200d8..777ccd58e 100644
--- a/deployment_scripts/puppet/modules/lma_collector/README.md
+++ b/deployment_scripts/puppet/modules/lma_collector/README.md
@@ -335,7 +335,8 @@ Main class. Install and configure the main components of the LMA collector.
 ##### Parameters
 
 * `tags`: *Optional*. Fields added to Heka messages. Valid options: a hash. Default: `{}`.
-* `groups`: *Optional*. Additional groups to add the `heka` user to. Valid options: an array of strings. Default: `['syslog', 'adm']`.
+* `user`: *Optional*. User the Heka service is run as. You may have to use `'root'` on some systems for the Heka service to be able to access log files, run additional commands, ... Valid options: a string.  Default: `'heka'`.
+* `groups`: *Optional*. Additional groups to add to the user running the Heka service. Ignored if the Heka service is run as "root". Valid options: an array of strings. Default: `['syslog', 'adm']`.
 
 #### Class: `lma_collector::elasticsearch`
 
diff --git a/deployment_scripts/puppet/modules/lma_collector/manifests/init.pp b/deployment_scripts/puppet/modules/lma_collector/manifests/init.pp
index 8ff196d23..26324bd01 100644
--- a/deployment_scripts/puppet/modules/lma_collector/manifests/init.pp
+++ b/deployment_scripts/puppet/modules/lma_collector/manifests/init.pp
@@ -31,9 +31,8 @@
 #
 class lma_collector (
   $tags = $lma_collector::params::tags,
+  $user = undef,
   $groups = [],
-  $pacemaker_managed = $lma_collector::params::pacemaker_managed,
-  $rabbitmq_resource = undef,
 ) inherits lma_collector::params {
   include heka::params
 
@@ -44,11 +43,16 @@ class lma_collector (
   $plugins_dir = $lma_collector::params::plugins_dir
   $lua_modules_dir = $heka::params::lua_modules_dir
 
+  $additional_groups = $user ? {
+    'root'  => [],
+    default => union($lma_collector::params::groups, $groups),
+  }
+
   class { 'heka':
     service_name        => $service_name,
     config_dir          => $config_dir,
-    run_as_root         => $lma_collector::params::run_as_root,
-    additional_groups   => union($lma_collector::params::groups, $groups),
+    user                => $user,
+    additional_groups   => $additional_groups,
     hostname            => $::hostname,
     internal_statistics => false,
     max_message_size    => $lma_collector::params::hekad_max_message_size,
@@ -56,88 +60,6 @@ class lma_collector (
     max_timer_inject    => $lma_collector::params::hekad_max_timer_inject,
   }
 
-  if $pacemaker_managed {
-    validate_string($rabbitmq_resource)
-
-    if $lma_collector::params::run_as_root {
-      $heka_user = 'root'
-    } else {
-      $heka_user = $heka::params::user
-    }
-
-    file { 'ocf-lma_collector':
-      ensure => present,
-      path   => '/usr/lib/ocf/resource.d/fuel/ocf-lma_collector',
-      source => 'puppet:///modules/lma_collector/ocf-lma_collector',
-      mode   => '0755',
-      owner  => 'root',
-      group  => 'root',
-    }
-
-    cs_resource { $service_name:
-      primitive_class => 'ocf',
-      provided_by     => 'fuel',
-      primitive_type  => 'ocf-lma_collector',
-      complex_type    => 'clone',
-      ms_metadata     => {
-        # the resource should start as soon as the dependent resources (eg
-        # RabbitMQ) are running *locally*
-        'interleave'          => true,
-        'migration-threshold' => '3',
-        'failure-timeout'     => '120',
-      },
-      parameters      => {
-        'config'   => $config_dir,
-        'log_file' => "/var/log/${service_name}.log",
-        'user'     => $heka_user,
-      },
-      operations      => {
-        'monitor' => {
-          'interval' => '20',
-          'timeout'  => '10',
-        },
-        'start'   => {
-          'timeout' => '30',
-        },
-        'stop'    => {
-          'timeout' => '30',
-        },
-      },
-      require         => [File['ocf-lma_collector'], Class['heka']],
-    }
-
-    cs_rsc_colocation { "${service_name}-with-rabbitmq":
-      ensure     => present,
-      alias      => $service_name,
-      primitives => ["clone_${service_name}", $rabbitmq_resource],
-      score      => 0,
-      require    => Cs_resource[$service_name],
-    }
-
-    cs_rsc_order { "${service_name}-after-rabbitmq":
-      ensure  => present,
-      alias   => $service_name,
-      first   => $rabbitmq_resource,
-      second  => "clone_${service_name}",
-      # Heka cannot start if RabbitMQ isn't ready to accept connections. But
-      # once it is initialized, it can recover from a RabbitMQ outage. This is
-      # why we set score to 0 (interleave) meaning that the collector should
-      # start once RabbitMQ is active but a restart of RabbitMQ
-      # won't trigger a restart of the LMA collector.
-      score   => 0,
-      require => Cs_rsc_colocation[$service_name]
-    }
-
-    class { 'lma_collector::service':
-      provider => 'pacemaker',
-      require  => Cs_rsc_order[$service_name]
-    }
-
-  } else {
-    # Use the default service class
-    include lma_collector::service
-  }
-
   # Copy our Lua modules to Heka's modules directory so they're available for
   # the sandboxes
   file { $lua_modules_dir:
@@ -196,4 +118,8 @@ class lma_collector (
       ensure => present,
     }
   }
+
+  class { 'lma_collector::service':
+    require => Class['heka'],
+  }
 }
diff --git a/deployment_scripts/puppet/modules/lma_collector/manifests/params.pp b/deployment_scripts/puppet/modules/lma_collector/manifests/params.pp
index 9f1da669e..e0f955744 100644
--- a/deployment_scripts/puppet/modules/lma_collector/manifests/params.pp
+++ b/deployment_scripts/puppet/modules/lma_collector/manifests/params.pp
@@ -52,13 +52,9 @@ class lma_collector::params {
   # required to read the log files
   case $::osfamily {
     'Debian': {
-      $run_as_root = false
       $groups = ['syslog', 'adm']
     }
     'RedHat': {
-      # For CentOS, the LMA collector needs to run as root because the files
-      # created by RSyslog aren't created with the correct mode for now.
-      $run_as_root = true
       $groups = []
     }
     default: {
diff --git a/deployment_scripts/puppet/modules/lma_collector/manifests/service.pp b/deployment_scripts/puppet/modules/lma_collector/manifests/service.pp
index 8f34f3b21..b31096e3c 100644
--- a/deployment_scripts/puppet/modules/lma_collector/manifests/service.pp
+++ b/deployment_scripts/puppet/modules/lma_collector/manifests/service.pp
@@ -47,19 +47,13 @@ class lma_collector::service (
     }
   }
 
-  if member(split($::pacemaker_resources, ','), $service_name) {
-    $real_provider = 'pacemaker'
-  } else {
-    $real_provider = $provider
-  }
-
   if $service_manage {
-    if $real_provider {
+    if $provider {
       service { 'lma_collector':
         ensure   => $_service_ensure,
         name     => $service_name,
         enable   => $service_enable,
-        provider => $real_provider,
+        provider => $provider,
       }
     } else {
       service { 'lma_collector':
diff --git a/pre_build_hook b/pre_build_hook
index 2440a4436..80a35232a 100755
--- a/pre_build_hook
+++ b/pre_build_hook
@@ -47,10 +47,10 @@ download_packages \
     http://mirrors.kernel.org/ubuntu/pool/main/libd/libdbi-drivers/libdbd-mysql_0.9.0-2ubuntu2_amd64.deb
 
 # Extract dependent manifests from fuel-library
-rm -rf "${MODULES_DIR:?}"/{haproxy,openstack,osnailyfacter,pacemaker,l23network}
+rm -rf "${MODULES_DIR:?}"/{haproxy,openstack,osnailyfacter,pacemaker,pacemaker_wrappers,l23network}
 wget -qO- "${FUEL_LIB_TARBALL_URL}" | \
     tar -C "${MODULES_DIR}" --strip-components=3 -zxvf - \
-    fuel-library-${FUEL_LIB_COMMIT/\//-}/deployment/puppet/{haproxy,openstack,osnailyfacter,pacemaker,l23network}
+    fuel-library-${FUEL_LIB_COMMIT/\//-}/deployment/puppet/{haproxy,openstack,osnailyfacter,pacemaker,pacemaker_wrappers,l23network}
 # Remove this dead link otherwise fpb fails
 rm -f "${MODULES_DIR:?}"/osnailyfacter/modular/master