CHANGELOG.md

@@ -2,6 +2,14 @@
 
 All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org).
 
+## [3.0.1](https://github.com/puppetlabs/puppetlabs-haproxy/tree/3.0.1) (2019-02-19)
+
+[Full Changelog](https://github.com/puppetlabs/puppetlabs-haproxy/compare/3.0.0...3.0.1)
+
+### Fixed
+
+- \(MODULES-8566\) Only create entries for defined settings [\#350](https://github.com/puppetlabs/puppetlabs-haproxy/pull/350) ([genebean](https://github.com/genebean))
+
 ## [3.0.0](https://github.com/puppetlabs/puppetlabs-haproxy/tree/3.0.0) (2019-02-01)
 
 [Full Changelog](https://github.com/puppetlabs/puppetlabs-haproxy/compare/2.2.0...3.0.0)

manifests/listen.pp

@@ -114,7 +114,7 @@
   if $ipaddress and $bind {
     fail('The use of $ipaddress and $bind is mutually exclusive, please choose either one')
   }
-  if $ipaddress                               == undef and $bind == undef {
+  if $ipaddress == undef and $bind == undef {
     fail('Either $ipaddress or $bind is needed, please choose one')
   }
   if $bind_options                            != '' {

metadata.json

@@ -1,6 +1,6 @@
 {
   "name": "puppetlabs-haproxy",
-  "version": "3.0.0",
+  "version": "3.0.1",
   "author": "puppetlabs",
   "summary": "Configures HAProxy servers and manages the configuration of backend member servers.",
   "license": "Apache-2.0",

spec/acceptance/basic_spec.rb

@@ -22,7 +22,9 @@ class { 'haproxy':
       }
     PUPPETCODE
     it 'does not listen on any ports' do
-      apply_manifest(pp_one, catch_failures: true)
+      retry_on_error_matching do
+        apply_manifest(pp_one, catch_failures: true)
+      end
     end
 
     describe port('9090') do
@@ -45,7 +47,9 @@ class { 'haproxy': }
         }
       PUPPETCODE
       it 'is able to listen on an array of ports' do
-        apply_manifest(pp_two, catch_failures: true)
+        retry_on_error_matching do
+          apply_manifest(pp_two, catch_failures: true)
+        end
       end
 
       it 'has stats listening on each port' do
@@ -72,7 +76,9 @@ class { 'haproxy': }
         }
       PUPPETCODE
       it 'starts' do
-        apply_manifest(pp_three, catch_failures: true)
+        retry_on_error_matching do
+          apply_manifest(pp_three, catch_failures: true)
+        end
       end
 
       it 'has stats listening on each port' do
@@ -95,7 +101,9 @@ class { 'haproxy':
         }
     PUPPETCODE
     it 'removes it' do
-      apply_manifest(pp_four, catch_failures: true)
+      retry_on_error_matching do
+        apply_manifest(pp_four, catch_failures: true)
+      end
     end
     describe package('haproxy') do
       it { is_expected.not_to be_installed }
@@ -114,7 +122,9 @@ class { 'haproxy':
         }
     PUPPETCODE
     it 'stops the service' do
-      apply_manifest(pp_five, catch_failures: true)
+      retry_on_error_matching do
+        apply_manifest(pp_five, catch_failures: true)
+      end
     end
     describe service('haproxy') do
       it { is_expected.not_to be_running }

spec/acceptance/defaults_spec.rb

@@ -49,11 +49,12 @@ class { 'haproxy': }
       }
   PUPPETCODE
   it 'is able to configure defaults with puppet' do
-    apply_manifest(pp_one, catch_failures: true)
+    retry_on_error_matching do
+      apply_manifest(pp_one, catch_failures: true)
+    end
   end
 
   it 'does a curl against the LB to make sure it gets a response from each port' do
-    # shell('cat /etc/haproxy/haproxy.cfg').stdout.should match(/^$/)
     shell('curl localhost:5555').stdout.chomp.should match(%r{Response on 555(6|7)})
     shell('curl localhost:5555').stdout.chomp.should match(%r{Response on 555(6|7)})
   end
@@ -115,11 +116,12 @@ class { 'haproxy': }
       }
   PUPPETCODE
   it 'is able to configure defaults and old style with puppet' do
-    apply_manifest(pp_two, catch_failures: true)
+    retry_on_error_matching do
+      apply_manifest(pp_two, catch_failures: true)
+    end
   end
 
   it 'does a curl against the LB to make sure it gets a response from each port #oldstyle' do
-    # shell('cat /etc/haproxy/haproxy.cfg').stdout.should match(/^$/)
     shell('curl localhost:5555').stdout.chomp.should match(%r{Response on 5556})
     shell('curl localhost:6666').stdout.chomp.should match(%r{Response on 5557})
   end

spec/acceptance/frontbackend_spec.rb

@@ -23,15 +23,22 @@ class { 'haproxy': }
       }
   PUPPETCODE
   it 'is able to configure the frontend/backend with puppet' do
-    apply_manifest(pp_one, catch_failures: true)
+    retry_on_error_matching do
+      apply_manifest(pp_one, catch_failures: true)
+    end
   end
 
   # This is not great since it depends on the ordering served by the load
   # balancer. Something with retries would be better.
   # C9945
   it 'does a curl against the LB to make sure it gets a response from each port' do
-    shell('curl localhost:5555').stdout.chomp.should match(%r{Response on 555(6|7)})
-    shell('curl localhost:5555').stdout.chomp.should match(%r{Response on 555(6|7)})
+    response_connection = shell('curl localhost:5555').stdout.chomp
+    response_connection.should match(%r{Response on 555(6|7)})
+    if response_connection == 'Response on 5556'
+      shell('curl localhost:5555').stdout.chomp.should match(%r{Response on 5557})
+    else
+      shell('curl localhost:5555').stdout.chomp.should match(%r{Response on 5556})
+    end
   end
 
   pp_two = <<-PUPPETCODE
@@ -56,7 +63,9 @@ class { 'haproxy': }
       }
   PUPPETCODE
   it 'is able to configure the frontend/backend with one node up' do
-    apply_manifest(pp_two, catch_failures: true)
+    retry_on_error_matching do
+      apply_manifest(pp_two, catch_failures: true)
+    end
   end
 
   # C9951
@@ -74,6 +83,8 @@ class { 'haproxy': }
         }
   PUPPETCODE
   it 'having no address set but setting bind' do
-    apply_manifest(pp_three, catch_failures: true)
+    retry_on_error_matching do
+      apply_manifest(pp_three, catch_failures: true)
+    end
   end
 end

spec/acceptance/listen_spec.rb

@@ -18,7 +18,9 @@ class { 'haproxy': }
       }
   PUPPETCODE
   it 'is able to configure the listen with puppet' do
-    apply_manifest(pp_one, catch_failures: true)
+    retry_on_error_matching do
+      apply_manifest(pp_one, catch_failures: true)
+    end
   end
 
   # This is not great since it depends on the ordering served by the load
@@ -50,8 +52,9 @@ class { 'haproxy': }
       }
   PUPPETCODE
   it 'is able to configure the listen active/passive' do
-    apply_manifest(pp_two, catch_failures: true)
-    apply_manifest(pp_two, catch_changes: true)
+    retry_on_error_matching do
+      idempotent_apply(default, pp_two, {})
+    end
   end
 
   it 'does a curl against the LB to make sure it only gets a response from the active port' do
@@ -80,7 +83,9 @@ class { 'haproxy': }
       }
   PUPPETCODE
   it 'is able to configure the listen with only one node up' do
-    apply_manifest(pp_three, catch_failures: true)
+    retry_on_error_matching do
+      apply_manifest(pp_three, catch_failures: true)
+    end
   end
 
   it 'does a curl against the LB to make sure it gets a response from each port #onenodeup' do
@@ -97,6 +102,8 @@ class { 'haproxy': }
         }
   PUPPETCODE
   it 'having no address set but setting bind' do
-    apply_manifest(pp_four, catch_failures: true)
+    retry_on_error_matching do
+      apply_manifest(pp_four, catch_failures: true)
+    end
   end
 end

spec/acceptance/nodesets/default.yml

@@ -1,10 +1,18 @@
+---
 HOSTS:
-  ubuntu-1404-x64:
+  redhat5-64-1:
+    pe_dir:
+    pe_ver:
+    pe_upgrade_dir:
+    pe_upgrade_ver:
+    platform: el-5-x86_64
+    packaging_platform: el-5-x86_64
+    template: redhat-5-x86_64
+    hypervisor: vmpooler
     roles:
-      - agent
-      - default
-    platform: ubuntu-14.04-amd64
-    hypervisor: vagrant
-    box: puppetlabs/ubuntu-14.04-64-nocm
+    - agent
+    - default
 CONFIG:
-  type: foss
+  nfs_server: none
+  consoleport: 443
+  pooling_api: http://vmpooler.delivery.puppetlabs.net/

spec/acceptance/userlist_spec.rb

@@ -1,9 +1,6 @@
 require 'spec_helper_acceptance'
 
-# lucid ships with haproxy 1.3 which does not have userlist support by default
-describe 'userlist define', unless: (['Darwin', 'Suse', 'windows', 'AIX', 'Solaris'].include?(fact('osfamily')) || (fact('lsbdistcodename') == 'lucid') ||
-                                    (fact('osfamily') == 'RedHat' && fact('operatingsystemmajrelease') == '5')) do
-
+describe 'userlist define', unless: (fact('osfamily') == 'RedHat' && fact('operatingsystemmajrelease') == '5') do
   pp_one = <<-PUPPETCODE
       class { 'haproxy': }
       haproxy::userlist { 'users_groups':
@@ -50,7 +47,9 @@ class { 'haproxy': }
   PUPPETCODE
   it 'is able to configure the listen with puppet' do
     # C9966 C9970
-    apply_manifest(pp_one, catch_failures: true)
+    retry_on_error_matching do
+      apply_manifest(pp_one, catch_failures: true)
+    end
   end
 
   # C9957

spec/classes/haproxy_spec.rb

@@ -393,13 +393,7 @@
         verify_contents(catalogue, '/etc/default/haproxy', ['ENABLED=1'])
       end
     end
-    context 'when only on RedHat family operatingsystems' do
-      let(:facts) do
-        { osfamily: 'RedHat' }.merge default_facts
-      end
 
-      pending('Not yet implemented')
-    end
     context 'when only on Gentoo family operatingsystems' do
       let(:facts) do
         { osfamily: 'Gentoo' }.merge default_facts

spec/defines/frontend_spec.rb

@@ -173,23 +173,6 @@
       )
     }
   end
-  context 'when a comma-separated list of ports is provided' do
-    let(:params) do
-      {
-        name: 'apache',
-        ipaddress: '23.23.23.23',
-        ports: '80,443',
-      }
-    end
-
-    it {
-      is_expected.to contain_concat__fragment('haproxy-apache_frontend_block').with(
-        'order'   => '15-apache-00',
-        'target'  => '/etc/haproxy/haproxy.cfg',
-        'content' => "\nfrontend apache\n  bind 23.23.23.23:80 \n  bind 23.23.23.23:443 \n  option tcplog\n",
-      )
-    }
-  end
 
   context 'when bind parameter is used without ipaddress parameter' do
     let(:params) do

spec/defines/listen_spec.rb

@@ -174,6 +174,30 @@
       expect { catalogue }.to raise_error Puppet::Error, %r{mutually exclusive}
     end
   end
+  context 'when an ipaddress parameter and a bind parameter are passed' do
+    let(:params) do
+      {
+        name: 'apache',
+        ipaddress: '1.1.1.1',
+        bind: { '192.168.0.1:80' => ['ssl'] },
+      }
+    end
+
+    it 'raises error' do
+      expect { catalogue }.to raise_error Puppet::Error, %r{mutually exclusive}
+    end
+  end
+  context 'when no ipaddress or bind parameter are passed' do
+    let(:params) do
+      {
+        name: 'apache',
+      }
+    end
+
+    it 'raises error' do
+      expect { catalogue }.to raise_error Puppet::Error, %r{is needed}
+    end
+  end
   # C9977
   context 'when an invalid hostname is passed' do
     let(:params) do

spec/spec_helper.rb

@@ -26,6 +26,7 @@
 end
 
 RSpec.configure do |c|
+  c.mock_with :rspec
   c.default_facts = default_facts
   c.before :each do
     # set to strictest setting for testing

spec/spec_helper_acceptance.rb

@@ -6,6 +6,49 @@
 run_puppet_install_helper
 configure_type_defaults_on(hosts)
 
+def idempotent_apply(hosts, manifest, opts = {}, &block)
+  block_on hosts, opts do |host|
+    file_path = host.tmpfile('apply_manifest.pp')
+    create_remote_file(host, file_path, manifest + "\n")
+
+    puppet_apply_opts = { :verbose => nil, 'detailed-exitcodes' => nil }
+    on_options = { acceptable_exit_codes: [0, 2] }
+    on host, puppet('apply', file_path, puppet_apply_opts), on_options, &block
+    puppet_apply_opts2 = { :verbose => nil, 'detailed-exitcodes' => nil }
+    on_options2 = { acceptable_exit_codes: [0] }
+    on host, puppet('apply', file_path, puppet_apply_opts2), on_options2, &block
+  end
+end
+
+UNSUPPORTED_PLATFORMS = ['RedHat', 'Suse', 'windows', 'AIX', 'Solaris'].freeze
+MAX_RETRY_COUNT       = 12
+RETRY_WAIT            = 10
+ERROR_MATCHER         = %r{(no valid OpenPGP data found|keyserver timed out|keyserver receive failed)}
+
+# This method allows a block to be passed in and if an exception is raised
+# that matches the 'error_matcher' matcher, the block will wait a set number
+# of seconds before retrying.
+# Params:
+# - max_retry_count - Max number of retries
+# - retry_wait_interval_secs - Number of seconds to wait before retry
+# - error_matcher - Matcher which the exception raised must match to allow retry
+# Example Usage:
+# retry_on_error_matching(3, 5, /OpenGPG Error/) do
+#   apply_manifest(pp, :catch_failures => true)
+# end
+def retry_on_error_matching(max_retry_count = MAX_RETRY_COUNT, retry_wait_interval_secs = RETRY_WAIT, error_matcher = ERROR_MATCHER)
+  try = 0
+  begin
+    puts "retry_on_error_matching: try #{try}" unless try.zero?
+    try += 1
+    yield
+  rescue StandardError => e
+    raise unless try < max_retry_count && (error_matcher.nil? || e.message =~ error_matcher)
+    sleep retry_wait_interval_secs
+    retry
+  end
+end
+
 RSpec.configure do |c|
   # Project root
   proj_root = File.expand_path(File.join(File.dirname(__FILE__), '..'))