(PDB-3587) Add puppetlabs-postgresql 5.x support and integrate rspec-puppetfacts #260

dhollinger · 2017-06-26T22:22:32Z

Casts $database_port in postgresql.pp to an integer to make module compatible with puppetlabs-postgresql 5.x

Fix an issue where puppetlabs-puppetdb would pass true to managed_pg_repo by default for all supported OSes, however puppetlabs-postgresql can only manage the pg repo if the osfamily is RedHat or Debian.

Added rspec-puppet-facts to the unit tests and updated the beaker tests with beaker-puppet_install_helper and beaker-module_install_helper. These simplify test setup (it also happened to be the easiest way to get tests working consistently).

Converted $database_port into an integer for compatibility with puppetlabs-postgresql 5.x

The manage_pg_repo defaulted to 'true' in all cases. This proved to be a problem on supported OSes that weren't of the Redhat or Debian families as puppetlabs-postgresql only supports managing Debian or Redhat Postgresql repositories. The parameter now defaults to true only when $osfamily is Redhat or Debian

bastelfreak · 2017-06-26T22:25:11Z

manifests/database/postgresql.pp

@@ -20,7 +20,7 @@
    class { '::postgresql::server':
      ip_mask_allow_all_users => '0.0.0.0/0',
      listen_addresses        => $listen_addresses,
-      port                    => $database_port,
+      port                    => 0 + $database_port,


Is this the cheap way of casting $database_port to an integer?

Yes, I was trying to avoid breaking any functionality with this PR.

There is the scanf function as well, but required a minimum of Puppet 3.7.5, so that would also be a breaking change.

We're going to roll a major release soon and bring the minimum required version to Puppet 4, so scanf should be ok here.

@mullr Sounds good, I'll update that line of code

bastelfreak · 2017-06-26T22:25:45Z

spec/spec_helper.rb

+  default_facts = {
+      puppetversion: Puppet.version,
+      facterversion: Facter.version
+  }


Vox Pupuli configs \o/

bastelfreak · 2017-06-26T22:27:25Z

spec/unit/classes/init_spec.rb

+  on_supported_os.each do |os, facts|
+    context "on #{os}" do
+      let(:facts) do
+        facts.merge({


do you want to move the selinux and iptables fact to default_module_facts.yml?

The selinux fact changes in different tests, but the iptables fact never changes.

bastelfreak · 2017-06-26T22:29:21Z

spec/unit/classes/master/config_spec.rb

+      let(:facts) do
+        facts.merge({
+            :puppetversion => Puppet.version,
+            :fqdn => 'puppetdb.example.com',


instead of mocking the fqdn fact you can set the node: https://github.com/voxpupuli/puppet-zabbix/blob/master/spec/classes/agent_spec.rb#L4-L6

Apparently mocking node for some reason doesn't carry over to fqdn in these tests

bastelfreak · 2017-06-26T22:29:54Z

spec/unit/classes/master/config_spec.rb

+          it { should contain_puppetdb_conn_validator('puppetdb_conn').with(
+              :puppetdb_server => 'puppetdb.example.com',
+              :puppetdb_port => '8081',
+              :use_ssl => 'true') }


can you pull the } into the next line?

Updated the spec_helper_acceptance.rb file to utilize a few more beaker helper gems to simplify agent and dependency installs. Updated the puppetserver and puppetlabs release install logic to use Puppet Collections and supported packages.

Updated base_spec.rb to utilize the newer, supported directory paths and corrected the ntp panic attribute which is no longer a boolean value.

ekohl · 2017-06-27T15:19:50Z

manifests/params.pp

@@ -17,7 +17,12 @@
  $puppetdb_version          = $puppetdb::globals::version
  $database                  = $puppetdb::globals::database
  $manage_dbserver           = true
-  $manage_pg_repo            = true
+
+  if $::osfamily =~ /RedHat|Debian/ {


Isn't this equal to $manage_pg_repo = $::osfamily =~ /RedHat|Debian/?

@ekohl You're right, that does to the same thing.

Unless @mullr requests it for this PR, though, I'm going to keep this PR as is. We (or Puppet) could always submit that change as a part of a PR to bring the module in-line with Puppet 4.

can you use the new facts hash here? $facts['os']['family']

@bastelfreak I'll leave that for a Puppet 4 or Facts Hash PR. Wanted to keep the scope of this PR as close as possible to just making the module compatible with puppetlabs-postgresql 5.x for now.

ekohl · 2017-06-28T09:56:51Z

metadata.json

@@ -87,6 +87,10 @@
        {
            "name": "puppetlabs/stdlib",
            "version_requirement": ">= 4.2.2 <5.0.0"
+        },
+        {
+            "name": "puppetlabs/ntp",


This is only needed for acceptance testing. It feels wrong to add this as a regular dependency and should be fixed in the spec_helper_acceptance.

Let's discuss this in #263

mullr · 2017-06-29T20:59:35Z

@dhollinger THANK YOU THANK YOU THANK YOU for this PR.

mmoll · 2017-06-29T22:42:23Z

to the forge, pretty plz?

mullr · 2017-06-30T20:27:54Z

mullr · 2017-06-30T20:29:31Z

@dhollinger I'd like to do in next wednesday, after US holidays.

dhollinger · 2017-06-30T23:18:01Z

@mullr sounds good to me!

Sent from my Google Pixel using FastHub

David Hollinger added 7 commits June 26, 2017 15:29

Add Jetbrains project folder to gitignore

f6f9d95

Add rspec-puppet-facts gem

640185e

Enable rspec-puppet-facts in the spec_helper.rb file

720999c

Convert $database_port into an integer

be976df

Converted $database_port into an integer for compatibility with puppetlabs-postgresql 5.x

Add rspec-puppet-facts integration

7e20c5f

Added postgresql 5.x support to the module

7ad8f78

bastelfreak reviewed Jun 26, 2017

View reviewed changes

dhollinger added 9 commits June 26, 2017 22:02

Updated travis config to be inline with other puppetlabs modules

b753ae7

Migrated some common facts to a default facts file

99d573d

Remove redundant fact declarations

7b4e7a0

Update gitignore with additional beaker files

9dcbade

Add additional Beaker gems

dbc47e4

Add puppetlabs-ntp as a dependency

aac7181

Add more current nodesets using docker

8b7dc23

Update spec_helper_acceptance

21ddc2f

Updated the spec_helper_acceptance.rb file to utilize a few more beaker helper gems to simplify agent and dependency installs. Updated the puppetserver and puppetlabs release install logic to use Puppet Collections and supported packages.

Update acceptance tests to use current configs

cce7d2c

Updated base_spec.rb to utilize the newer, supported directory paths and corrected the ntp panic attribute which is no longer a boolean value.

dhollinger force-pushed the PDB-3587 branch from a9db903 to cce7d2c Compare June 27, 2017 04:53

David Hollinger added 2 commits June 27, 2017 09:36

Fix typo that was causing centos beaker tests to fail

bfec67b

Use scanf() to cast to an integer

74bb289

ekohl reviewed Jun 27, 2017

View reviewed changes

ekohl reviewed Jun 28, 2017

View reviewed changes

ekohl mentioned this pull request Jun 28, 2017

Pin postgres and concat theforeman/foreman-installer#232

Merged

mullr merged commit 8a7f600 into puppetlabs:master Jun 29, 2017

smortex added the enhancement label Apr 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(PDB-3587) Add puppetlabs-postgresql 5.x support and integrate rspec-puppetfacts #260

(PDB-3587) Add puppetlabs-postgresql 5.x support and integrate rspec-puppetfacts #260

dhollinger commented Jun 26, 2017 •

edited

bastelfreak Jun 26, 2017

dhollinger Jun 26, 2017 •

edited

mullr Jun 27, 2017

dhollinger Jun 27, 2017

bastelfreak Jun 26, 2017

bastelfreak Jun 26, 2017

dhollinger Jun 27, 2017

bastelfreak Jun 26, 2017

dhollinger Jun 27, 2017

bastelfreak Jun 26, 2017

ekohl Jun 27, 2017

dhollinger Jun 27, 2017

bastelfreak Jun 27, 2017

dhollinger Jun 27, 2017

ekohl Jun 28, 2017

ekohl Jun 30, 2017

mullr commented Jun 29, 2017

mmoll commented Jun 29, 2017

mullr commented Jun 30, 2017 •

edited

mullr commented Jun 30, 2017

dhollinger commented Jun 30, 2017

(PDB-3587) Add puppetlabs-postgresql 5.x support and integrate rspec-puppetfacts #260

(PDB-3587) Add puppetlabs-postgresql 5.x support and integrate rspec-puppetfacts #260

Conversation

dhollinger commented Jun 26, 2017 • edited

Choose a reason for hiding this comment

dhollinger Jun 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mullr commented Jun 29, 2017

mmoll commented Jun 29, 2017

mullr commented Jun 30, 2017 • edited

mullr commented Jun 30, 2017

dhollinger commented Jun 30, 2017

dhollinger commented Jun 26, 2017 •

edited

dhollinger Jun 26, 2017 •

edited

mullr commented Jun 30, 2017 •

edited