Aerospike migrations module #49138

Alb0t · 2018-11-26T16:55:25Z

SUMMARY

Aerospike is a shared-nothing NoSQL database which is typically ran against 10-120 servers. With the typical replication and redundancy being able to withstand the loss of a limited amount of servers at a time, we need a module that allows us to check for replication or 'migrations' is complete to ensure data availability - before taking a system down. This ansible module should solve that need.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

aerospike_migrations module

ADDITIONAL INFORMATION

More info on Migrations:
https://www.aerospike.com/docs/operations/manage/migration/


PLAY [upgrade aerospike] *********************************************************************************************************************************************************************************************************************

TASK [Gathering Facts] ***********************************************************************************************************************************************************************************************************************
ok: [192.168.50.15]

TASK [Install dependencies] ******************************************************************************************************************************************************************************************************************
ok: [192.168.50.15]

TASK [setup aerospike] ***********************************************************************************************************************************************************************************************************************
ok: [192.168.50.15]

TASK [wait_for_migs] *************************************************************************************************************************************************************************************************************************
fatal: [192.168.50.15]: FAILED! => {"changed": false, "message": "Migrations still found after reaching limit.", "msg": "Migrations still found after reaching tries limit.", "original_message": ""}                                        
        to retry, use: --limit @/Users/alb0t/code/ansible_ansible/albot_ansible/test.retry

PLAY RECAP ***********************************************************************************************************************************************************************************************************************************
192.168.50.15              : ok=3    changed=0    unreachable=0    failed=1    skipped=0

(venv) alb0t @ ~/code/ansible_ansible/albot_ansible - [aerospike_migrations] $ ansible-playbook -i test.inv  test.yml -u root                                                                                                         

PLAY [upgrade aerospike] *********************************************************************************************************************************************************************************************************************

TASK [Gathering Facts] ***********************************************************************************************************************************************************************************************************************
ok: [192.168.50.15]

TASK [Install dependencies] ******************************************************************************************************************************************************************************************************************
ok: [192.168.50.15]

TASK [setup aerospike] ***********************************************************************************************************************************************************************************************************************
ok: [192.168.50.15]

TASK [wait_for_migs] *************************************************************************************************************************************************************************************************************************
ok: [192.168.50.15]

TASK [reboot] ********************************************************************************************************************************************************************************************************************************
^C

ansibot · 2018-11-26T19:11:14Z

Hi @Alb0t, thank you for submitting this pull-request!

click here for bot help

ansibot · 2018-11-26T19:11:15Z

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/database/aerospike/aerospike_migrations.py:0:0: E305 DOCUMENTATION.author: Invalid author for dictionary value @ data['author']. Got 'Albert Autin github.com/Alb0t'

click here for bot help

ansibot · 2018-11-26T21:10:30Z

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/database/aerospike/aerospike_migrations.py:26:8: W291 trailing whitespace

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/database/aerospike/aerospike_migrations.py:0:0: E305 DOCUMENTATION.author: Invalid author for dictionary value @ data['author']. Got ['Albert Autin @Alb0t']

click here for bot help

Alb0t · 2018-11-27T03:20:01Z

ready_for_review

Alb0t · 2018-12-13T06:56:26Z

bot_status

ansibot · 2018-12-13T07:04:15Z

Components

lib/ansible/modules/database/aerospike/init.py
support: community
maintainers:

lib/ansible/modules/database/aerospike/aerospike_migrations.py
support: community
maintainers:

Metadata

waiting_on: maintainer
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainer or core team member): []
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

felixfontein

I'll continue with the second part tomorrow.

lib/ansible/modules/database/aerospike/aerospike_migrations.py

felixfontein · 2018-12-15T10:22:10Z

lib/ansible/modules/database/aerospike/aerospike_migrations.py

+        )
+
+    if module.check_mode:
+        return result


You should always use module.exit_json() or module.jail_json() to return.

Also, this implementation of check_mode looks strange. A check_mode run should behave like a proper run from what it returns (if that can be determined in a reasonable way), with the exception that no change is made.

Does the fail_json code on 215 not look right?

That one's ok. Just using return is not, though :)

lib/ansible/modules/database/aerospike/aerospike_migrations.py

felixfontein · 2018-12-15T10:25:02Z

lib/ansible/modules/database/aerospike/aerospike_migrations.py

+        result['message'] = "No migrations"
+    else:
+        result['message'] = skip_reason
+        module.fail_json(


I would change this to module.fail_json(msg="Failed.", skip_reason=skip_reason), so that skip_reason is returned explicitly.

felixfontein · 2018-12-15T10:26:06Z

lib/ansible/modules/database/aerospike/aerospike_migrations.py

+    # a bool to specify checking local node only or entire cluster
+    has_migrations, skip_reason = migrations.has_migs(module.params['local_only'])
+    if not has_migrations:
+        result['message'] = "No migrations"


I don't think you need to return this fixed string on success. Just not returning anything should be fine.

lib/ansible/modules/database/aerospike/aerospike_migrations.py

felixfontein · 2018-12-15T10:59:02Z

Also, you might want to reduce your existing commit count (by squashing some of them together). Especially when you rebase anyway :)

Alb0t · 2018-12-19T08:37:45Z

welp, i failed at squash.

Alb0t · 2018-12-19T21:18:25Z

ready_for_review

felixfontein · 2018-12-19T22:12:30Z

welp, i failed at squash.

The good thing is, you can try again the next time ;)

Alb0t · 2018-12-27T04:42:49Z

ready_for_review

lib/ansible/modules/database/aerospike/aerospike_migrations.py

exception handling

Alb0t · 2019-01-01T20:47:37Z

ready_for_review

felixfontein

shipit

(Like I said before, I'm not an aerospike user, so I didn't test this, but the code looks good. Assuming that @Alb0t did test it, it's fine to merge this for me.)

Alb0t · 2019-01-13T00:31:19Z

thanks @felixfontein for all the help! We're using this actively in production with AWX. It's beautiful

Alb0t · 2019-01-13T00:32:12Z

bot_status

ansibot · 2019-01-13T00:36:39Z

Components

lib/ansible/modules/database/aerospike/init.py
support: community
maintainers:

lib/ansible/modules/database/aerospike/aerospike_migrations.py
support: community
maintainers:

Metadata

waiting_on: maintainer
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 1
shipit_actors (maintainers or core team members): felixfontein
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

felixfontein · 2019-01-13T20:41:56Z

You need at least one more review before this can get merged. You could for example bring your PR up in a Public Ansible Meeting on IRC (Freenode); see ansible/community#418 for details. @gundalow might also have some ideas :)

Alb0t · 2019-01-14T01:23:09Z

Thanks Felix. I dropped it into ansible/community#418 and will be on the core meeting Tuesday at 19:00 UTC ! :)

bcoca · 2019-01-15T19:34:30Z

rebuild_merge

* Push aerospike migrations module * maxsplit only valid for python3 exception handling

Alb0t force-pushed the aerospike_migrations_module branch from d9c1537 to b4f07d6 Compare November 26, 2018 20:25

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Nov 26, 2018

Alb0t force-pushed the aerospike_migrations_module branch from 8fc1e02 to 8dc65a9 Compare November 26, 2018 21:09

ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 26, 2018

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 5, 2018

felixfontein reviewed Dec 14, 2018

View reviewed changes

Alb0t force-pushed the aerospike_migrations_module branch from 8dc65a9 to 405b2fc Compare December 14, 2018 23:23

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 Dec 15, 2018

felixfontein reviewed Dec 15, 2018

View reviewed changes

Alb0t force-pushed the aerospike_migrations_module branch from 405b2fc to d8ce3b7 Compare December 19, 2018 08:36

felixfontein reviewed Dec 27, 2018

View reviewed changes

lib/ansible/modules/database/aerospike/aerospike_migrations.py Show resolved Hide resolved

felixfontein reviewed Dec 27, 2018

View reviewed changes

lib/ansible/modules/database/aerospike/aerospike_migrations.py Outdated Show resolved Hide resolved

Albert Autin added 2 commits January 1, 2019 13:44

Push aerospike migrations module

fc33b64

maxsplit only valid for python3

78badbe

exception handling

Alb0t force-pushed the aerospike_migrations_module branch from c85bff8 to 78badbe Compare January 1, 2019 20:46

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 10, 2019

felixfontein approved these changes Jan 12, 2019

View reviewed changes

Alb0t mentioned this pull request Jan 14, 2019

Public Ansible Project Meeting Agenda - January 2019 ansible/community#418

Closed

gundalow approved these changes Jan 15, 2019

View reviewed changes

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. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jan 15, 2019

ansibot merged commit 066ea55 into ansible:devel Jan 15, 2019

sivel removed the needs_triage Needs a first human triage before being processed. label Jan 15, 2019

knumskull pushed a commit to knumskull/ansible that referenced this pull request Jan 21, 2019

Aerospike migrations module (ansible#49138)

8c53145

* Push aerospike migrations module * maxsplit only valid for python3 exception handling

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Aerospike migrations module #49138

Aerospike migrations module #49138

Alb0t commented Nov 26, 2018 •

edited

ansibot commented Nov 26, 2018

ansibot commented Nov 26, 2018

ansibot commented Nov 26, 2018

Alb0t commented Nov 27, 2018

Alb0t commented Dec 13, 2018

ansibot commented Dec 13, 2018

felixfontein left a comment

felixfontein Dec 15, 2018

Alb0t Dec 19, 2018

felixfontein Dec 26, 2018

felixfontein Dec 15, 2018

felixfontein Dec 15, 2018

felixfontein commented Dec 15, 2018

Alb0t commented Dec 19, 2018

Alb0t commented Dec 19, 2018

felixfontein commented Dec 19, 2018

Alb0t commented Dec 27, 2018

Alb0t commented Jan 1, 2019

felixfontein left a comment

Alb0t commented Jan 13, 2019

Alb0t commented Jan 13, 2019

ansibot commented Jan 13, 2019

felixfontein commented Jan 13, 2019

Alb0t commented Jan 14, 2019

bcoca commented Jan 15, 2019

Aerospike migrations module #49138

Aerospike migrations module #49138

Conversation

Alb0t commented Nov 26, 2018 • edited

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibot commented Nov 26, 2018

ansibot commented Nov 26, 2018

ansibot commented Nov 26, 2018

Alb0t commented Nov 27, 2018

Alb0t commented Dec 13, 2018

ansibot commented Dec 13, 2018

Components

Metadata

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein Dec 15, 2018

Choose a reason for hiding this comment

Alb0t Dec 19, 2018

Choose a reason for hiding this comment

felixfontein Dec 26, 2018

Choose a reason for hiding this comment

felixfontein Dec 15, 2018

Choose a reason for hiding this comment

felixfontein Dec 15, 2018

Choose a reason for hiding this comment

felixfontein commented Dec 15, 2018

Alb0t commented Dec 19, 2018

Alb0t commented Dec 19, 2018

felixfontein commented Dec 19, 2018

Alb0t commented Dec 27, 2018

Alb0t commented Jan 1, 2019

felixfontein left a comment

Choose a reason for hiding this comment

Alb0t commented Jan 13, 2019

Alb0t commented Jan 13, 2019

ansibot commented Jan 13, 2019

Components

Metadata

felixfontein commented Jan 13, 2019

Alb0t commented Jan 14, 2019

bcoca commented Jan 15, 2019

Alb0t commented Nov 26, 2018 •

edited