Skip to content

Add puppetdb manage dbserver#125

Closed
chrisowensboston wants to merge 20 commits intoabstractitptyltd:productionfrom
chrisowensboston:add_puppetdb_manage_dbserver
Closed

Add puppetdb manage dbserver#125
chrisowensboston wants to merge 20 commits intoabstractitptyltd:productionfrom
chrisowensboston:add_puppetdb_manage_dbserver

Conversation

@chrisowensboston
Copy link
Copy Markdown
Contributor

Added manage_dbserver param to profile/puppetdb, to make it consistent with profile/master.

@rendhalver
Copy link
Copy Markdown
Member

Can you also remove the Gemfile.lock changes from this please.

@chrisowensboston
Copy link
Copy Markdown
Contributor Author

I probably can’t get to that until tomorrow. Stand by.

From: Pete Brown [mailto:notifications@github.com]
Sent: 11/02/2016 4:47 PM
To: abstractitptyltd/abstractit-puppet abstractit-puppet@noreply.github.com
Cc: chrisowensboston ctag@chris-owens.com; Author author@noreply.github.com
Subject: Re: [abstractitptyltd/abstractit-puppet] Add puppetdb manage dbserver (#125)

Can you also remove the Gemfile.lock changes from this please.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #125 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/AQKb58iSqL3EhrbP9mlB0wMeOV_Ar1aUks5q6PbIgaJpZM4Knuin . https://github.com/notifications/beacon/AQKb50_a7-UNbQzJeQ-qzVSDzY51GVuPks5q6PbIgaJpZM4Knuin.gif

:disable_ssl => false,
:listen_address => '127.0.0.1',
:ssl_listen_address => '0.0.0.0',
:manage_dbserver => true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you also add a test for when this is not set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adding that test surfaced a bug in the puppetlabs/postgresql module: https://tickets.puppetlabs.com/browse/MODULES-4054.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ouch.
Maybe we need to looks at later versions of the module.
I will have a look later and se what we can do about this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given discussion in main thread lets leave this for now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Where is discussion?

Comment thread CHANGELOG.md
in Gemfile, forced json_pure to be < 2.0.1 when ruby < 2.0 @chrisowensboston
in .travis.yml, added a line for rvm 2.2.0, puppet gem 4.7.0, facter gem 2.4.6, hiera gem 3.2.1 @chrisowensboston


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can remove your changes to this file too.

@chrisowensboston
Copy link
Copy Markdown
Contributor Author

The problem is probably theoretical… it occurs in testing, because declaring puppet::profile::puppetdb with manage_dbserver false, invokes the puppetdb class with manage_dbserver false, which declares an instance of postgresql::server::db but (because it’s not trying to manage the server) does not declare postgresql::server.

In actual deployments, the the likely reason for setting manage_dbserver to false would be because you’re managing the Postgresql server out of some other catalog entry, i.e. postgresql::server is going to be declared on that host somewhere, somehow. The exception would be a host that has a postgresql server on it that is not under the control of puppet.

I’m not sure what the right answer would be. I don’t love the whole postgresql::globals pattern, but perhaps the right thing would be for postgresql::server::db and friends to include postgresql::globals and get things like character encoding from that class?

From: Pete Brown [mailto:notifications@github.com]
Sent: 11/04/2016 7:48 AM
To: abstractitptyltd/abstractit-puppet abstractit-puppet@noreply.github.com
Cc: chrisowensboston ctag@chris-owens.com; Author author@noreply.github.com
Subject: Re: [abstractitptyltd/abstractit-puppet] Add puppetdb manage dbserver (#125)

@rendhalver commented on this pull request.


In spec/classes/puppet_profile_puppetdb_spec.rb #125 :

@@ -67,6 +67,7 @@
:disable_ssl => false,
:listen_address => '127.0.0.1',
:ssl_listen_address => '0.0.0.0',

  •        :manage_dbserver    => true,
    

Ouch.
Maybe we need to looks at later versions of the module.
I will have a look later and se what we can do about this.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #125 , or mute the thread https://github.com/notifications/unsubscribe-auth/AQKb5y2npg0tBfC87z-ya-qAgYhR8vo0ks5q6xtbgaJpZM4Knuin . https://github.com/notifications/beacon/AQKb53KC9W7c_68Ufkg90FYMhScON6S0ks5q6xtbgaJpZM4Knuin.gif

@rendhalver
Copy link
Copy Markdown
Member

Yeah that makes sense.
Annoying but it explains what the issue is.
I think I did run into something similar when I was trying to spec test puppetdb in this module.
Maybe we should just leave it off the tests and trust that puppet is testing things properly.

@chrisowensboston
Copy link
Copy Markdown
Contributor Author

I can't make sense of the pull request chronology... are you still waiting for changes from me?

rendhalver and others added 4 commits March 25, 2017 19:15
* Attempt to fix Travis builds

* Adjust travis version matrix
* Drop log format config for puppetsyntax

* Set max version for puppetserevr_gem

* Lock puppet-syntax to 2.1.0

* Lock puppet-lint to 2.0.2

* lock concat to 1.2.2

* Code cleanup

* cleanup versions
@rendhalver
Copy link
Copy Markdown
Member

@chrisowensboston Please rebase and remove your changes to the CHANGELOG.md

@chrisowensboston
Copy link
Copy Markdown
Contributor Author

chrisowensboston commented Apr 5, 2017 via email

@chrisowensboston
Copy link
Copy Markdown
Contributor Author

chrisowensboston commented Apr 7, 2017 via email

@rendhalver
Copy link
Copy Markdown
Member

Interesting.
That's going to break a few setups.
I have been seeing some interesting things on some recent servers that could be explained by that.
Let me do some investigation and get back to you.

@rendhalver
Copy link
Copy Markdown
Member

Yep it looks like that is what's happening.
I migrated a few of my servers to a new puppet server recently.
The old server listens on puppet.$::domain. The new servers uses a different address.
The servers I migrated are contacting the old puppet server and getting auth denied.

@rendhalver
Copy link
Copy Markdown
Member

@chrisowensboston If you are happy with this I am going to merge it.

@rendhalver
Copy link
Copy Markdown
Member

Actually can you please rebase @chrisowensboston ?

@chrisowensboston
Copy link
Copy Markdown
Contributor Author

chrisowensboston commented Apr 8, 2017 via email

@calmenergy
Copy link
Copy Markdown
Contributor

It's possible that this branch is now munted due to me having made a mess of the rebase.

@rendhalver
Copy link
Copy Markdown
Member

Yes it looks like it.
It's best to just setup an upstream remote and sync up your local production branch with that and then do a manual rebase.
I don't use the sync buttons in GitHub at all.
This will show you how to sync a fork: https://help.github.com/articles/syncing-a-fork/

@calmenergy
Copy link
Copy Markdown
Contributor

calmenergy commented Apr 10, 2017 via email

@rendhalver
Copy link
Copy Markdown
Member

git cherry-pick is a good way to copy commits to a new branch.

@calmenergy
Copy link
Copy Markdown
Contributor

See PR #144

@rendhalver rendhalver closed this Apr 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants