(MODULES-3319) Hash argument for create_resources #775

jcpunk · 2016-06-27T16:50:24Z

Without this patch, seperate modules and classes need to be built
to declare the provided defines for the postgresql server.
This patch permits the passing of a hash into the postgresql::server
function for the various resources.

The puppetlabs-mysql module provides similar behavior whereby
databases can be managed as values from Hiera or an ENC. This
commit brings the puppetlabs-postgresql module into feature
parity with the puppetlabs-mysql module.

jcpunk · 2016-06-27T16:51:03Z

Was #765

jcpunk · 2016-06-30T14:25:09Z

Mentioning @bflad as I think this fixes the report.
Mentioning @domcleal since I had to switch PRs

gerhardsam · 2016-07-11T07:49:42Z

@jcpunk, it seems that the vendor modules are lint-checked and some fail. Could you review this and re-trigger the build? I'm highly interested in seeing your pull request being merged.

jcpunk · 2016-07-11T15:01:39Z

hmmm.... I'm a bit concerned to merge puppet-lint fixes in with this patch. I'll see about getting another PR opened up to fix the linting issues.

gerhardsam · 2016-07-12T07:31:06Z

@jcpunk, ah sorry, I should have expressed that more precisely. What I meant is, that in your pull request the vendor-modules are lint-checked.
This has been fixed in the meantime by @tphoney in 0794b2c.

In your pull request this fix has not been merged yet. I think after merging/rebasing against the master your PR should be fine.

jcpunk · 2016-07-12T14:21:55Z

Ahh, got it. I've rebased off of master and pushed again.

tphoney · 2016-07-13T11:17:14Z

I re-kicked travis, the lint issues are gone. Now there are just test failures.

rspec ./spec/unit/classes/client_spec.rb:32 # postgresql::client with parameters should modify package
rspec ./spec/unit/classes/client_spec.rb:40 # postgresql::client with parameters should have specified validate connexion
rspec ./spec/unit/classes/server_spec.rb:193 # postgresql::server manage extension entries should contain Postgresql_psql[Add postgis extension to template_postgis] that requires Postgresql::Server::Database[template_postgis]

jcpunk · 2016-07-13T15:48:33Z

hmmmm.... I have no idea why the client_spec tests are failing....

Any suggestions?

domcleal · 2016-07-14T07:04:39Z

spec/unit/classes/client_spec.rb

+        :package_name         => 'mypackage',
+        :file_ensure          => 'file',
+        :validate_connections => { :test => {
+                                              :database_host => 'test',


Try changing symbols to strings throughout this validate_connections hash, like you've got in server_spec.rb. I don't think rspec-puppet can convert hash keys properly in a manifest.

gerhardsam · 2016-07-14T07:48:40Z

@jcpunk, your fix got in the wrong direction. The keys in the hash should be quoted.
I fixed it like this.
Now the rspec-tests are running through.

jcpunk · 2016-07-14T21:45:51Z

Gave it a shot and pushed it up.

gerhardsam · 2016-07-15T07:47:42Z

The rspec-tests are running fine now but there seems something wrong with the connection to the docker-containers in the beaker-tests.
Eventually you could re-trigger the build by squashing the previous commit with your first in the PR.

Without this patch, seperate modules and classes need to be built to declare the provided defines for the postgresql server. This patch permits the passing of a hash into the postgresql::server function for the various resources. The puppetlabs-mysql module provides similar behavior whereby databases can be managed as values from Hiera or an ENC. This commit brings the puppetlabs-postgresql module into feature parity with the puppetlabs-mysql module.

gerhardsam · 2016-07-21T10:56:27Z

@jcpunk Could you please rebase on current master and see if the beaker-tests run through now?

Conflicts: manifests/server/schema.pp

jcpunk · 2016-07-21T14:44:41Z

Attempted rebase

gerhardsam · 2016-07-22T08:14:28Z

@jcpunk Looking good now!
@hunner Could you please accept this PR and merge it also in the 4.8.0 release-branch?

gerhardsam · 2016-07-25T11:11:12Z

👍

gerhardsam · 2016-07-28T11:37:44Z

@bmjen Would you mind merging this PR?

tphoney · 2016-07-28T18:12:31Z

Hi @jcpunk @gerhardsam we spent time discussing this PR in the community meeting. This is not a simple merge. Our notes are here, https://github.com/voxpupuli/community-triage/blob/master/modules/notes/2016-07-28.md#discussion it would be great if you could come to the community meeting next week to discuss it further. It is on Thursdays -17:00 UTC, @ http://bluejeans.com/280736660

jcpunk · 2016-07-28T19:35:14Z

Unfortunately I've got a weekly meeting that conflicts. Is there a pointer to some doc that could bring me up to speed on "adoption of proper profile-level abstraction practices"?

antaflos · 2016-07-28T23:03:23Z

FWIW, issues like this have been discussed in the past. Here are a few PRs I remember because I participated in the discussion: #465, #467, puppetlabs/puppetlabs-apache#794, puppetlabs/puppetlabs-haproxy#172

In the cases above others and I argued that the way to go was not including Hiera support in the module but to encourage abstraction, like profile or wrapper modules. AFAIK there has never been an official statement or stance on this topic from Puppetlabs, so maybe this is an opportunity.

An example of Hiera support "going too far" is, IMHO, presented in voxpupuli/puppet-php#160. In that module/class it is impossible to set various parameters without using Hiera, and trying to refactor the module in that regard has been a nightmare on which I quickly had to give up. The coupling between Puppet code and Hiera is just way too tight.

tphoney · 2016-07-29T08:58:08Z

There is no documentation on this as yet. I have created https://tickets.puppetlabs.com/browse/MODULES-3661 for investigation work. That is not to say that is something we are starting immediately.

gerhardsam · 2016-08-02T13:52:30Z

@tphoney Thanks for the invitation. I'm not sure, whether I'm able to attend the meeting. I try to, but i cannot guarantee it.

gerhardsam · 2016-08-04T14:37:01Z

@tphoney Unfortunatly I'm not able to attend the meeting. Please Informationen me about the results of your discussion.

hunner · 2017-03-02T17:42:09Z

manifests/server.pp

+  $schemas         = {},
+  $table_grants    = {},
+  $tablespaces     = {},
+


Instead of adding a lot of extra hash parameters that are implicitly for hiera or ENC use, what about making this a bit more explicit? eg:

class postgresql::profile::hiera ( $config_entries = {}, $databases = {}, $database_grants = {}, $dbs = {}, $extensions = {}, $pg_hba_rules = {}, $pg_ident_rules = {}, $recovery = {}, $roles = {}, $schemas = {}, $table_grants = {}, $tablespaces = {}, ) { create_resources('postgresql::server::config_entry', $config_entries) create_resources('postgresql::server::database', $databases) create_resources('postgresql::server::database_grant', $database_grants) create_resources('postgresql::server::db', $dbs) create_resources('postgresql::server::extension', $extensions) create_resources('postgresql::server::pg_hba_rule', $pg_hba_rules) create_resources('postgresql::server::pg_ident_rule', $pg_ident_rules) create_resources('postgresql::server::recovery', $recovery) create_resources('postgresql::server::role', $roles) create_resources('postgresql::server::schema', $schemas) create_resources('postgresql::server::table_grant', $table_grants) create_resources('postgresql::server::tablespace', $tablespaces) }

The related discussion to come up with this pattern is in https://github.com/voxpupuli/community-triage/blob/master/modules/notes/2017-03-02.md . tl;dr is that "this isn't a great pattern to be stuck on, but it seems like most people still have to move through it before they arrive at profile abstraction patterns so how can we facilitate that path of learning?"

eputnam · 2017-12-07T17:14:47Z

Closing this due to inactivity. If this is still something you'd like to see merged, please feel free to re-open.

domcleal reviewed Jul 14, 2016
View reviewed changes

tphoney and others added 3 commits July 15, 2016 09:23

{maint} modulesync 0794b2c

03b36db

(MODULES-3581) modulesync [067d08a]

e6c0d8d

Merge remote-tracking branch 'upstream/master' into create-resources

a6cc071

Conflicts: manifests/server/schema.pp

hunner reviewed Mar 2, 2017

View reviewed changes

willmeek added the needs-rebase label Oct 31, 2017

eputnam closed this Dec 7, 2017

Ramesh7 added the maintenance label Jun 12, 2023

(MODULES-3319) Hash argument for create_resources #775

(MODULES-3319) Hash argument for create_resources #775

Uh oh!

Conversation

jcpunk commented Jun 27, 2016

Uh oh!

jcpunk commented Jun 27, 2016

Uh oh!

jcpunk commented Jun 30, 2016

Uh oh!

gerhardsam commented Jul 11, 2016

Uh oh!

jcpunk commented Jul 11, 2016

Uh oh!

gerhardsam commented Jul 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcpunk commented Jul 12, 2016

Uh oh!

tphoney commented Jul 13, 2016

Uh oh!

jcpunk commented Jul 13, 2016

Uh oh!

domcleal Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

gerhardsam commented Jul 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcpunk commented Jul 14, 2016

Uh oh!

gerhardsam commented Jul 15, 2016

Uh oh!

gerhardsam commented Jul 21, 2016

Uh oh!

jcpunk commented Jul 21, 2016

Uh oh!

gerhardsam commented Jul 22, 2016

Uh oh!

gerhardsam commented Jul 25, 2016

Uh oh!

gerhardsam commented Jul 28, 2016

Uh oh!

tphoney commented Jul 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcpunk commented Jul 28, 2016

Uh oh!

antaflos commented Jul 28, 2016

Uh oh!

tphoney commented Jul 29, 2016

Uh oh!

gerhardsam commented Aug 2, 2016

Uh oh!

gerhardsam commented Aug 4, 2016

Uh oh!

hunner Mar 2, 2017

Choose a reason for hiding this comment

Uh oh!

hunner Mar 2, 2017

Choose a reason for hiding this comment

Uh oh!

eputnam commented Dec 7, 2017

Uh oh!

Uh oh!

gerhardsam commented Jul 12, 2016 •

edited

Loading

gerhardsam commented Jul 14, 2016 •

edited

Loading

tphoney commented Jul 28, 2016 •

edited

Loading