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.

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

Added multiple postgres instances and fixed pgbouncer #372

Merged
merged 2 commits into from Dec 12, 2017

Conversation

rothgar
Copy link
Contributor

@rothgar rothgar commented Nov 30, 2017

This fixes a problem with pgbouncer class not including the datadog_agent (error if not otherwise included)

It also allows multiple postgres instances on the same node. It's based on the mysql template and manifest but now can be applied to postgres.

@rothgar
Copy link
Contributor Author

rothgar commented Nov 30, 2017

Just for reference/testing, here's an example to declare multiple postgres instances in hiera for a node

datadog_agent::integrations::postgres::instances:
  - host: foo
    port: 5432
    username: postgres
    password: foo_pass
    dbname:
    ssl: False
    tags:
      - Foo DB
      - postgresql
  - host: bar
    port: 5400
    username: postgres
    password: bar_pass
    ssl: False
    tags:
      - Bar DB
      - postgresql

Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

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

I think this is mostly great. Thank you so much @rothgar 🙇

It would be awesome if we could update the spec tests to cover the case where we pass an array of instances.

There's also a small stylistic issue on postgres.pp:96

$_instances = $instances
}

file { "$dst":
Copy link
Member

Choose a reason for hiding this comment

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

This line has a minor stylistic issue:

manifests/integrations/postgres.pp:96:only_variable_string:WARNING:string containing only a variable
manifests/integrations/postgres.pp:96:variables_not_enclosed:WARNING:variable not enclosed in {}

The line was originally:

  file { $dst:

which I think is fine, but if you have your reasons for changing that you might have to address it some other way.

@truthbk truthbk added this to the 1.13.0 milestone Dec 1, 2017
@rothgar
Copy link
Contributor Author

rothgar commented Dec 1, 2017

I reverted the file definition. Do you know of an example for testing passing an array of instances? I looked in the mysql tests but didn't see a test for an array so I'm not sure how to do it.

@truthbk
Copy link
Member

truthbk commented Dec 12, 2017

@rothgar that's OK, I'm working on puppet right now, so I'll add the tests to the suite myself - there are tests for postgres already and as they're passing the output YAML is sane. Thank you very much for your work.

@truthbk truthbk modified the milestones: 1.13.0, 1.12.0 Dec 12, 2017
@truthbk truthbk merged commit e02286d into DataDog:master Dec 12, 2017
cegeka-jenkins pushed a commit to cegeka/puppet-datadog_agent that referenced this pull request Jan 31, 2018
* Added postgress instances and fixed pgbouncer

* Revert file name variable quoting
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.

None yet

2 participants