Fix resource defaults class

Currently the resource defaults type has a few problems:
1) Calls pcs directly and so will break in case of concurrent
   CIB updates
2) The delete method is broken because it will not expand the
   name of the variable so nothing will be deleted

Move to the common pcs() function which takes care of
1) and also fix 2) as well. Also add the tries/try_sleep parameters
to be passed to the common pcs() function. Let's also add a test
case to cover for this class.

Change-Id: I1408c359ba9047f4295c9d820e055a8bef1891ee

lib/puppet/provider/pcmk_resource_default/pcs.rb

@@ -1,3 +1,5 @@
+require_relative '../pcmk_common'
+
 # Currently the implementation is somewhat naive (will not work great
 # with ensure => absent, unless the correct current value is also
 # specified). For more proper handling, prefetching should be
@@ -13,42 +15,26 @@ Puppet::Type.type(:pcmk_resource_default).provide(:pcs) do
 
     cmd = "resource defaults #{name}='#{value}'"
 
-    pcs(cmd)
+    pcs('create', name, cmd, @resource[:tries], @resource[:try_sleep],
+        @resource[:verify_on_create], @resource[:post_success_sleep])
   end
 
   def destroy
     name = @resource[:name]
 
-    cmd = 'resource defaults #{name}='
-    pcs(cmd)
+    cmd = "resource defaults #{name}="
+    pcs('create', name, cmd, @resource[:tries], @resource[:try_sleep],
+        @resource[:verify_on_create], @resource[:post_success_sleep])
   end
 
   def exists?
     name = @resource[:name]
     value = @resource[:value]
 
-    cmd = 'resource defaults'
-    status, _ = pcs(cmd,
-                    :grep => "^#{name}: #{value}$",
-                    :allow_failure => true)
-    status == 0
-  end
-
-  def pcs(cmd, options={})
-    full_cmd = "pcs #{cmd}"
-
-    if options[:grep]
-      full_cmd += " | grep '#{options[:grep]}'"
-    end
-
-    Puppet.debug("Running #{full_cmd}")
-    output = `#{full_cmd}`
-    status = $?.exitstatus
-
-    if status != 0 && !options[:allow_failure]
-      raise Puppet::Error("#{full_cmd} returned #{status}, output: #{output}")
-    end
-
-    [status, output]
+    cmd = "resource defaults | grep '^#{name}: #{value}\$'"
+    Puppet.debug("defaults exists #{cmd}")
+    status = pcs('show', name, cmd, @resource[:tries], @resource[:try_sleep],
+                 @resource[:verify_on_create], @resource[:post_success_sleep])
+    return status == false ? false : true
   end
 end

lib/puppet/type/pcmk_resource_default.rb

@@ -13,4 +13,69 @@ Puppet::Type.newtype(:pcmk_resource_default) do
     desc "A default value for the option"
   end
 
+  newparam(:post_success_sleep) do
+    desc "The time to sleep after successful pcs action.  The reason to set
+          this is to avoid immediate back-to-back 'pcs resource create' calls
+          when creating multiple resources.  Defaults to '0'."
+
+    munge do |value|
+      if value.is_a?(String)
+        unless value =~ /^[-\d.]+$/
+          raise ArgumentError, "post_success_sleep must be a number"
+        end
+        value = Float(value)
+      end
+      raise ArgumentError, "post_success_sleep cannot be a negative number" if value < 0
+      value
+    end
+
+    defaultto 0
+  end
+
+  ## borrowed from exec.rb
+  newparam(:tries) do
+    desc "The number of times to attempt to create a pcs resource.
+      Defaults to '1'."
+
+    munge do |value|
+      if value.is_a?(String)
+        unless value =~ /^[\d]+$/
+          raise ArgumentError, "Tries must be an integer"
+        end
+        value = Integer(value)
+      end
+      raise ArgumentError, "Tries must be an integer >= 1" if value < 1
+      value
+    end
+
+    defaultto 1
+  end
+
+  newparam(:try_sleep) do
+    desc "The time to sleep in seconds between 'tries'."
+
+    munge do |value|
+      if value.is_a?(String)
+        unless value =~ /^[-\d.]+$/
+          raise ArgumentError, "try_sleep must be a number"
+        end
+        value = Float(value)
+      end
+      raise ArgumentError, "try_sleep cannot be a negative number" if value < 0
+      value
+    end
+
+    defaultto 0
+  end
+
+  newparam(:verify_on_create, :boolean => true, :parent => Puppet::Parameter::Boolean) do
+     desc "Whether to verify pcs resource creation with an additional
+     call to 'pcs resource show' rather than just relying on the exit
+     status of 'pcs resource create'.  When true, $try_sleep
+     determines how long to wait to verify and $post_success_sleep is
+     ignored.  Defaults to `false`."
+
+     defaultto false
+   end
+
 end

manifests/resource_defaults.pp

@@ -7,6 +7,22 @@
 # [*defaults*]
 #   (required) Comma separated string of key=value pairs specifying defaults.
 #
+# [*post_success_sleep*]
+#   (optional) How long to wait acfter successful action
+#   Defaults to 0
+#
+# [*tries*]
+#   (optional) How many times to attempt to create the constraint
+#   Defaults to 1
+#
+# [*try_sleep*]
+#   (optional) How long to wait between tries, in seconds
+#   Defaults to 0
+#
+# [*verify_on_create*]
+#   (optional) Whether to verify creation of resource
+#   Defaults to false
+#
 # [*ensure*]
 #   (optional) Whether to create or remove the defaults
 #   Defaults to present
@@ -38,13 +54,21 @@
 #
 class pacemaker::resource_defaults(
   $defaults,
-  $ensure = 'present',
+  $post_success_sleep = 0,
+  $tries              = 1,
+  $try_sleep          = 0,
+  $verify_on_create   = false,
+  $ensure             = 'present',
 ) {
   create_resources(
     'pcmk_resource_default',
     $defaults,
     {
-      ensure => $ensure,
+      ensure             => $ensure,
+      post_success_sleep => $post_success_sleep,
+      tries              => $tries,
+      try_sleep          => $try_sleep,
+      verify_on_create   => $verify_on_create,
     }
   )
 }

spec/classes/pacemaker_resource_defaults_spec.rb

@@ -0,0 +1,52 @@
+#
+# Copyright (C) 2017 Red Hat, 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 'pacemaker::resource_defaults', type: :class do
+
+  let(:title) { 'foo' }
+
+  shared_examples_for 'pacemaker::resource_defaults' do
+    context 'with params' do
+      let(:params) { {
+        :defaults  => { 'fencingtest' => { 'name' => 'requires', 'value' => 'fencing', }, },
+        :tries     => 10,
+        :try_sleep => 20,
+      } }
+      it { is_expected.to compile.with_all_deps }
+
+      it {
+        is_expected.to contain_pcmk_resource_default('fencingtest').with(
+          :name  => 'requires',
+          :value => 'fencing',
+          :tries     => 10,
+          :try_sleep => 20,
+        )
+      }
+    end
+
+  end
+
+  on_supported_os.each do |os, facts|
+    context "on #{os}" do
+      let(:facts) do
+        facts.merge({ :hostname => 'node.example.com' })
+      end
+
+      it_behaves_like 'pacemaker::resource_defaults'
+    end
+  end
+end