Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ubuntu 12.04 bug fixes. Python drivers class #2

Open
wants to merge 1 commit into
base: feature/master/align-with-puppetlabs-mysql
Choose a base branch
from
Open

Ubuntu 12.04 bug fixes. Python drivers class #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions manifests/config/beforeservice.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,21 @@
notify => Service['postgresqld'],
}

file { 'postgresql.conf':
ensure => file,
path => $postgresql_conf_path,
content => template("postgresql/postgresql.conf.erb"),
notify => Service['postgresqld'],
}

# We must set a "listen_addresses" line in the postgresql.conf if we
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a reason that this file_line approach was not working for you? I was trying to avoid having to manage the entire file, because I'm concerned about compatibility between postgres 8.x and 9.x.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting an matching error and it was making the puppet run fail. I think I may have an old version of stdlib? Perhaps we could put separate templates for each version instead? That would also give more configuration options down the road.

Actually, I think the matcher didn't find anything because that line is preceded with a # in 9.1. Maybe I should revert and just try to fix the regex.

Found the problem. Issue with stdlibs. I've upgraded it and I am re-resting. I'll update the pull request later.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do templates for postgres.conf, then I like the idea of having a separate template for postgres 8 vs postgres 9.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Chris,

Are you still working on this module actively? I'm at a different company (MaestroDev) and I'd like to help you wrap this up if you're not done yet. We are using postgresql 9.0 on CentOS.

Etienne

On Jun 15, 2012, at 5:39 PM, Chris Price [email protected] wrote:

We must set a "listen_addresses" line in the postgresql.conf if we

want to allow any connections from remote hosts.

  • file_line { 'postgresql.conf':
  • path => $postgresql_conf_path,
  • match => '^listen_addresses\s_=._$',
  • line => "listen_addresses = '${listen_addresses}'",
  • notify => Service['postgresqld'],
  • }

If we do templates for postgres.conf, then I like the idea of having a separate template for postgres 8 vs postgres 9.

Reply to this email directly or view it on GitHub:
https://github.com/cprice-puppet/puppet-postgresql/pull/2/files#r996699

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we're just about to do a release to the forge. All of my work so far has been merged back into the original repo that I forked from, though... inkling/puppet-postgres. Feel free to email me to discuss further, we would love additional submissions!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My email is just chris@ puppetlabs if you don't have it already.

# want to allow any connections from remote hosts.
file_line { 'postgresql.conf':
path => $postgresql_conf_path,
match => '^listen_addresses\s*=.*$',
line => "listen_addresses = '${listen_addresses}'",
notify => Service['postgresqld'],
}
# file_line { 'postgresql.conf':
# path => $postgresql_conf_path,
# match => '^listen_addresses\s*=.*$',
# line => "listen_addresses = '${listen_addresses}'",
# notify => Service['postgresqld'],
# }

# TODO: is this a reasonable place for this firewall stuff?
# TODO: figure out a way to make this not platform-specific; debian and ubuntu have
Expand Down
13 changes: 2 additions & 11 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,10 @@
}

'Debian': {
case $::operatingsystem {
'Debian': {
$service_name = "postgresql"
}

'Ubuntu': {
$service_name = "postgresql-${::postgres_default_version}"
}
}

$service_name = "postgresql"
$client_package_name = 'postgresql-client'
$server_package_name = 'postgresql'
$needs_initdb = false
$needs_initdb = true
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on Ubuntu 10.04 it seemed like the initdb had already been done as part of the package installation. Is that no longer the case for 12.04?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is no longer the case. You have to run the initdb specifically. I think it might have been to better align it with the RHEL/CentOS distribution. Won't work with it disabled.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, good to know. Perhaps we can use "operatingsystemrelease" here as well.

$initdb_path = "/usr/lib/postgresql/${::postgres_default_version}/bin/initdb"
$createdb_path = "/usr/lib/postgresql/${::postgres_default_version}/bin/createdb"
$psql_path = "/usr/lib/postgresql/${::postgres_default_version}/bin/psql"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On ubuntu 10.04 the service name is, e.g., postgresql-8.4 rather than just postgresql. I presume that on ubuntu 12.04 they've changed it to just be 'postgresql'? It would be nice if we could figure out a way to make the module compatible with both versions...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to query the release number or flavor (oneiric, precise, etc)? We could use that.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are these facter facts:

operatingsystem => CentOS
operatingsystemrelease => 6.0
osfamily => RedHat

Expand Down
23 changes: 23 additions & 0 deletions manifests/python.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# vim: tabstop=2 shiftwidth=2 softtabstop=2
# This class installs postgresql server and configures it using the conf template
class postgresql::python {

#Default client installed by postgresql module
include postgresql

# Add what's missing
$pg_packages=['python-psycopg2', 'libpq-dev', 'python-egenix-mx-base-dev']

package { $pg_packages:
ensure => installed,
}
if ! defined(Package['python-dev']) {
package { 'python-dev':
ensure => installed,
}
}

# package { 'mcloud-psycopg':
# ensure => installed,
# }
}
2 changes: 1 addition & 1 deletion templates/pg_hba.conf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ local all postgres ident
# "local" is for Unix domain socket connections only
local all all ident
# IPv4 local connections:
host all postgres <%= @ip_mask_deny_postgres_user + "\t" %> reject
# host all postgres <%= @ip_mask_deny_postgres_user + "\t" %> reject
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this causing you problems?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this line needs to be optional. We didn't want or need it in our configuration.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that the default ip_mask that I had put in there would never match any real ip address ( 0.0.0.0/32 ), which should make that line effectively a no-op. That was my goal anyway...

host all all <%= @ip_mask_allow_all_users + "\t" %> md5
# IPv6 local connections:
host all all ::1/128 md5
Expand Down
Loading