Skip checking flagIN value before inserting rules

[squirrel532]

SUMMARY

Fix #107

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

proxysql_query_rules_fast_routing

ADDITIONAL INFORMATION

[markuman]

@squirrel532 thanks for your PR. Can you also please add a changelog fragment: https://docs.ansible.com/ansible/latest/community/development_process.html#creating-changelog-fragments

And another think that would be very helpful is to expand the intergration test for proxysql_query_rules_fast_routing.

• https://github.com/ansible-collections/community.proxysql/tree/main/tests/integration/targets/test_proxysql_query_rules_fast_routing/tasks

Just add a new task file 107-update_destination_hostgroup.yml for example. basically you can put there the tasks to reproduce the bug and include it in the main.yml, somewhere here: https://github.com/ansible-collections/community.proxysql/blob/main/tests/integration/targets/test_proxysql_query_rules_fast_routing/tasks/main.yml#L84

Once it's done, you can run the integrationtest locally
ansible-test integration -v test_proxysql_query_rules_fast_routing --docker default

[squirrel532]

I found a way to reuse existed test case.

[markuman]

@squirrel532 just the two tihngs, otherwise LGTM!

[squirrel532]

Remove addition testing mentioned before, it's ready to merge.

[markuman]

@squirrel532 I've committed the integration test. You can run it with

ansible-test integration test_proxysql_query_rules_fast_routing --docker default

You'll see that it is/was failing.

TASK [test_proxysql_query_rules_fast_routing : test change destination hostgroup] ***
included: /root/ansible_collections/community/proxysql/tests/output/.tmp/integration/test_proxysql_query_rules_fast_routing-360nphmu-ÅÑŚÌβŁÈ/tests/integration/targets/test_proxysql_query_rules_fast_routing/tasks/107-update_destination_hostgroup.yml for testhost

TASK [test_proxysql_query_rules_fast_routing : Create fast routing rules] ******
changed: [testhost]

TASK [test_proxysql_query_rules_fast_routing : debug] **************************
ok: [testhost] => {
    "out": {
        "changed": true,
        "failed": false,
        "msg": "Added rule to mysql_query_rules_fast_routing.",
        "rules": [
            {
                "comment": "",
                "destination_hostgroup": "1",
                "flagIN": "0",
                "schemaname": "test_database",
                "username": "user"
            }
        ],
        "state": "present"
    }
}

TASK [test_proxysql_query_rules_fast_routing : verify create] ******************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [test_proxysql_query_rules_fast_routing : Update destination hostgroup of previous fast routing rule] ***
changed: [testhost]

TASK [test_proxysql_query_rules_fast_routing : debug] **************************
ok: [testhost] => {
    "out": {
        "changed": true,
        "failed": false,
        "msg": "Updated rule in mysql_query_rules_fast_routing.",
        "rules": [],
        "state": "present"
    }
}

TASK [test_proxysql_query_rules_fast_routing : verify change] ******************
fatal: [testhost]: FAILED! => {"msg": "The conditional check 'out.rules[0].destination_hostgroup == \"2\"' failed. The error was: error while evaluating conditional (out.rules[0].destination_hostgroup == \"2\"): list object has no element 0"}

PLAY RECAP *********************************************************************
testhost                   : ok=232  changed=114  unreachable=0    failed=1    skipped=44   rescued=0    ignored=0  

So the current fix d25ef07 just prevents that the module is falling in an uncaught exception. The change itself is still not happen.

The update_rule_config method is buggy https://github.com/ansible-collections/community.proxysql/blob/main/plugins/modules/proxysql_query_rules_fast_routing.py#L231

0.0s main: 'initial update'
 0.0s update_rule_config: ['user', 'test_database', 0]
 0.0s update_rule_config: 
      {'comment': '', 'destination_hostgroup': 2, 'flagIN': 0, 'schemaname': 'test_database', 'username': 'user'}
 0.0s update_rule_config: ['user', 'test_database', 0, 2, '']
 0.0s update_rule_config: 
      'UPDATE mysql_query_rules_fast_routing SET destin...username = %s AND schemaname = %s AND flagIN = %s'

After digging deeper, I've figured out that the query_data (consists of the where statements parameter values) is appended by the parameters that are requested to change, are in a wrong order.
So "append",....as it says, append the values at the end. ['user', 'test_database', 0] becomes ['user', 'test_database', 0, 2, '']
The 2 is for the destination hostgroup and '' for the comment is added.
And because the build query begins with SET destination_group .... and ends with WHERE username ..., the order is wrong. The destination hostgroup and comment values must be moved to index 0 and 1.

After that 48bd3f9, it works

TASK [test_proxysql_query_rules_fast_routing : test change destination hostgroup] ***
included: /root/ansible_collections/community/proxysql/tests/output/.tmp/integration/test_proxysql_query_rules_fast_routing-k7oma19i-ÅÑŚÌβŁÈ/tests/integration/targets/test_proxysql_query_rules_fast_routing/tasks/107-update_destination_hostgroup.yml for testhost

TASK [test_proxysql_query_rules_fast_routing : Create fast routing rules] ******
changed: [testhost]

TASK [test_proxysql_query_rules_fast_routing : debug] **************************
ok: [testhost] => {
    "out": {
        "changed": true,
        "failed": false,
        "msg": "Added rule to mysql_query_rules_fast_routing.",
        "rules": [
            {
                "comment": "",
                "destination_hostgroup": "1",
                "flagIN": "0",
                "schemaname": "test_database",
                "username": "user"
            }
        ],
        "state": "present"
    }
}

TASK [test_proxysql_query_rules_fast_routing : verify create] ******************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [test_proxysql_query_rules_fast_routing : Update destination hostgroup of previous fast routing rule] ***
changed: [testhost]

TASK [test_proxysql_query_rules_fast_routing : debug] **************************
ok: [testhost] => {
    "out": {
        "changed": true,
        "failed": false,
        "msg": "Updated rule in mysql_query_rules_fast_routing.",
        "rules": [
            {
                "comment": "",
                "destination_hostgroup": "2",
                "flagIN": "0",
                "schemaname": "test_database",
                "username": "user"
            }
        ],
        "state": "present"
    }
}

TASK [test_proxysql_query_rules_fast_routing : verify change] ******************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [test_proxysql_query_rules_fast_routing : Update destination hostgroup of previous fast routing rule (idempotent)] ***
ok: [testhost]

TASK [test_proxysql_query_rules_fast_routing : debug] **************************
ok: [testhost] => {
    "out": {
        "changed": false,
        "failed": false,
        "msg": "The rule already exists in mysql_query_rules_fast_routing and doesn't need to be updated.",
        "rules": [
            {
                "comment": "",
                "destination_hostgroup": "2",
                "flagIN": "0",
                "schemaname": "test_database",
                "username": "user"
            }
        ],
        "state": "present"
    }
}

TASK [test_proxysql_query_rules_fast_routing : verify no change] ***************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [test_proxysql_query_rules_fast_routing : test_proxysql_query_rules_fast_routing | teardown | uninstall proxysql] ***
changed: [testhost]

PLAY RECAP *********************************************************************
testhost                   : ok=237  changed=115  unreachable=0    failed=0    skipped=44   rescued=0    ignored=0   

[Andersson007]

I'm not a specialist in the underlying technology.
If it's backwards compatible, LGTM in terms of general stuff like the changelog and integration test coverage

@squirrel532 @markuman thanks a lot for working on the fix!

[Andersson007]

Thank you all for the contribution!

@squirrel532 please continue to contribute to ansible! https://docs.ansible.com/ansible/devel//community/contributor_path.html