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

mongodb_replicaset module and test exception #49690

Merged
merged 13 commits into from
Feb 25, 2019
Merged

mongodb_replicaset module and test exception #49690

merged 13 commits into from
Feb 25, 2019

Conversation

rhysmeister
Copy link
Contributor

SUMMARY

New module allowing the creation of mongodb replicasets. Based on existing mongodb modules.

Tidy up of previous PR: #43674

Includes changes requested by @maxamillion and @acozine

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

mongodb_replicaset

ADDITIONAL INFORMATION
ansible 2.7.1
  config file = None
  configured module search path = ['/Users/rhyscampbell/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/Cellar/python/3.6.4_2/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.6.4 (default, Jan  6 2018, 11:51:59) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)]

@ansibot
Copy link
Contributor

ansibot commented Dec 9, 2018

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 core_review In order to be merged, this PR must follow the core 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. python3 support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Dec 9, 2018
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Dec 11, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Dec 22, 2018
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 11, 2019
@ansibot

This comment has been minimized.

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jan 11, 2019
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 11, 2019
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 19, 2019
@dagwieers dagwieers added the mongodb MongoDB community label Jan 28, 2019
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 2, 2019
@dagwieers
Copy link
Contributor

Please add integration tests for this module. Look at test/integration/targets/ and more specifically check what is being done for postgresql and mysql to see how they set up a database, and then use it for testing.

We currently have no mongodb integration tests, and we have to fix that for all modules.

@dagwieers dagwieers added the database Database category label Feb 13, 2019
@ansibot

This comment has been minimized.

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Feb 13, 2019
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Feb 13, 2019
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Feb 21, 2019
@rhysmeister
Copy link
Contributor Author

@dagwieers Good to go here? PR for testing should be ready soon.

@ansibot ansibot removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Feb 22, 2019
@ansibot

This comment has been minimized.

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Feb 22, 2019
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Feb 22, 2019
@rhysmeister
Copy link
Contributor Author

Caught a bug in this module for MongoDB 4.0 found thanks to integration tests. Integration tests are working for MongoDB versions 3.2, 3.4, 3.6 and 4.0.

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.

Given that this has been tested, and we are expecting integration tests as well. I am going to approve this PR.

@dagwieers dagwieers merged commit 7f50f46 into ansible:devel Feb 25, 2019
@dagwieers dagwieers removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Feb 25, 2019
@rhysmeister
Copy link
Contributor Author

Great stuff. Thanks for your efforts in getting this merged.

@netjordan
Copy link

Hi @rhysmeister

At line 336 should
if arbiter_at_index is not None and len(members) - 1 > arbiter_at_index: module.fail_json(msg="MongoDB Replicaset validation failed. Invalid arbiter index.")

Should this actually be:
if arbiter_at_index is not None and len(members) - 1 < arbiter_at_index:

I can raise a PR if you agree?

@rhysmeister
Copy link
Contributor Author

@netjordan Yes. Good catch. I'll ensure there's a test for this as well.

@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. mongodb MongoDB community new_module This PR includes a new module. new_plugin This PR includes a new plugin. python3 support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants