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

New module postgresql_ping: Checks remote PostgreSQL server availability. #51477

Merged
merged 10 commits into from
Mar 6, 2019

Conversation

Andersson007
Copy link
Contributor

@Andersson007 Andersson007 commented Jan 30, 2019

SUMMARY

New module postgresql_ping: Simple module to check remote PostgreSQL server availability.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

postgresql_ping

ANSIBLE VERSION
2.8
RETURNS
is_available:
    description: PostgreSQL server availability.
    returned: always
    type: bool
    sample: true

server_version:
    description: PostgreSQL server version.
    returned: always
    type: dict
    sample: { major: 10, minor: 1 }
EXAMPLES
# PostgreSQL ping dbsrv server from the shell:
ansible dbsrv -m postgresql_ping
# In the example below you need to generate sertificates previously.
# See https://www.postgresql.org/docs/current/libpq-ssl.html for more information.
- name: PostgreSQL ping dbsrv server using not default credentials and ssl
  postgresql_ping:
    db: protected_db
    login_host: dbsrv
    login_user: secret
    login_password: secret_pass
    ssl_rootcert: /root/root.crt
    ssl_mode: verify-full
OUTPUT EXAMPLES

In following examples Postgres on dbsrv1 is OK, on dbsrv2 it is not running:

dbsrv1 | SUCCESS => {
    "changed": false,
    "is_available": true,
    "server_version": {
        "major": 10, 
        "minor": 6
    }
}
 [WARNING]: PostgreSQL server is unavailable: could not connect to server: No such file or directory         Is the server running locally and accepting         connections
on Unix domain socket "/var/run/postgresql/.s.PGSQL.5432"?

dbsrv2 | SUCCESS => {
    "changed": false,
    "is_available": false,
    "server_version": {}
}

@ansibot
Copy link
Contributor

ansibot commented Jan 30, 2019

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_module This PR includes a new module. new_plugin This PR includes a new plugin. postgresql PostgreSQL community support:community This issue/PR relates to code supported by the Ansible community. labels Jan 30, 2019
@ansibot
Copy link
Contributor

ansibot commented Jan 30, 2019

@Dorn- @andytom @b6d @dschep @jensdepuydt @matburt @nerzhul

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

@ansibot

This comment has been minimized.

@ansibot ansibot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html test This PR relates to tests. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Feb 4, 2019
@Andersson007
Copy link
Contributor Author

Andersson007 commented Feb 4, 2019

@dagwieers , hello. I added my first integration test for this module. It's a very simple module so the test very simple too.
This module don't change anything on the target, therefore I made three check: return values, return values if the database to connect doesn't exist and check mode.
All checks were passed as expected. Could you look at it once, please? If it's OK - good and I'm going to continue to make tests for other modules by myself.
Test output example:

TASK [postgresql : postgresql_ping - test return values] ***********************
ok: [testhost] => {"changed": false, "is_available": true, "server_version": {"major": 10, "minor": 6}}

TASK [postgresql : assert] *****************************************************
ok: [testhost] => {
    "changed": false, 
    "msg": "All assertions passed"
}

TASK [postgresql : postgresql_ping - check ping of non-existing database doesn't return anything] ***
ok: [testhost] => {"changed": false, "is_available": false, "server_version": {}}

TASK [postgresql : assert] *****************************************************
ok: [testhost] => {
    "changed": false, 
    "msg": "All assertions passed"
}

@ansibot
Copy link
Contributor

ansibot commented Feb 4, 2019

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 4, 2019
Copy link
Contributor

@dagwieers dagwieers left a comment

Choose a reason for hiding this comment

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

I made some cosmetic and docs changes to speed up this process.
LGTM, but I haven't tested the code myself.

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Feb 12, 2019
@dagwieers dagwieers added the database Database category label Feb 13, 2019
@ansibot
Copy link
Contributor

ansibot commented Feb 13, 2019

@Andersson007
Copy link
Contributor Author

cc @sebasmannem @amenonsen

Copy link
Contributor

@sebasmannem sebasmannem left a comment

Choose a reason for hiding this comment

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

Hmm, I see some pylint recommendations on variables like e and kw.
I know these examples are out there in other ansible modules, but I would fix them.
Makes it just a little bit cleaner. Also some docstring recommendations.
But I'm okay with shipping it in its current form also.

Generall, I like this module. It will be beneficcial into some crude checking if postgres is up, and init'ed as expected.

@Andersson007
Copy link
Contributor Author

@sebasmannem thank you for the review!

@dagwieers
Copy link
Contributor

dagwieers commented Feb 28, 2019

@sebasmannem As a reviewer you can add a shipit to any PR simply by adding a comment/review and mention shipit on a new line. A PR requires 2 shipits (a green CI check and no change requests) to get merged automagically by @ansibot.

The only exception is a PR that contains a new module (or that modifies something outside of the control of the PostgreSQL Working Group). So for this PR a core member still needs to validate the module before merging, but the shipits do help understand that it is vetted by maintainers/reviewers.

We currently have 19 (not new_module) PostgreSQL-related pull-requests that need shipits from reviewers to get merged. So please help spread the gospel and let those shipits count :-)

@sebasmannem
Copy link
Contributor

shipit

Copy link
Contributor

@sebasmannem sebasmannem left a comment

Choose a reason for hiding this comment

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

shipit

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Mar 2, 2019
@cmfrtl
Copy link

cmfrtl commented Mar 5, 2019

shipit

@kostiantyn-nemchenko
Copy link
Contributor

shipit

@gundalow gundalow merged commit e246e74 into ansible:devel Mar 6, 2019
@gundalow
Copy link
Contributor

gundalow commented Mar 6, 2019

Thanks everybody, merged into devel for release in Ansible 2.8

@Andersson007
Copy link
Contributor Author

Thanks all !

@ansible ansible locked and limited conversation to collaborators Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 database Database category module This issue/PR relates to a module. new_module This PR includes a new module. new_plugin This PR includes a new plugin. postgresql PostgreSQL community shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants