aerospike_migrations: check_mode does not check state

[russoz]

Summary

The code for check_mode simply returns changed=false without validating any state. Arguably, it does not support check_mode.

Issue Type

Bug Report

Component Name

aerospike_migrations

Ansible Version

$ ansible --version

Community.general Version

10.2.0 (verified in the code)

Configuration

$ ansible-config dump --only-changed

OS / Environment

No response

Steps to Reproduce

N/A

Expected Results

I expected the module to verify the state and return whether the module execution without check_mode would incur in a change or not.

Actual Results

The module always states there was no change in check_mode without even connecting to the aerospike database.

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/modules/aerospike_migrations.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @Alb0t
click here for bot help

[felixfontein]

The module also returns with changed=false outside check mode, so this behavior is not per se wrong.

It probably doesn't hurt to add details to attributes.check_mode to explain in more detail what check mode does and why.

[russoz]

The problem is not whether changed is false or not, within or without check mode. The problem is that literally nothing is being checked. The state of the backend service is never accessed in order to set the value of changed.

[felixfontein]

Depending on how you define check mode that is totally fine. One big motivation for check mode is to find out whether the module would cause a change or not. The implementation does that. I know too little about Aerospike to be able to say which requests that the module executes would be safe, so I don't know much more the module could check. I guess it also depends a lot on your workflow.

It's probably a good idea to document what the module does, and potentially do more in check mode, without breaking existing workflows.

[Alb0t]

Thanks for fielding this so far @felixfontein .
@russoz I can agree there would be value in check mode actually checking for pending replication. It's been a few years since I authored it, so I'm not sure why I wanted it to be false - I think it was to speed up the rest of the playbook development. In the case of pending migrations, check mode would wait for very long times and in the case of large clusters the connection est time was long too.

To make it have the behavior you want, I think you just need to get rid of this conditional and have it just skip to L214:

community.general/plugins/modules/aerospike_migrations.py

Line 211 in 7c54c70

if module.check_mode:

I managed to get this working in aerolab and pinning aerospike library version to 10.0.0. The change I mentioned should do the trick. It will make the playbooks that use CD mode take much longer though.. but I don't think there is much risk. This module just checks replication metrics by default and doesn't actually modify anything or run dangerous commands.